From d2d7aed475556286ee8453fda5dd3170c00acdf9 Mon Sep 17 00:00:00 2001 From: Helen Walsh Date: Tue, 18 Feb 2020 17:10:30 +0000 Subject: [PATCH] PowerMax Driver - Allowing for default volume type in group If the volumetype(s) used in group operations does not capture the array serial_number an exception is thrown. This is only a factor when consistent_group_snapshot_enabled True. The fix requires getting the array serial_number from the conf file. Closes-Bug: #1866871 Change-Id: Ia2fa7cb7b47ae45564e4eef7f0ed62c9830fd47d --- .../dell_emc/powermax/test_powermax_common.py | 14 +++++++ .../drivers/dell_emc/powermax/common.py | 41 +++++++++++++------ .../volume/drivers/dell_emc/powermax/utils.py | 22 +++++----- .../bug-fix-1866871-f9d61defc00f4007.yaml | 6 +++ 4 files changed, 60 insertions(+), 23 deletions(-) create mode 100644 releasenotes/notes/bug-fix-1866871-f9d61defc00f4007.yaml 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 e6f26ba6c63..8777a0ffc27 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 @@ -3179,3 +3179,17 @@ class PowerMaxCommonTest(test.TestCase): remote_vol = self.common.get_and_set_remote_device_uuid( extra_specs, rep_extra_specs, volume_dict) self.assertEqual(remote_vol, self.data.device_id2) + + @mock.patch.object(utils.PowerMaxUtils, 'get_volume_group_utils', + return_value=(None, {'interval': 1, 'retries': 1})) + def test_get_volume_group_info(self, mock_group_utils): + self.common.interval = 1 + self.common.retries = 1 + with mock.patch.object( + self.common, '_get_configuration_value') as mock_array: + self.common._get_volume_group_info( + self.data.test_group_1) + mock_group_utils.assert_called_once_with( + self.data.test_group_1, self.common.interval, + self.common.retries) + mock_array.assert_called_once() diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index e7a21eee599..712a5a820ff 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -4402,8 +4402,7 @@ class PowerMaxCommon(object): vol_grp_name = self.utils.update_volume_group_name(group) try: - array, interval_retries_dict = self.utils.get_volume_group_utils( - group, self.interval, self.retries) + array, interval_retries_dict = self._get_volume_group_info(group) self.provision.create_volume_group( array, vol_grp_name, interval_retries_dict) if group.is_replicated: @@ -4452,8 +4451,7 @@ class PowerMaxCommon(object): :returns: model_update, volumes_model_update """ volumes_model_update = [] - array, interval_retries_dict = self.utils.get_volume_group_utils( - group, self.interval, self.retries) + array, interval_retries_dict = self._get_volume_group_info(group) vol_grp_name = None volume_group = self._find_volume_group( @@ -4668,8 +4666,8 @@ class PowerMaxCommon(object): :param source_group: the group object :param snap_name: the name of the snapshot """ - array, interval_retries_dict = self.utils.get_volume_group_utils( - source_group, self.interval, self.retries) + array, interval_retries_dict = self._get_volume_group_info( + source_group) vol_grp_name = None volume_group = ( self._find_volume_group(array, source_group)) @@ -4721,8 +4719,8 @@ class PowerMaxCommon(object): vol_grp_name = None try: # Get the array serial - array, extra_specs = self.utils.get_volume_group_utils( - source_group, self.interval, self.retries) + array, extra_specs = self._get_volume_group_info( + source_group) # Get the volume group dict for getting the group name volume_group = (self._find_volume_group(array, source_group)) if volume_group and volume_group.get('name'): @@ -4811,8 +4809,7 @@ class PowerMaxCommon(object): and not group.is_replicated): raise NotImplementedError() - array, interval_retries_dict = self.utils.get_volume_group_utils( - group, self.interval, self.retries) + array, interval_retries_dict = self._get_volume_group_info(group) model_update = {'status': fields.GroupStatus.AVAILABLE} add_vols = [vol for vol in add_volumes] if add_volumes else [] add_device_ids = self._get_volume_device_ids(add_vols, array) @@ -4940,8 +4937,7 @@ class PowerMaxCommon(object): tgt_name = self.utils.update_volume_group_name(group) rollback_dict = {} - array, interval_retries_dict = self.utils.get_volume_group_utils( - group, self.interval, self.retries) + array, interval_retries_dict = self._get_volume_group_info(group) source_sg = self._find_volume_group(array, actual_source_grp) if source_sg is not None: src_grp_name = (source_sg['name'] @@ -5435,6 +5431,27 @@ class PowerMaxCommon(object): return_value = self.configuration.safe_get(second_key) return return_value + def _get_volume_group_info(self, group): + """Get the volume group array, retries and intervals + + :param group: the group object + :returns: array -- str + interval_retries_dict -- dict + """ + array, interval_retries_dict = self.utils.get_volume_group_utils( + group, self.interval, self.retries) + if not array: + array = self._get_configuration_value( + utils.VMAX_ARRAY, utils.POWERMAX_ARRAY) + if not array: + exception_message = _( + "Cannot get the array serial_number") + + LOG.error(exception_message) + raise exception.VolumeBackendAPIException( + message=exception_message) + return array, interval_retries_dict + def _get_unlink_configuration_value(self, first_key, second_key): """Get the configuration value of snapvx_unlink_limit diff --git a/cinder/volume/drivers/dell_emc/powermax/utils.py b/cinder/volume/drivers/dell_emc/powermax/utils.py index 1d7d9919307..b3cc86baf4c 100644 --- a/cinder/volume/drivers/dell_emc/powermax/utils.py +++ b/cinder/volume/drivers/dell_emc/powermax/utils.py @@ -602,28 +602,28 @@ class PowerMaxUtils(object): """ arrays = set() # Check if it is a generic volume group instance + intervals_retries_dict = {INTERVAL: interval, RETRIES: retries} if isinstance(group, Group): for volume_type in group.volume_types: extra_specs = self.update_extra_specs(volume_type.extra_specs) - arrays.add(extra_specs[ARRAY]) + try: + arrays.add(extra_specs[ARRAY]) + except KeyError: + return None, intervals_retries_dict else: msg = (_("Unable to get volume type ids.")) LOG.error(msg) raise exception.VolumeBackendAPIException(message=msg) - if len(arrays) != 1: - if not arrays: - msg = (_("Failed to get an array associated with " - "volume group: %(groupid)s.") - % {'groupid': group.id}) - else: - msg = (_("There are multiple arrays " - "associated with volume group: %(groupid)s.") - % {'groupid': group.id}) + if len(arrays) > 1: + msg = (_("There are multiple arrays " + "associated with volume group: %(groupid)s.") + % {'groupid': group.id}) LOG.error(msg) raise exception.VolumeBackendAPIException(message=msg) array = arrays.pop() - intervals_retries_dict = {INTERVAL: interval, RETRIES: retries} + LOG.debug("Serial number %s retrieved from the volume type extra " + "specs.", array) return array, intervals_retries_dict def update_volume_group_name(self, group): diff --git a/releasenotes/notes/bug-fix-1866871-f9d61defc00f4007.yaml b/releasenotes/notes/bug-fix-1866871-f9d61defc00f4007.yaml new file mode 100644 index 00000000000..c969430dd3c --- /dev/null +++ b/releasenotes/notes/bug-fix-1866871-f9d61defc00f4007.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + PowerMax Driver - Allowing for default volume type in group operations + where the array serial number is retrieved from the cinder.conf instead + of the pool_name on the extra specs.