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 a8b63906a22..481b6a3f484 100644 --- a/cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py +++ b/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): diff --git a/cinder/volume/drivers/emc/emc_vmax_masking.py b/cinder/volume/drivers/emc/emc_vmax_masking.py index 67548af6fc4..dbf2ba51cbd 100644 --- a/cinder/volume/drivers/emc/emc_vmax_masking.py +++ b/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( - conn, controllerConfigService, maskingViewName, - maskingViewInstanceName, extraSpecs) + if maskingViewInstanceName: + # Existing masking view needs to be deleted before + # 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, diff --git a/cinder/volume/drivers/emc/emc_vmax_provision.py b/cinder/volume/drivers/emc/emc_vmax_provision.py index a3b49fe0ec3..9f25ccfa3c3 100644 --- a/cinder/volume/drivers/emc/emc_vmax_provision.py +++ b/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})