From 0905c41f07b426fa50822c826333b711b659bd16 Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Sun, 7 Oct 2018 21:01:42 +0000 Subject: [PATCH] Support API version discovery This commit adds version discovery when user doesn't explicitly request a version or request the latest version. The latest version means the most recently version between server and client. Change-Id: I17be26342add9e9f4e3c384cc4aad613902e04a9 Implements: blueprint api-version-negotiation --- zunclient/api_versions.py | 59 ++++++++++++++- zunclient/client.py | 5 -- zunclient/osc/plugin.py | 52 +++++++++---- zunclient/shell.py | 42 ++++++++++- zunclient/tests/unit/osc/test_plugin.py | 5 +- zunclient/tests/unit/test_api_versions.py | 88 +++++++++++++++++++++- zunclient/tests/unit/test_shell.py | 28 +++++-- zunclient/tests/unit/v1/shell_test_base.py | 5 ++ zunclient/v1/versions.py | 6 ++ 9 files changed, 256 insertions(+), 34 deletions(-) diff --git a/zunclient/api_versions.py b/zunclient/api_versions.py index 29cb31e4..01550c21 100644 --- a/zunclient/api_versions.py +++ b/zunclient/api_versions.py @@ -32,7 +32,7 @@ HEADER_NAME = "OpenStack-API-Version" SERVICE_TYPE = "container" MIN_API_VERSION = '1.1' MAX_API_VERSION = '1.25' -DEFAULT_API_VERSION = MAX_API_VERSION +DEFAULT_API_VERSION = '1.latest' _SUBSTITUTIONS = {} @@ -231,13 +231,68 @@ def get_api_version(version_string): """Returns checked APIVersion object""" version_string = str(version_string) if strutils.is_int_like(version_string): - version_string = "%s.0" % version_string + version_string = "%s.latest" % version_string api_version = APIVersion(version_string) check_major_version(api_version) return api_version +def _get_server_version_range(client): + version = client.versions.get_current() + + if not hasattr(version, 'max_version') or not version.max_version: + return APIVersion(), APIVersion() + + return APIVersion(version.min_version), APIVersion(version.max_version) + + +def discover_version(client, requested_version): + server_start_version, server_end_version = _get_server_version_range( + client) + + if (not requested_version.is_latest() and + requested_version != APIVersion('1.1')): + if server_start_version.is_null() and server_end_version.is_null(): + raise exceptions.UnsupportedVersion( + _("Server doesn't support microversions")) + if not requested_version.matches(server_start_version, + server_end_version): + raise exceptions.UnsupportedVersion( + _("The specified version isn't supported by server. The valid " + "version range is '%(min)s' to '%(max)s'") % { + "min": server_start_version.get_string(), + "max": server_end_version.get_string()}) + return requested_version + + min_version = APIVersion(MIN_API_VERSION) + max_version = APIVersion(MAX_API_VERSION) + if server_start_version.is_null() and server_end_version.is_null(): + return APIVersion("1.1") + elif min_version > server_end_version: + raise exceptions.UnsupportedVersion( + _("Server version is too old. The client valid version range is " + "'%(client_min)s' to '%(client_max)s'. The server valid version " + "range is '%(server_min)s' to '%(server_max)s'.") % { + 'client_min': min_version.get_string(), + 'client_max': max_version.get_string(), + 'server_min': server_start_version.get_string(), + 'server_max': server_end_version.get_string()}) + elif max_version < server_start_version: + raise exceptions.UnsupportedVersion( + _("Server version is too new. The client valid version range is " + "'%(client_min)s' to '%(client_max)s'. The server valid version " + "range is '%(server_min)s' to '%(server_max)s'.") % { + 'client_min': min_version.get_string(), + 'client_max': max_version.get_string(), + 'server_min': server_start_version.get_string(), + 'server_max': server_end_version.get_string()}) + elif max_version <= server_end_version: + return max_version + elif server_end_version < max_version: + return server_end_version + + def update_headers(headers, api_version): """Set microversion headers if api_version is not null""" diff --git a/zunclient/client.py b/zunclient/client.py index 363296f2..2717b59a 100644 --- a/zunclient/client.py +++ b/zunclient/client.py @@ -17,8 +17,6 @@ import warnings from oslo_utils import importutils from zunclient import api_versions -from zunclient import exceptions -from zunclient.i18n import _ osprofiler_profiler = importutils.try_import("osprofiler.profiler") @@ -28,9 +26,6 @@ def _get_client_class_and_version(version): version = api_versions.get_api_version(version) else: api_versions.check_major_version(version) - if version.is_latest(): - raise exceptions.UnsupportedVersion( - _('The version should be explicit, not latest.')) return version, importutils.import_class( 'zunclient.v%s.client.Client' % version.ver_major) diff --git a/zunclient/osc/plugin.py b/zunclient/osc/plugin.py index 955f0696..7517e3dc 100644 --- a/zunclient/osc/plugin.py +++ b/zunclient/osc/plugin.py @@ -22,26 +22,36 @@ LOG = logging.getLogger(__name__) DEFAULT_CONTAINER_API_VERSION = api_versions.DEFAULT_API_VERSION API_VERSION_OPTION = "os_container_api_version" API_NAME = "container" -LAST_KNOWN_API_VERSION = int(DEFAULT_CONTAINER_API_VERSION.split('.')[1]) +CLIENT_CLASS = 'zunclient.v1.client.Client' +LAST_KNOWN_API_VERSION = int(api_versions.MAX_API_VERSION.split('.')[1]) API_VERSIONS = { - '1.%d' % i: 'zunclient.v1.client.Client' + '1.%d' % i: CLIENT_CLASS for i in range(1, LAST_KNOWN_API_VERSION + 1) } -API_VERSIONS['1'] = API_VERSIONS[DEFAULT_CONTAINER_API_VERSION] +API_VERSIONS['1'] = CLIENT_CLASS def make_client(instance): """Returns a zun service client""" + requested_api_version = instance._api_version[API_NAME] + zun_client = utils.get_client_class( API_NAME, - instance._api_version[API_NAME], + requested_api_version, API_VERSIONS) LOG.debug("Instantiating zun client: {0}".format( zun_client)) - # TODO(hongbin): Instead of hard-coding api-version to 'latest', it is - # better to read micro-version from CLI (bug #1701939). - api_version = api_versions.get_api_version(instance._api_version[API_NAME]) + api_version = api_versions.get_api_version(requested_api_version) + if api_version.is_latest(): + client = zun_client( + region_name=instance._region_name, + session=instance.session, + service_type='container', + api_version=api_versions.APIVersion("1.1"), + ) + api_version = api_versions.discover_version(client, api_version) + client = zun_client( region_name=instance._region_name, session=instance.session, @@ -56,26 +66,38 @@ def build_option_parser(parser): parser.add_argument( '--os-container-api-version', metavar='', - default=utils.env( - 'OS_CONTAINER_API_VERSION', - default=DEFAULT_CONTAINER_API_VERSION), + default=_get_environment_version(DEFAULT_CONTAINER_API_VERSION), action=ReplaceLatestVersion, choices=sorted( API_VERSIONS, - key=lambda k: [int(x) for x in k.split('.')]) + ['latest'], + key=lambda k: [int(x) for x in k.split('.')]) + ['1.latest'], help=("Container API version, default={0}" "(Env:OS_CONTAINER_API_VERSION)").format( DEFAULT_CONTAINER_API_VERSION)) return parser +def _get_environment_version(default): + env_value = utils.env('OS_CONTAINER_API_VERSION') or default + latest = env_value == '1.latest' + if latest: + # NOTE(hongbin): '1.latest' means enabling negotiation of the + # latest version between server and client but due to how OSC works + # we cannot just add "1.latest" to the list of supported versions. + # Use '1' in this case. + env_value = '1' + return env_value + + class ReplaceLatestVersion(argparse.Action): """Replaces `latest` keyword by last known version.""" def __call__(self, parser, namespace, values, option_string=None): - latest = values == 'latest' + latest = values == '1.latest' if latest: - values = '1.%d' % LAST_KNOWN_API_VERSION - LOG.debug("Replacing 'latest' API version with the " - "latest known version '%s'", values) + # NOTE(hongbin): '1.latest' means enabling negotiation of the + # latest version between server and client but due to how OSC works + # we cannot just add "1.latest" to the list of supported versions. + # Use '1' in this case. + values = '1' setattr(namespace, self.dest, values) diff --git a/zunclient/shell.py b/zunclient/shell.py index e794575f..1a19922d 100644 --- a/zunclient/shell.py +++ b/zunclient/shell.py @@ -335,7 +335,8 @@ class OpenStackZunShell(object): 'ZUN_API_VERSION', default=DEFAULT_API_VERSION), help='Accepts X, X.Y (where X is major, Y is minor' - ' part), defaults to env[ZUN_API_VERSION].') + ' part) or "X.latest", defaults to' + ' env[ZUN_API_VERSION].') parser.add_argument('--zun_api_version', help=argparse.SUPPRESS) @@ -517,8 +518,9 @@ class OpenStackZunShell(object): spot = argv.index('--endpoint_type') argv[spot] = '--endpoint-type' + do_help = "help" in args subcommand_parser = self.get_subcommand_parser( - api_version, do_help=("help" in args)) + api_version, do_help=do_help) self.parser = subcommand_parser @@ -625,6 +627,42 @@ class OpenStackZunShell(object): client = base_client + if not do_help: + if api_version.is_latest(): + # This client is just used to discover api version. + # Version API needn't microversion, so we just pass + # version 1.1 at here. + self.cs = client.Client( + version=api_versions.APIVersion("1.1"), + username=os_username, + password=os_password, + project_id=os_project_id, + project_name=os_project_name, + user_domain_id=os_user_domain_id, + user_domain_name=os_user_domain_name, + project_domain_id=os_project_domain_id, + project_domain_name=os_project_domain_name, + auth_url=os_auth_url, + service_type=service_type, + region_name=args.os_region_name, + endpoint_override=bypass_url, + interface=endpoint_type, + insecure=insecure, + cacert=os_cacert) + api_version = api_versions.discover_version(self.cs, + api_version) + + min_version = api_versions.APIVersion(api_versions.MIN_API_VERSION) + max_version = api_versions.APIVersion(api_versions.MAX_API_VERSION) + if not api_version.matches(min_version, max_version): + raise exc.CommandError( + _("The specified version isn't supported by " + "client. The valid version range is '%(min)s' " + "to '%(max)s'") % { + "min": min_version.get_string(), + "max": max_version.get_string()} + ) + kwargs = {} if profiler: kwargs["profile"] = args.profile diff --git a/zunclient/tests/unit/osc/test_plugin.py b/zunclient/tests/unit/osc/test_plugin.py index 7f35f6d2..c164ba50 100644 --- a/zunclient/tests/unit/osc/test_plugin.py +++ b/zunclient/tests/unit/osc/test_plugin.py @@ -12,6 +12,7 @@ import mock +from zunclient import api_versions from zunclient.osc import plugin from zunclient.tests.unit import base @@ -26,10 +27,10 @@ class TestContainerPlugin(base.TestCase): instance._api_version = {"container": '1'} instance._region_name = 'zun_region' instance.session = 'zun_session' - mock_get_api_version.return_value = '1.2' + mock_get_api_version.return_value = api_versions.APIVersion('1.2') plugin.make_client(instance) p_client.assert_called_with(region_name='zun_region', session='zun_session', service_type='container', - api_version='1.2') + api_version=api_versions.APIVersion('1.2')) diff --git a/zunclient/tests/unit/test_api_versions.py b/zunclient/tests/unit/test_api_versions.py index 44588e46..52cbaedf 100644 --- a/zunclient/tests/unit/test_api_versions.py +++ b/zunclient/tests/unit/test_api_versions.py @@ -18,6 +18,7 @@ import mock from zunclient import api_versions from zunclient import exceptions from zunclient.tests.unit import utils +from zunclient.v1 import versions class APIVersionTestCase(utils.TestCase): @@ -157,7 +158,7 @@ class GetAPIVersionTestCase(utils.TestCase): version = 7 self.assertEqual(mock_apiversion.return_value, api_versions.get_api_version(version)) - mock_apiversion.assert_called_once_with("%s.0" % str(version)) + mock_apiversion.assert_called_once_with("%s.latest" % str(version)) @mock.patch("zunclient.api_versions.APIVersion") def test_major_and_minor_parts_is_presented(self, mock_apiversion): @@ -244,3 +245,88 @@ class WrapsTestCase(utils.TestCase): some_func(obj, *some_args, **some_kwargs) checker.assert_called_once_with(*((obj,) + some_args), **some_kwargs) + + +class DiscoverVersionTestCase(utils.TestCase): + def setUp(self): + super(DiscoverVersionTestCase, self).setUp() + self.orig_max = api_versions.MAX_API_VERSION + self.orig_min = api_versions.MIN_API_VERSION + self.addCleanup(self._clear_fake_version) + + def _clear_fake_version(self): + api_versions.MAX_API_VERSION = self.orig_max + api_versions.MIN_API_VERSION = self.orig_min + + def test_server_is_too_new(self): + fake_client = mock.MagicMock() + fake_client.versions.get_current.return_value = mock.MagicMock( + max_version="1.7", min_version="1.4") + api_versions.MAX_API_VERSION = "1.3" + api_versions.MIN_API_VERSION = "1.1" + self.assertRaises(exceptions.UnsupportedVersion, + api_versions.discover_version, fake_client, + api_versions.APIVersion('1.latest')) + + def test_server_is_too_old(self): + fake_client = mock.MagicMock() + fake_client.versions.get_current.return_value = mock.MagicMock( + max_version="1.7", min_version="1.4") + api_versions.MAX_API_VERSION = "1.10" + api_versions.MIN_API_VERSION = "1.9" + + self.assertRaises(exceptions.UnsupportedVersion, + api_versions.discover_version, fake_client, + api_versions.APIVersion('1.latest')) + + def test_server_end_version_is_the_latest_one(self): + fake_client = mock.MagicMock() + fake_client.versions.get_current.return_value = mock.MagicMock( + max_version="1.7", min_version="1.4") + api_versions.MAX_API_VERSION = "1.11" + api_versions.MIN_API_VERSION = "1.1" + + self.assertEqual( + "1.7", + api_versions.discover_version( + fake_client, + api_versions.APIVersion('1.latest')).get_string()) + + def test_client_end_version_is_the_latest_one(self): + fake_client = mock.MagicMock() + fake_client.versions.get_current.return_value = mock.MagicMock( + max_version="1.16", min_version="1.4") + api_versions.MAX_API_VERSION = "1.11" + api_versions.MIN_API_VERSION = "1.1" + + self.assertEqual( + "1.11", + api_versions.discover_version( + fake_client, + api_versions.APIVersion('1.latest')).get_string()) + + def test_server_without_microversion(self): + fake_client = mock.MagicMock() + fake_client.versions.get_current.return_value = mock.MagicMock( + max_version='', min_version='') + api_versions.MAX_API_VERSION = "1.11" + api_versions.MIN_API_VERSION = "1.1" + + self.assertEqual( + "1.1", + api_versions.discover_version( + fake_client, + api_versions.APIVersion('1.latest')).get_string()) + + def test_server_without_microversion_and_no_version_field(self): + fake_client = mock.MagicMock() + fake_client.versions.get_current.return_value = versions.Version( + None, {}) + api_versions.MAX_API_VERSION = "1.11" + api_versions.MIN_API_VERSION = "1.1" + + self.assertEqual( + "1.1", + api_versions.discover_version( + fake_client, + api_versions.APIVersion('1.latest')).get_string()) diff --git a/zunclient/tests/unit/test_shell.py b/zunclient/tests/unit/test_shell.py index d0da26fe..2a313b1e 100644 --- a/zunclient/tests/unit/test_shell.py +++ b/zunclient/tests/unit/test_shell.py @@ -82,6 +82,9 @@ class ShellTest(utils.TestCase): self.nc_util = mock.patch( 'zunclient.common.cliutils.isunauthenticated').start() self.nc_util.return_value = False + self.discover_version = mock.patch( + 'zunclient.api_versions.discover_version').start() + self.discover_version.return_value = api_versions.APIVersion('1.1') def test_help_unknown_command(self): self.assertRaises(exceptions.CommandError, self.shell, 'help foofoo') @@ -250,22 +253,29 @@ class ShellTest(utils.TestCase): def test_main_option_region(self): self.make_env() - self._test_main_region('--os-region-name=myregion service-list', - 'myregion') + self._test_main_region( + '--zun-api-version 1.25 ' + '--os-region-name=myregion service-list', 'myregion') def test_main_env_region(self): fake_env = dict(utils.FAKE_ENV, OS_REGION_NAME='myregion') self.make_env(fake_env=fake_env) - self._test_main_region('service-list', 'myregion') + self._test_main_region( + '--zun-api-version 1.25 ' + 'service-list', 'myregion') def test_main_no_region(self): self.make_env() - self._test_main_region('service-list', None) + self._test_main_region( + '--zun-api-version 1.25 ' + 'service-list', None) @mock.patch('zunclient.client.Client') def test_main_endpoint_public(self, mock_client): self.make_env() - self.shell('--endpoint-type publicURL service-list') + self.shell( + '--zun-api-version 1.25 ' + '--endpoint-type publicURL service-list') mock_client.assert_called_once_with( username='username', password='password', interface='publicURL', project_id=None, @@ -279,7 +289,9 @@ class ShellTest(utils.TestCase): @mock.patch('zunclient.client.Client') def test_main_endpoint_internal(self, mock_client): self.make_env() - self.shell('--endpoint-type internalURL service-list') + self.shell( + '--zun-api-version 1.25 ' + '--endpoint-type internalURL service-list') mock_client.assert_called_once_with( username='username', password='password', interface='internalURL', project_id=None, @@ -310,7 +322,9 @@ class ShellTestKeystoneV3(ShellTest): @mock.patch('zunclient.client.Client') def test_main_endpoint_public(self, mock_client): self.make_env(fake_env=FAKE_ENV4) - self.shell('--endpoint-type publicURL service-list') + self.shell( + '--zun-api-version 1.25 ' + '--endpoint-type publicURL service-list') mock_client.assert_called_once_with( username='username', password='password', interface='publicURL', project_id='project_id', diff --git a/zunclient/tests/unit/v1/shell_test_base.py b/zunclient/tests/unit/v1/shell_test_base.py index 9bb5bace..321828fc 100644 --- a/zunclient/tests/unit/v1/shell_test_base.py +++ b/zunclient/tests/unit/v1/shell_test_base.py @@ -17,6 +17,7 @@ import re import mock from testtools import matchers +from zunclient import api_versions from zunclient.tests.unit import utils FAKE_ENV = {'OS_USERNAME': 'username', @@ -67,10 +68,14 @@ class TestCommandLineArgument(utils.TestCase): loader.start() session = mock.patch('keystoneauth1.session.Session') session.start() + discover = mock.patch('zunclient.api_versions.discover_version', + return_value=api_versions.APIVersion('1.1')) + discover.start() self.addCleanup(session_client.stop) self.addCleanup(loader.stop) self.addCleanup(session.stop) + self.addCleanup(discover.stop) def _test_arg_success(self, command): stdout, stderr = self.shell(command) diff --git a/zunclient/v1/versions.py b/zunclient/v1/versions.py index ba5d45c2..63c76570 100644 --- a/zunclient/v1/versions.py +++ b/zunclient/v1/versions.py @@ -25,3 +25,9 @@ class VersionManager(base.Manager): url = "%s" % self.api.get_endpoint() url = "%s/" % url.rsplit("/", 1)[0] return self._list(url, "versions") + + def get_current(self): + for version in self.list(): + if version.status == "CURRENT": + return version + return None