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 <samueldmq@gmail.com>
Change-Id: I78019717cdee79cab43b0d11e737327aa281fd03

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.

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

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[
+            endpoint=self._identity_client.get_endpoint(),
-                'identity_endpoint_override'],
+            endpoint_override=self._identity_client.get_endpoint())
-            endpoint_override=self.cloud_config.config[
-                'identity_endpoint_override'])
 
     # Set the ironic API microversion to a known-good
     # supported/tested with the contents of shade.

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):
+    def _get_major_version_id(self, version):
-        return _adapter.ShadeAdapter(
+        if isinstance(version, int):
-            manager=self.manager,
+            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),
+            manager=self.manager,
-            service_name=self.cloud_config.get_service_name(service_key),
+            service_type=self.cloud_config.get_service_type(service_type),
-            interface=self.cloud_config.get_interface(service_key),
+            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)
+        client_name = '_{client}_client'.format(client=client)
-        return api_version.startswith(str(version))
+        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_client = self._get_versioned_client(
-            dns_url = self._discover_endpoint(
+                'dns', min_version=2, max_version='2.latest')
-                'dns', version_required=True)
-            dns_client.endpoint_override = dns_url
             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')
+            self._raw_clients['identity'] = self._get_versioned_client(
-            identity_url = self._discover_endpoint(
+                'identity', min_version=2, max_version='3.latest')
-                '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
         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')
+            self._raw_clients['image'] = self._get_versioned_client(
-            image_url = self._discover_endpoint(
+                'image', min_version=1, max_version='2.latest')
-                'image', version_required=True, versions=['2', '1'])
-            image_client.endpoint_override = image_url
-            self._raw_clients['image'] = image_client
         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()

shade/tests/unit/fixtures/catalog-versioned-image.json

@@ -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"
-    }
-  }
-}

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')]
+        api_calls = [mock.call('identity', 3), mock.call('identity', 3)]
-        mock_api_version.assert_has_calls(api_calls)
+        mock_is_client_version.assert_has_calls(api_calls)
         self.assertIn('tenant_id', ret)
         self.assertEqual(ret['tenant_id'], 1234)
         self.assertNotIn('domain', ret)

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):
+    def test_create_image_put_v1(
-        mock_api_version.return_value = '1'
+            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):
+            self, mock_image_client, mock_is_client_version):
-        mock_api_version.return_value = '1'
+        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):
+    def test_update_image_no_patch(
-        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 = []
@@ -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):
+            self, mock_image_client, mock_is_client_version):
-        mock_api_version.return_value = '2'
+        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):
+            self, mock_image_client, mock_is_client_version):
-        mock_api_version.return_value = '2'
+        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):
+            self, mock_image_client, mock_is_client_version):
-        mock_api_version.return_value = '2'
+        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):
+            self, mock_image_client, mock_is_client_version):
-        mock_api_version.return_value = '2'
+        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):
+            self, mock_image_client, mock_is_client_version):
-        mock_api_version.return_value = '2'
+        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):
+            self, mock_image_client, mock_is_client_version):
-        mock_api_version.return_value = '2'
+        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(
+        self.assertTrue(self.cloud._is_client_version('image', 1))
-            '1', self.cloud_config.get_api_version('image'))
 
     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(
+        self.assertFalse(self.cloud._is_client_version('image', 2))
-            '1', self.cloud_config.get_api_version('image'))
 
 
 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(
+        self.assertTrue(self.cloud._is_client_version('image', 2))
-            '2', self.cloud_config.get_api_version('image'))
 
     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(
+        self.assertTrue(self.cloud._is_client_version('image', 2))
-            '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()
 
 
 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()