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 aef7fe44c1)
2020-04-16 11:22:03 +00:00 · 2020-04-16 11:22:03 +00:00 · 20a55fc0ca
parent e7798968f1
commit 20a55fc0ca
8 changed files with 133 additions and 97 deletions
--- 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
--- 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,
--- 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=[
--- 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:
--- 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"
--- 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"
--- 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
--- 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.