From 09a40d4d8432d907c02eca4e9995812f20bf2ca1 Mon Sep 17 00:00:00 2001 From: Helen Walsh Date: Fri, 19 Aug 2016 18:34:28 +0100 Subject: [PATCH] VMAX driver - failed rollback on VMAX3 when MV issue On a failed masking view creation, the device was not being returned to the default storage group, stale storage groups were not being deleted and stale initiator groups were not being deleted. This patch fixes this issue. Change-Id: I96a2dac73792558850affee1287742bd3b7bfcf4 Closes-Bug: #1552426 --- .../unit/volume/drivers/emc/test_emc_vmax.py | 113 +++++++++++++- cinder/volume/drivers/emc/emc_vmax_masking.py | 142 ++++++++++++------ 2 files changed, 207 insertions(+), 48 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py b/cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py index 285a56eb517..facbbb76035 100644 --- a/cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py +++ b/cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py @@ -2324,14 +2324,17 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): rollbackDict['defaultStorageGroupInstanceName'] = ( self.data.default_storage_group) rollbackDict['sgName'] = self.data.storagegroupname + rollbackDict['sgGroupName'] = self.data.storagegroupname rollbackDict['volumeName'] = 'vol1' rollbackDict['fastPolicyName'] = 'GOLD1' rollbackDict['volumeInstance'] = vol rollbackDict['controllerConfigService'] = controllerConfigService rollbackDict['extraSpecs'] = extraSpecs + rollbackDict['igGroupName'] = self.data.initiatorgroup_name + rollbackDict['connector'] = self.data.connector # Path 1 - The volume is in another storage group that isn't the # default storage group - expectedmessage = (_("V2 rollback - Volume in another storage " + expectedmessage = (_("Rollback - Volume in another storage " "group besides default storage group.")) message = ( self.driver.common.masking. @@ -2340,6 +2343,7 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): self.assertEqual(expectedmessage, message) # Path 2 - The volume is not in any storage group rollbackDict['sgName'] = 'sq_not_exist' + rollbackDict['sgGroupName'] = 'sq_not_exist' expectedmessage = (_("V2 rollback, volume is not in any storage " "group.")) message = ( @@ -7904,6 +7908,113 @@ class EMCVMAXMaskingTest(test.TestCase): conn, controllerConfigService, storageGroupInstanceName, volumeInstance, extraSpecs) + # Bug 1552426 - failed rollback on V3 when MV issue + def test_check_ig_rollback(self): + # called on masking view rollback + masking = self.driver.common.masking + conn = self.fake_ecom_connection() + controllerConfigService = ( + self.driver.utils.find_controller_configuration_service( + conn, self.data.storage_system)) + connector = self.data.connector + extraSpecs = {'volume_backend_name': 'V3_BE', + 'isV3': True, + 'slo': 'Bronze', + 'pool': 'SRP_1', + } + igGroupName = self.data.initiatorgroup_name + host = igGroupName.split("-")[1] + igInstance = masking._find_initiator_masking_group( + conn, controllerConfigService, self.data.initiatorNames) + # path 1: The masking view creation process created a now stale + # initiator group before it failed. + with mock.patch.object(masking, + '_last_volume_delete_initiator_group'): + masking._check_ig_rollback(conn, controllerConfigService, + igGroupName, connector, extraSpecs) + (masking._last_volume_delete_initiator_group. + assert_called_once_with(conn, controllerConfigService, + igInstance, extraSpecs, host)) + # path 2: No initiator group was created before the masking + # view process failed. + with mock.patch.object(masking, + '_find_initiator_masking_group', + return_value=None): + masking._last_volume_delete_initiator_group.reset_mock() + masking._check_ig_rollback(conn, controllerConfigService, + igGroupName, connector, extraSpecs) + (masking._last_volume_delete_initiator_group. + assert_not_called()) + + @mock.patch.object( + emc_vmax_masking.EMCVMAXMasking, + 'get_associated_masking_groups_from_device', + return_value=EMCVMAXCommonData.storagegroups) + @mock.patch.object( + emc_vmax_masking.EMCVMAXMasking, + 'return_volume_to_default_storage_group_v3', + return_value='Returning volume to default sg') + def test_check_if_rollback_action_required_v3( + self, mock_return, mock_group): + conn = self.fake_ecom_connection() + masking = self.driver.common.masking + controllerConfigService = ( + self.driver.utils.find_controller_configuration_service( + conn, self.data.storage_system)) + extraSpecs_v3 = {'volume_backend_name': 'V3_BE', + 'isV3': True, + 'slo': 'Bronze', + 'pool': 'SRP_1', + 'connector': self.data.connector} + + vol = EMC_StorageVolume() + vol['name'] = self.data.test_volume['name'] + vol['CreationClassName'] = 'Symm_StorageVolume' + vol['ElementName'] = self.data.test_volume['id'] + vol['DeviceID'] = self.data.test_volume['device_id'] + vol['Id'] = self.data.test_volume['id'] + vol['SystemName'] = self.data.storage_system + vol['NumberOfBlocks'] = self.data.test_volume['NumberOfBlocks'] + vol['BlockSize'] = self.data.test_volume['BlockSize'] + + # Added vol to vol.path + vol['SystemCreationClassName'] = 'Symm_StorageSystem' + vol.path = vol + vol.path.classname = vol['CreationClassName'] + rollbackDict = {} + rollbackDict['isV3'] = True + rollbackDict['defaultStorageGroupInstanceName'] = ( + self.data.default_storage_group) + rollbackDict['sgGroupName'] = self.data.storagegroupname + rollbackDict['sgName'] = self.data.storagegroupname + rollbackDict['volumeName'] = 'vol1' + rollbackDict['slo'] = 'Bronze' + rollbackDict['volumeInstance'] = vol + rollbackDict['controllerConfigService'] = controllerConfigService + rollbackDict['extraSpecs'] = extraSpecs_v3 + rollbackDict['igGroupName'] = self.data.initiatorgroup_name + rollbackDict['connector'] = self.data.connector + # v3 Path 1 - The volume is in another storage group that isn't the + # default storage group + expectedmessage = (_("Rollback - Volume in another storage " + "group besides default storage group.")) + message = ( + masking. + _check_if_rollback_action_for_masking_required(conn, + rollbackDict)) + self.assertEqual(expectedmessage, message) + # v3 Path 2 - The volume is not in any storage group + rollbackDict['sgGroupName'] = 'sq_not_exist' + (rollbackDict + ['defaultStorageGroupInstanceName']) = (self.data. + default_sg_instance_name) + expectedmessage = (_("V3 rollback")) + message = ( + masking. + _check_if_rollback_action_for_masking_required(conn, + rollbackDict)) + self.assertEqual(expectedmessage, message) + class EMCVMAXFCTest(test.TestCase): def setUp(self): diff --git a/cinder/volume/drivers/emc/emc_vmax_masking.py b/cinder/volume/drivers/emc/emc_vmax_masking.py index f48fbb89eb4..4424933b8e9 100644 --- a/cinder/volume/drivers/emc/emc_vmax_masking.py +++ b/cinder/volume/drivers/emc/emc_vmax_masking.py @@ -88,7 +88,6 @@ class EMCVMAXMasking(object): maskingViewDict['extraSpecs'] = extraSpecs defaultStorageGroupInstanceName = None fastPolicyName = None - assocStorageGroupName = None storageGroupInstanceName = None if isLiveMigration is False: if isV3: @@ -153,7 +152,9 @@ class EMCVMAXMasking(object): rollbackDict['fastPolicyName'] = fastPolicyName rollbackDict['isV3'] = isV3 rollbackDict['extraSpecs'] = extraSpecs - rollbackDict['sgName'] = maskingViewDict['sgGroupName'] + rollbackDict['sgGroupName'] = maskingViewDict['sgGroupName'] + rollbackDict['igGroupName'] = maskingViewDict['igGroupName'] + rollbackDict['connector'] = maskingViewDict['connector'] if errorMessage: # Rollback code if we cannot complete any of the steps above @@ -165,11 +166,19 @@ class EMCVMAXMasking(object): self._check_if_rollback_action_for_masking_required( conn, rollbackDict) if isV3: - rollbackDict['sgGroupName'] = assocStorageGroupName - rollbackDict['storageSystemName'] = ( - maskingViewDict['storageSystemName']) - self._check_if_rollback_action_for_masking_required( - conn, rollbackDict) + if maskingViewDict['slo'] is not None: + rollbackDict['storageSystemName'] = ( + maskingViewDict['storageSystemName']) + rollbackDict['slo'] = maskingViewDict['slo'] + self._check_if_rollback_action_for_masking_required( + conn, rollbackDict) + + else: + errorMessage = self._check_adding_volume_to_storage_group( + conn, rollbackDict, + rollbackDict['defaultStorageGroupInstanceName']) + if errorMessage: + LOG.error(errorMessage) exceptionMessage = (_( "Failed to get, create or add volume %(volumeName)s " @@ -1271,7 +1280,8 @@ class EMCVMAXMasking(object): We need to be able to return the volume to the default storage group if anything has gone wrong. The volume can also potentially belong to a storage group that is not the default depending on where - the exception occurred. + the exception occurred. We also may need to clean up any unused + initiator groups. :param conn: the connection to the ecom server :param rollbackDict: the rollback dictionary @@ -1279,23 +1289,29 @@ class EMCVMAXMasking(object): :raises: VolumeBackendAPIException """ message = None + # Check if ig has been created. If so, check for other + # masking views associated with the ig. If none, remove + # initiators and delete ig. + self._check_ig_rollback( + conn, rollbackDict['controllerConfigService'], + rollbackDict['igGroupName'], rollbackDict['connector'], + rollbackDict['extraSpecs']) try: - if rollbackDict['isV3']: - errorMessage = self._check_adding_volume_to_storage_group( - conn, rollbackDict, - rollbackDict['defaultStorageGroupInstanceName']) - if errorMessage: - LOG.error(errorMessage) - message = (_("V3 rollback")) - - else: - foundStorageGroupInstanceName = ( - self.utils.get_storage_group_from_volume( - conn, rollbackDict['volumeInstance'].path, - rollbackDict['sgName'])) - # Volume is not associated with any storage group so add - # it back to the default. - if not foundStorageGroupInstanceName: + foundStorageGroupInstanceName = ( + self.utils.get_storage_group_from_volume( + conn, rollbackDict['volumeInstance'].path, + rollbackDict['sgGroupName'])) + # Volume is not associated with any storage group so add + # it back to the default. + if not foundStorageGroupInstanceName: + if rollbackDict['isV3']: + errorMessage = self._check_adding_volume_to_storage_group( + conn, rollbackDict, + rollbackDict['defaultStorageGroupInstanceName']) + if errorMessage: + LOG.error(errorMessage) + message = (_("V3 rollback")) + else: LOG.warning(_LW( "No storage group found. " "Performing rollback on Volume: %(volumeName)s " @@ -1323,35 +1339,35 @@ class EMCVMAXMasking(object): 'fastPolicyName': rollbackDict['fastPolicyName']}) message = (_("V2 rollback, volume is not in any storage " "group.")) - else: - LOG.info(_LI( - "The storage group found is " - "%(foundStorageGroupInstanceName)s."), - {'foundStorageGroupInstanceName': - foundStorageGroupInstanceName}) + else: + LOG.info(_LI( + "The storage group found is " + "%(foundStorageGroupInstanceName)s."), + {'foundStorageGroupInstanceName': + foundStorageGroupInstanceName}) - # Check the name, see is it the default storage group - # or another. - if (foundStorageGroupInstanceName != - rollbackDict['defaultStorageGroupInstanceName']): - # Remove it from its current masking view and return it - # to its default masking view if fast is enabled. - self.remove_and_reset_members( - conn, - rollbackDict['controllerConfigService'], - rollbackDict['volumeInstance'], - rollbackDict['volumeName'], - rollbackDict['extraSpecs']) - message = (_("V2 rollback - Volume in another storage " + # Check the name, see if it is the default storage group + # or another. + if (foundStorageGroupInstanceName != + rollbackDict['defaultStorageGroupInstanceName']): + # Remove it from its current masking view and return it + # to its default masking view if fast is enabled or slo + # is defined. + self.remove_and_reset_members( + conn, + rollbackDict['controllerConfigService'], + rollbackDict['volumeInstance'], + rollbackDict['volumeName'], + rollbackDict['extraSpecs']) + message = (_("Rollback - Volume in another storage " "group besides default storage group.")) except Exception: errorMessage = (_( "Rollback for Volume: %(volumeName)s has failed. " "Please contact your system administrator to manually return " - "your volume to the default storage group for fast policy " - "%(fastPolicyName)s failed.") - % {'volumeName': rollbackDict['volumeName'], - 'fastPolicyName': rollbackDict['fastPolicyName']}) + "your volume to the default storage group for fast policy/ " + "slo.") + % {'volumeName': rollbackDict['volumeName']}) LOG.exception(errorMessage) raise exception.VolumeBackendAPIException(data=errorMessage) return message @@ -1551,6 +1567,38 @@ class EMCVMAXMasking(object): return foundInitiatorGroupInstanceName + def _check_ig_rollback( + self, conn, controllerConfigService, + igGroupName, connector, extraSpecs): + """Check if rollback action is required on an initiator group. + + If anything goes wrong on a masking view creation, we need to check if + the process created a now-stale initiator group before failing, i.e. + an initiator group a) matching the name used in the mv process and + b) not associated with any other masking views. + If a stale ig exists, remove the initiators and delete the ig. + + :param conn: the ecom connection + :param controllerConfigService: controller config service + :param igGroupName: the initiator group name + :param connector: the connector object + :param extraSpecs: extra specifications + """ + initiatorNames = self._find_initiator_names(conn, connector) + foundInitiatorGroupInstanceName = self._find_initiator_masking_group( + conn, controllerConfigService, initiatorNames) + if foundInitiatorGroupInstanceName: + initiatorGroupInstance = conn.GetInstance( + foundInitiatorGroupInstanceName, LocalOnly=False) + if initiatorGroupInstance['ElementName'] == igGroupName: + host = igGroupName.split("-")[1] + LOG.debug("Searching for masking views associated with " + "%(igGroupName)s", + {'igGroupName': igGroupName}) + self._last_volume_delete_initiator_group( + conn, controllerConfigService, + foundInitiatorGroupInstanceName, extraSpecs, host) + def _get_port_group_from_masking_view( self, conn, maskingViewName, storageSystemName): """Given the masking view name get the port group from it.