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

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):
-        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,

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")
 

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.