Merge "VMAX driver - Duplicate initiator group name error."

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

@@ -8008,6 +8008,100 @@ class EMCVMAXMaskingTest(test.TestCase):
                               igGroupName, hardwareIdinstanceNames,
                               extraSpecs)
 
+    @mock.patch.object(
+        emc_vmax_masking.EMCVMAXMasking,
+        "_delete_initiators_from_initiator_group")
+    @mock.patch.object(
+        emc_vmax_masking.EMCVMAXMasking,
+        "_delete_initiator_group")
+    @mock.patch.object(
+        emc_vmax_masking.EMCVMAXMasking,
+        "_create_initiator_Group",
+        return_value=EMCVMAXCommonData.initiatorgroup_name)
+    # bug 1579934: duplicate IG name error from SMI-S
+    def test_verify_initiator_group_from_masking_view(
+            self, create_ig, delete_ig, delete_initiators):
+        utils = self.driver.common.utils
+        masking = self.driver.common.masking
+        conn = self.fake_ecom_connection()
+        controllerConfigService = (
+            utils.find_controller_configuration_service(
+                conn, self.data.storage_system))
+        connector = self.data.connector
+        maskingViewName = self.data.lunmaskctrl_name
+        storageSystemName = self.data.storage_system
+        igGroupName = self.data.initiatorgroup_name
+        extraSpecs = self.data.extra_specs
+        initiatorNames = (
+            self.driver.common.masking._find_initiator_names(conn, connector))
+        storageHardwareIDInstanceNames = (
+            masking._get_storage_hardware_id_instance_names(
+                conn, initiatorNames, storageSystemName))
+        foundInitiatorGroupFromMaskingView = (
+            masking._get_initiator_group_from_masking_view(
+                conn, maskingViewName, storageSystemName))
+        # path 1: initiator group from masking view matches initiator
+        # group from connector
+        verify = masking._verify_initiator_group_from_masking_view(
+            conn, controllerConfigService, maskingViewName, connector,
+            storageSystemName, igGroupName, extraSpecs)
+        masking._create_initiator_Group.assert_not_called()
+        self.assertTrue(verify)
+        # path 2: initiator group from masking view does not match
+        # initiator group from connector
+        with mock.patch.object(
+                masking, "_find_initiator_masking_group",
+                return_value="not_a_match"):
+            # path 2a: initiator group from connector is not None
+            # - no new initiator group created
+            verify = masking._verify_initiator_group_from_masking_view(
+                conn, controllerConfigService, maskingViewName,
+                connector, storageSystemName, igGroupName,
+                extraSpecs)
+            self.assertTrue(verify)
+            masking._create_initiator_Group.assert_not_called()
+            # path 2b: initiator group from connector is None
+            # - new initiator group created
+            with mock.patch.object(
+                    masking, "_find_initiator_masking_group",
+                    return_value=None):
+                masking._verify_initiator_group_from_masking_view(
+                    conn, controllerConfigService, maskingViewName,
+                    connector, storageSystemName, igGroupName,
+                    extraSpecs)
+                (masking._create_initiator_Group.
+                 assert_called_once_with(conn, controllerConfigService,
+                                         igGroupName,
+                                         storageHardwareIDInstanceNames,
+                                         extraSpecs))
+                # path 2b(i) - the name of the initiator group from the
+                # masking view is the same as the provided igGroupName
+                # - existing ig must be deleted
+                (masking._delete_initiator_group.
+                 assert_called_once_with(conn, controllerConfigService,
+                                         foundInitiatorGroupFromMaskingView,
+                                         igGroupName, extraSpecs))
+                # path 2b(ii) - the name of the ig from the masking view
+                # is different - do not delete the existing ig
+                masking._delete_initiator_group.reset_mock()
+                with mock.patch.object(
+                        conn, "GetInstance",
+                        return_value={'ElementName': "different_name"}):
+                    masking._verify_initiator_group_from_masking_view(
+                        conn, controllerConfigService, maskingViewName,
+                        connector, storageSystemName, igGroupName,
+                        extraSpecs)
+                    masking._delete_initiator_group.assert_not_called()
+            # path 3 - the masking view cannot be verified
+            with mock.patch.object(
+                    masking, "_get_storage_group_from_masking_view",
+                    return_value=None):
+                verify = masking._verify_initiator_group_from_masking_view(
+                    conn, controllerConfigService, maskingViewName,
+                    connector, storageSystemName, igGroupName,
+                    extraSpecs)
+                self.assertFalse(verify)
+
 
 class EMCVMAXFCTest(test.TestCase):
     def setUp(self):

cinder/volume/drivers/emc/emc_vmax_masking.py

@@ -1452,6 +1452,11 @@ class EMCVMAXMasking(object):
             if foundInitiatorGroupFromMaskingView is not None:
                 maskingViewInstanceName = self._find_masking_view(
                     conn, maskingViewName, storageSystemName)
+                storageGroupInstanceName = (
+                    self._get_storage_group_from_masking_view(
+                        conn, maskingViewName, storageSystemName))
+                portGroupInstanceName = self._get_port_group_from_masking_view(
+                    conn, maskingViewName, storageSystemName)
                 if foundInitiatorGroupFromConnector is None:
                     storageHardwareIDInstanceNames = (
                         self._get_storage_hardware_id_instance_names(
@@ -1472,21 +1477,41 @@ class EMCVMAXMasking(object):
                                 {'storageSystemName': storageSystemName})
                             return False
 
+                    igFromMaskingViewInstance = conn.GetInstance(
+                        foundInitiatorGroupFromMaskingView, LocalOnly=False)
+                    # if the current foundInitiatorGroupFromMaskingView name
+                    # matches the igGroupName supplied for the new group, the
+                    # existing ig needs to be deleted before the new one with
+                    # the correct initiators can be created.
+                    if (igFromMaskingViewInstance['ElementName'] ==
+                            igGroupName):
+                        # Masking view needs to be deleted before IG
+                        # can be deleted.
+                        self._delete_masking_view(
+                            conn, controllerConfigService, maskingViewName,
+                            maskingViewInstanceName, extraSpecs)
+                        maskingViewInstanceName = None
+                        self._delete_initiators_from_initiator_group(
+                            conn, controllerConfigService,
+                            foundInitiatorGroupFromMaskingView,
+                            igGroupName)
+                        self._delete_initiator_group(
+                            conn, controllerConfigService,
+                            foundInitiatorGroupFromMaskingView,
+                            igGroupName, extraSpecs)
                     foundInitiatorGroupFromConnector = (
                         self._create_initiator_Group(
                             conn, controllerConfigService, igGroupName,
                             storageHardwareIDInstanceNames, extraSpecs))
-                storageGroupInstanceName = (
-                    self._get_storage_group_from_masking_view(
-                        conn, maskingViewName, storageSystemName))
-                portGroupInstanceName = self._get_port_group_from_masking_view(
-                    conn, maskingViewName, storageSystemName)
                 if (foundInitiatorGroupFromConnector is not None and
                         storageGroupInstanceName is not None and
                         portGroupInstanceName is not None):
-                    self._delete_masking_view(
+                    if maskingViewInstanceName:
-                        conn, controllerConfigService, maskingViewName,
+                        # Existing masking view needs to be deleted before
-                        maskingViewInstanceName, extraSpecs)
+                        # a new one can be created.
+                        self._delete_masking_view(
+                            conn, controllerConfigService, maskingViewName,
+                            maskingViewInstanceName, extraSpecs)
                     newMaskingViewInstanceName = (
                         self._get_masking_view_instance_name(
                             conn, controllerConfigService, maskingViewName,

cinder/volume/drivers/emc/emc_vmax_provision.py

@@ -310,7 +310,7 @@ class EMCVMAXProvision(object):
                 if rc != 0:
                     exceptionMessage = (_(
                         "Error removing volume %(vol)s from %(sg)s. "
-                        "%(error)s.")
+                        "Error is: %(error)s.")
                         % {'vol': volumeName,
                            'sg': storageGroupInstance['ElementName'],
                            'error': errorDesc})