From c96512aab0ee00201e26b0efa9c87c7f62fd463e Mon Sep 17 00:00:00 2001 From: rajinir Date: Wed, 25 Oct 2017 10:28:30 -0500 Subject: [PATCH] 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 22c09d57687b98faf4193cb1be3d738ddf3bbd28) --- .../unit/volume/drivers/dell_emc/test_ps.py | 20 ++++++++++++- cinder/volume/drivers/dell_emc/ps.py | 30 +++++++++++++------ .../ps-duplicate-ACL-5aa447c50f2474e7.yaml | 5 ++++ 3 files changed, 45 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/ps-duplicate-ACL-5aa447c50f2474e7.yaml diff --git a/cinder/tests/unit/volume/drivers/dell_emc/test_ps.py b/cinder/tests/unit/volume/drivers/dell_emc/test_ps.py index ba7ba77c3e9..aa324f4887d 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/test_ps.py +++ b/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' diff --git a/cinder/volume/drivers/dell_emc/ps.py b/cinder/volume/drivers/dell_emc/ps.py index a0d8a013e05..4af48225e9f 100644 --- a/cinder/volume/drivers/dell_emc/ps.py +++ b/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) diff --git a/releasenotes/notes/ps-duplicate-ACL-5aa447c50f2474e7.yaml b/releasenotes/notes/ps-duplicate-ACL-5aa447c50f2474e7.yaml new file mode 100644 index 00000000000..037af0c312e --- /dev/null +++ b/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.