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