From 8b47d15fd5a3f67072e82ca8fc4928f57d9c96c8 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Mon, 14 Aug 2017 10:05:19 -0500 Subject: [PATCH] Use new keystoneauth version discovery We can remove a ton of our logic. We still need SOME because we have a different fallback strategy that isn't appropriate for keystoneauth generally. There are a couple of tests removed, since they were testing shade code that was removed. Also it's worth noting that there are tests in test_domain_params and test_image that are mocking the client rather than mocking the requests and we should fix those. Co-authored-by: Samuel de Medeiros Queiroz Change-Id: I78019717cdee79cab43b0d11e737327aa281fd03 --- .../version-discovery-a501c4e9e9869f77.yaml | 13 + shade/_adapter.py | 6 + shade/_legacy_clients.py | 6 +- shade/openstackcloud.py | 264 +++++++----------- .../fixtures/catalog-versioned-image.json | 71 ----- shade/tests/unit/test_domain_params.py | 31 +- shade/tests/unit/test_image.py | 108 +++---- 7 files changed, 178 insertions(+), 321 deletions(-) create mode 100644 releasenotes/notes/version-discovery-a501c4e9e9869f77.yaml delete mode 100644 shade/tests/unit/fixtures/catalog-versioned-image.json diff --git a/releasenotes/notes/version-discovery-a501c4e9e9869f77.yaml b/releasenotes/notes/version-discovery-a501c4e9e9869f77.yaml new file mode 100644 index 000000000..c55792fe8 --- /dev/null +++ b/releasenotes/notes/version-discovery-a501c4e9e9869f77.yaml @@ -0,0 +1,13 @@ +--- +features: + - Version discovery is now done via the keystoneauth + library. shade still has one behavioral difference + from default keystoneauth behavior, which is that + shade will use a version it understands if it can + find one even if the user has requested a different + version. This change opens the door for shade to + start being able to consume API microversions as + needed. +upgrade: + - keystoneauth version 3.2.0 or higher is required + because of version discovery. diff --git a/shade/_adapter.py b/shade/_adapter.py index 6293c2533..bd88d6fd6 100644 --- a/shade/_adapter.py +++ b/shade/_adapter.py @@ -156,3 +156,9 @@ class ShadeAdapter(adapter.Adapter): return response else: return self._munch_response(response, error_message=error_message) + + def _version_matches(self, version): + api_version = self.get_api_major_version() + if api_version: + return api_version[0] == version + return False diff --git a/shade/_legacy_clients.py b/shade/_legacy_clients.py index c18eea1a4..b5ae535aa 100644 --- a/shade/_legacy_clients.py +++ b/shade/_legacy_clients.py @@ -104,10 +104,8 @@ class LegacyClientFactoryMixin(object): 'keystone', 'identity', client_class=client_class.Client, deprecated=False, - endpoint=self.cloud_config.config[ - 'identity_endpoint_override'], - endpoint_override=self.cloud_config.config[ - 'identity_endpoint_override']) + endpoint=self._identity_client.get_endpoint(), + endpoint_override=self._identity_client.get_endpoint()) # Set the ironic API microversion to a known-good # supported/tested with the contents of shade. diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index 4e8bd54d9..8f02b5e5a 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -17,6 +17,7 @@ import hashlib import ipaddress import json import jsonpatch +import keystoneauth1.session import operator import os import os_client_config @@ -352,19 +353,108 @@ class OpenStackCloud( service=service_key)) return client - def _get_raw_client(self, service_key): - return _adapter.ShadeAdapter( - manager=self.manager, + def _get_major_version_id(self, version): + if isinstance(version, int): + return version + elif isinstance(version, six.string_types + (tuple,)): + return int(version[0]) + return version + + def _get_versioned_client( + self, service_type, min_version=None, max_version=None): + config_version = self.cloud_config.get_api_version(service_type) + config_major = self._get_major_version_id(config_version) + max_major = self._get_major_version_id(max_version) + min_major = self._get_major_version_id(min_version) + # NOTE(mordred) The shade logic for versions is slightly different + # than the ksa Adapter constructor logic. shade knows the versions + # it knows, and uses them when it detects them. However, if a user + # requests a version, and it's not found, and a different one shade + # does know about it found, that's a warning in shade. + if config_version: + if min_major and config_major < min_major: + raise OpenStackCloudException( + "Version {config_version} requested for {service_type}" + " but shade understands a minimum of {min_version}".format( + config_version=config_version, + service_type=service_type, + min_version=min_version)) + elif max_major and config_major > max_major: + raise OpenStackCloudException( + "Version {config_version} requested for {service_type}" + " but shade understands a maximum of {max_version}".format( + config_version=config_version, + service_type=service_type, + max_version=max_version)) + request_min_version = config_version + request_max_version = '{version}.latest'.format( + version=config_major) + adapter = _adapter.ShadeAdapter( + session=self.keystone_session, + manager=self.manager, + service_type=self.cloud_config.get_service_type(service_type), + service_name=self.cloud_config.get_service_name(service_type), + interface=self.cloud_config.get_interface(service_type), + endpoint_override=self.cloud_config.get_endpoint(service_type), + region_name=self.cloud_config.region, + min_version=request_min_version, + max_version=request_max_version, + shade_logger=self.log) + if adapter.get_endpoint(): + return adapter + + adapter = _adapter.ShadeAdapter( session=self.keystone_session, - service_type=self.cloud_config.get_service_type(service_key), - service_name=self.cloud_config.get_service_name(service_key), - interface=self.cloud_config.get_interface(service_key), + manager=self.manager, + service_type=self.cloud_config.get_service_type(service_type), + service_name=self.cloud_config.get_service_name(service_type), + interface=self.cloud_config.get_interface(service_type), + endpoint_override=self.cloud_config.get_endpoint(service_type), + region_name=self.cloud_config.region, + min_version=min_version, + max_version=max_version, + shade_logger=self.log) + + # data.api_version can be None if no version was detected, such + # as with neutron + api_version = adapter.get_api_major_version() + api_major = self._get_major_version_id(api_version) + + # If we detect a different version that was configured, warn the user. + # shade still knows what to do - but if the user gave us an explicit + # version and we couldn't find it, they may want to investigate. + if api_version and (api_major != config_major): + warning_msg = ( + '{service_type} is configured for {config_version}' + ' but only {api_version} is available. shade is happy' + ' with this version, but if you were trying to force an' + ' override, that did not happen. You may want to check' + ' your cloud, or remove the version specification from' + ' your config.'.format( + service_type=service_type, + config_version=config_version, + api_version='.'.join([str(f) for f in api_version]))) + self.log.debug(warning_msg) + warnings.warn(warning_msg) + return adapter + + def _get_raw_client( + self, service_type, api_version=None, endpoint_override=None): + return _adapter.ShadeAdapter( + session=self.keystone_session, + manager=self.manager, + service_type=self.cloud_config.get_service_type(service_type), + service_name=self.cloud_config.get_service_name(service_type), + interface=self.cloud_config.get_interface(service_type), + endpoint_override=self.cloud_config.get_endpoint( + service_type) or endpoint_override, region_name=self.cloud_config.region, shade_logger=self.log) def _is_client_version(self, client, version): - api_version = self.cloud_config.get_api_version(client) - return api_version.startswith(str(version)) + client_name = '_{client}_client'.format(client=client) + client = getattr(self, client_name) + return client._version_matches(version) @property def _application_catalog_client(self): @@ -408,23 +498,16 @@ class OpenStackCloud( @property def _dns_client(self): if 'dns' not in self._raw_clients: - dns_client = self._get_raw_client('dns') - dns_url = self._discover_endpoint( - 'dns', version_required=True) - dns_client.endpoint_override = dns_url + dns_client = self._get_versioned_client( + 'dns', min_version=2, max_version='2.latest') self._raw_clients['dns'] = dns_client return self._raw_clients['dns'] @property def _identity_client(self): if 'identity' not in self._raw_clients: - identity_client = self._get_raw_client('identity') - identity_url = self._discover_endpoint( - 'identity', version_required=True, versions=['3', '2']) - identity_client.endpoint_override = identity_url - self.cloud_config.config['identity_endpoint_override'] = \ - identity_url - self._raw_clients['identity'] = identity_client + self._raw_clients['identity'] = self._get_versioned_client( + 'identity', min_version=2, max_version='3.latest') return self._raw_clients['identity'] @property @@ -434,154 +517,19 @@ class OpenStackCloud( self._raw_clients['raw-image'] = image_client return self._raw_clients['raw-image'] - def _get_version_and_base_from_endpoint(self, endpoint): - url, version = endpoint.rstrip('/').rsplit('/', 1) - if version.endswith(self.current_project_id): - url, version = endpoint.rstrip('/').rsplit('/', 1) - if version.startswith('v'): - return url, version[1] - return "/".join([url, version]), None - - def _get_version_from_endpoint(self, endpoint): - return self._get_version_and_base_from_endpoint(endpoint)[1] - - def _strip_version_from_endpoint(self, endpoint): - return self._get_version_and_base_from_endpoint(endpoint)[0] - - def _match_given_endpoint(self, given, version): - given_version = self._get_version_from_endpoint(given) - if given_version and version == given_version: - return True - return False - - def _discover_endpoint( - self, service_type, version_required=False, - versions=None): - # If endpoint_override is set, do nothing - service_endpoint = self.cloud_config.get_endpoint(service_type) - if service_endpoint: - return service_endpoint - - client = self._get_raw_client(service_type) - config_version = self.cloud_config.get_api_version(service_type) - - if versions and config_version[0] not in versions: - raise OpenStackCloudException( - "Version {version} was requested for service {service_type}" - " but shade only understands how to handle versions" - " {versions}".format( - version=config_version, - service_type=service_type, - versions=versions)) - - # shade only groks major versions at the moment - if config_version: - config_version = config_version[0] - - # First - quick check to see if the endpoint in the catalog - # is a versioned endpoint that matches the version we requested. - # If it is, don't do any additional work. - catalog_endpoint = client.get_endpoint() - if self._match_given_endpoint( - catalog_endpoint, config_version): - return catalog_endpoint - - if not versions and config_version: - versions = [config_version] - - candidate_endpoints = [] - version_key = '{service}_api_version'.format(service=service_type) - - # Next, try built-in keystoneauth discovery so that we can take - # advantage of the discovery cache - base_url = self._strip_version_from_endpoint(catalog_endpoint) - try: - discovery = self.keystone_session.auth.get_discovery( - self.keystone_session, base_url) - - version_list = discovery.version_data(reverse=True) - if config_version: - # If we have a specific version request, look for it first. - for version_data in version_list: - if str(version_data['version'][0]) == config_version: - candidate_endpoints.append(version_data) - - if not candidate_endpoints: - # If we didn't find anything, look again, this time either - # for the range, or just grab everything if we don't have - # a range - if str(version_data['version'][0]) in versions: - candidate_endpoints.append(version_data) - elif not config_version and not versions: - candidate_endpoints.append(version_data) - - except keystoneauth1.exceptions.DiscoveryFailure as e: - self.log.debug( - "Version discovery failed, assuming endpoint in" - " the catalog is already versioned. {e}".format(e=str(e))) - - if candidate_endpoints: - # If we got more than one, pick the highest - endpoint_description = candidate_endpoints[0] - service_endpoint = endpoint_description['url'] - api_version = str(endpoint_description['version'][0]) - else: - # Can't discover a version. Do best-attempt at inferring - # version from URL so that later logic can do its best - api_version = self._get_version_from_endpoint(catalog_endpoint) - if not api_version: - if not config_version and version_required: - raise OpenStackCloudException( - "No version for {service_type} could be detected," - " and also {version_key} is not set. It is impossible" - " to continue without knowing what version this" - " service is.") - if not config_version: - return catalog_endpoint - api_version = config_version - service_endpoint = catalog_endpoint - - # If we detect a different version that was configured, - # set the version in occ because we have logic elsewhere - # that is different depending on which version we're using - if config_version != api_version: - warning_msg = ( - '{version_key} is {config_version}' - ' but only {api_version} is available.'.format( - version_key=version_key, - config_version=config_version, - api_version=api_version)) - self.log.debug(warning_msg) - warnings.warn(warning_msg) - self.cloud_config.config[version_key] = api_version - - # Sometimes version discovery documents have broken endpoints, but - # the catalog has good ones (what?) - catalog_endpoint = urllib.parse.urlparse(client.get_endpoint()) - discovered_endpoint = urllib.parse.urlparse(service_endpoint) - - return urllib.parse.ParseResult( - catalog_endpoint.scheme, - catalog_endpoint.netloc, - discovered_endpoint.path, - discovered_endpoint.params, - discovered_endpoint.query, - discovered_endpoint.fragment).geturl() - @property def _image_client(self): if 'image' not in self._raw_clients: - image_client = self._get_raw_client('image') - image_url = self._discover_endpoint( - 'image', version_required=True, versions=['2', '1']) - image_client.endpoint_override = image_url - self._raw_clients['image'] = image_client + self._raw_clients['image'] = self._get_versioned_client( + 'image', min_version=1, max_version='2.latest') return self._raw_clients['image'] @property def _network_client(self): if 'network' not in self._raw_clients: client = self._get_raw_client('network') + # TODO(mordred) I don't care if this is what neutronclient does, + # fix this. # Don't bother with version discovery - there is only one version # of neutron. This is what neutronclient does, fwiw. endpoint = client.get_endpoint() diff --git a/shade/tests/unit/fixtures/catalog-versioned-image.json b/shade/tests/unit/fixtures/catalog-versioned-image.json deleted file mode 100644 index cb33dd5c4..000000000 --- a/shade/tests/unit/fixtures/catalog-versioned-image.json +++ /dev/null @@ -1,71 +0,0 @@ -{ - "token": { - "audit_ids": [ - "Rvn7eHkiSeOwucBIPaKdYA" - ], - "catalog": [ - { - "endpoints": [ - { - "id": "5a64de3c4a614d8d8f8d1ba3dee5f45f", - "interface": "public", - "region": "RegionOne", - "url": "https://image.example.com/v2" - } - ], - "name": "glance", - "type": "image" - }, - { - "endpoints": [ - { - "id": "4deb4d0504a044a395d4480741ba628c", - "interface": "public", - "region": "RegionOne", - "url": "https://identity.example.com" - }, - { - "id": "012322eeedcd459edabb4933021112bc", - "interface": "admin", - "region": "RegionOne", - "url": "https://identity.example.com" - } - ], - "endpoints_links": [], - "name": "keystone", - "type": "identity" - } - ], - "expires_at": "9999-12-31T23:59:59Z", - "issued_at": "2016-12-17T14:25:05.000000Z", - "methods": [ - "password" - ], - "project": { - "domain": { - "id": "default", - "name": "default" - }, - "id": "1c36b64c840a42cd9e9b931a369337f0", - "name": "Default Project" - }, - "roles": [ - { - "id": "9fe2ff9ee4384b1894a90878d3e92bab", - "name": "_member_" - }, - { - "id": "37071fc082e14c2284c32a2761f71c63", - "name": "swiftoperator" - } - ], - "user": { - "domain": { - "id": "default", - "name": "default" - }, - "id": "c17534835f8f42bf98fc367e0bf35e09", - "name": "mordred" - } - } -} diff --git a/shade/tests/unit/test_domain_params.py b/shade/tests/unit/test_domain_params.py index 1ff5586fc..62bfcc51f 100644 --- a/shade/tests/unit/test_domain_params.py +++ b/shade/tests/unit/test_domain_params.py @@ -11,7 +11,6 @@ # under the License. import mock -import os_client_config as occ import munch @@ -22,11 +21,12 @@ from shade.tests.unit import base class TestDomainParams(base.TestCase): - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, 'get_project') - def test_identity_params_v3(self, mock_get_project, mock_api_version): + def test_identity_params_v3(self, mock_get_project, + mock_is_client_version): mock_get_project.return_value = munch.Munch(id=1234) - mock_api_version.return_value = '3' + mock_is_client_version.return_value = True ret = self.cloud._get_identity_params(domain_id='5678', project='bar') self.assertIn('default_project_id', ret) @@ -34,39 +34,40 @@ class TestDomainParams(base.TestCase): self.assertIn('domain_id', ret) self.assertEqual(ret['domain_id'], '5678') - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, 'get_project') def test_identity_params_v3_no_domain( - self, mock_get_project, mock_api_version): + self, mock_get_project, mock_is_client_version): mock_get_project.return_value = munch.Munch(id=1234) - mock_api_version.return_value = '3' + mock_is_client_version.return_value = True self.assertRaises( exc.OpenStackCloudException, self.cloud._get_identity_params, domain_id=None, project='bar') - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, 'get_project') - def test_identity_params_v2(self, mock_get_project, mock_api_version): + def test_identity_params_v2(self, mock_get_project, + mock_is_client_version): mock_get_project.return_value = munch.Munch(id=1234) - mock_api_version.return_value = '2' + mock_is_client_version.return_value = False ret = self.cloud._get_identity_params(domain_id='foo', project='bar') self.assertIn('tenant_id', ret) self.assertEqual(ret['tenant_id'], 1234) self.assertNotIn('domain', ret) - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, 'get_project') def test_identity_params_v2_no_domain(self, mock_get_project, - mock_api_version): + mock_is_client_version): mock_get_project.return_value = munch.Munch(id=1234) - mock_api_version.return_value = '2' + mock_is_client_version.return_value = False ret = self.cloud._get_identity_params(domain_id=None, project='bar') - api_calls = [mock.call('identity'), mock.call('identity')] - mock_api_version.assert_has_calls(api_calls) + api_calls = [mock.call('identity', 3), mock.call('identity', 3)] + mock_is_client_version.assert_has_calls(api_calls) self.assertIn('tenant_id', ret) self.assertEqual(ret['tenant_id'], 1234) self.assertNotIn('domain', ret) diff --git a/shade/tests/unit/test_image.py b/shade/tests/unit/test_image.py index ba07d8918..f0be4b990 100644 --- a/shade/tests/unit/test_image.py +++ b/shade/tests/unit/test_image.py @@ -21,7 +21,6 @@ import uuid import mock import munch -import os_client_config as occ import six import shade @@ -385,10 +384,12 @@ class TestImage(BaseTestImage): name, imagefile.name, wait=True, timeout=1, is_public=False, **kwargs) - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, '_image_client') - def test_create_image_put_v1(self, mock_image_client, mock_api_version): - mock_api_version.return_value = '1' + def test_create_image_put_v1( + self, mock_image_client, mock_is_client_version): + # TODO(mordred) Fix this to use requests_mock + mock_is_client_version.return_value = False mock_image_client.get.return_value = [] self.assertEqual([], self.cloud.list_images()) @@ -421,11 +422,11 @@ class TestImage(BaseTestImage): self.assertEqual( self._munch_images(ret), self.cloud.list_images()) - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, '_image_client') def test_create_image_put_v1_bad_delete( - self, mock_image_client, mock_api_version): - mock_api_version.return_value = '1' + self, mock_image_client, mock_is_client_version): + mock_is_client_version.return_value = False mock_image_client.get.return_value = [] self.assertEqual([], self.cloud.list_images()) @@ -459,10 +460,11 @@ class TestImage(BaseTestImage): }) mock_image_client.delete.assert_called_with('/images/42') - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, '_image_client') - def test_update_image_no_patch(self, mock_image_client, mock_api_version): - mock_api_version.return_value = '2' + def test_update_image_no_patch( + self, mock_image_client, mock_is_client_version): + mock_is_client_version.return_value = True self.cloud.image_api_use_tasks = False mock_image_client.get.return_value = [] @@ -489,11 +491,11 @@ class TestImage(BaseTestImage): mock_image_client.get.assert_called_with('/images', params={}) mock_image_client.patch.assert_not_called() - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, '_image_client') def test_create_image_put_v2_bad_delete( - self, mock_image_client, mock_api_version): - mock_api_version.return_value = '2' + self, mock_image_client, mock_is_client_version): + mock_is_client_version.return_value = True self.cloud.image_api_use_tasks = False mock_image_client.get.return_value = [] @@ -528,11 +530,11 @@ class TestImage(BaseTestImage): data=mock.ANY) mock_image_client.delete.assert_called_with('/images/42') - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, '_image_client') def test_create_image_put_bad_int( - self, mock_image_client, mock_api_version): - mock_api_version.return_value = '2' + self, mock_image_client, mock_is_client_version): + mock_is_client_version.return_value = True self.cloud.image_api_use_tasks = False self.assertRaises( @@ -540,11 +542,11 @@ class TestImage(BaseTestImage): self._call_create_image, '42 name', min_disk='fish', min_ram=0) mock_image_client.post.assert_not_called() - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, '_image_client') def test_create_image_put_user_int( - self, mock_image_client, mock_api_version): - mock_api_version.return_value = '2' + self, mock_image_client, mock_is_client_version): + mock_is_client_version.return_value = True self.cloud.image_api_use_tasks = False args = {'name': '42 name', @@ -575,11 +577,11 @@ class TestImage(BaseTestImage): self.assertEqual( self._munch_images(ret), self.cloud.list_images()) - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, '_image_client') def test_create_image_put_meta_int( - self, mock_image_client, mock_api_version): - mock_api_version.return_value = '2' + self, mock_image_client, mock_is_client_version): + mock_is_client_version.return_value = True self.cloud.image_api_use_tasks = False mock_image_client.get.return_value = [] @@ -604,11 +606,11 @@ class TestImage(BaseTestImage): self.assertEqual( self._munch_images(ret), self.cloud.list_images()) - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, '_image_client') def test_create_image_put_protected( - self, mock_image_client, mock_api_version): - mock_api_version.return_value = '2' + self, mock_image_client, mock_is_client_version): + mock_is_client_version.return_value = True self.cloud.image_api_use_tasks = False mock_image_client.get.return_value = [] @@ -642,11 +644,11 @@ class TestImage(BaseTestImage): headers={'Content-Type': 'application/octet-stream'}) self.assertEqual(self._munch_images(ret), self.cloud.list_images()) - @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') + @mock.patch.object(shade.OpenStackCloud, '_is_client_version') @mock.patch.object(shade.OpenStackCloud, '_image_client') def test_create_image_put_user_prop( - self, mock_image_client, mock_api_version): - mock_api_version.return_value = '2' + self, mock_image_client, mock_is_client_version): + mock_is_client_version.return_value = True self.cloud.image_api_use_tasks = False mock_image_client.get.return_value = [] @@ -687,8 +689,7 @@ class TestImageV1Only(base.RequestsMockTestCase): self.assertEqual( 'https://image.example.com/v1/', self.cloud._image_client.get_endpoint()) - self.assertEqual( - '1', self.cloud_config.get_api_version('image')) + self.assertTrue(self.cloud._is_client_version('image', 1)) def test_config_v2(self): self.cloud.cloud_config.config['image_api_version'] = '2' @@ -697,8 +698,7 @@ class TestImageV1Only(base.RequestsMockTestCase): self.assertEqual( 'https://image.example.com/v1/', self.cloud._image_client.get_endpoint()) - self.assertEqual( - '1', self.cloud_config.get_api_version('image')) + self.assertFalse(self.cloud._is_client_version('image', 2)) class TestImageV2Only(base.RequestsMockTestCase): @@ -714,8 +714,7 @@ class TestImageV2Only(base.RequestsMockTestCase): self.assertEqual( 'https://image.example.com/v2/', self.cloud._image_client.get_endpoint()) - self.assertEqual( - '2', self.cloud_config.get_api_version('image')) + self.assertTrue(self.cloud._is_client_version('image', 2)) def test_config_v2(self): self.cloud.cloud_config.config['image_api_version'] = '2' @@ -724,26 +723,7 @@ class TestImageV2Only(base.RequestsMockTestCase): self.assertEqual( 'https://image.example.com/v2/', self.cloud._image_client.get_endpoint()) - self.assertEqual( - '2', self.cloud_config.get_api_version('image')) - - -class TestImageVersionDiscovery(BaseTestImage): - - def test_version_discovery_skip(self): - self.cloud.cloud_config.config['image_endpoint_override'] = \ - 'https://image.example.com/v2/override' - - self.register_uris([ - dict(method='GET', - uri='https://image.example.com/v2/override/images', - json={'images': []}) - ]) - self.assertEqual([], self.cloud.list_images()) - self.assertEqual( - self.cloud._image_client.endpoint_override, - 'https://image.example.com/v2/override') - self.assert_calls() + self.assertTrue(self.cloud._is_client_version('image', 2)) class TestImageVolume(BaseTestImage): @@ -829,24 +809,6 @@ class TestImageBrokenDiscovery(base.RequestsMockTestCase): ]) self.assertEqual([], self.cloud.list_images()) self.assertEqual( - self.cloud._image_client.endpoint_override, + self.cloud._image_client.get_endpoint(), 'https://image.example.com/v2/') self.assert_calls() - - -class TestImageDiscoveryOptimization(base.RequestsMockTestCase): - - def setUp(self): - super(TestImageDiscoveryOptimization, self).setUp() - self.use_keystone_v3(catalog='catalog-versioned-image.json') - - def test_version_discovery_skip(self): - self.cloud.cloud_config.config['image_api_version'] = '2' - - self.register_uris([ - dict(method='GET', - uri='https://image.example.com/v2/images', - json={'images': []}) - ]) - self.assertEqual([], self.cloud.list_images()) - self.assert_calls()