Defer all endpoint discovery to keystoneauth

keystoneauth has support for service type aliases and version discovery.
Stop doing it locally and just pass data to keystoneauth.

Depends-On: https://review.openstack.org/567602
Change-Id: If60d02a8216ca0719fa628431515a0c3b37bf607

lower-constraints.txt

@@ -13,7 +13,7 @@ jmespath==0.9.0
 jsonpatch==1.16
 jsonpointer==1.13
 jsonschema==2.6.0
-keystoneauth1==3.4.0
+keystoneauth1==3.6.0
 linecache2==1.0.0
 mock==2.0.0
 mox3==0.20.0

openstack/block_storage/block_storage_service.py

@@ -21,4 +21,5 @@ class BlockStorageService(service_filter.ServiceFilter):
     def __init__(self, version=None):
         """Create a block storage service."""
         super(BlockStorageService, self).__init__(
-            service_type='volume', version=version, requires_project_id=True)
+            service_type='block-storage',
+            version=version, requires_project_id=True)

openstack/cloud/openstackcloud.py

@@ -603,9 +603,10 @@ class OpenStackCloud(_normalize.Normalizer):
 
     @property
     def _volume_client(self):
-        if 'volume' not in self._raw_clients:
+        if 'block-storage' not in self._raw_clients:
-            self._raw_clients['volume'] = self._get_raw_client('volume')
+            client = self._get_raw_client('block-storage')
-        return self._raw_clients['volume']
+            self._raw_clients['block-storage'] = client
+        return self._raw_clients['block-storage']
 
     def pprint(self, resource):
         """Wrapper aroud pprint that groks munch objects"""

openstack/config/cloud_region.py

@@ -13,7 +13,6 @@
 # under the License.
 
 import copy
-import math
 import warnings
 
 from keystoneauth1 import adapter
@@ -199,26 +198,13 @@ class CloudRegion(object):
         return self.config.get(key, None)
 
     def get_service_type(self, service_type):
+        # People requesting 'volume' are doing so because os-client-config
+        # let them. What they want is block-storage, not explicitly the
+        # v1 of cinder. If someone actually wants v1, they'll have api_version
+        # set to 1, in which case block-storage will still work properly.
+        if service_type == 'volume':
+            service_type = 'block-storage'
         key = _make_key('service_type', service_type)
-        # Cinder did an evil thing where they defined a second service
-        # type in the catalog. Of course, that's insane, so let's hide this
-        # atrocity from the as-yet-unsullied eyes of our users.
-        # Of course, if the user requests a volumev2, that structure should
-        # still work.
-        # What's even more amazing is that they did it AGAIN with cinder v3
-        # And then I learned that mistral copied it.
-        # TODO(shade) This should get removed when we have os-service-types
-        # alias support landed in keystoneauth.
-        if service_type in ('volume', 'block-storage'):
-            vol_ver = self.get_api_version('volume')
-            if vol_ver and vol_ver.startswith('2'):
-                service_type = 'volumev2'
-            elif vol_ver and vol_ver.startswith('3'):
-                service_type = 'volumev3'
-        elif service_type == 'workflow':
-            wk_ver = self.get_api_version(service_type)
-            if wk_ver and wk_ver.startswith('2'):
-                service_type = 'workflowv2'
         return self.config.get(key, service_type)
 
     def get_service_name(self, service_type):
@@ -326,7 +312,7 @@ class CloudRegion(object):
     def get_session_client(
             self, service_key, version=None, constructor=adapter.Adapter,
             **kwargs):
-        """Return a prepped requests adapter for a given service.
+        """Return a prepped keystoneauth Adapter for a given service.
 
         This is useful for making direct requests calls against a
         'mounted' endpoint. That is, if you do:
@@ -353,17 +339,6 @@ class CloudRegion(object):
             endpoint_override=self.get_endpoint(service_key),
             **kwargs)
 
-    def _get_highest_endpoint(self, service_types, kwargs):
-        session = self.get_session()
-        for service_type in service_types:
-            kwargs['service_type'] = service_type
-            try:
-                # Return the highest version we find that matches
-                # the request
-                return session.get_endpoint(**kwargs)
-            except keystoneauth1.exceptions.catalog.EndpointNotFound:
-                pass
-
     def get_session_endpoint(
             self, service_key, min_version=None, max_version=None):
         """Return the endpoint from config or the catalog.
@@ -380,38 +355,38 @@ class CloudRegion(object):
         override_endpoint = self.get_endpoint(service_key)
         if override_endpoint:
             return override_endpoint
-        endpoint = None
+
-        kwargs = {
+        service_name = self.get_service_name(service_key)
-            'service_name': self.get_service_name(service_key),
+        interface = self.get_interface(service_key)
-            'region_name': self.region_name
+        session = self.get_session()
-        }
+        # Do this as kwargs because of os-client-config unittest mocking
-        kwargs['interface'] = self.get_interface(service_key)
+        version_kwargs = {}
-        if service_key == 'volume' and not self.get_api_version('volume'):
+        if min_version:
-            # If we don't have a configured cinder version, we can't know
+            version_kwargs['min_version'] = min_version
-            # to request a different service_type
+        if max_version:
-            min_version = float(min_version or 1)
+            version_kwargs['max_version'] = max_version
-            max_version = float(max_version or 3)
+        try:
-            min_major = math.trunc(float(min_version))
+            # Return the highest version we find that matches
-            max_major = math.trunc(float(max_version))
+            # the request
-            versions = range(int(max_major) + 1, int(min_major), -1)
+            endpoint = session.get_endpoint(
-            service_types = []
+                service_type=service_key,
-            for version in versions:
+                region_name=self.region_name,
-                if version == 1:
+                interface=interface,
-                    service_types.append('volume')
+                service_name=service_name,
-                else:
+                **version_kwargs
-                    service_types.append('volumev{v}'.format(v=version))
+            )
-        else:
+        except keystoneauth1.exceptions.catalog.EndpointNotFound:
-            service_types = [self.get_service_type(service_key)]
+            endpoint = None
-        endpoint = self._get_highest_endpoint(service_types, kwargs)
         if not endpoint:
             self.log.warning(
                 "Keystone catalog entry not found ("
                 "service_type=%s,service_name=%s"
                 "interface=%s,region_name=%s)",
                 service_key,
-                kwargs['service_name'],
+                service_name,
-                kwargs['interface'],
+                interface,
-                kwargs['region_name'])
+                self.region_name,
+            )
         return endpoint
 
     def get_cache_expiration_time(self):

openstack/tests/unit/block_storage/test_block_storage_service.py

@@ -19,7 +19,7 @@ class TestBlockStorageService(base.TestCase):
 
     def test_service(self):
         sot = block_storage_service.BlockStorageService()
-        self.assertEqual("volume", sot.service_type)
+        self.assertEqual("block-storage", sot.service_type)
         self.assertEqual("public", sot.interface)
         self.assertIsNone(sot.region)
         self.assertIsNone(sot.service_name)

openstack/tests/unit/block_storage/v2/test_snapshot.py

@@ -45,7 +45,7 @@ class TestSnapshot(base.TestCase):
         self.assertEqual("snapshot", sot.resource_key)
         self.assertEqual("snapshots", sot.resources_key)
         self.assertEqual("/snapshots", sot.base_path)
-        self.assertEqual("volume", sot.service.service_type)
+        self.assertEqual("block-storage", sot.service.service_type)
         self.assertTrue(sot.allow_get)
         self.assertTrue(sot.allow_update)
         self.assertTrue(sot.allow_create)

openstack/tests/unit/block_storage/v2/test_type.py

@@ -31,7 +31,7 @@ class TestType(base.TestCase):
         self.assertEqual("volume_type", sot.resource_key)
         self.assertEqual("volume_types", sot.resources_key)
         self.assertEqual("/types", sot.base_path)
-        self.assertEqual("volume", sot.service.service_type)
+        self.assertEqual("block-storage", sot.service.service_type)
         self.assertTrue(sot.allow_create)
         self.assertTrue(sot.allow_get)
         self.assertTrue(sot.allow_delete)

openstack/tests/unit/block_storage/v2/test_volume.py

@@ -68,7 +68,7 @@ class TestVolume(base.TestCase):
         self.assertEqual("volume", sot.resource_key)
         self.assertEqual("volumes", sot.resources_key)
         self.assertEqual("/volumes", sot.base_path)
-        self.assertEqual("volume", sot.service.service_type)
+        self.assertEqual("block-storage", sot.service.service_type)
         self.assertTrue(sot.allow_get)
         self.assertTrue(sot.allow_create)
         self.assertTrue(sot.allow_update)

openstack/tests/unit/config/test_cloud_config.py

@@ -143,32 +143,17 @@ class TestCloudRegion(base.TestCase):
         self.assertEqual('mage', cc.get_service_type('image'))
         self.assertEqual('compute', cc.get_service_type('compute'))
         self.assertEqual('1', cc.get_api_version('volume'))
-        self.assertEqual('volume', cc.get_service_type('volume'))
+        self.assertEqual('block-storage', cc.get_service_type('volume'))
         self.assertEqual('http://compute.example.com',
                          cc.get_endpoint('compute'))
         self.assertIsNone(cc.get_endpoint('image'))
         self.assertIsNone(cc.get_service_name('compute'))
         self.assertEqual('locks', cc.get_service_name('identity'))
 
-    def test_volume_override(self):
-        cc = cloud_region.CloudRegion("test1", "region-al", fake_services_dict)
-        cc.config['volume_api_version'] = '2'
-        self.assertEqual('volumev2', cc.get_service_type('volume'))
-
-    def test_volume_override_v3(self):
-        cc = cloud_region.CloudRegion("test1", "region-al", fake_services_dict)
-        cc.config['volume_api_version'] = '3'
-        self.assertEqual('volumev3', cc.get_service_type('volume'))
-
-    def test_workflow_override_v2(self):
-        cc = cloud_region.CloudRegion("test1", "region-al", fake_services_dict)
-        cc.config['workflow_api_version'] = '2'
-        self.assertEqual('workflowv2', cc.get_service_type('workflow'))
-
     def test_no_override(self):
         """Test no override happens when defaults are not configured"""
         cc = cloud_region.CloudRegion("test1", "region-al", fake_services_dict)
-        self.assertEqual('volume', cc.get_service_type('volume'))
+        self.assertEqual('block-storage', cc.get_service_type('volume'))
         self.assertEqual('workflow', cc.get_service_type('workflow'))
         self.assertEqual('not-exist', cc.get_service_type('not-exist'))
 

releasenotes/notes/ksa-discovery-86a4ef00d85ea87f.yaml

@@ -0,0 +1,5 @@
+---
+other:
+  - |
+    All endpoint discovery logic is now handled by keystoneauth. There should
+    be no behavior differences.

requirements.txt

@@ -8,7 +8,7 @@ requestsexceptions>=1.2.0 # Apache-2.0
 jsonpatch!=1.20,>=1.16 # BSD
 six>=1.10.0 # MIT
 os-service-types>=1.2.0 # Apache-2.0
-keystoneauth1>=3.4.0 # Apache-2.0
+keystoneauth1>=3.6.0 # Apache-2.0
 deprecation>=1.0 # Apache-2.0
 
 munch>=2.1.0 # MIT