Merge "VMAX driver - failed rollback on VMAX3 when MV issue"

cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py

@@ -2334,14 +2334,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.
@@ -2350,6 +2353,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 = (
@@ -7841,6 +7845,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):

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
+                if maskingViewDict['slo'] is not None:
-                rollbackDict['storageSystemName'] = (
+                    rollbackDict['storageSystemName'] = (
-                    maskingViewDict['storageSystemName'])
+                        maskingViewDict['storageSystemName'])
-                self._check_if_rollback_action_for_masking_required(
+                    rollbackDict['slo'] = maskingViewDict['slo']
-                    conn, rollbackDict)
+                    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']:
+            foundStorageGroupInstanceName = (
-                errorMessage = self._check_adding_volume_to_storage_group(
+                self.utils.get_storage_group_from_volume(
-                    conn, rollbackDict,
+                    conn, rollbackDict['volumeInstance'].path,
-                    rollbackDict['defaultStorageGroupInstanceName'])
+                    rollbackDict['sgGroupName']))
-                if errorMessage:
+            # Volume is not associated with any storage group so add
-                    LOG.error(errorMessage)
+            # it back to the default.
-                message = (_("V3 rollback"))
+            if not foundStorageGroupInstanceName:
-
+                if rollbackDict['isV3']:
-            else:
+                    errorMessage = self._check_adding_volume_to_storage_group(
-                foundStorageGroupInstanceName = (
+                        conn, rollbackDict,
-                    self.utils.get_storage_group_from_volume(
+                        rollbackDict['defaultStorageGroupInstanceName'])
-                        conn, rollbackDict['volumeInstance'].path,
+                    if errorMessage:
-                        rollbackDict['sgName']))
+                        LOG.error(errorMessage)
-                # Volume is not associated with any storage group so add
+                    message = (_("V3 rollback"))
-                # it back to the default.
+                else:
-                if not foundStorageGroupInstanceName:
                     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:
+            else:
-                    LOG.info(_LI(
+                LOG.info(_LI(
-                        "The storage group found is "
+                    "The storage group found is "
-                        "%(foundStorageGroupInstanceName)s."),
+                    "%(foundStorageGroupInstanceName)s."),
-                        {'foundStorageGroupInstanceName':
+                    {'foundStorageGroupInstanceName':
-                         foundStorageGroupInstanceName})
+                     foundStorageGroupInstanceName})
 
-                    # Check the name, see is it the default storage group
+                # Check the name, see if it is the default storage group
-                    # or another.
+                # or another.
-                    if (foundStorageGroupInstanceName !=
+                if (foundStorageGroupInstanceName !=
-                            rollbackDict['defaultStorageGroupInstanceName']):
+                        rollbackDict['defaultStorageGroupInstanceName']):
-                        # Remove it from its current masking view and return it
+                    # Remove it from its current masking view and return it
-                        # to its default masking view if fast is enabled.
+                    # to its default masking view if fast is enabled or slo
-                        self.remove_and_reset_members(
+                    # is defined.
-                            conn,
+                    self.remove_and_reset_members(
-                            rollbackDict['controllerConfigService'],
+                        conn,
-                            rollbackDict['volumeInstance'],
+                        rollbackDict['controllerConfigService'],
-                            rollbackDict['volumeName'],
+                        rollbackDict['volumeInstance'],
-                            rollbackDict['extraSpecs'])
+                        rollbackDict['volumeName'],
-                    message = (_("V2 rollback - Volume in another storage "
+                        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 "
+                "your volume to the default storage group for fast policy/ "
-                "%(fastPolicyName)s failed.")
+                "slo.")
-                % {'volumeName': rollbackDict['volumeName'],
+                % {'volumeName': rollbackDict['volumeName']})
-                   'fastPolicyName': rollbackDict['fastPolicyName']})
             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.