IBM Storwize driver: Add local variable assignment to "ctxt"

* The method get_vdisk_params in helpers.py is missing a local variable
assignment for "ctxt", causing "UnboundLocalError: local variable
'ctxt' referenced before assignment. Adding the assignment should
resolve this issue.

* Add the unit tests coverage for get_vdisk_params.

Change-Id: I564b1ef8cd1b6504d5ac8c9af0bb11bf29767d9a
closes-bug: #1379654
(cherry picked from commit afcbfe053c9165ab84af30c8e663dfaee2a79e81)

diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py
index cbe681540931e44a4744b560dbe7463f56859634..b6838179f310b739358669369b4f006bafd67f23 100644 (file)
--- a/cinder/tests/test_storwize_svc.py
+++ b/cinder/tests/test_storwize_svc.py
@@ -39,6 +39,7 @@ from cinder.volume import configuration as conf
 from cinder.volume.drivers.ibm import storwize_svc
 from cinder.volume.drivers.ibm.storwize_svc import helpers
 from cinder.volume.drivers.ibm.storwize_svc import ssh
+from cinder.volume import qos_specs
 from cinder.volume import volume_types
 
 LOG = logging.getLogger(__name__)
@@ -1513,7 +1514,8 @@ class StorwizeSVCDriverTestCase(test.TestCase):
                                'storwize_svc_flashcopy_timeout': 20,
                                # Test ignore capitalization
                                'storwize_svc_connection_protocol': 'iScSi',
-                               'storwize_svc_multipath_enabled': False}
+                               'storwize_svc_multipath_enabled': False,
+                               'storwize_svc_allow_tenant_qos': True}
             wwpns = [str(random.randint(0, 9999999999999999)).zfill(16),
                      str(random.randint(0, 9999999999999999)).zfill(16)]
             initiator = 'test.initiator.%s' % str(random.randint(10000, 99999))
@@ -1535,6 +1537,7 @@ class StorwizeSVCDriverTestCase(test.TestCase):
                                # Test ignore capitalization
                                'storwize_svc_connection_protocol': 'iScSi',
                                'storwize_svc_multipath_enabled': False,
+                               'storwize_svc_allow_tenant_qos': True,
                                'ssh_conn_timeout': 0}
             config_group = self.driver.configuration.config_group
             self.driver.configuration.set_override('rootwrap_config',
@@ -1697,7 +1700,9 @@ class StorwizeSVCDriverTestCase(test.TestCase):
                'protocol': 'iSCSI',
                'multipath': False,
                'iogrp': 0,
-               'qos': None}
+               'qos': None,
+               'replication': False,
+               'stretched_cluster': None}
         return opt
 
     @mock.patch.object(helpers.StorwizeHelpers, 'add_vdisk_qos')
@@ -2442,6 +2447,107 @@ class StorwizeSVCDriverTestCase(test.TestCase):
         self.driver.migrate_volume(ctxt, volume, host)
         self._delete_volume(volume)
 
+    def test_storwize_svc_get_vdisk_params(self):
+        self.driver.do_setup(None)
+        fake_qos = {'qos:IOThrottling': 5000}
+        expected_qos = {'IOThrottling': 5000}
+        fake_opts = self._get_default_opts()
+        # The parameters retured should be the same to the default options,
+        # if the QoS is empty.
+        vol_type_empty_qos = self._create_volume_type_qos(True, None)
+        type_id = vol_type_empty_qos['id']
+        params = self.driver._get_vdisk_params(type_id,
+                                               volume_type=vol_type_empty_qos,
+                                               volume_metadata=None)
+        self.assertEqual(fake_opts, params)
+        volume_types.destroy(self.ctxt, type_id)
+
+        # If the QoS is set via the qos association with the volume type,
+        # qos value should be set in the retured parameters.
+        vol_type_qos = self._create_volume_type_qos(False, fake_qos)
+        type_id = vol_type_qos['id']
+        # If type_id is not none and volume_type is none, it should work fine.
+        params = self.driver._get_vdisk_params(type_id, volume_type=None,
+                                               volume_metadata=None)
+        self.assertEqual(expected_qos, params['qos'])
+        # If type_id is not none and volume_type is not none, it should
+        # work fine.
+        params = self.driver._get_vdisk_params(type_id,
+                                               volume_type=vol_type_qos,
+                                               volume_metadata=None)
+        self.assertEqual(expected_qos, params['qos'])
+        # If type_id is none and volume_type is not none, it should work fine.
+        params = self.driver._get_vdisk_params(None,
+                                               volume_type=vol_type_qos,
+                                               volume_metadata=None)
+        self.assertEqual(expected_qos, params['qos'])
+        # If both type_id and volume_type are none, no qos will be returned
+        # in the parameter.
+        params = self.driver._get_vdisk_params(None, volume_type=None,
+                                               volume_metadata=None)
+        self.assertEqual(None, params['qos'])
+        qos_spec = volume_types.get_volume_type_qos_specs(type_id)
+        volume_types.destroy(self.ctxt, type_id)
+        qos_specs.delete(self.ctxt, qos_spec['qos_specs']['id'])
+
+        # If the QoS is set via the extra specs in the volume type,
+        # qos value should be set in the retured parameters.
+        vol_type_qos = self._create_volume_type_qos(True, fake_qos)
+        type_id = vol_type_qos['id']
+        # If type_id is not none and volume_type is none, it should work fine.
+        params = self.driver._get_vdisk_params(type_id, volume_type=None,
+                                               volume_metadata=None)
+        self.assertEqual(expected_qos, params['qos'])
+        # If type_id is not none and volume_type is not none,
+        # it should work fine.
+        params = self.driver._get_vdisk_params(type_id,
+                                               volume_type=vol_type_qos,
+                                               volume_metadata=None)
+        self.assertEqual(expected_qos, params['qos'])
+        # If type_id is none and volume_type is not none,
+        # it should work fine.
+        params = self.driver._get_vdisk_params(None,
+                                               volume_type=vol_type_qos,
+                                               volume_metadata=None)
+        self.assertEqual(expected_qos, params['qos'])
+        # If both type_id and volume_type are none, no qos will be returned
+        # in the parameter.
+        params = self.driver._get_vdisk_params(None, volume_type=None,
+                                               volume_metadata=None)
+        self.assertEqual(None, params['qos'])
+        volume_types.destroy(self.ctxt, type_id)
+
+        # If the QoS is set in the volume metadata,
+        # qos value should be set in the retured parameters.
+        metadata = [{'key': 'qos:IOThrottling', 'value': 4000}]
+        expected_qos_metadata = {'IOThrottling': 4000}
+        params = self.driver._get_vdisk_params(None, volume_type=None,
+                                               volume_metadata=metadata)
+        self.assertEqual(expected_qos_metadata, params['qos'])
+
+        # If the QoS is set both in the metadata and the volume type, the one
+        # in the volume type will take effect.
+        vol_type_qos = self._create_volume_type_qos(True, fake_qos)
+        type_id = vol_type_qos['id']
+        params = self.driver._get_vdisk_params(type_id, volume_type=None,
+                                               volume_metadata=metadata)
+        self.assertEqual(expected_qos, params['qos'])
+        volume_types.destroy(self.ctxt, type_id)
+
+        # If the QoS is set both via the qos association and the
+        # extra specs, the one from the qos association will take effect.
+        fake_qos_associate = {'qos:IOThrottling': 6000}
+        expected_qos_associate = {'IOThrottling': 6000}
+        vol_type_qos = self._create_volume_type_qos_both(fake_qos,
+                                                         fake_qos_associate)
+        type_id = vol_type_qos['id']
+        params = self.driver._get_vdisk_params(type_id, volume_type=None,
+                                               volume_metadata=None)
+        self.assertEqual(expected_qos_associate, params['qos'])
+        qos_spec = volume_types.get_volume_type_qos_specs(type_id)
+        volume_types.destroy(self.ctxt, type_id)
+        qos_specs.delete(self.ctxt, qos_spec['qos_specs']['id'])
+
     @mock.patch.object(helpers.StorwizeHelpers, 'disable_vdisk_qos')
     @mock.patch.object(helpers.StorwizeHelpers, 'update_vdisk_qos')
     def test_storwize_svc_retype_no_copy(self, update_vdisk_qos,
@@ -3095,6 +3201,29 @@ class StorwizeSVCDriverTestCase(test.TestCase):
 
         self.assertEqual(term_data, term_ret)
 
+    def _create_volume_type_qos(self, extra_specs, fake_qos):
+        # Generate a QoS volume type for volume.
+        if extra_specs:
+            spec = fake_qos
+            type_ref = volume_types.create(self.ctxt, "qos_extra_specs", spec)
+        else:
+            type_ref = volume_types.create(self.ctxt, "qos_associate", None)
+            if fake_qos:
+                qos_ref = qos_specs.create(self.ctxt, 'qos-specs', fake_qos)
+                qos_specs.associate_qos_with_type(self.ctxt, qos_ref['id'],
+                                                  type_ref['id'])
+
+        qos_type = volume_types.get_volume_type(self.ctxt, type_ref['id'])
+        return qos_type
+
+    def _create_volume_type_qos_both(self, fake_qos, fake_qos_associate):
+        type_ref = volume_types.create(self.ctxt, "qos_extra_specs", fake_qos)
+        qos_ref = qos_specs.create(self.ctxt, 'qos-specs', fake_qos_associate)
+        qos_specs.associate_qos_with_type(self.ctxt, qos_ref['id'],
+                                          type_ref['id'])
+        qos_type = volume_types.get_volume_type(self.ctxt, type_ref['id'])
+        return qos_type
+
     def _create_replication_volume_type(self, enable):
         # Generate a volume type for volume repliation.
         if enable:
@@ -3335,6 +3464,3 @@ class StorwizeHelpersTestCase(test.TestCase):
         with mock.patch.object(ssh.StorwizeSSH, 'lslicense') as lslicense:
             lslicense.return_value = fake_license
             self.assertTrue(self.helpers.compression_enabled())
-
-    def test_get_vdisk_params(self):
-        pass

diff --git a/cinder/volume/drivers/ibm/storwize_svc/helpers.py b/cinder/volume/drivers/ibm/storwize_svc/helpers.py
index 6f8540211042ce3791461510780db86c89afaa14..23248a5099f3b76e52793a6b855d2de82eed1fce 100644 (file)
--- a/cinder/volume/drivers/ibm/storwize_svc/helpers.py
+++ b/cinder/volume/drivers/ibm/storwize_svc/helpers.py
@@ -547,8 +547,8 @@ class StorwizeHelpers(object):
         Takes volume type and defaults from config options into account.
         """
         opts = self.build_default_opts(config)
+        ctxt = context.get_admin_context()
         if volume_type is None and type_id is not None:
-            ctxt = context.get_admin_context()
             volume_type = volume_types.get_volume_type(ctxt, type_id)
         if volume_type:
             qos_specs_id = volume_type.get('qos_specs_id')