diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py index e44615a8c7a..5cf1ed30cfa 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py @@ -571,7 +571,7 @@ class PowerMaxCommonTest(test.TestCase): deepcopy(self.data.test_volume_attachment)] extra_specs = deepcopy(self.data.rep_extra_specs_rep_config) array = extra_specs[utils.ARRAY] - extra_specs[utils.FORCE_VOL_REMOVE] = True + extra_specs[utils.FORCE_VOL_EDIT] = True self.common._unmap_lun(volume, connector) mck_rem.assert_called_once_with(array, volume, device_info, extra_specs, connector, False, @@ -597,7 +597,7 @@ class PowerMaxCommonTest(test.TestCase): volume.volume_attachment.objects = [ deepcopy(self.data.test_volume_attachment)] extra_specs = deepcopy(self.data.rep_extra_specs_rep_config) - extra_specs[utils.FORCE_VOL_REMOVE] = True + extra_specs[utils.FORCE_VOL_EDIT] = True self.common._unmap_lun(volume, connector) self.assertEqual(2, mck_rem.call_count) @@ -621,7 +621,7 @@ class PowerMaxCommonTest(test.TestCase): volume.volume_attachment.objects = [ deepcopy(self.data.test_volume_attachment)] extra_specs = deepcopy(self.data.rep_extra_specs_rep_config) - extra_specs[utils.FORCE_VOL_REMOVE] = True + extra_specs[utils.FORCE_VOL_EDIT] = True self.common.promotion = True self.common._unmap_lun(volume, connector) self.common.promotion = False @@ -2726,7 +2726,7 @@ class PowerMaxCommonTest(test.TestCase): group_name = self.data.storagegroup_name_source interval_retries_dict = {utils.INTERVAL: 1, utils.RETRIES: 1, - utils.FORCE_VOL_REMOVE: True} + utils.FORCE_VOL_EDIT: True} self.common._update_group_promotion(group, add_vols, remove_vols) mck_rem.assert_called_once_with( remote_array, device_id, group_name, interval_retries_dict) diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py index 90002890ef1..eedd0f6f384 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py @@ -106,7 +106,7 @@ class PowerMaxReplicationTest(test.TestCase): extra_specs = deepcopy(self.extra_specs) extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f extra_specs[utils.IS_RE] = True - extra_specs[utils.FORCE_VOL_REMOVE] = True + extra_specs[utils.FORCE_VOL_EDIT] = True rep_config = self.data.rep_config_sync rep_config[utils.RDF_CONS_EXEMPT] = False extra_specs[utils.REP_CONFIG] = rep_config @@ -711,6 +711,9 @@ class PowerMaxReplicationTest(test.TestCase): self.common._delete_group(group, []) mock_cleanup.assert_called_once() + @mock.patch.object(rest.PowerMaxRest, 'is_volume_in_storagegroup', + return_value=True) + @mock.patch.object(masking.PowerMaxMasking, 'add_volumes_to_storage_group') @mock.patch.object(masking.PowerMaxMasking, 'remove_volumes_from_storage_group') @mock.patch.object(utils.PowerMaxUtils, 'check_rep_status_enabled') @@ -721,15 +724,27 @@ class PowerMaxReplicationTest(test.TestCase): @mock.patch.object(volume_utils, 'is_group_a_type', return_value=True) @mock.patch.object(volume_utils, 'is_group_a_cg_snapshot_type', return_value=True) - def test_update_replicated_group(self, mock_cg_type, mock_type_check, - mock_add, mock_remove, mock_check, - mock_rm): + def test_update_replicated_group( + self, mock_cg_type, mock_type_check, mock_add_remote, + mock_remove_remote, mock_check, mock_remove_local, mock_add_local, + mock_vol_in_sg): + array = self.data.array add_vols = [self.data.test_volume] + add_vols_id = [self.data.device_id] remove_vols = [self.data.test_clone_volume] - self.common.update_group( - self.data.test_group_1, add_vols, remove_vols) - mock_add.assert_called_once() - mock_remove.assert_called_once() + remove_vols_id = [self.data.device_id2] + group = self.data.test_group_1 + group_sg = self.data.storagegroup_name_source + extra_specs = { + utils.INTERVAL: 1, utils.RETRIES: 1, utils.FORCE_VOL_EDIT: True} + self.common.update_group(group, add_vols, remove_vols) + mock_add_local.assert_called_once_with( + array, add_vols_id, group_sg, extra_specs) + mock_add_remote.assert_called_once_with(add_vols, group, extra_specs) + mock_remove_local.assert_called_once_with( + array, remove_vols_id, group_sg, extra_specs) + mock_remove_remote.assert_called_once_with( + array, remove_vols, group, extra_specs) @mock.patch.object(masking.PowerMaxMasking, 'remove_volumes_from_storage_group') @@ -1580,7 +1595,7 @@ class PowerMaxReplicationTest(test.TestCase): self.common.break_rdf_device_pair_session( array, device_id, volume_name, extra_specs, volume)) - extra_specs[utils.REP_CONFIG]['force_vol_remove'] = True + extra_specs[utils.REP_CONFIG][utils.FORCE_VOL_EDIT] = True self.assertEqual(extra_specs[utils.REP_CONFIG], rep_extra_specs) self.assertFalse(resume_rdf) @@ -1626,7 +1641,7 @@ class PowerMaxReplicationTest(test.TestCase): self.common.break_rdf_device_pair_session( array, device_id, volume_name, extra_specs, volume)) - extra_specs[utils.REP_CONFIG]['force_vol_remove'] = True + extra_specs[utils.REP_CONFIG][utils.FORCE_VOL_EDIT] = True extra_specs[utils.REP_CONFIG]['mgmt_sg_name'] = ( self.data.default_sg_no_slo_re_enabled) diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py index bd777b92725..2b9f76587a4 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py @@ -549,7 +549,7 @@ class PowerMaxRestTest(test.TestCase): def test_remove_vol_from_sg_force_true(self, mck_wait): device_id = self.data.device_id extra_specs = deepcopy(self.data.extra_specs) - extra_specs[utils.FORCE_VOL_REMOVE] = True + extra_specs[utils.FORCE_VOL_EDIT] = True expected_payload = ( {"executionOption": "ASYNCHRONOUS", "editStorageGroupActionParam": { @@ -571,7 +571,7 @@ class PowerMaxRestTest(test.TestCase): def test_remove_vol_from_sg_force_false(self, mck_wait): device_id = self.data.device_id extra_specs = deepcopy(self.data.extra_specs) - extra_specs.pop(utils.FORCE_VOL_REMOVE, None) + extra_specs.pop(utils.FORCE_VOL_EDIT, None) expected_payload = ( {"executionOption": "ASYNCHRONOUS", "editStorageGroupActionParam": { diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index 83e8207ed2d..1c5bb1e0116 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -807,7 +807,7 @@ class PowerMaxCommon(object): backend_id = self._get_replicated_volume_backend_id(volume) rep_config = self.utils.get_rep_config( backend_id, self.rep_configs) - extra_specs[utils.FORCE_VOL_REMOVE] = True + extra_specs[utils.FORCE_VOL_EDIT] = True rep_extra_specs = self._get_replication_extra_specs( extra_specs, rep_config) if self.utils.is_volume_failed_over(volume): @@ -2582,7 +2582,7 @@ class PowerMaxCommon(object): rep_config = extra_specs[utils.REP_CONFIG] rep_mode = extra_specs['rep_mode'] if rep_mode in [utils.REP_METRO, utils.REP_ASYNC]: - extra_specs['force_vol_remove'] = True + extra_specs[utils.FORCE_VOL_EDIT] = True rdf_group_no, remote_array = self.get_rdf_details(array, rep_config) rep_extra_specs = self._get_replication_extra_specs( extra_specs, rep_config) @@ -4868,8 +4868,8 @@ class PowerMaxCommon(object): array, rdfg_no, device_id) remote_device_id = remote_device['remoteVolumeName'] - extra_specs['force_vol_remove'] = True - rep_extra_specs['force_vol_remove'] = True + extra_specs[utils.FORCE_VOL_EDIT] = True + rep_extra_specs[utils.FORCE_VOL_EDIT] = True # Get the names of the SGs associated with the volume on the R2 array # before any operations are carried out - this will be used later for @@ -5020,7 +5020,7 @@ class PowerMaxCommon(object): mgmt_sg_name = None rep_config = extra_specs[utils.REP_CONFIG] rdfg_no = extra_specs['rdf_group_no'] - extra_specs['force_vol_remove'] = True + extra_specs[utils.FORCE_VOL_EDIT] = True if rep_config['mode'] in [utils.REP_ASYNC, utils.REP_METRO]: mgmt_sg_name = self.utils.get_rdf_management_group_name( rep_config) @@ -5885,6 +5885,9 @@ class PowerMaxCommon(object): vol_grp_name = volume_group['name'] if vol_grp_name is None: raise exception.GroupNotFound(group_id=group.id) + if group.is_replicated: + # Need force flag when manipulating RDF enabled SGs + interval_retries_dict[utils.FORCE_VOL_EDIT] = True # Add volume(s) to the group if add_device_ids: self.utils.check_rep_status_enabled(group) @@ -5900,9 +5903,6 @@ class PowerMaxCommon(object): add_vols, group, interval_retries_dict) # Remove volume(s) from the group if remove_device_ids: - if group.is_replicated: - # Need force flag when manipulating RDF enabled SGs - interval_retries_dict[utils.FORCE_VOL_REMOVE] = True # Check if the volumes exist in the storage group temp_list = deepcopy(remove_device_ids) for device_id in temp_list: @@ -5975,7 +5975,7 @@ class PowerMaxCommon(object): remove_device_ids = self._get_volume_device_ids( remove_volumes, remote_array) if remove_device_ids: - interval_retries_dict[utils.FORCE_VOL_REMOVE] = True + interval_retries_dict[utils.FORCE_VOL_EDIT] = True # Check if the volumes exist in the storage group temp_list = deepcopy(remove_device_ids) for device_id in temp_list: diff --git a/cinder/volume/drivers/dell_emc/powermax/masking.py b/cinder/volume/drivers/dell_emc/powermax/masking.py index fe26597a43b..750978621f5 100644 --- a/cinder/volume/drivers/dell_emc/powermax/masking.py +++ b/cinder/volume/drivers/dell_emc/powermax/masking.py @@ -743,6 +743,7 @@ class PowerMaxMasking(object): return start_time = time.time() temp_device_id_list = list_device_id + force = extra_specs.get(utils.FORCE_VOL_EDIT, False) @coordination.synchronized("emc-sg-{sg_name}-{serial_number}") def do_add_volumes_to_sg(sg_name, serial_number): @@ -759,7 +760,8 @@ class PowerMaxMasking(object): # Remove this device id from the list temp_device_id_list.remove(volume) self.rest.add_vol_to_sg(serial_number, storagegroup_name, - temp_device_id_list, extra_specs) + temp_device_id_list, extra_specs, + force=force) do_add_volumes_to_sg(storagegroup_name, serial_number) LOG.debug("Add volumes to storagegroup took: %(delta)s H:MM:SS.", diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py index 369b0356637..75b4fa25ef9 100644 --- a/cinder/volume/drivers/dell_emc/powermax/rest.py +++ b/cinder/volume/drivers/dell_emc/powermax/rest.py @@ -1265,8 +1265,8 @@ class PowerMaxRest(object): :param extra_specs: the extra specifications """ - force_vol_remove = ( - "true" if utils.FORCE_VOL_REMOVE in extra_specs else "false") + force_vol_edit = ( + "true" if utils.FORCE_VOL_EDIT in extra_specs else "false") if not isinstance(device_id, list): device_id = [device_id] payload = ({"executionOption": "ASYNCHRONOUS", @@ -1274,7 +1274,7 @@ class PowerMaxRest(object): "removeVolumeParam": { "volumeId": device_id, "remoteSymmSGInfoParam": { - "force": force_vol_remove}}}}) + "force": force_vol_edit}}}}) status_code, job = self.modify_storage_group( array, storagegroup_name, payload) diff --git a/cinder/volume/drivers/dell_emc/powermax/utils.py b/cinder/volume/drivers/dell_emc/powermax/utils.py index aac8436de66..e635e41b1e3 100644 --- a/cinder/volume/drivers/dell_emc/powermax/utils.py +++ b/cinder/volume/drivers/dell_emc/powermax/utils.py @@ -105,7 +105,7 @@ TAG_LIST = 'tag_list' USED_HOST_NAME = "used_host_name" RDF_SYNCED_STATES = [RDF_SYNC_STATE, RDF_CONSISTENT_STATE, RDF_ACTIVEACTIVE, RDF_ACTIVEBIAS] -FORCE_VOL_REMOVE = 'force_vol_remove' +FORCE_VOL_EDIT = 'force_vol_edit' PMAX_FAILOVER_START_ARRAY_PROMOTION = 'pmax_failover_start_array_promotion' # Multiattach constants