Fix Infinidat driver multi-attach feature

Added a check if there are multiple attachments to the volume
from the same connector and terminate connection only for the
last attachment from the corresponding host.

Closes-bug: #1982350
Change-Id: Ibda3a09a181160b8ee9129795429a7f1795e907d
Signed-off-by: Alexander Deiter <adeiter@infinidat.com>

cinder/tests/unit/volume/drivers/test_infinidat.py

@@ -39,6 +39,7 @@ TEST_IP_ADDRESS2 = '2.2.2.2'
 TEST_IP_ADDRESS3 = '3.3.3.3'
 TEST_IP_ADDRESS4 = '4.4.4.4'
 TEST_INITIATOR_IQN = 'iqn.2012-07.org.initiator:01'
+TEST_INITIATOR_IQN2 = 'iqn.2012-07.org.initiator:02'
 TEST_TARGET_IQN = 'iqn.2012-07.org.target:01'
 TEST_ISCSI_TCP_PORT1 = 3261
 TEST_ISCSI_TCP_PORT2 = 3262
@@ -58,16 +59,25 @@ TEST_SNAPSHOT_SOURCE_ID = 67890
 TEST_SNAPSHOT_METADATA = {'cinder_id': fake.SNAPSHOT_ID}
 
 test_volume = mock.Mock(id=fake.VOLUME_ID, name_id=fake.VOLUME_ID, size=1,
-                        volume_type_id=fake.VOLUME_TYPE_ID)
+                        volume_type_id=fake.VOLUME_TYPE_ID,
+                        multiattach=False, volume_attachment=None)
 test_volume2 = mock.Mock(id=fake.VOLUME2_ID, name_id=fake.VOLUME2_ID, size=1,
-                         volume_type_id=fake.VOLUME_TYPE_ID)
+                         volume_type_id=fake.VOLUME_TYPE_ID,
+                         multiattach=False, volume_attachment=None)
 test_snapshot = mock.Mock(id=fake.SNAPSHOT_ID, volume=test_volume,
                           volume_id=test_volume.id)
-test_clone = mock.Mock(id=fake.VOLUME4_ID, size=1)
+test_clone = mock.Mock(id=fake.VOLUME4_ID, size=1, multiattach=False,
+                       volume_attachment=None)
 test_group = mock.Mock(id=fake.GROUP_ID)
 test_snapgroup = mock.Mock(id=fake.GROUP_SNAPSHOT_ID, group=test_group)
 test_connector = dict(wwpns=[TEST_WWN_1],
                       initiator=TEST_INITIATOR_IQN)
+test_connector2 = dict(wwpns=[TEST_WWN_2],
+                       initiator=TEST_INITIATOR_IQN2)
+test_connector3 = dict(wwpns=None, initiator=None)
+test_attachment1 = mock.Mock(connector=test_connector)
+test_attachment2 = mock.Mock(connector=test_connector2)
+test_attachment3 = mock.Mock(connector=None)
 
 
 def skip_driver_setup(func):
@@ -261,15 +271,41 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
         self.driver.initialize_connection(test_volume, test_connector)
         self._validate_host_metadata()
 
+    @ddt.data({'connector': None, 'multiattach': True,
+               'attachment': [test_attachment1, test_attachment1]},
+              {'connector': test_connector3, 'multiattach': True,
+               'attachment': [test_attachment1, test_attachment1]},
+              {'connector': test_connector, 'multiattach': False,
+               'attachment': [test_attachment1]},
+              {'connector': test_connector, 'multiattach': True,
+               'attachment': None},
+              {'connector': test_connector, 'multiattach': True,
+               'attachment': [test_attachment2, test_attachment3]})
+    @ddt.unpack
+    def test__is_volume_multiattached_negative(self, connector,
+                                               multiattach, attachment):
+        volume = copy.deepcopy(test_volume)
+        volume.multiattach = multiattach
+        volume.volume_attachment = attachment
+        self.assertFalse(self.driver._is_volume_multiattached(volume,
+                                                              connector))
+
     def test_terminate_connection(self):
-        self.driver.terminate_connection(test_volume, test_connector)
+        volume = copy.deepcopy(test_volume)
+        volume.volume_attachment = [test_attachment1]
+        self.assertFalse(self.driver.terminate_connection(volume,
+                                                          test_connector))
 
     def test_terminate_connection_delete_host(self):
         self._mock_host.get_luns.return_value = [object()]
-        self.driver.terminate_connection(test_volume, test_connector)
+        volume = copy.deepcopy(test_volume)
+        volume.volume_attachment = [test_attachment1]
+        self.assertFalse(self.driver.terminate_connection(volume,
+                                                          test_connector))
         self.assertEqual(0, self._mock_host.safe_delete.call_count)
         self._mock_host.get_luns.return_value = []
-        self.driver.terminate_connection(test_volume, test_connector)
+        self.assertFalse(self.driver.terminate_connection(volume,
+                                                          test_connector))
         self.assertEqual(1, self._mock_host.safe_delete.call_count)
 
     def test_terminate_connection_volume_doesnt_exist(self):
@@ -617,8 +653,10 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
         mock_mapping = mock.Mock()
         mock_mapping.get_host.return_value = mock_infinidat_host
         self._mock_volume.get_logical_units.return_value = [mock_mapping]
+        volume = copy.deepcopy(test_volume)
+        volume.volume_attachment = [test_attachment1, test_attachment2]
         # connector is None - force detach - detach all mappings
-        self.driver.terminate_connection(test_volume, None)
+        self.assertTrue(self.driver.terminate_connection(volume, None))
         # make sure we actually detached the host mapping
         self._mock_host.unmap_volume.assert_called_once()
         self._mock_host.safe_delete.assert_called_once()
@@ -861,6 +899,27 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
     def test_unmanage_snapshot(self):
         self.driver.unmanage_snapshot(test_snapshot)
 
+    def test_terminate_connection_no_attachment_connector(self):
+        volume = copy.deepcopy(test_volume)
+        volume.multiattach = True
+        volume.volume_attachment = [test_attachment3]
+        self.assertFalse(self.driver.terminate_connection(volume,
+                                                          test_connector))
+
+    def test_terminate_connection_no_host(self):
+        self._system.hosts.safe_get.return_value = None
+        volume = copy.deepcopy(test_volume)
+        volume.volume_attachment = [test_attachment1]
+        self.assertFalse(self.driver.terminate_connection(volume,
+                                                          test_connector))
+
+    def test_terminate_connection_no_mapping(self):
+        self._mock_host.unmap_volume.side_effect = KeyError
+        volume = copy.deepcopy(test_volume)
+        volume.volume_attachment = [test_attachment1]
+        self.assertFalse(self.driver.terminate_connection(volume,
+                                                          test_connector))
+
     def test_update_migrated_volume_new_volume_not_found(self):
         self._system.volumes.safe_get.side_effect = [
             None, self._mock_volume]
@@ -918,6 +977,7 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
         self.assertEqual(0, self._log.error.call_count)
 
 
+@ddt.ddt
 class InfiniboxDriverTestCaseFC(InfiniboxDriverTestCaseBase):
     def test_initialize_connection_multiple_wwpns(self):
         connector = {'wwpns': [TEST_WWN_1, TEST_WWN_2]}
@@ -931,7 +991,27 @@ class InfiniboxDriverTestCaseFC(InfiniboxDriverTestCaseBase):
         self.assertRaises(exception.InvalidConnectorException,
                           self.driver.validate_connector, iscsi_connector)
 
+    @ddt.data({'connector': test_connector,
+               'attachment': [test_attachment1, test_attachment1]},
+              {'connector': test_connector2,
+               'attachment': [test_attachment2, test_attachment2]})
+    @ddt.unpack
+    def test__is_volume_multiattached_positive(self, connector, attachment):
+        volume = copy.deepcopy(test_volume)
+        volume.multiattach = True
+        volume.volume_attachment = attachment
+        self.assertTrue(self.driver._is_volume_multiattached(volume,
+                                                             connector))
 
+    def test_terminate_connection_multiattached_volume(self):
+        volume = copy.deepcopy(test_volume)
+        volume.multiattach = True
+        volume.volume_attachment = [test_attachment1, test_attachment1]
+        self.assertTrue(self.driver.terminate_connection(volume,
+                                                         test_connector))
+
+
+@ddt.ddt
 class InfiniboxDriverTestCaseISCSI(InfiniboxDriverTestCaseBase):
     def setUp(self):
         super(InfiniboxDriverTestCaseISCSI, self).setUp()
@@ -1114,8 +1194,23 @@ class InfiniboxDriverTestCaseISCSI(InfiniboxDriverTestCaseBase):
         }
         self.assertEqual(expected, result)
 
+    @ddt.data({'connector': test_connector,
+               'attachment': [test_attachment1, test_attachment1]},
+              {'connector': test_connector2,
+               'attachment': [test_attachment2, test_attachment2]})
+    @ddt.unpack
+    def test__is_volume_multiattached_positive(self, connector, attachment):
+        volume = copy.deepcopy(test_volume)
+        volume.multiattach = True
+        volume.volume_attachment = attachment
+        self.assertTrue(self.driver._is_volume_multiattached(volume,
+                                                             connector))
+
     def test_terminate_connection(self):
-        self.driver.terminate_connection(test_volume, test_connector)
+        volume = copy.deepcopy(test_volume)
+        volume.volume_attachment = [test_attachment1]
+        self.assertFalse(self.driver.terminate_connection(volume,
+                                                          test_connector))
 
     def test_validate_connector(self):
         fc_connector = {'wwpns': [TEST_WWN_1, TEST_WWN_2]}

cinder/volume/drivers/infinidat.py

@@ -126,10 +126,11 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
         1.9 - added manage/unmanage/manageable-list volume/snapshot
         1.10 - added support for TLS/SSL communication
         1.11 - fixed generic volume migration
+        1.12 - fixed volume multi-attach
 
     """
 
-    VERSION = '1.11'
+    VERSION = '1.12'
 
     # ThirdPartySystems wiki page
     CI_WIKI_NAME = "INFINIDAT_CI"
@@ -478,6 +479,32 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
             ports = [iqn.IQN(connector['initiator'])]
         return ports
 
+    def _is_volume_multiattached(self, volume, connector):
+        """Returns whether the volume is multiattached.
+
+        Check if there are multiple attachments to the volume
+        from the same connector. Terminate connection only for
+        the last attachment from the corresponding host.
+        """
+        if not (connector and volume.multiattach and
+                volume.volume_attachment):
+            return False
+        keys = ['system uuid']
+        if self._protocol == constants.FC:
+            keys.append('wwpns')
+        else:
+            keys.append('initiator')
+        for key in keys:
+            if not (key in connector and connector[key]):
+                continue
+            if sum(1 for attachment in volume.volume_attachment if
+                   attachment.connector and key in attachment.connector and
+                   attachment.connector[key] == connector[key]) > 1:
+                LOG.debug('Volume %s is multiattached to %s %s',
+                          volume.name_id, key, connector[key])
+                return True
+        return False
+
     @infinisdk_to_cinder_exceptions
     @coordination.synchronized('infinidat-{self.management_address}-lock')
     def initialize_connection(self, volume, connector):
@@ -491,6 +518,8 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
     @coordination.synchronized('infinidat-{self.management_address}-lock')
     def terminate_connection(self, volume, connector, **kwargs):
         """Unmap an InfiniBox volume from the host"""
+        if self._is_volume_multiattached(volume, connector):
+            return True
         infinidat_volume = self._get_infinidat_volume(volume)
         if self._protocol == constants.FC:
             volume_type = 'fibre_channel'
@@ -521,11 +550,11 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
                                                          target_wwpns))
                     result_data = dict(target_wwn=target_wwpns,
                                        initiator_target_map=target_map)
-        conn_info = dict(driver_volume_type=volume_type,
-                         data=result_data)
         if self._protocol == constants.FC:
+            conn_info = dict(driver_volume_type=volume_type,
+                             data=result_data)
             fczm_utils.remove_fc_zone(conn_info)
-        return conn_info
+        return volume.volume_attachment and len(volume.volume_attachment) > 1
 
     @infinisdk_to_cinder_exceptions
     def get_volume_stats(self, refresh=False):
@@ -665,7 +694,8 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
         # we need a cinder-volume-like object to map the clone by name
         # (which is derived from the cinder id) but the clone is internal
         # so there is no such object. mock one
-        clone = mock.Mock(name_id=str(volume.name_id) + '-internal')
+        clone = mock.Mock(name_id=str(volume.name_id) + '-internal',
+                          multiattach=False, volume_attachment=[])
         try:
             infinidat_volume = self._create_volume(volume)
             try:

doc/source/configuration/block-storage/drivers/infinidat-volume-driver.rst

@@ -25,6 +25,7 @@ Supported operations
 * Revert a volume to a snapshot.
 * Manage and unmanage volumes and snapshots.
 * List manageable volumes and snapshots.
+* Attach a volume to multiple instances at once (multi-attach).
 
 External package installation
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

releasenotes/notes/bug-1982350-infinidat-fix-multi-attach-19f62d182b675e59.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Infinidat Driver `bug #1982350
+    <https://bugs.launchpad.net/cinder/+bug/1982350>`_:
+    Fixed Infinidat driver multi-attach feature.
+    Added a check if there are multiple attachments to the volume
+    from the same connector and terminate connection only for the
+    last attachment from the corresponding host.