From b67e2de51521f1836003bdbbad9a5421251e530e Mon Sep 17 00:00:00 2001 From: Kumar Prashant Date: Mon, 17 Sep 2018 23:19:19 +0530 Subject: [PATCH] VMAX Driver - Rollback for manage existing volume Implements rollback for manage existing volume for the scenario when the manage operation fails after the volume has been already renamed on the array Change-Id: I40f861819d8ce43a128d411ff58a1a703d8b1e0b Closes-bug: 1792884 --- .../volume/drivers/dell_emc/vmax/test_vmax.py | 9 ++++-- cinder/volume/drivers/dell_emc/vmax/common.py | 30 +++++++++++++++---- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py b/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py index e4a7214ea9e..385bddd4f44 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py @@ -4877,7 +4877,8 @@ class VMAXCommonTest(test.TestCase): provider_location = {'device_id': u'00002', 'array': u'000197800123'} ref_update = {'provider_location': six.text_type(provider_location)} with mock.patch.object( - self.common, '_check_lun_valid_for_cinder_management'): + self.common, '_check_lun_valid_for_cinder_management', + return_value=('vol1')): model_update = self.common.manage_existing( self.data.test_volume, external_ref) self.assertEqual(ref_update, model_update) @@ -4890,9 +4891,10 @@ class VMAXCommonTest(test.TestCase): return_value=(False, False, None)) def test_check_lun_valid_for_cinder_management(self, mock_rep, mock_mv): external_ref = {u'source-name': u'00003'} - self.common._check_lun_valid_for_cinder_management( + vol_name = self.common._check_lun_valid_for_cinder_management( self.data.array, self.data.device_id3, self.data.test_volume.id, external_ref) + self.assertEqual(vol_name, '123') @mock.patch.object( rest.VMAXRest, 'get_volume', @@ -7648,7 +7650,8 @@ class VMAXCommonReplicationTest(test.TestCase): self.data.test_volume.id) provider_location = {'device_id': u'00002', 'array': self.data.array} with mock.patch.object( - self.common, '_check_lun_valid_for_cinder_management'): + self.common, '_check_lun_valid_for_cinder_management', + return_value=(volume_name)): self.common.manage_existing( self.data.test_volume, external_ref) mock_rep.assert_called_once_with( diff --git a/cinder/volume/drivers/dell_emc/vmax/common.py b/cinder/volume/drivers/dell_emc/vmax/common.py index 2b6b0e88812..4f0bea5f6e8 100644 --- a/cinder/volume/drivers/dell_emc/vmax/common.py +++ b/cinder/volume/drivers/dell_emc/vmax/common.py @@ -1989,14 +1989,17 @@ class VMAXCommon(object): volume, external_ref) volume_id = volume.id # Check if the existing volume is valid for cinder management - self._check_lun_valid_for_cinder_management( + orig_vol_name = self._check_lun_valid_for_cinder_management( array, device_id, volume_id, external_ref) + # If volume name is not present, then assign the device id as the name + if not orig_vol_name: + orig_vol_name = device_id extra_specs = self._initial_setup(volume) volume_name = self.utils.get_volume_element_name(volume_id) # Rename the volume LOG.debug("Rename volume %(vol)s to %(element_name)s.", - {'vol': volume_id, + {'vol': orig_vol_name, 'element_name': volume_name}) self.rest.rename_volume(array, device_id, volume_name) provider_location = {'device_id': device_id, 'array': array} @@ -2010,9 +2013,22 @@ class VMAXCommon(object): model_update.update(rep_update) else: - # Add volume to default storage group - self.masking.add_volume_to_default_storage_group( - array, device_id, volume_name, extra_specs) + try: + # Add volume to default storage group + self.masking.add_volume_to_default_storage_group( + array, device_id, volume_name, extra_specs) + except Exception as e: + exception_message = (_( + "Unable to move the volume to the default SG. " + "Exception received was %(e)s") % {'e': six.text_type(e)}) + LOG.error(exception_message) + # Try to rename the volume back to the original name + LOG.debug("Rename volume %(vol)s back to %(element_name)s.", + {'vol': volume_id, + 'element_name': orig_vol_name}) + self.rest.rename_volume(array, device_id, orig_vol_name) + raise exception.VolumeBackendAPIException( + message=exception_message) self.volume_metadata.capture_manage_existing( volume, rep_info_dict, device_id, extra_specs) @@ -2027,6 +2043,7 @@ class VMAXCommon(object): :param device_id: the device id :param volume_id: the cinder volume id :param external_ref: the external reference + :returns volume_identifier - name of the volume on VMAX :raises: ManageExistingInvalidReference, ManageExistingAlreadyManaged: """ # Ensure the volume exists on the array @@ -2036,7 +2053,7 @@ class VMAXCommon(object): 'device %(device_id)s') % {'device_id': device_id}) raise exception.ManageExistingInvalidReference( existing_ref=external_ref, reason=msg) - + volume_identifier = None # Check if volume is already cinder managed if volume_details.get('volume_identifier'): volume_identifier = volume_details['volume_identifier'] @@ -2067,6 +2084,7 @@ class VMAXCommon(object): % {'device_id': device_id}) raise exception.ManageExistingInvalidReference( existing_ref=external_ref, reason=msg) + return volume_identifier def manage_existing_get_size(self, volume, external_ref): """Return size of an existing VMAX volume to manage_existing.