VMAX driver - QoS, replacing SMI-S with REST

In VMAX driver version 3.0, SMI-S has been replaced with unisphere
REST. This is porting QoS from SMIS to REST.
See original https://review.openstack.org/#/c/307502/ for more
details

Change-Id: Iba516767a465138474832d8de487886ecf9b305f
Partially-Implements: blueprint vmax-rest

cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py

@@ -24,6 +24,7 @@ import mock
 import requests
 import six
 
+from cinder import context
 from cinder import exception
 from cinder import test
 from cinder.tests.unit import fake_snapshot
@@ -38,7 +39,6 @@ from cinder.volume.drivers.dell_emc.vmax import utils
 from cinder.volume import volume_types
 from cinder.zonemanager import utils as fczm_utils
 
-
 CINDER_EMC_CONFIG_DIR = '/etc/cinder/'
 
 
@@ -113,7 +113,7 @@ class VMAXCommonData(object):
                       'hostlunid': 3}
 
     # cinder volume info
-    ctx = 'context'
+    ctx = context.RequestContext('admin', 'fake', True)
     provider_location = {'array': six.text_type(array),
                          'device_id': '00001'}
 
@@ -123,10 +123,15 @@ class VMAXCommonData(object):
     snap_location = {'snap_name': '12345',
                      'source_id': '00001'}
 
+    test_volume_type = fake_volume.fake_volume_type_obj(
+        context=ctx
+    )
+
     test_volume = fake_volume.fake_volume_obj(
         context=ctx, name='vol1', size=2, provider_auth=None,
         provider_location=six.text_type(provider_location),
-        host=fake_host, volume_type_id='1e5177e7-95e5-4a0f-b170-e45f4b469f6a')
+        volume_type=test_volume_type,
+        host=fake_host)
 
     test_clone_volume = fake_volume.fake_volume_obj(
         context=ctx, name='vol1', size=2, provider_auth=None,
@@ -685,9 +690,11 @@ class VMAXUtilsTest(test.TestCase):
         with mock.patch.object(volume_types, 'get_volume_type_extra_specs',
                                return_value={'specs'}) as type_mock:
             # path 1: volume_type_id not passed in
+            self.data.test_volume.volume_type_id = (
+                self.data.test_volume_type.id)
             self.utils.get_volumetype_extra_specs(self.data.test_volume)
             volume_types.get_volume_type_extra_specs.assert_called_once_with(
-                self.data.test_volume.volume_type_id)
+                self.data.test_volume_type.id)
             type_mock.reset_mock()
             # path 2: volume_type_id passed in
             self.utils.get_volumetype_extra_specs(self.data.test_volume, '123')
@@ -1925,6 +1932,43 @@ class VMAXRestTest(test.TestCase):
             array, source_id, tgt_only=True)
         self.assertEqual(ref_sessions, sessions)
 
+    def test_update_storagegroup_qos(self):
+        sg_qos = {"srp": self.data.srp, "num_of_vols": 2, "cap_gb": 2,
+                  "storageGroupId": "OS-QOS-SG",
+                  "slo": self.data.slo, "workload": self.data.workload,
+                  "hostIOLimit": {"host_io_limit_io_sec": "4000",
+                                  "dynamicDistribution": "Always",
+                                  "host_io_limit_mb_sec": "4000"}}
+        self.data.sg_details.append(sg_qos)
+        array = self.data.array
+        extra_specs = self.data.extra_specs
+        extra_specs['qos'] = {
+            'maxIOPS': '4000', 'DistributionType': 'Always'}
+        return_value = self.rest.update_storagegroup_qos(
+            array, "OS-QOS-SG", extra_specs)
+        self.assertEqual(False, return_value)
+        extra_specs['qos'] = {
+            'DistributionType': 'onFailure', 'maxMBPS': '4000'}
+        return_value = self.rest.update_storagegroup_qos(
+            array, "OS-QOS-SG", extra_specs)
+        self.assertTrue(return_value)
+
+    def test_update_storagegroup_qos_exception(self):
+        array = self.data.array
+        storage_group = self.data.defaultstoragegroup_name
+        extra_specs = self.data.extra_specs
+        extra_specs['qos'] = {
+            'maxIOPS': '4000', 'DistributionType': 'Wrong', 'maxMBPS': '4000'}
+        with mock.patch.object(self.rest, 'check_status_code_success',
+                               side_effect=[None, None, None, Exception]):
+            self.assertRaises(exception.VolumeBackendAPIException,
+                              self.rest.update_storagegroup_qos, array,
+                              storage_group, extra_specs)
+            extra_specs['qos']['DistributionType'] = 'Always'
+            return_value = self.rest.update_storagegroup_qos(
+                array, "OS-QOS-SG", extra_specs)
+            self.assertFalse(return_value)
+
 
 class VMAXProvisionTest(test.TestCase):
     def setUp(self):
@@ -2477,7 +2521,7 @@ class VMAXCommonTest(test.TestCase):
 
     def test_set_config_file_and_get_extra_specs(self):
         volume = self.data.test_volume
-        extra_specs, config_file = (
+        extra_specs, config_file, qos_specs = (
             self.common._set_config_file_and_get_extra_specs(volume))
         self.assertEqual(self.data.vol_type_extra_specs, extra_specs)
         self.assertEqual(self.fake_xml, config_file)
@@ -2487,7 +2531,7 @@ class VMAXCommonTest(test.TestCase):
         ref_config = '/etc/cinder/cinder_dell_emc_config.xml'
         with mock.patch.object(self.utils, 'get_volumetype_extra_specs',
                                return_value=None):
-            extra_specs, config_file = (
+            extra_specs, config_file, qos_specs = (
                 self.common._set_config_file_and_get_extra_specs(volume))
             self.assertIsNone(extra_specs)
             self.assertEqual(ref_config, config_file)

cinder/volume/drivers/dell_emc/vmax/common.py

@@ -650,15 +650,19 @@ class VMAXCommon(object):
         :returns: dict -- the extra specs dict
         :returns: string -- configuration file
         """
+        qos_specs = {}
         extra_specs = self.utils.get_volumetype_extra_specs(
             volume, volume_type_id)
+        if hasattr(volume, "volume_type") and (
+                volume.volume_type and volume.volume_type.qos_specs):
+            qos_specs = volume.volume_type.qos_specs
         config_group = None
         # If there are no extra specs then the default case is assumed.
         if extra_specs:
             config_group = self.configuration.config_group
         config_file = self._register_config_file_from_config_group(
             config_group)
-        return extra_specs, config_file
+        return extra_specs, config_file, qos_specs
 
     def _find_device_on_array(self, volume, extra_specs):
         """Given the volume get the VMAX device Id.
@@ -812,7 +816,7 @@ class VMAXCommon(object):
         :raises VolumeBackendAPIException:
         """
         try:
-            extra_specs, config_file = (
+            extra_specs, config_file, qos_specs = (
                 self._set_config_file_and_get_extra_specs(
                     volume, volume_type_id))
             array_info = self.utils.parse_file_to_get_array_map(
@@ -826,6 +830,9 @@ class VMAXCommon(object):
             self.rest.set_rest_credentials(array_info)
 
             extra_specs = self._set_vmax_extra_specs(extra_specs, array_info)
+            if (qos_specs and qos_specs.specs
+                    and qos_specs.consumer != "front-end"):
+                extra_specs['qos'] = qos_specs.specs
         except Exception:
             exception_message = (_(
                 "Unable to get configuration information necessary to "
@@ -1151,7 +1158,7 @@ class VMAXCommon(object):
         LOG.debug("Retries are set at: %(retries)s.",
                   {'retries': self.retries})
 
-        # set pool_name slo and workload
+        # Set pool_name slo and workload
         if 'pool_name' in extra_specs:
             pool_name = extra_specs['pool_name']
         else:
@@ -1173,7 +1180,7 @@ class VMAXCommon(object):
         pool_details = pool_name.split('+')
         slo_from_extra_spec = pool_details[0]
         workload_from_extra_spec = pool_details[1]
-        # standardize slo and workload 'NONE' naming conventions
+        # Standardize slo and workload 'NONE' naming conventions
         if workload_from_extra_spec.lower() == 'none':
             workload_from_extra_spec = 'NONE'
         if slo_from_extra_spec.lower() == 'none':

cinder/volume/drivers/dell_emc/vmax/fc.py

@@ -77,6 +77,7 @@ class VMAXFCDriver(driver.FibreChannelDriver):
               - rename and restructure driver (bp vmax-rename-dell-emc)
         3.0.0 - REST based driver
               - Retype (storage-assisted migration)
+              - QoS support
     """
 
     VERSION = "3.0.0"

cinder/volume/drivers/dell_emc/vmax/iscsi.py

@@ -82,6 +82,7 @@ class VMAXISCSIDriver(driver.ISCSIDriver):
               - rename and restructure driver (bp vmax-rename-dell-emc)
         3.0.0 - REST based driver
               - Retype (storage-assisted migration)
+              - QoS support
     """
 
     VERSION = "3.0.0"

cinder/volume/drivers/dell_emc/vmax/masking.py

@@ -365,6 +365,12 @@ class VMAXMasking(object):
                    % {'storagegroup_name': storagegroup_name,
                       'volume_name': masking_view_dict[utils.VOL_NAME]})
             LOG.error(msg)
+
+        # If qos exists, update storage group to reflect qos parameters
+        if 'qos' in extra_specs:
+            self.rest.update_storagegroup_qos(
+                serial_number, storagegroup_name, extra_specs)
+
         return msg
 
     def _check_existing_storage_group(
@@ -1244,6 +1250,10 @@ class VMAXMasking(object):
                 LOG.error(exception_message)
                 raise exception.VolumeBackendAPIException(
                     data=exception_message)
+        # If qos exists, update storage group to reflect qos parameters
+        if 'qos' in extra_specs:
+            self.rest.update_storagegroup_qos(
+                serial_number, storagegroup_name, extra_specs)
 
         return storagegroup_name
 

cinder/volume/drivers/dell_emc/vmax/rest.py

@@ -709,6 +709,80 @@ class VMAXRest(object):
 
         self.wait_for_job('Remove vol from sg', status_code, job, extra_specs)
 
+    def update_storagegroup_qos(self, array, storage_group_name, extra_specs):
+        """Update the storagegroupinstance with qos details.
+
+        If maxIOPS or maxMBPS is in extra_specs, then DistributionType can be
+        modified in addition to maxIOPS or/and maxMBPS
+        If maxIOPS or maxMBPS is NOT in extra_specs, we check to see if
+        either is set in StorageGroup. If so, then DistributionType can be
+        modified
+        :param array: the array serial number
+        :param storage_group_name: the storagegroup instance name
+        :param extra_specs: extra specifications
+        :returns: bool, True if updated, else False
+        """
+        return_value = False
+        sg_details = self.get_storage_group(array, storage_group_name)
+        sg_qos_details = None
+        sg_maxiops = None
+        sg_maxmbps = None
+        sg_distribution_type = None
+        maxiops = "nolimit"
+        maxmbps = "nolimit"
+        distribution_type = "never"
+        propertylist = []
+        try:
+            sg_qos_details = sg_details['hostIOLimit']
+            sg_maxiops = sg_qos_details['host_io_limit_io_sec']
+            sg_maxmbps = sg_qos_details['host_io_limit_mb_sec']
+            sg_distribution_type = sg_qos_details['dynamicDistribution']
+        except KeyError:
+            LOG.debug("Unable to get storage group QoS details.")
+        if 'maxIOPS' in extra_specs.get('qos'):
+            maxiops = extra_specs['qos']['maxIOPS']
+            if maxiops != sg_maxiops:
+                propertylist.append(maxiops)
+        if 'maxMBPS' in extra_specs.get('qos'):
+            maxmbps = extra_specs['qos']['maxMBPS']
+            if maxmbps != sg_maxmbps:
+                propertylist.append(maxmbps)
+        if 'DistributionType' in extra_specs.get('qos') and (
+                propertylist or sg_qos_details):
+            dynamic_list = ['never', 'onfailure', 'always']
+            if (extra_specs.get('qos').get('DistributionType').lower() not
+                    in dynamic_list):
+                exception_message = (_(
+                    "Wrong Distribution type value %(dt)s entered. "
+                    "Please enter one of: %(dl)s") %
+                    {'dt': extra_specs.get('qos').get('DistributionType'),
+                     'dl': dynamic_list
+                     })
+                LOG.error(exception_message)
+                raise exception.VolumeBackendAPIException(
+                    data=exception_message)
+            else:
+                distribution_type = extra_specs['qos']['DistributionType']
+                if distribution_type != sg_distribution_type:
+                    propertylist.append(distribution_type)
+        if propertylist:
+            payload = {"editStorageGroupActionParam": {
+                "setHostIOLimitsParam": {
+                    "host_io_limit_io_sec": maxiops,
+                    "host_io_limit_mb_sec": maxmbps,
+                    "dynamicDistribution": distribution_type}}}
+            status_code, message = (
+                self.modify_storage_group(array, storage_group_name, payload))
+            try:
+                self.check_status_code_success('Add qos specs', status_code,
+                                               message)
+                return_value = True
+            except Exception as e:
+                LOG.error("Error setting qos. Exception received was: "
+                          "%(e)s", {'e': e})
+                return_value = False
+        return return_value
+
     def get_vmax_default_storage_group(self, array, srp, slo, workload):
         """Get the default storage group.
 

releasenotes/notes/vmax-rest-qos-6bb4073b92c932c6.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    Adding Qos functionality to VMAX driver version 3.0.