From 8776c81f64bfee814573f217b140408f57fb302d 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. Change-Id: I7a6d3655cac173f2720a7f4ef87ce37ac4f022d7 Closes-bug: #1783582 --- .../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 d1f0e43b64e..2c5219cedbc 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 7d964b8f7b3..074cff8c661 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 411f2c3cb46..c6147969931 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