From 20a55fc0ca05b36423e89574a57a7d510e3bc0ff Mon Sep 17 00:00:00 2001 From: michael-mcaleer Date: Thu, 16 Apr 2020 11:22:03 +0000 Subject: [PATCH] PowerMax Driver - PowerMax Pools Fix This change fixes bug 1873253 whereby live migration was broken for volume-types with 'None' set as workload. The PowerMax for Cinder driver now accounts for both 'None' and 'NONE' variations of service level and workload. Change-Id: Ia26993f8c352d539fa8834d368a9c39109d6485d Closes-Bug: #1873253 (cherry picked from commit aef7fe44c10c9689bb889e866031c101c69fca4b) --- .../dell_emc/powermax/powermax_data.py | 4 + .../dell_emc/powermax/test_powermax_common.py | 68 ++++------- .../powermax/test_powermax_masking.py | 21 ++++ .../drivers/dell_emc/powermax/common.py | 112 ++++++++++-------- cinder/volume/drivers/dell_emc/powermax/fc.py | 3 +- .../volume/drivers/dell_emc/powermax/iscsi.py | 3 +- .../drivers/dell_emc/powermax/masking.py | 2 +- .../volume/drivers/dell_emc/powermax/utils.py | 17 ++- 8 files changed, 133 insertions(+), 97 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py index 9aa8e272684..92965da3058 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py @@ -270,6 +270,10 @@ class PowerMaxData(object): 'array': array, 'interval': 3, 'retries': 120} + extra_specs_optimized = { + 'pool_name': u'Optimized+None+SRP_1+000197800123', + 'slo': 'Optimized', 'workload': 'None', + 'srp': srp, 'array': array, 'interval': 3, 'retries': 120} extra_specs_migrate = deepcopy(extra_specs) extra_specs_migrate[utils.PORTGROUPNAME] = port_group_name_f 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 322427ee17b..442cc1d1c5a 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 @@ -82,55 +82,35 @@ class PowerMaxCommonTest(test.TestCase): self.assertEqual(self.common.ucode_level, self.data.next_gen_ucode) def test_get_slo_workload_combinations_powermax(self): - array_info = self.common.get_attributes_from_cinder_config() - finalarrayinfolist = self.common._get_slo_workload_combinations( - array_info) - self.assertTrue(len(finalarrayinfolist) > 1) - - @mock.patch.object( - rest.PowerMaxRest, 'get_vmax_model', - return_value=(tpd.PowerMaxData.vmax_model_details['model'])) - @mock.patch.object( - rest.PowerMaxRest, 'get_slo_list', - return_value=(tpd.PowerMaxData.vmax_slo_details['sloId'])) - def test_get_slo_workload_combinations_vmax(self, mck_slo, mck_model): - array_info = self.common.get_attributes_from_cinder_config() - finalarrayinfolist = self.common._get_slo_workload_combinations( - array_info) - self.assertTrue(len(finalarrayinfolist) > 1) - - @mock.patch.object( - rest.PowerMaxRest, 'get_vmax_model', - return_value=tpd.PowerMaxData.powermax_model_details['model']) - @mock.patch.object(rest.PowerMaxRest, 'get_workload_settings', - return_value=[]) - @mock.patch.object( - rest.PowerMaxRest, 'get_slo_list', - return_value=tpd.PowerMaxData.powermax_slo_details['sloId']) - def test_get_slo_workload_combinations_next_gen(self, mck_slo, mck_wl, - mck_model): self.common.next_gen = True - self.common.array_model = 'PowerMax 2000' - finalarrayinfolist = self.common._get_slo_workload_combinations( - self.data.array_info_no_wl) - self.assertTrue(len(finalarrayinfolist) == 14) + self.common.array_model = 'PowerMax_2000' + array_info = {} + pools = self.common._get_slo_workload_combinations(array_info) + self.assertTrue(len(pools) == 24) - @mock.patch.object( - rest.PowerMaxRest, 'get_vmax_model', - return_value=tpd.PowerMaxData.vmax_model_details['model']) - @mock.patch.object(rest.PowerMaxRest, 'get_workload_settings', - return_value=[]) - @mock.patch.object( - rest.PowerMaxRest, 'get_slo_list', - return_value=tpd.PowerMaxData.powermax_slo_details['sloId']) - def test_get_slo_workload_combinations_next_gen_vmax( - self, mck_slo, mck_wl, mck_model): + def test_get_slo_workload_combinations_afa_powermax(self): self.common.next_gen = True - finalarrayinfolist = self.common._get_slo_workload_combinations( - self.data.array_info_no_wl) - self.assertTrue(len(finalarrayinfolist) == 18) + self.common.array_model = 'VMAX250F' + array_info = {} + pools = self.common._get_slo_workload_combinations(array_info) + self.assertTrue(len(pools) == 28) + + def test_get_slo_workload_combinations_afa_hypermax(self): + self.common.next_gen = False + self.common.array_model = 'VMAX250F' + array_info = {} + pools = self.common._get_slo_workload_combinations(array_info) + self.assertTrue(len(pools) == 16) + + def test_get_slo_workload_combinations_hybrid(self): + self.common.next_gen = False + self.common.array_model = 'VMAX100K' + array_info = {} + pools = self.common._get_slo_workload_combinations(array_info) + self.assertTrue(len(pools) == 44) def test_get_slo_workload_combinations_failed(self): + self.common.array_model = 'xxxxxx' array_info = {} self.assertRaises( exception.VolumeBackendAPIException, diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py index 89a771620c9..241a36ed207 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py @@ -983,6 +983,27 @@ class PowerMaxMaskingTest(test.TestCase): utils.PowerMaxUtils.truncate_string.assert_called_once_with( 'DiamondDSS', 10) + @mock.patch.object(masking.PowerMaxMasking, + '_return_volume_to_fast_managed_group') + @mock.patch.object(masking.PowerMaxMasking, + '_clean_up_child_storage_group') + @mock.patch.object(masking.PowerMaxMasking, + 'move_volume_between_storage_groups') + @mock.patch.object(masking.PowerMaxMasking, + '_check_add_child_sg_to_parent_sg') + @mock.patch.object(rest.PowerMaxRest, 'get_storage_group', + return_value='Test_SG') + def test_pre_multiattach_pool_none_workload( + self, mck_get, mck_check, mck_move, mck_clean, mock_return): + with mock.patch.object(utils.PowerMaxUtils, 'truncate_string', + return_value='OptimdNONE'): + self.mask.pre_multiattach( + self.data.array, self.data.device_id, + self.data.masking_view_dict_multiattach, + self.data.extra_specs_optimized) + utils.PowerMaxUtils.truncate_string.assert_called_once_with( + 'OptimizedNONE', 10) + @mock.patch.object( rest.PowerMaxRest, 'get_storage_group_list', side_effect=[ diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index a33479ca9e3..5b8146054e4 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -347,58 +347,72 @@ class PowerMaxCommon(object): :raises: VolumeBackendAPIException: """ try: - array = array_info['SerialNumber'] - if self.failover: - array = self.active_backend_id - - slo_settings = self.rest.get_slo_list( - array, self.next_gen, self.array_model) - slo_list = [x for x in slo_settings - if x.lower() not in ['none', 'optimized']] - workload_settings = self.rest.get_workload_settings( - array, self.next_gen) - workload_settings.append('None') - slo_workload_set = set( - ['%(slo)s:%(workload)s' % {'slo': slo, - 'workload': workload} - for slo in slo_list for workload in workload_settings]) - slo_workload_set.add('None:None') + upgraded_afa = False + if self.array_model in utils.VMAX_HYBRID_MODELS: + sls = deepcopy(utils.HYBRID_SLS) + wls = deepcopy(utils.HYBRID_WLS) + elif self.array_model in utils.VMAX_AFA_MODELS: + wls = deepcopy(utils.AFA_WLS) + if not self.next_gen: + sls = deepcopy(utils.AFA_H_SLS) + else: + sls = deepcopy(utils.AFA_P_SLS) + upgraded_afa = True + elif self.array_model in utils.PMAX_MODELS: + sls, wls = deepcopy(utils.PMAX_SLS), deepcopy(utils.PMAX_WLS) + else: + raise exception.VolumeBackendAPIException( + message="Unable to determine array model.") if self.next_gen: - LOG.warning("Workloads have been deprecated for arrays " - "running PowerMax OS uCode level 5978 or higher. " - "Any supplied workloads will be treated as None " - "values. It is highly recommended to create a new " - "volume type without a workload specified.") - for slo in slo_list: - slo_workload_set.add(slo) - slo_workload_set.add('None') - slo_workload_set.add('Optimized') - slo_workload_set.add('Optimized:None') - # If array is 5978 or greater and a VMAX AFA add legacy SL/WL - # combinations - if any(self.array_model in x for x in - utils.VMAX_AFA_MODELS): - slo_workload_set.add('Diamond:OLTP') - slo_workload_set.add('Diamond:OLTP_REP') - slo_workload_set.add('Diamond:DSS') - slo_workload_set.add('Diamond:DSS_REP') - slo_workload_set.add('Diamond:None') + LOG.warning( + "Workloads have been deprecated for arrays running " + "PowerMax OS uCode level 5978 or higher. Any supplied " + "workloads will be treated as None values. It is " + "recommended to create a new volume type without a " + "workload specified.") - if not any(self.array_model in x for x in - utils.VMAX_AFA_MODELS): - slo_workload_set.add('Optimized:None') + # Add service levels: + pools = sls + # Array Specific SL/WL Combos + pools += ( + ['{}:{}'.format(x, y) for x in sls for y in wls + if x.lower() not in ['optimized', 'none']]) + # Add Optimized & None combinations + pools += ( + ['{}:{}'.format(x, y) for x in ['Optimized', 'NONE', 'None'] + for y in ['NONE', 'None']]) - finalarrayinfolist = [] - for sloWorkload in slo_workload_set: - temparray_info = array_info.copy() + if upgraded_afa: + # Cleanup is required here for service levels that were not + # present in AFA HyperMax but added for AFA PowerMax, we + # do not need these SL/WL combinations for backwards + # compatibility but we do for Diamond SL + afa_pool = list() + for p in pools: + try: + pl = p.split(':') + if (pl[0] not in [ + 'Platinum', 'Gold', 'Silver', 'Bronze']) or ( + pl[1] not in [ + 'OLTP', 'OLTP_REP', 'DSS', 'DSS_REP']): + afa_pool.append(p) + except IndexError: + # Pool has no workload present + afa_pool.append(p) + pools = afa_pool + + # Build array pool of SL/WL combinations + array_pool = list() + for pool in pools: + _array_info = array_info.copy() try: - slo, workload = sloWorkload.split(':') - temparray_info['SLO'] = slo - temparray_info['Workload'] = workload + slo, workload = pool.split(':') + _array_info['SLO'] = slo + _array_info['Workload'] = workload except ValueError: - temparray_info['SLO'] = sloWorkload - finalarrayinfolist.append(temparray_info) + _array_info['SLO'] = pool + array_pool.append(_array_info) except Exception as e: exception_message = (_( "Unable to get the SLO/Workload combinations from the array. " @@ -406,7 +420,7 @@ class PowerMaxCommon(object): LOG.error(exception_message) raise exception.VolumeBackendAPIException( message=exception_message) - return finalarrayinfolist + return array_pool def create_volume(self, volume): """Creates a EMC(PowerMax/VMAX) volume from a storage group. @@ -1483,8 +1497,8 @@ class PowerMaxCommon(object): other_maskedvols.append(devicedict) if len(other_maskedvols) > 0: LOG.debug("Volume is masked to a different host " - "than %(host)s - multiattach case.", - {'host': host}) + "than %(host)s - Live Migration or Multi-Attach " + "use case.", {'host': host}) is_multiattach = True else: diff --git a/cinder/volume/drivers/dell_emc/powermax/fc.py b/cinder/volume/drivers/dell_emc/powermax/fc.py index ab67dee17f1..86172039e6d 100644 --- a/cinder/volume/drivers/dell_emc/powermax/fc.py +++ b/cinder/volume/drivers/dell_emc/powermax/fc.py @@ -121,9 +121,10 @@ class PowerMaxFCDriver(san.SanDriver, driver.FibreChannelDriver): 4.1.3 - Retype attached replication fix (#1851371) 4.1.4 - Legacy volume not found fix (#1867163) 4.1.5 - Allowing for default volume type in group (#1866871) + 4.1.6 - Pools bug fix allowing 'None' variants (bug #1873253) """ - VERSION = "4.1.5" + VERSION = "4.1.6" # ThirdPartySystems wiki CI_WIKI_NAME = "EMC_VMAX_CI" diff --git a/cinder/volume/drivers/dell_emc/powermax/iscsi.py b/cinder/volume/drivers/dell_emc/powermax/iscsi.py index cf876d02b88..32f447e65b5 100644 --- a/cinder/volume/drivers/dell_emc/powermax/iscsi.py +++ b/cinder/volume/drivers/dell_emc/powermax/iscsi.py @@ -126,9 +126,10 @@ class PowerMaxISCSIDriver(san.SanISCSIDriver): 4.1.3 - Retype attached replication fix (#1851371) 4.1.4 - Legacy volume not found fix (#1867163) 4.1.5 - Allowing for default volume type in group (#1866871) + 4.1.6 - Pools bug fix allowing 'None' variants (bug #1873253) """ - VERSION = "4.1.5" + VERSION = "4.1.6" # ThirdPartySystems wiki CI_WIKI_NAME = "EMC_VMAX_CI" diff --git a/cinder/volume/drivers/dell_emc/powermax/masking.py b/cinder/volume/drivers/dell_emc/powermax/masking.py index cd398e691dc..abeb8778dfa 100644 --- a/cinder/volume/drivers/dell_emc/powermax/masking.py +++ b/cinder/volume/drivers/dell_emc/powermax/masking.py @@ -1662,7 +1662,7 @@ class PowerMaxMasking(object): split_pool = extra_specs['pool_name'].split('+') src_slo = split_pool[0] src_wl = split_pool[1] if len(split_pool) == 4 else 'NONE' - slo_wl_combo = self.utils.truncate_string(src_slo + src_wl, 10) + slo_wl_combo = self.utils.truncate_string(src_slo + src_wl.upper(), 10) for sg in sg_list.get('storageGroupId', []): if slo_wl_combo in sg: fast_source_sg_name = sg diff --git a/cinder/volume/drivers/dell_emc/powermax/utils.py b/cinder/volume/drivers/dell_emc/powermax/utils.py index 23627b3f33f..c48e65684c6 100644 --- a/cinder/volume/drivers/dell_emc/powermax/utils.py +++ b/cinder/volume/drivers/dell_emc/powermax/utils.py @@ -38,7 +38,6 @@ FC = 'fc' INTERVAL = 'interval' RETRIES = 'retries' VOLUME_ELEMENT_NAME_PREFIX = 'OS-' -VMAX_AFA_MODELS = ['VMAX250F', 'VMAX450F', 'VMAX850F', 'VMAX950F'] MAX_SRP_LENGTH = 16 TRUNCATE_5 = 5 TRUNCATE_27 = 27 @@ -110,6 +109,22 @@ POWERMAX_SERVICE_LEVEL = 'powermax_service_level' POWERMAX_PORT_GROUPS = 'powermax_port_groups' POWERMAX_SNAPVX_UNLINK_LIMIT = 'powermax_snapvx_unlink_limit' +# Array Models, Service Levels & Workloads +VMAX_HYBRID_MODELS = ['VMAX100K', 'VMAX200K', 'VMAX400K'] +VMAX_AFA_MODELS = ['VMAX250F', 'VMAX450F', 'VMAX850F', 'VMAX950F'] +PMAX_MODELS = ['PowerMax_2000', 'PowerMax_8000'] + +HYBRID_SLS = ['Diamond', 'Platinum', 'Gold', 'Silver', 'Bronze', 'Optimized', + 'None', 'NONE'] +HYBRID_WLS = ['OLTP', 'OLTP_REP', 'DSS', 'DSS_REP', 'NONE', 'None'] +AFA_H_SLS = ['Diamond', 'Optimized', 'None', 'NONE'] +AFA_P_SLS = ['Diamond', 'Platinum', 'Gold', 'Silver', 'Bronze', 'Optimized', + 'None', 'NONE'] +AFA_WLS = ['OLTP', 'OLTP_REP', 'DSS', 'DSS_REP', 'NONE', 'None'] +PMAX_SLS = ['Diamond', 'Platinum', 'Gold', 'Silver', 'Bronze', 'Optimized', + 'None', 'NONE'] +PMAX_WLS = ['NONE', 'None'] + class PowerMaxUtils(object): """Utility class for Rest based PowerMax volume drivers.