From 767bdf1a5e28f18c12dba03adc429766f9be79d6 Mon Sep 17 00:00:00 2001 From: rajinir Date: Thu, 2 May 2019 12:43:40 -0500 Subject: [PATCH] Dell EMC SC: Handle the mappings of multiattached volume For volumes attached to multiple instances, this handles the deletion of mappings in the backend when the volume is attached to multiple instances on the same host. Change-Id: I33d691d1dfa835b70726bcfacf868faab52de16a Closes-Bug: #1822229 Co-Authored-By: Sam Wan (cherry picked from commit 08d35b099113679eb7d343f7463d63cb6f1adbbc) --- .../volume/drivers/dell_emc/sc/test_fc.py | 101 ++++++++++++++++++ .../volume/drivers/dell_emc/sc/test_sc.py | 72 +++++++++++++ .../dell_emc/sc/storagecenter_common.py | 23 ++++ .../drivers/dell_emc/sc/storagecenter_fc.py | 17 ++- .../dell_emc/sc/storagecenter_iscsi.py | 22 +++- ...tiattach-onterminate-6ab1f96f21bb284d.yaml | 7 ++ 6 files changed, 237 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/sc-handle-multiattach-onterminate-6ab1f96f21bb284d.yaml diff --git a/cinder/tests/unit/volume/drivers/dell_emc/sc/test_fc.py b/cinder/tests/unit/volume/drivers/dell_emc/sc/test_fc.py index b0f63a8b9f4..a31eb5959d7 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/sc/test_fc.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/sc/test_fc.py @@ -18,6 +18,7 @@ from cinder import context from cinder import exception from cinder import test from cinder.tests.unit import fake_constants as fake +from cinder.tests.unit import fake_volume from cinder.volume.drivers.dell_emc.sc import storagecenter_api from cinder.volume.drivers.dell_emc.sc import storagecenter_fc @@ -939,6 +940,106 @@ class DellSCSanFCDriverTestCase(test.TestCase): 'driver_volume_type': 'fibre_channel'} self.assertEqual(expected, res, 'Unexpected return data') + @mock.patch.object(storagecenter_api.SCApi, + 'find_server', + return_value=SCSERVER) + @mock.patch.object(storagecenter_api.SCApi, + 'find_volume', + return_value=VOLUME) + @mock.patch.object(storagecenter_api.SCApi, + 'unmap_volume', + return_value=True) + @mock.patch.object(storagecenter_api.SCApi, + 'find_wwns', + return_value=(1, + [u'5000D31000FCBE3D', + u'5000D31000FCBE35'], + {u'21000024FF30441C': + [u'5000D31000FCBE35'], + u'21000024FF30441D': + [u'5000D31000FCBE3D']})) + @mock.patch.object(storagecenter_api.SCApi, + 'get_volume_count', + return_value=1) + def test_terminate_connection_multiattached_host(self, + mock_get_volume_count, + mock_find_wwns, + mock_unmap_volume, + mock_find_volume, + mock_find_server, + mock_close_connection, + mock_open_connection, + mock_init): + connector = self.connector + + attachment1 = fake_volume.volume_attachment_ovo(self._context) + attachment1.connector = connector + attachment1.attached_host = connector['host'] + attachment1.attach_status = 'attached' + + attachment2 = fake_volume.volume_attachment_ovo(self._context) + attachment2.connector = connector + attachment2.attached_host = connector['host'] + attachment2.attach_status = 'attached' + + vol = fake_volume.fake_volume_obj(self._context) + vol.multiattach = True + vol.volume_attachment.objects.append(attachment1) + vol.volume_attachment.objects.append(attachment2) + + self.driver.terminate_connection(vol, connector) + mock_unmap_volume.assert_not_called() + + @mock.patch.object(storagecenter_api.SCApi, + 'find_server', + return_value=SCSERVER) + @mock.patch.object(storagecenter_api.SCApi, + 'find_volume', + return_value=VOLUME) + @mock.patch.object(storagecenter_api.SCApi, + 'unmap_volume', + return_value=True) + @mock.patch.object(storagecenter_api.SCApi, + 'find_wwns', + return_value=(1, + [u'5000D31000FCBE3D', + u'5000D31000FCBE35'], + {u'21000024FF30441C': + [u'5000D31000FCBE35'], + u'21000024FF30441D': + [u'5000D31000FCBE3D']})) + @mock.patch.object(storagecenter_api.SCApi, + 'get_volume_count', + return_value=1) + def test_terminate_connection_multiattached_diffhost(self, + mock_get_volume_count, + mock_find_wwns, + mock_unmap_volume, + mock_find_volume, + mock_find_server, + mock_close_connection, + mock_open_connection, + mock_init): + connector = self.connector + + attachment1 = fake_volume.volume_attachment_ovo(self._context) + attachment1.connector = connector + attachment1.attached_host = connector['host'] + attachment1.attach_status = 'attached' + + attachment2 = fake_volume.volume_attachment_ovo(self._context) + attachment2.connector = connector + attachment2.attached_host = 'host2' + attachment2.attach_status = 'attached' + + vol = fake_volume.fake_volume_obj(self._context) + vol.multiattach = True + vol.volume_attachment.objects.append(attachment1) + vol.volume_attachment.objects.append(attachment2) + + self.driver.terminate_connection(vol, connector) + mock_unmap_volume.assert_called_once_with(self.VOLUME, self.SCSERVER) + def test_terminate_secondary(self, mock_close_connection, mock_open_connection, diff --git a/cinder/tests/unit/volume/drivers/dell_emc/sc/test_sc.py b/cinder/tests/unit/volume/drivers/dell_emc/sc/test_sc.py index e12166fd027..62663e98041 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/sc/test_sc.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/sc/test_sc.py @@ -1543,6 +1543,78 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): volume, connector) + @mock.patch.object(storagecenter_api.SCApi, + 'find_server', + return_value=SCSERVER) + @mock.patch.object(storagecenter_api.SCApi, + 'find_volume', + return_value=VOLUME) + @mock.patch.object(storagecenter_api.SCApi, + 'unmap_volume', + return_value=True) + def test_terminate_connection_multiattached_host(self, + mock_unmap_volume, + mock_find_volume, + mock_find_server, + mock_close_connection, + mock_open_connection, + mock_init): + connector = self.connector + + attachment1 = fake_volume.volume_attachment_ovo(self._context) + attachment1.connector = connector + attachment1.attached_host = connector['host'] + attachment1.attach_status = 'attached' + + attachment2 = fake_volume.volume_attachment_ovo(self._context) + attachment2.connector = connector + attachment2.attached_host = connector['host'] + attachment2.attach_status = 'attached' + + vol = fake_volume.fake_volume_obj(self._context) + vol.multiattach = True + vol.volume_attachment.objects.append(attachment1) + vol.volume_attachment.objects.append(attachment2) + + self.driver.terminate_connection(vol, connector) + mock_unmap_volume.assert_not_called() + + @mock.patch.object(storagecenter_api.SCApi, + 'find_server', + return_value=SCSERVER) + @mock.patch.object(storagecenter_api.SCApi, + 'find_volume', + return_value=VOLUME) + @mock.patch.object(storagecenter_api.SCApi, + 'unmap_volume', + return_value=True) + def test_terminate_connection_multiattached_diffhost(self, + mock_unmap_volume, + mock_find_volume, + mock_find_server, + mock_close_connection, + mock_open_connection, + mock_init): + connector = self.connector + + attachment1 = fake_volume.volume_attachment_ovo(self._context) + attachment1.connector = connector + attachment1.attached_host = connector['host'] + attachment1.attach_status = 'attached' + + attachment2 = fake_volume.volume_attachment_ovo(self._context) + attachment2.connector = connector + attachment2.attached_host = 'host2' + attachment2.attach_status = 'attached' + + vol = fake_volume.fake_volume_obj(self._context) + vol.multiattach = True + vol.volume_attachment.objects.append(attachment1) + vol.volume_attachment.objects.append(attachment2) + + self.driver.terminate_connection(vol, connector) + mock_unmap_volume.assert_called_once_with(self.VOLUME, self.SCSERVER) + def _simple_volume(self, **kwargs): updates = {'display_name': fake.VOLUME_NAME, 'id': fake.VOLUME_ID, diff --git a/cinder/volume/drivers/dell_emc/sc/storagecenter_common.py b/cinder/volume/drivers/dell_emc/sc/storagecenter_common.py index e55dba541ab..b3a57ecd0ea 100644 --- a/cinder/volume/drivers/dell_emc/sc/storagecenter_common.py +++ b/cinder/volume/drivers/dell_emc/sc/storagecenter_common.py @@ -2021,3 +2021,26 @@ class SCCommonDriver(driver.ManageableVD, raise exception.Invalid(reason=msg) return True + + def is_multiattach_to_host(self, volume_attachment, host_name): + # When multiattach is enabled, a volume could be attached to two or + # more instances which are hosted on one nova host. + # Because the backend cannot recognize the volume is attached to two + # or more instances, we should keep the volume attached to the nova + # host until the volume is detached from the last instance. + LOG.info('is_multiattach_to_host: volume_attachment %s.', + volume_attachment) + LOG.info('is_multiattach_to_host: host_name %s.', host_name) + if not volume_attachment: + return False + + for a in volume_attachment: + LOG.debug('attachment %s.', a) + + attachment = [a for a in volume_attachment + if a['attach_status'] == + fields.VolumeAttachStatus.ATTACHED + and a['attached_host'] == host_name] + LOG.info('is_multiattach_to_host: attachment %s.', attachment) + + return len(attachment) > 1 diff --git a/cinder/volume/drivers/dell_emc/sc/storagecenter_fc.py b/cinder/volume/drivers/dell_emc/sc/storagecenter_fc.py index ad9f1f84b8d..c6b583abace 100644 --- a/cinder/volume/drivers/dell_emc/sc/storagecenter_fc.py +++ b/cinder/volume/drivers/dell_emc/sc/storagecenter_fc.py @@ -20,6 +20,7 @@ from oslo_utils import excutils from cinder import exception from cinder.i18n import _ from cinder import interface +from cinder import utils from cinder.volume import driver from cinder.volume.drivers.dell_emc.sc import storagecenter_common from cinder.zonemanager import utils as fczm_utils @@ -239,6 +240,7 @@ class SCFCDriver(storagecenter_common.SCCommonDriver, 'data': {}} return info + @utils.synchronized('{self.driver_prefix}-{volume.id}') def terminate_connection(self, volume, connector, force=False, **kwargs): # Special case if connector is None: @@ -248,6 +250,20 @@ class SCFCDriver(storagecenter_common.SCCommonDriver, volume_name = volume.get('id') provider_id = volume.get('provider_id') LOG.debug('Terminate connection: %s', volume_name) + LOG.debug('Volume details %s', volume) + + # None `connector` indicates force detach, then detach all even if the + # volume is multi-attached. + is_multiattached = (hasattr(volume, 'volume_attachment') and + self.is_multiattach_to_host( + volume.get('volume_attachment'), + connector['host'])) + if is_multiattached: + LOG.info('Cannot terminate connection: ' + '%(vol)s is multiattached.', + {'vol': volume_name}) + + return True with self._client.open_connection() as api: try: @@ -291,7 +307,6 @@ class SCFCDriver(storagecenter_common.SCCommonDriver, raise exception.VolumeBackendAPIException( data=_('Terminate connection failed')) - # basic return info... info = {'driver_volume_type': 'fibre_channel', 'data': {}} diff --git a/cinder/volume/drivers/dell_emc/sc/storagecenter_iscsi.py b/cinder/volume/drivers/dell_emc/sc/storagecenter_iscsi.py index 3ac4ea05aa2..b66b19896c1 100644 --- a/cinder/volume/drivers/dell_emc/sc/storagecenter_iscsi.py +++ b/cinder/volume/drivers/dell_emc/sc/storagecenter_iscsi.py @@ -20,6 +20,7 @@ from oslo_utils import excutils from cinder import exception from cinder.i18n import _ from cinder import interface +from cinder import utils from cinder.volume import driver from cinder.volume.drivers.dell_emc.sc import storagecenter_common @@ -250,8 +251,11 @@ class SCISCSIDriver(storagecenter_common.SCCommonDriver, raise exception.VolumeBackendAPIException( _('Terminate connection failed')) + @utils.synchronized('{self.driver_prefix}-{volume.id}') def terminate_connection(self, volume, connector, force=False, **kwargs): # Special case + # None `connector` indicates force detach, then detach all even if the + # volume is multi-attached. if connector is None: return self.force_detach(volume) @@ -260,9 +264,18 @@ class SCISCSIDriver(storagecenter_common.SCCommonDriver, volume_name = volume.get('id') provider_id = volume.get('provider_id') initiator_name = None if not connector else connector.get('initiator') - LOG.debug('Terminate connection: %(vol)s:%(initiator)s', - {'vol': volume_name, - 'initiator': initiator_name}) + LOG.info('Volume in terminate connection: %(vol)s', + {'vol': volume}) + + is_multiattached = (hasattr(volume, 'volume_attachment') and + self.is_multiattach_to_host( + volume.get('volume_attachment'), + connector['host'])) + if is_multiattached: + LOG.info('Cannot terminate connection: ' + '%(vol)s is multiattached.', + {'vol': volume_name}) + return True with self._client.open_connection() as api: try: @@ -288,7 +301,7 @@ class SCISCSIDriver(storagecenter_common.SCCommonDriver, scserver = (None if not initiator_name else api.find_server(initiator_name, ssn)) - # If we have a server and a volume lets pull them apart. + # If we have a server and a volume lets pull them apart if ((scserver and api.unmap_volume(scvolume, scserver) is True) or (not scserver and api.unmap_all(scvolume))): @@ -308,6 +321,7 @@ class SCISCSIDriver(storagecenter_common.SCCommonDriver, rtn = True secondaryvol = api.get_volume( sclivevolume['secondaryVolume']['instanceId']) + if secondaryvol: if initiatorname: # Find our server. diff --git a/releasenotes/notes/sc-handle-multiattach-onterminate-6ab1f96f21bb284d.yaml b/releasenotes/notes/sc-handle-multiattach-onterminate-6ab1f96f21bb284d.yaml new file mode 100644 index 00000000000..973815970c3 --- /dev/null +++ b/releasenotes/notes/sc-handle-multiattach-onterminate-6ab1f96f21bb284d.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Dell EMC SC Driver: Fixes `bug 1822229 + `__ + to handle the volume mappings in the backend when a volume + is attached to multiple instances on the same host.