From 8e5330c029b538252e8743f7dd2276f423a8ec1b Mon Sep 17 00:00:00 2001 From: Alexander Deiter Date: Thu, 21 Jul 2022 10:57:05 +0000 Subject: [PATCH] Fix Infinidat driver multi-attach feature Added a check if there are multiple attachments to the volume from the same connector and terminate connection only for the last attachment from the corresponding host. Closes-bug: #1982350 Change-Id: Ibda3a09a181160b8ee9129795429a7f1795e907d Signed-off-by: Alexander Deiter (cherry picked from commit d8ca81e7c911add4f99f888d7315130ac3b60e22) --- .../unit/volume/drivers/test_infinidat.py | 111 ++++++++++++++++-- cinder/volume/drivers/infinidat.py | 40 ++++++- .../drivers/infinidat-volume-driver.rst | 1 + ...dat-fix-multi-attach-19f62d182b675e59.yaml | 9 ++ 4 files changed, 148 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/bug-1982350-infinidat-fix-multi-attach-19f62d182b675e59.yaml diff --git a/cinder/tests/unit/volume/drivers/test_infinidat.py b/cinder/tests/unit/volume/drivers/test_infinidat.py index 53ed70e19a0..33a5b5d87df 100644 --- a/cinder/tests/unit/volume/drivers/test_infinidat.py +++ b/cinder/tests/unit/volume/drivers/test_infinidat.py @@ -39,6 +39,7 @@ TEST_IP_ADDRESS2 = '2.2.2.2' TEST_IP_ADDRESS3 = '3.3.3.3' TEST_IP_ADDRESS4 = '4.4.4.4' TEST_INITIATOR_IQN = 'iqn.2012-07.org.initiator:01' +TEST_INITIATOR_IQN2 = 'iqn.2012-07.org.initiator:02' TEST_TARGET_IQN = 'iqn.2012-07.org.target:01' TEST_ISCSI_TCP_PORT1 = 3261 TEST_ISCSI_TCP_PORT2 = 3262 @@ -58,16 +59,25 @@ TEST_SNAPSHOT_SOURCE_ID = 67890 TEST_SNAPSHOT_METADATA = {'cinder_id': fake.SNAPSHOT_ID} test_volume = mock.Mock(id=fake.VOLUME_ID, name_id=fake.VOLUME_ID, size=1, - volume_type_id=fake.VOLUME_TYPE_ID) + volume_type_id=fake.VOLUME_TYPE_ID, + multiattach=False, volume_attachment=None) test_volume2 = mock.Mock(id=fake.VOLUME2_ID, name_id=fake.VOLUME2_ID, size=1, - volume_type_id=fake.VOLUME_TYPE_ID) + volume_type_id=fake.VOLUME_TYPE_ID, + multiattach=False, volume_attachment=None) test_snapshot = mock.Mock(id=fake.SNAPSHOT_ID, volume=test_volume, volume_id=test_volume.id) -test_clone = mock.Mock(id=fake.VOLUME4_ID, size=1) +test_clone = mock.Mock(id=fake.VOLUME4_ID, size=1, multiattach=False, + volume_attachment=None) test_group = mock.Mock(id=fake.GROUP_ID) test_snapgroup = mock.Mock(id=fake.GROUP_SNAPSHOT_ID, group=test_group) test_connector = dict(wwpns=[TEST_WWN_1], initiator=TEST_INITIATOR_IQN) +test_connector2 = dict(wwpns=[TEST_WWN_2], + initiator=TEST_INITIATOR_IQN2) +test_connector3 = dict(wwpns=None, initiator=None) +test_attachment1 = mock.Mock(connector=test_connector) +test_attachment2 = mock.Mock(connector=test_connector2) +test_attachment3 = mock.Mock(connector=None) def skip_driver_setup(func): @@ -261,15 +271,41 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase): self.driver.initialize_connection(test_volume, test_connector) self._validate_host_metadata() + @ddt.data({'connector': None, 'multiattach': True, + 'attachment': [test_attachment1, test_attachment1]}, + {'connector': test_connector3, 'multiattach': True, + 'attachment': [test_attachment1, test_attachment1]}, + {'connector': test_connector, 'multiattach': False, + 'attachment': [test_attachment1]}, + {'connector': test_connector, 'multiattach': True, + 'attachment': None}, + {'connector': test_connector, 'multiattach': True, + 'attachment': [test_attachment2, test_attachment3]}) + @ddt.unpack + def test__is_volume_multiattached_negative(self, connector, + multiattach, attachment): + volume = copy.deepcopy(test_volume) + volume.multiattach = multiattach + volume.volume_attachment = attachment + self.assertFalse(self.driver._is_volume_multiattached(volume, + connector)) + def test_terminate_connection(self): - self.driver.terminate_connection(test_volume, test_connector) + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) def test_terminate_connection_delete_host(self): self._mock_host.get_luns.return_value = [object()] - self.driver.terminate_connection(test_volume, test_connector) + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) self.assertEqual(0, self._mock_host.safe_delete.call_count) self._mock_host.get_luns.return_value = [] - self.driver.terminate_connection(test_volume, test_connector) + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) self.assertEqual(1, self._mock_host.safe_delete.call_count) def test_terminate_connection_volume_doesnt_exist(self): @@ -617,8 +653,10 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase): mock_mapping = mock.Mock() mock_mapping.get_host.return_value = mock_infinidat_host self._mock_volume.get_logical_units.return_value = [mock_mapping] + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1, test_attachment2] # connector is None - force detach - detach all mappings - self.driver.terminate_connection(test_volume, None) + self.assertTrue(self.driver.terminate_connection(volume, None)) # make sure we actually detached the host mapping self._mock_host.unmap_volume.assert_called_once() self._mock_host.safe_delete.assert_called_once() @@ -861,6 +899,27 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase): def test_unmanage_snapshot(self): self.driver.unmanage_snapshot(test_snapshot) + def test_terminate_connection_no_attachment_connector(self): + volume = copy.deepcopy(test_volume) + volume.multiattach = True + volume.volume_attachment = [test_attachment3] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) + + def test_terminate_connection_no_host(self): + self._system.hosts.safe_get.return_value = None + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) + + def test_terminate_connection_no_mapping(self): + self._mock_host.unmap_volume.side_effect = KeyError + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) + def test_update_migrated_volume_new_volume_not_found(self): self._system.volumes.safe_get.side_effect = [ None, self._mock_volume] @@ -918,6 +977,7 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase): self.assertEqual(0, self._log.error.call_count) +@ddt.ddt class InfiniboxDriverTestCaseFC(InfiniboxDriverTestCaseBase): def test_initialize_connection_multiple_wwpns(self): connector = {'wwpns': [TEST_WWN_1, TEST_WWN_2]} @@ -931,7 +991,27 @@ class InfiniboxDriverTestCaseFC(InfiniboxDriverTestCaseBase): self.assertRaises(exception.InvalidConnectorException, self.driver.validate_connector, iscsi_connector) + @ddt.data({'connector': test_connector, + 'attachment': [test_attachment1, test_attachment1]}, + {'connector': test_connector2, + 'attachment': [test_attachment2, test_attachment2]}) + @ddt.unpack + def test__is_volume_multiattached_positive(self, connector, attachment): + volume = copy.deepcopy(test_volume) + volume.multiattach = True + volume.volume_attachment = attachment + self.assertTrue(self.driver._is_volume_multiattached(volume, + connector)) + def test_terminate_connection_multiattached_volume(self): + volume = copy.deepcopy(test_volume) + volume.multiattach = True + volume.volume_attachment = [test_attachment1, test_attachment1] + self.assertTrue(self.driver.terminate_connection(volume, + test_connector)) + + +@ddt.ddt class InfiniboxDriverTestCaseISCSI(InfiniboxDriverTestCaseBase): def setUp(self): super(InfiniboxDriverTestCaseISCSI, self).setUp() @@ -1114,8 +1194,23 @@ class InfiniboxDriverTestCaseISCSI(InfiniboxDriverTestCaseBase): } self.assertEqual(expected, result) + @ddt.data({'connector': test_connector, + 'attachment': [test_attachment1, test_attachment1]}, + {'connector': test_connector2, + 'attachment': [test_attachment2, test_attachment2]}) + @ddt.unpack + def test__is_volume_multiattached_positive(self, connector, attachment): + volume = copy.deepcopy(test_volume) + volume.multiattach = True + volume.volume_attachment = attachment + self.assertTrue(self.driver._is_volume_multiattached(volume, + connector)) + def test_terminate_connection(self): - self.driver.terminate_connection(test_volume, test_connector) + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) def test_validate_connector(self): fc_connector = {'wwpns': [TEST_WWN_1, TEST_WWN_2]} diff --git a/cinder/volume/drivers/infinidat.py b/cinder/volume/drivers/infinidat.py index 3e7bbcfa783..0995cecf9ce 100644 --- a/cinder/volume/drivers/infinidat.py +++ b/cinder/volume/drivers/infinidat.py @@ -126,10 +126,11 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver): 1.9 - added manage/unmanage/manageable-list volume/snapshot 1.10 - added support for TLS/SSL communication 1.11 - fixed generic volume migration + 1.12 - fixed volume multi-attach """ - VERSION = '1.11' + VERSION = '1.12' # ThirdPartySystems wiki page CI_WIKI_NAME = "INFINIDAT_CI" @@ -478,6 +479,32 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver): ports = [iqn.IQN(connector['initiator'])] return ports + def _is_volume_multiattached(self, volume, connector): + """Returns whether the volume is multiattached. + + Check if there are multiple attachments to the volume + from the same connector. Terminate connection only for + the last attachment from the corresponding host. + """ + if not (connector and volume.multiattach and + volume.volume_attachment): + return False + keys = ['system uuid'] + if self._protocol == constants.FC: + keys.append('wwpns') + else: + keys.append('initiator') + for key in keys: + if not (key in connector and connector[key]): + continue + if sum(1 for attachment in volume.volume_attachment if + attachment.connector and key in attachment.connector and + attachment.connector[key] == connector[key]) > 1: + LOG.debug('Volume %s is multiattached to %s %s', + volume.name_id, key, connector[key]) + return True + return False + @infinisdk_to_cinder_exceptions @coordination.synchronized('infinidat-{self.management_address}-lock') def initialize_connection(self, volume, connector): @@ -491,6 +518,8 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver): @coordination.synchronized('infinidat-{self.management_address}-lock') def terminate_connection(self, volume, connector, **kwargs): """Unmap an InfiniBox volume from the host""" + if self._is_volume_multiattached(volume, connector): + return True infinidat_volume = self._get_infinidat_volume(volume) if self._protocol == constants.FC: volume_type = 'fibre_channel' @@ -521,11 +550,11 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver): target_wwpns)) result_data = dict(target_wwn=target_wwpns, initiator_target_map=target_map) - conn_info = dict(driver_volume_type=volume_type, - data=result_data) if self._protocol == constants.FC: + conn_info = dict(driver_volume_type=volume_type, + data=result_data) fczm_utils.remove_fc_zone(conn_info) - return conn_info + return volume.volume_attachment and len(volume.volume_attachment) > 1 @infinisdk_to_cinder_exceptions def get_volume_stats(self, refresh=False): @@ -665,7 +694,8 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver): # we need a cinder-volume-like object to map the clone by name # (which is derived from the cinder id) but the clone is internal # so there is no such object. mock one - clone = mock.Mock(name_id=str(volume.name_id) + '-internal') + clone = mock.Mock(name_id=str(volume.name_id) + '-internal', + multiattach=False, volume_attachment=[]) try: infinidat_volume = self._create_volume(volume) try: diff --git a/doc/source/configuration/block-storage/drivers/infinidat-volume-driver.rst b/doc/source/configuration/block-storage/drivers/infinidat-volume-driver.rst index 65e58a94623..3b730ee2580 100644 --- a/doc/source/configuration/block-storage/drivers/infinidat-volume-driver.rst +++ b/doc/source/configuration/block-storage/drivers/infinidat-volume-driver.rst @@ -25,6 +25,7 @@ Supported operations * Revert a volume to a snapshot. * Manage and unmanage volumes and snapshots. * List manageable volumes and snapshots. +* Attach a volume to multiple instances at once (multi-attach). External package installation ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/releasenotes/notes/bug-1982350-infinidat-fix-multi-attach-19f62d182b675e59.yaml b/releasenotes/notes/bug-1982350-infinidat-fix-multi-attach-19f62d182b675e59.yaml new file mode 100644 index 00000000000..1ca0a83906c --- /dev/null +++ b/releasenotes/notes/bug-1982350-infinidat-fix-multi-attach-19f62d182b675e59.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Infinidat Driver `bug #1982350 + `_: + Fixed Infinidat driver multi-attach feature. + Added a check if there are multiple attachments to the volume + from the same connector and terminate connection only for the + last attachment from the corresponding host.