Dell EMC PS: Fix Duplicate ACL records Issue

Live migration was creating duplicate ACL records for the compute
nodes. Fixed it by not creating one when an ACL exists during
initialization of the volume.

Closes Bug: #1726591

Change-Id: Ib78293efa626c098a572f4c64119ee2ff296bd40
(cherry picked from commit 22c09d5768)

cinder/tests/unit/volume/drivers/dell_emc/test_ps.py

@@ -76,7 +76,7 @@ class PSSeriesISCSIDriverTestCase(test.TestCase):
             "--- --------------- ------------- ---------- ---------- --------",
             "1   iqn.1993-08.org.debian:01:222 *.*.*.*       none        both",
             "       7dab76162"]
-
+        self.fake_access_id = '1'
         self.fake_iqn = 'iqn.2003-10.com.equallogic:group01:25366:fakev'
         self.fake_iqn_return = ['iSCSI target name is %s.' % self.fake_iqn]
         self.fake_volume_output = ["Size: 5GB",
@@ -345,6 +345,24 @@ class PSSeriesISCSIDriverTestCase(test.TestCase):
             mock_eql_execute.side_effect = my_side_effect
             self.driver.terminate_connection(volume, self.connector)
 
+    def test_get_access_record(self):
+        attrs = ('volume', 'select', self.volume['name'], 'access', 'show')
+        with mock.patch.object(self.driver,
+                               '_eql_execute') as mock_eql_execute:
+            mock_eql_execute.return_value = self.access_record_output
+            data = self.driver._get_access_record(self.volume, self.connector)
+            mock_eql_execute.assert_called_with(*attrs)
+            self.assertEqual(self.fake_access_id, data)
+
+    def test_get_access_record_negative(self):
+        attrs = ('volume', 'select', self.volume['name'], 'access', 'show')
+        with mock.patch.object(self.driver,
+                               '_eql_execute') as mock_eql_execute:
+            mock_eql_execute.return_value = []
+            data = self.driver._get_access_record(self.volume, self.connector)
+            mock_eql_execute.assert_called_with(*attrs)
+            self.assertIsNone(data)
+
     def test_do_setup(self):
         fake_group_ip = '10.1.2.3'
 

cinder/volume/drivers/dell_emc/ps.py

@@ -368,6 +368,17 @@ class PSSeriesISCSIDriver(san.SanISCSIDriver):
                               volume['name'])
                     raise exception.VolumeNotFound(volume_id=volume['id'])
 
+    def _get_access_record(self, volume, connector):
+        """Returns access record id for the initiator"""
+        try:
+            out = self._eql_execute('volume', 'select', volume['name'],
+                                    'access', 'show')
+            return self._parse_connection(connector, out)
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                LOG.error(_LE('Failed to get access records '
+                          'to volume "%s".'), volume['name'])
+
     def _parse_connection(self, connector, out):
         """Returns the correct connection id for the initiator.
 
@@ -539,12 +550,15 @@ class PSSeriesISCSIDriver(san.SanISCSIDriver):
     def initialize_connection(self, volume, connector):
         """Restrict access to a volume."""
         try:
-            cmd = ['volume', 'select', volume['name'], 'access', 'create',
-                   'initiator', connector['initiator']]
-            if self.configuration.use_chap_auth:
-                cmd.extend(['authmethod', 'chap', 'username',
-                            self.configuration.chap_username])
-            self._eql_execute(*cmd)
+            connection_id = self._get_access_record(volume, connector)
+            if connection_id is None:
+                cmd = ['volume', 'select', volume['name'], 'access', 'create',
+                       'initiator', connector['initiator']]
+                if self.configuration.use_chap_auth:
+                    cmd.extend(['authmethod', 'chap', 'username',
+                                self.configuration.chap_username])
+                self._eql_execute(*cmd)
+
             iscsi_properties = self._get_iscsi_properties(volume)
             return {
                 'driver_volume_type': 'iscsi',
@@ -559,9 +573,7 @@ class PSSeriesISCSIDriver(san.SanISCSIDriver):
     def terminate_connection(self, volume, connector, force=False, **kwargs):
         """Remove access restrictions from a volume."""
         try:
-            out = self._eql_execute('volume', 'select', volume['name'],
-                                    'access', 'show')
-            connection_id = self._parse_connection(connector, out)
+            connection_id = self._get_access_record(volume, connector)
             if connection_id is not None:
                 self._eql_execute('volume', 'select', volume['name'],
                                   'access', 'delete', connection_id)

releasenotes/notes/ps-duplicate-ACL-5aa447c50f2474e7.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - Dell EMC PS Series Driver code was creating duplicate ACL records during
+    live migration. Fixes the initialize_connection code to not create access
+    record for a host if one exists previously. This change fixes bug 1726591.