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

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.