NetApp ONTAP: Fix iSCSI multiattach volume terminates connection

When detaching a volume from an instance, NetApp ONTAP driver terminates
the iSCSI connection, even if there're other connections to the LUN from
the same initiator. This terminates connections to the LUN from other
instances running on the same host.

This patch fixes this bug by leaving the connection untouched if there
are other connections from the same initiator. Now the connection is
terminated only if it's the last one from the same initiator.

Change-Id: I0b8b5930c9e8f3c049b82e92c5fb7b05d6ae9291
Closes-Bug: #1839384
(cherry picked from commit e27d83f4d0)
(cherry picked from commit 87b933db77)

cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py

@@ -538,6 +538,15 @@ test_snapshot.volume_id = 'fake_volume_id'
 test_snapshot.provider_location = PROVIDER_LOCATION
 
 
+class test_iscsi_attachment(object):
+    def __getattr__(self, key):
+        return getattr(self, key)
+
+
+test_iscsi_attachment = test_iscsi_attachment()
+test_iscsi_attachment.connector = ISCSI_CONNECTOR
+
+
 def get_fake_net_interface_get_iter_response():
     return etree.XML("""<results status="passed">
         <num-records>1</num-records>

cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py

@@ -260,6 +260,33 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
                           'fake_volume', fake.FC_FORMATTED_INITIATORS,
                           protocol, None)
 
+    def test__is_multiattached_true(self):
+        volume = copy.deepcopy(fake.test_volume)
+        volume.multiattach = True
+        volume.volume_attachment = [
+            fake.test_iscsi_attachment,
+            fake.test_iscsi_attachment,
+        ]
+
+        self.assertTrue(self.library._is_multiattached(
+            volume,
+            fake.ISCSI_CONNECTOR))
+
+    def test__is_multiattached_false(self):
+        volume1 = copy.deepcopy(fake.test_volume)
+        volume1.multiattach = True
+        volume1.volume_attachment = []
+        volume2 = copy.deepcopy(fake.test_volume)
+        volume2.multiattach = False
+        volume2.volume_attachment = []
+
+        self.assertFalse(self.library._is_multiattached(
+            volume1,
+            fake.ISCSI_CONNECTOR))
+        self.assertFalse(self.library._is_multiattached(
+            volume2,
+            fake.ISCSI_CONNECTOR))
+
     @mock.patch.object(block_base.NetAppBlockStorageLibrary,
                        '_find_mapped_lun_igroup')
     def test_unmap_lun_empty(self, mock_find_mapped_lun_igroup):
@@ -302,6 +329,50 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
                            fake.ISCSI_MULTI_MAP_LIST[1]['initiator-group'])]
         self.zapi_client.unmap_lun.assert_has_calls(calls)
 
+    @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_unmap_lun')
+    def test_terminate_connection_iscsi_multiattach(self, mock_unmap_lun):
+        volume = copy.deepcopy(fake.test_volume)
+        volume.multiattach = True
+        volume.volume_attachment = [
+            fake.test_iscsi_attachment,
+            fake.test_iscsi_attachment,
+        ]
+
+        self.library.terminate_connection_iscsi(volume, fake.ISCSI_CONNECTOR)
+
+        mock_unmap_lun.assert_not_called()
+
+    @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_unmap_lun')
+    @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_get_lun_attr')
+    def test_terminate_connection_iscsi_last_attachment(self,
+                                                        mock_get_lun_attr,
+                                                        mock_unmap_lun):
+        mock_get_lun_attr.return_value = {'Path': fake.PATH}
+        volume = copy.deepcopy(fake.test_volume)
+        volume.multiattach = True
+        volume.volume_attachment = [fake.test_iscsi_attachment]
+
+        self.library.terminate_connection_iscsi(volume, fake.ISCSI_CONNECTOR)
+
+        mock_unmap_lun.assert_called_once_with(
+            fake.PATH, [fake.ISCSI_CONNECTOR['initiator']])
+
+    @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_unmap_lun')
+    @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_get_lun_attr')
+    def test_terminate_connection_iscsi_all_initiators(self, mock_get_lun_attr,
+                                                       mock_unmap_lun):
+        mock_get_lun_attr.return_value = {'Path': fake.PATH}
+        volume = copy.deepcopy(fake.test_volume)
+        volume.multiattach = True
+        volume.volume_attachment = [
+            fake.test_iscsi_attachment,
+            fake.test_iscsi_attachment,
+        ]
+
+        self.library.terminate_connection_iscsi(volume, None)
+
+        mock_unmap_lun.assert_called_once_with(fake.PATH, [])
+
     def test_find_mapped_lun_igroup(self):
         self.assertRaises(NotImplementedError,
                           self.library._find_mapped_lun_igroup,

cinder/volume/drivers/netapp/dataontap/block_base.py

@@ -888,12 +888,30 @@ class NetAppBlockStorageLibrary(object):
             targets = target_details_list
         return targets
 
+    def _is_multiattached(self, volume, connector):
+        """Returns whether the volume is multiattached.
+
+        Returns True if the volume is attached to multiple instances using the
+        same initiator as the given one.
+        Returns False otherwise.
+        """
+        if not volume.multiattach or not volume.volume_attachment:
+            return False
+
+        same_connector = (True for at in volume.volume_attachment
+                          if at.connector and
+                          at.connector['initiator'] == connector['initiator'])
+        next(same_connector, False)
+        return next(same_connector, False)
+
     def terminate_connection_iscsi(self, volume, connector, **kwargs):
         """Driver entry point to unattach a volume from an instance.
 
         Unmask the LUN on the storage system so the given initiator can no
         longer access it.
         """
+        if connector and self._is_multiattached(volume, connector):
+            return
 
         name = volume['name']
         if connector is None:

releasenotes/notes/netapp-ontap-fix-detach-multiattach-d99d33dff2fefb4c.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    NetApp ONTAP: Fixes `bug 1839384
+    <https://bugs.launchpad.net/cinder/+bug/1839384>`__ Detaching any instance
+    from multiattached volume terminates connection. Now the connection is
+    terminated only if there're no other instances using the same initiator.