From b0a97cc8c8545a60d8e81a0e28a516cd49e09427 Mon Sep 17 00:00:00 2001 From: odonos12 Date: Tue, 18 Aug 2020 17:16:13 +0100 Subject: [PATCH] PowerMax Driver - Force add rep group volume Force flag is needed to add volumes to replication enabled storage groups since U4P 91. Added this flag when adding volumes to replication enabled volume groups. Edited string convention used when editing replication volumes to better reflect its intended usage. Change-Id: I67d8b0dd4879db86a910f89769c21cb5d145027a Closes-Bug: 1892057 --- .../dell_emc/powermax/test_powermax_common.py | 8 ++--- .../powermax/test_powermax_replication.py | 35 +++++++++++++------ .../dell_emc/powermax/test_powermax_rest.py | 4 +-- .../drivers/dell_emc/powermax/common.py | 18 +++++----- .../drivers/dell_emc/powermax/masking.py | 4 ++- .../volume/drivers/dell_emc/powermax/rest.py | 6 ++-- .../volume/drivers/dell_emc/powermax/utils.py | 2 +- 7 files changed, 47 insertions(+), 30 deletions(-) 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