Merge "NetApp ONTAP: Fix driver force detach operations"

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 = {

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):
+    def test_unmap_lun_empty(self, mock_find_mapped_lun_igroup):
-        mock_find_mapped_lun_igroup.return_value = (fake.IGROUP1_NAME, 1)
+        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,
+        mock_find_mapped_lun_igroup.assert_not_called()
-                                                           fake.IGROUP1_NAME)
+        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,

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)
+        if len(initiator_list) != 0:
-        self.zapi_client.unmap_lun(path, igroup_name)
+            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 "
+        self._unmap_lun(path, initiators)
-                  "%(initiator_name)s",
-                  {'name': name, 'initiator_name': initiator_name})
 
     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
         """
 
+        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']]
-        name = volume['name']
+            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")
 

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.