diff options
7 files changed, 149 insertions, 4 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 d032902bf..161f9b31b 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 @@ -2866,6 +2866,8 @@ class PowerMaxCommonTest(test.TestCase): mck_update.assert_called_once_with(group, add_vols, remove_vols) self.assertEqual(ref_model_update, model_update) + @mock.patch.object(common.PowerMaxCommon, '_find_volume_group', + return_value=tpd.PowerMaxData.test_rep_group) @mock.patch.object(rest.PowerMaxRest, 'is_volume_in_storagegroup', return_value=True) @mock.patch.object( @@ -2882,7 +2884,7 @@ class PowerMaxCommonTest(test.TestCase): masking.PowerMaxMasking, 'remove_volumes_from_storage_group') def test_update_group_promotion( self, mck_rem, mock_cg_type, mock_type_check, mck_setup, mck_rep, - mck_in_sg): + mck_in_sg, mck_group): group = self.data.test_rep_group add_vols = [] remove_vols = [self.data.test_volume_group_member] 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 66c52ea6d..a5eae2b7c 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 @@ -695,7 +695,10 @@ class PowerMaxReplicationTest(test.TestCase): mck_validate.assert_called_once_with( self.common.rep_configs, extra_specs_list) - def test_enable_replication(self): + @mock.patch.object(common.PowerMaxCommon, '_find_volume_group', + side_effect=[tpd.PowerMaxData.test_group, + None]) + def test_enable_replication(self, mock_vg): # Case 1: Group not replicated with mock.patch.object(volume_utils, 'is_group_a_type', return_value=False): @@ -720,7 +723,10 @@ class PowerMaxReplicationTest(test.TestCase): self.assertEqual(fields.ReplicationStatus.ERROR, model_update['replication_status']) - def test_disable_replication(self): + @mock.patch.object(common.PowerMaxCommon, '_find_volume_group', + side_effect=[tpd.PowerMaxData.test_group, + None]) + def test_disable_replication(self, mock_vg): # Case 1: Group not replicated with mock.patch.object(volume_utils, 'is_group_a_type', return_value=False): 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 45084ae4e..a3fac79c9 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 @@ -2469,3 +2469,69 @@ class PowerMaxRestTest(test.TestCase): self.data.array, self.data.device_id, self.data.test_snapshot_snap_name) self.assertEqual('0', snap_id) + + @mock.patch.object( + rest.PowerMaxRest, 'get_storage_group_list') + @mock.patch.object( + rest.PowerMaxRest, 'get_storage_group_rep', + side_effect=[{'rdf': False}, None]) + def test_get_or_rename_storage_group_rep( + self, mock_sg_rep, mock_sg_list): + # Success - no need for rename + rep_info = self.rest.get_or_rename_storage_group_rep( + self.data.array, self.data.storagegroup_name_f, + self.data.extra_specs) + mock_sg_list.assert_not_called() + self.assertIsNotNone(rep_info) + + # Fail - cannot find sg but no filter set + rep_info = self.rest.get_or_rename_storage_group_rep( + self.data.array, self.data.storagegroup_name_f, + self.data.extra_specs) + mock_sg_list.assert_not_called() + self.assertIsNone(rep_info) + + @mock.patch.object( + rest.PowerMaxRest, '_rename_storage_group') + @mock.patch.object( + rest.PowerMaxRest, 'get_storage_group_list', + return_value=({'storageGroupId': ['user-name+uuid']})) + @mock.patch.object( + rest.PowerMaxRest, 'get_storage_group_rep', + side_effect=[None, ({'rdf': False}), ({'rdf': False})]) + def test_get_or_rename_storage_group_rep_exists( + self, mock_sg_rep, mock_sg_list, mock_rename): + sg_filter = '<like>uuid' + rep_info = self.rest.get_or_rename_storage_group_rep( + self.data.array, self.data.storagegroup_name_f, + self.data.extra_specs, sg_filter=sg_filter) + mock_sg_list.assert_called_once_with( + self.data.array, + params={'storageGroupId': sg_filter}) + group_list_return = {'storageGroupId': ['user-name+uuid']} + mock_rename.assert_called_once_with( + self.data.array, + group_list_return['storageGroupId'][0], + self.data.storagegroup_name_f, + self.data.extra_specs) + self.assertIsNotNone(rep_info) + + @mock.patch.object( + rest.PowerMaxRest, '_rename_storage_group') + @mock.patch.object( + rest.PowerMaxRest, 'get_storage_group_list', + return_value=({'storageGroupId': ['user-name+uuid']})) + @mock.patch.object( + rest.PowerMaxRest, 'get_storage_group_rep', + side_effect=[None, None]) + def test_get_or_rename_storage_group_rep_does_not_exist( + self, mock_sg_rep, mock_sg_list, mock_rename): + sg_filter = '<like>uuid' + rep_info = self.rest.get_or_rename_storage_group_rep( + self.data.array, self.data.storagegroup_name_f, + self.data.extra_specs, sg_filter=sg_filter) + mock_sg_list.assert_called_once_with( + self.data.array, + params={'storageGroupId': sg_filter}) + mock_rename.assert_not_called() + self.assertIsNone(rep_info) diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index 1935ef35b..2022621ed 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -535,6 +535,7 @@ class PowerMaxCommon(object): if group_id is not None: if group and (volume_utils.is_group_a_cg_snapshot_type(group) or group.is_replicated): + self._find_volume_group(extra_specs[utils.ARRAY], group) extra_specs[utils.FORCE_VOL_EDIT] = True group_name = self._add_new_volume_to_volume_group( volume, device_id, volume_name, @@ -6071,8 +6072,12 @@ class PowerMaxCommon(object): :param group: the group object :returns: volume group dictionary """ + __, interval_retries_dict = self._get_volume_group_info(group) group_name = self.utils.update_volume_group_name(group) - volume_group = self.rest.get_storage_group_rep(array, group_name) + sg_name_filter = utils.LIKE_FILTER + group.id + volume_group = self.rest.get_or_rename_storage_group_rep( + array, group_name, interval_retries_dict, + sg_filter=sg_name_filter) if not volume_group: LOG.warning("Volume group %(group_id)s cannot be found", {'group_id': group_name}) diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py index f18cf6b12..2ee58b8dd 100644 --- a/cinder/volume/drivers/dell_emc/powermax/rest.py +++ b/cinder/volume/drivers/dell_emc/powermax/rest.py @@ -3072,6 +3072,44 @@ class PowerMaxRest(object): 'src_device': device_id, 'tgt_device': r2_device_id, 'session_info': session_info} + def get_or_rename_storage_group_rep( + self, array, storage_group_name, extra_specs, sg_filter=None): + """Get storage group rep info if it exist. + + If a generic volume group has been renamed we also need + to rename it on the array based on the uuid component. + We check for uuid if we cannot find it based on its old name. + + :param array: the array serial number + :param storage_group_name: the name of the storage group + :param extra_specs: extra specification + :param sg_filter: uuid substring <like> + :returns: storage group dict or None + """ + rep_details = self.get_storage_group_rep(array, storage_group_name) + if not rep_details: + # It is possible that the group has been renamed + if sg_filter: + sg_dict = self.get_storage_group_list( + array, params={ + 'storageGroupId': sg_filter}) + sg_list = sg_dict.get('storageGroupId') if sg_dict else None + if sg_list and len(sg_list) == 1: + rep_details = self.get_storage_group_rep( + array, sg_list[0]) + # Update the new storage group name + if rep_details: + self._rename_storage_group( + array, sg_list[0], storage_group_name, extra_specs) + rep_details = self.get_storage_group_rep( + array, storage_group_name) + LOG.warning( + "Volume group %(old)s has been renamed to %(new)s " + "due to a rename operation in OpenStack.", + {'old': sg_list[0], 'new': storage_group_name}) + + return rep_details + def get_storage_group_rep(self, array, storage_group_name): """Given a name, return storage group details wrt replication. @@ -3429,3 +3467,20 @@ class PowerMaxRest(object): """ return (self.ucode_major_level >= utils.UCODE_5978 and self.ucode_minor_level >= utils.UCODE_5978_HICKORY) + + def _rename_storage_group( + self, array, old_name, new_name, extra_specs): + """Rename the storage group. + + :param array: the array serial number + :param old_name: the original name + :param new_name: the new name + :param extra_specs: the extra specifications + """ + payload = {"editStorageGroupActionParam": { + "renameStorageGroupParam": { + "new_storage_Group_name": new_name}}} + status_code, job = self.modify_storage_group( + array, old_name, payload) + self.wait_for_job( + 'Rename storage group', status_code, job, extra_specs) diff --git a/cinder/volume/drivers/dell_emc/powermax/utils.py b/cinder/volume/drivers/dell_emc/powermax/utils.py index 6956329ad..fc2b51294 100644 --- a/cinder/volume/drivers/dell_emc/powermax/utils.py +++ b/cinder/volume/drivers/dell_emc/powermax/utils.py @@ -216,6 +216,9 @@ REVERT_SS_EXC = 'Link must be fully copied for this operation to proceed' IS_TRUE = ['<is> True', 'True', 'true', True] IS_FALSE = ['<is> False', 'False', 'false', False] +# <like> filter +LIKE_FILTER = '<like>' + class PowerMaxUtils(object): """Utility class for Rest based PowerMax volume drivers. diff --git a/releasenotes/notes/bug-1936848-6ecc78e0e970419a.yaml b/releasenotes/notes/bug-1936848-6ecc78e0e970419a.yaml new file mode 100644 index 000000000..5cf26d331 --- /dev/null +++ b/releasenotes/notes/bug-1936848-6ecc78e0e970419a.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + PowerMax driver `bug #1936848 + <https://bugs.launchpad.net/cinder/+bug/1936848>`_: Fixed + Generic Volume Group error where the name has been changed + in OpenStack and is not reflected on the corresponding storage + group on the PowerMax. |