From 93399a32bf994a3129a798be72f27a9731cb2750 Mon Sep 17 00:00:00 2001 From: "Erlon R. Cruz" Date: Mon, 23 Jul 2018 16:34:47 -0300 Subject: [PATCH] NetApp ONTAP: Fix driver force detach operations Cinder API supports os-force_detach, which once invoked can call the driver finalize_connection passing a 'None' connector. When receiving this, the driver should handle this call by terminating every connection between the specified volume and any host it is mapped to. This patch fixes this behavior for NetApp ONTAP iSCSI and FC drivers. Closes-bug: #1783582 (cherry picked from commit I2a55961b62297f0fa4e678929f69cafc5aa17bd9) Change-Id: Id3ad0fb4ea46d09033435641f39af04cbffea77f --- .../volume/drivers/netapp/dataontap/fakes.py | 9 ++++ .../netapp/dataontap/test_block_base.py | 42 +++++++++++++-- .../drivers/netapp/dataontap/block_base.py | 51 ++++++++++++++----- ...tap-fix-force-detach-55be3f4ac962b493.yaml | 5 ++ 4 files changed, 88 insertions(+), 19 deletions(-) create mode 100644 releasenotes/notes/netapp-ontap-fix-force-detach-55be3f4ac962b493.yaml diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py index 34a82fe0839..e03ed552d00 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py @@ -185,6 +185,15 @@ FC_TARGET_INFO_UNMAP = {'driver_volume_type': 'fibre_channel', 'data': {'target_wwn': FC_TARGET_WWPNS, 'initiator_target_map': FC_I_T_MAP}} +ISCSI_ONE_MAP_LIST = [{'initiator-group': 'openstack-faketgt1', + 'vserver': 'vserver_123', 'lun-id': '1'}] +ISCSI_MULTI_MAP_LIST = [{'initiator-group': 'openstack-faketgt1', + 'vserver': 'vserver_123', 'lun-id': '1'}, + {'initiator-group': 'openstack-faketgt2', + 'vserver': 'vserver_123', 'lun-id': '2'} + ] +ISCSI_EMPTY_MAP_LIST = [] + IGROUP1_NAME = 'openstack-igroup1' IGROUP1 = { diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py index 4499916e8f1..77e54cfff2c 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py @@ -262,13 +262,45 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_find_mapped_lun_igroup') - def test_unmap_lun(self, mock_find_mapped_lun_igroup): - mock_find_mapped_lun_igroup.return_value = (fake.IGROUP1_NAME, 1) + def test_unmap_lun_empty(self, mock_find_mapped_lun_igroup): + self.zapi_client.get_lun_map.return_value = fake.ISCSI_ONE_MAP_LIST - self.library._unmap_lun(fake.LUN_PATH, fake.FC_FORMATTED_INITIATORS) + self.library._unmap_lun(fake.LUN_PATH, fake.ISCSI_EMPTY_MAP_LIST) - self.zapi_client.unmap_lun.assert_called_once_with(fake.LUN_PATH, - fake.IGROUP1_NAME) + mock_find_mapped_lun_igroup.assert_not_called() + self.zapi_client.get_lun_map.assert_called_once_with(fake.LUN_PATH) + self.zapi_client.unmap_lun.assert_called_once_with( + fake.LUN_PATH, fake.ISCSI_ONE_MAP_LIST[0]['initiator-group']) + + @mock.patch.object(block_base.NetAppBlockStorageLibrary, + '_find_mapped_lun_igroup') + def test_unmap_lun_detach_one(self, mock_find_mapped_lun_igroup): + fake_ini_group = fake.ISCSI_ONE_MAP_LIST[0]['initiator-group'] + mock_find_mapped_lun_igroup.return_value = (fake_ini_group, 1) + self.zapi_client.get_lun_map.return_value = fake.ISCSI_ONE_MAP_LIST + + self.library._unmap_lun(fake.LUN_PATH, fake.ISCSI_ONE_MAP_LIST) + + mock_find_mapped_lun_igroup.assert_called_once_with( + fake.LUN_PATH, fake.ISCSI_ONE_MAP_LIST) + self.zapi_client.get_lun_map.assert_not_called() + self.zapi_client.unmap_lun.assert_called_once_with( + fake.LUN_PATH, fake_ini_group) + + @mock.patch.object(block_base.NetAppBlockStorageLibrary, + '_find_mapped_lun_igroup') + def test_unmap_lun_empty_detach_all(self, mock_find_mapped_lun_igroup): + self.zapi_client.get_lun_map.return_value = fake.ISCSI_MULTI_MAP_LIST + + self.library._unmap_lun(fake.LUN_PATH, fake.ISCSI_EMPTY_MAP_LIST) + + mock_find_mapped_lun_igroup.assert_not_called() + self.zapi_client.get_lun_map.assert_called_once_with(fake.LUN_PATH) + calls = [mock.call(fake.LUN_PATH, + fake.ISCSI_MULTI_MAP_LIST[0]['initiator-group']), + mock.call(fake.LUN_PATH, + fake.ISCSI_MULTI_MAP_LIST[1]['initiator-group'])] + self.zapi_client.unmap_lun.assert_has_calls(calls) def test_find_mapped_lun_igroup(self): self.assertRaises(NotImplementedError, diff --git a/cinder/volume/drivers/netapp/dataontap/block_base.py b/cinder/volume/drivers/netapp/dataontap/block_base.py index 1612e4d2369..e5e51cfd57f 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_base.py +++ b/cinder/volume/drivers/netapp/dataontap/block_base.py @@ -440,9 +440,19 @@ class NetAppBlockStorageLibrary(object): def _unmap_lun(self, path, initiator_list): """Unmaps a LUN from given initiator.""" - (igroup_name, _lun_id) = self._find_mapped_lun_igroup(path, - initiator_list) - self.zapi_client.unmap_lun(path, igroup_name) + + if len(initiator_list) != 0: + lun_unmap_list = [] + (igroup_name, _) = self._find_mapped_lun_igroup( + path, initiator_list) + lun_unmap_list.append((path, igroup_name)) + else: + lun_maps = self.zapi_client.get_lun_map(path) + lun_unmap_list = [(path, lun_m['initiator-group']) + for lun_m in lun_maps] + + for _path, _igroup_name in lun_unmap_list: + self.zapi_client.unmap_lun(_path, _igroup_name) def _find_mapped_lun_igroup(self, path, initiator_list): """Find an igroup for a LUN mapped to the given initiator(s).""" @@ -891,14 +901,21 @@ class NetAppBlockStorageLibrary(object): longer access it. """ - initiator_name = connector['initiator'] name = volume['name'] + if connector is None: + initiators = [] + LOG.debug('Unmapping LUN %(name)s from all initiators', + {'name': name}) + else: + initiators = [connector['initiator']] + LOG.debug("Unmapping LUN %(name)s from the initiator " + "%(initiator_name)s", {'name': name, + 'initiator_name': initiators}) + metadata = self._get_lun_attr(name, 'metadata') path = metadata['Path'] - self._unmap_lun(path, [initiator_name]) - LOG.debug("Unmapped LUN %(name)s from the initiator " - "%(initiator_name)s", - {'name': name, 'initiator_name': initiator_name}) + + self._unmap_lun(path, initiators) def initialize_connection_fc(self, volume, connector): """Initializes the connection and returns connection info. @@ -990,21 +1007,27 @@ class NetAppBlockStorageLibrary(object): an empty dict for the 'data' key """ - initiators = [fczm_utils.get_formatted_wwn(wwpn) - for wwpn in connector['wwpns']] name = volume['name'] + if connector is None: + initiators = [] + LOG.debug('Unmapping LUN %(name)s from all initiators', + {'name': name}) + else: + initiators = [fczm_utils.get_formatted_wwn(wwpn) + for wwpn in connector['wwpns']] + LOG.debug("Unmapping LUN %(name)s from the initiators " + "%(initiator_name)s", {'name': name, + 'initiator_name': initiators}) + metadata = self._get_lun_attr(name, 'metadata') path = metadata['Path'] self._unmap_lun(path, initiators) - LOG.debug("Unmapped LUN %(name)s from the initiator %(initiators)s", - {'name': name, 'initiators': initiators}) - info = {'driver_volume_type': 'fibre_channel', 'data': {}} - if not self._has_luns_mapped_to_initiators(initiators): + if connector and not self._has_luns_mapped_to_initiators(initiators): # No more exports for this host, so tear down zone. LOG.info("Need to remove FC Zone, building initiator target map") diff --git a/releasenotes/notes/netapp-ontap-fix-force-detach-55be3f4ac962b493.yaml b/releasenotes/notes/netapp-ontap-fix-force-detach-55be3f4ac962b493.yaml new file mode 100644 index 00000000000..fd020eaf163 --- /dev/null +++ b/releasenotes/notes/netapp-ontap-fix-force-detach-55be3f4ac962b493.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixed bug #1783582, where calls to os-force_detach were failing on NetApp + ONTAP iSCSI/FC drivers. \ No newline at end of file