VMAX driver - Duplicate initiator group name error.

As part of the validation of each of the components in a
masking view, the code checks if the initiators in the
initiator group associated with the masking view match
those from the connector. If the initiators don't match,
and there is no other initiator group associated with any
of the initiators from the connector, a new initiator
group with the correct initiators is created. However,
the current code does not first check if the ig to be
created has the same name as the ig from the masking
view. If so, the ig from the masking view must be first
deleted before a new ig with the same name can be created.

Change-Id: I51111c66b1954c82ed7df01d737d57c7bc57171e
Closes-Bug: #1579934

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})