From 000da0f8fc2c0f6b69d9c6f8d32518c36794b19c Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Tue, 15 Jan 2019 04:23:08 +0900 Subject: [PATCH] api.cinder: refactor microversioning logic There are sevral similar logics to handle microversioning in Cinder API wrapper. This commit refactors them and introduces _cinderclient_with_features() function. The parent commit to fix bug 1810309 does not introduce this logic to make it easy to backport the fix. Change-Id: I378e58c7a80e2d00481a582eb3fa449f51c3612a Related-Bug: #1810309 --- openstack_dashboard/api/cinder.py | 53 +++++++++++-------- openstack_dashboard/api/microversions.py | 29 +++++++--- .../test/unit/api/test_cinder.py | 6 +-- 3 files changed, 56 insertions(+), 32 deletions(-) diff --git a/openstack_dashboard/api/cinder.py b/openstack_dashboard/api/cinder.py index d283bff06a..0c52eaf910 100644 --- a/openstack_dashboard/api/cinder.py +++ b/openstack_dashboard/api/cinder.py @@ -259,17 +259,33 @@ def get_microversion(request, features): else: return None min_ver, max_ver = cinder_client.get_server_version(cinder_url) - return (microversions.get_microversion_for_features( - 'cinder', features, api_versions.APIVersion, min_ver, max_ver)) + return microversions.get_microversion_for_features( + 'cinder', features, api_versions.APIVersion, min_ver, max_ver) -def _cinderclient_with_generic_groups(request): - version = get_microversion(request, 'groups') +def _cinderclient_with_features(request, features, + raise_exc=False, message=False): + version = get_microversion(request, features) + if version is None: + if message: + versions = microversions.get_requested_versions('cinder', features) + if message is True: + message = ('Insufficient microversion for cinder feature(s) ' + '%(features)s. One of the following API ' + 'microversion(s) is required: %(versions).') + LOG.warning(message, + {'features': features, 'versions': versions}) + if raise_exc: + raise microversions.MicroVersionNotFound(features) if version is not None: version = version.get_string() return cinderclient(request, version=version) +def _cinderclient_with_generic_groups(request): + return _cinderclient_with_features(request, 'groups') + + def version_get(): api_version = VERSIONS.get_active_version() return api_version['version'] @@ -986,23 +1002,15 @@ def qos_specs_list(request): return [QosSpecs(s) for s in qos_spec_list(request)] -def _cinderclient_with_limits_project_id_query(request): - version = get_microversion(request, ['limits_project_id_query']) - if version is None: - cinder_microversions = microversions.MICROVERSION_FEATURES['cinder'] - LOG.warning('Insufficient microversion for GET limits with ' - 'project_id query. One of the following API micro ' - 'version is required: %s', - cinder_microversions['limits_project_id_query']) - else: - version = version.get_string() - return cinderclient(request, version=version) - - @profiler.trace @memoized def tenant_absolute_limits(request, tenant_id=None): - _cinderclient = _cinderclient_with_limits_project_id_query(request) + _cinderclient = _cinderclient_with_features( + request, ['limits_project_id_query'], + message=('Insufficient microversion for GET limits with ' + 'project_id query. One of the following API micro ' + 'version is required: %(versions)s. ' + 'This causes bug 1810309 on updating quotas.')) limits = _cinderclient.limits.get(tenant_id=tenant_id).absolute limits_dict = {} for limit in limits: @@ -1097,11 +1105,12 @@ def pool_list(request, detailed=False): @profiler.trace def message_list(request, search_opts=None): - version = get_microversion(request, ['message_list']) - if version is None: - LOG.warning("insufficient microversion for message_list") + try: + c_client = _cinderclient_with_features(request, ['message_list'], + raise_exc=True, message=True) + except microversions.MicroVersionNotFound: + LOG.warning("Insufficient microversion for message_list") return [] - c_client = cinderclient(request, version=version) return c_client.messages.list(search_opts) diff --git a/openstack_dashboard/api/microversions.py b/openstack_dashboard/api/microversions.py index a1091eabff..e28bbe03dd 100644 --- a/openstack_dashboard/api/microversions.py +++ b/openstack_dashboard/api/microversions.py @@ -45,12 +45,15 @@ MICROVERSION_FEATURES = { } -# NOTE(robcresswell): Since each client implements their own wrapper class for -# API objects, we'll need to allow that to be passed in. In the future this -# should be replaced by some common handling in Oslo. -def get_microversion_for_features(service, features, wrapper_class, - min_ver, max_ver): - """Retrieves that highest known functional microversion for features""" +class MicroVersionNotFound(Exception): + def __init__(self, features): + self.features = features + + def __str__(self): + return "Insufficient microversion for %s" % self.features + + +def get_requested_versions(service, features): if not features: return None # Convert a single feature string into a list for backward compatibility. @@ -68,10 +71,22 @@ def get_microversion_for_features(service, features, wrapper_class, feature_versions &= set(service_features[feature]) if not feature_versions: return None - # Sort version candidates from larger versins feature_versions = sorted(feature_versions, reverse=True, key=lambda v: [int(i) for i in v.split('.')]) + return feature_versions + + +# NOTE(robcresswell): Since each client implements their own wrapper class for +# API objects, we'll need to allow that to be passed in. In the future this +# should be replaced by some common handling in Oslo. +def get_microversion_for_features(service, features, wrapper_class, + min_ver, max_ver): + """Retrieves that highest known functional microversion for features""" + feature_versions = get_requested_versions(service, features) + if not feature_versions: + return None + for version in feature_versions: microversion = wrapper_class(version) if microversion.matches(min_ver, max_ver): diff --git a/openstack_dashboard/test/unit/api/test_cinder.py b/openstack_dashboard/test/unit/api/test_cinder.py index 5805824ed0..70531c4340 100644 --- a/openstack_dashboard/test/unit/api/test_cinder.py +++ b/openstack_dashboard/test/unit/api/test_cinder.py @@ -394,8 +394,7 @@ class CinderApiTests(test.APIMockTestCase): qos_associations_mock.assert_called_once_with(qos_specs_only_one[0].id) self.assertEqual(associate_spec, qos_specs_only_one[0].name) - @mock.patch.object(api.cinder, - '_cinderclient_with_limits_project_id_query') + @mock.patch.object(api.cinder, '_cinderclient_with_features') def test_absolute_limits_with_negative_values(self, mock_cinderclient): values = {"maxTotalVolumes": -1, "totalVolumesUsed": -1} expected_results = {"maxTotalVolumes": float("inf"), @@ -422,7 +421,8 @@ class CinderApiTests(test.APIMockTestCase): self.assertEqual(expected_results[key], ret_val[key]) mock_limit.assert_called_once() - mock_cinderclient.assert_called_once_with(self.request) + mock_cinderclient.assert_called_once_with( + self.request, ['limits_project_id_query'], message=test.IsA(str)) @mock.patch.object(api.cinder, 'cinderclient') def test_pool_list(self, mock_cinderclient):