Ensure we discover only when we should
There are a two interrelated pieces in this patch which are around fixing up places where discovery was being re-run inappropriately. They fall out from adding tests for the functionality and couldn't be sanely shifted back further in the stack without a big dance. Switch the default for "discover_versions" on all of the calls that return an EndpointData to "True". It's a new feature and is a thing that doesn't make a ton of sense to call if you don't want discovery run. However, get_endpoint uses it, so needs to be able to pass in discover_version=False, so the option is still useful. Make sure that get_endpoint and other places where ksa calls get_endpoint_data on behalf of the user work as before without unneeded discovery. Add tests to show that we use actually use the discovery cache properly when we've previously done discovery that can satisfy the new request. This works from the microversion optimization patch, but we had to clean up a couple of things to show it fully in a test. Change-Id: I54053336edf1b3c2bd35a77dbd78f56388b8e806
This commit is contained in:
Monty Taylor
parent
d061a61aae
commit
429b19c88c
|
|
@ -209,8 +209,6 @@ class Adapter(object):
|
|||
self._set_endpoint_filter_kwargs(kwargs)
|
||||
if self.endpoint_override:
|
||||
kwargs['endpoint_override'] = self.endpoint_override
|
||||
# Default discover_versions to True, but allow override.
|
||||
kwargs.setdefault('discover_versions', True)
|
||||
|
||||
return self.session.get_endpoint_data(auth or self.auth, **kwargs)
|
||||
|
||||
|
|
|
|||
|
|
@ -544,7 +544,7 @@ class EndpointData(object):
|
|||
def get_versioned_data(self, session, version=None,
|
||||
authenticated=False, allow=None, cache=None,
|
||||
allow_version_hack=True, project_id=None,
|
||||
discover_versions=False,
|
||||
discover_versions=True,
|
||||
min_version=None, max_version=None):
|
||||
"""Run version discovery for the service described.
|
||||
|
||||
|
|
@ -571,10 +571,11 @@ class EndpointData(object):
|
|||
(optional)
|
||||
:param bool authenticated: Include a token in the discovery call.
|
||||
(optional) Defaults to False.
|
||||
:param bool discover_versions: Whether to perform version discovery
|
||||
even if a version string wasn't
|
||||
requested. This is useful for getting
|
||||
microversion information.
|
||||
:param bool discover_versions: Whether to get version metadata from
|
||||
the version discovery document even
|
||||
if it's not neccessary to fulfill the
|
||||
major version request. (optional,
|
||||
defaults to True)
|
||||
:param min_version: The minimum version that is acceptable. Mutually
|
||||
exclusive with version. If min_version is given
|
||||
with no max_version it is as if max version is
|
||||
|
|
|
|||
|
|
@ -159,7 +159,7 @@ class BaseIdentityPlugin(plugin.BaseAuthPlugin):
|
|||
def get_endpoint_data(self, session, service_type=None, interface=None,
|
||||
region_name=None, service_name=None, version=None,
|
||||
allow={}, allow_version_hack=True,
|
||||
discover_versions=False, skip_discovery=False,
|
||||
discover_versions=True, skip_discovery=False,
|
||||
min_version=None, max_version=None,
|
||||
endpoint_override=None,
|
||||
**kwargs):
|
||||
|
|
@ -197,10 +197,11 @@ class BaseIdentityPlugin(plugin.BaseAuthPlugin):
|
|||
:param bool allow_version_hack: Allow keystoneauth to hack up catalog
|
||||
URLS to support older schemes.
|
||||
(optional, default True)
|
||||
:param bool discover_versions: Whether to perform version discovery
|
||||
even if a version string wasn't
|
||||
requested. This is useful for getting
|
||||
microversion information.
|
||||
:param bool discover_versions: Whether to get version metadata from
|
||||
the version discovery document even
|
||||
if it's not neccessary to fulfill the
|
||||
major version request. (optional,
|
||||
defaults to True)
|
||||
:param bool skip_discovery: Whether to skip version discovery even
|
||||
if a version has been given. This is useful
|
||||
if endpoint_override or similar has been
|
||||
|
|
|
|||
|
|
@ -883,9 +883,6 @@ class Session(object):
|
|||
:returns: Endpoint data if available or None.
|
||||
:rtype: keystoneauth1.discover.EndpointData
|
||||
"""
|
||||
if 'endpoint_override' in kwargs:
|
||||
# Default discover_versions to True, but allow override.
|
||||
kwargs.setdefault('discover_versions', True)
|
||||
auth = self._auth_required(auth, 'determine endpoint URL')
|
||||
return auth.get_endpoint_data(self, **kwargs)
|
||||
|
||||
|
|
|
|||
|
|
@ -487,6 +487,13 @@ class CommonIdentityTests(object):
|
|||
self.assertIsNone(endpoint_v3)
|
||||
|
||||
def test_endpoint_data_no_version(self):
|
||||
path = self.TEST_COMPUTE_ADMIN[self.TEST_COMPUTE_ADMIN.find(':') + 1:]
|
||||
|
||||
disc = fixture.DiscoveryList(v2=False, v3=False)
|
||||
disc.add_v2(path + '/v2.0')
|
||||
disc.add_v3(path + '/v3')
|
||||
|
||||
self.stub_url('GET', [], base_url=self.TEST_COMPUTE_ADMIN, json=disc)
|
||||
a = self.create_auth_plugin()
|
||||
s = session.Session(auth=a)
|
||||
|
||||
|
|
@ -494,8 +501,29 @@ class CommonIdentityTests(object):
|
|||
service_type='compute',
|
||||
interface='admin')
|
||||
|
||||
self.assertEqual(self.TEST_COMPUTE_ADMIN + '/v3', data.url)
|
||||
|
||||
def test_endpoint_data_no_version_no_discovery(self):
|
||||
a = self.create_auth_plugin()
|
||||
s = session.Session(auth=a)
|
||||
|
||||
data = a.get_endpoint_data(session=s,
|
||||
service_type='compute',
|
||||
interface='admin',
|
||||
discover_versions=False)
|
||||
|
||||
self.assertEqual(self.TEST_COMPUTE_ADMIN, data.url)
|
||||
|
||||
def test_endpoint_no_version(self):
|
||||
a = self.create_auth_plugin()
|
||||
s = session.Session(auth=a)
|
||||
|
||||
data = a.get_endpoint(session=s,
|
||||
service_type='compute',
|
||||
interface='admin')
|
||||
|
||||
self.assertEqual(self.TEST_COMPUTE_ADMIN, data)
|
||||
|
||||
def test_endpoint_data_relative_version(self):
|
||||
# need to construct list this way for relative
|
||||
disc = fixture.DiscoveryList(v2=False, v3=False)
|
||||
|
|
@ -542,7 +570,7 @@ class CommonIdentityTests(object):
|
|||
data = a.get_endpoint_data(session=s,
|
||||
service_type='compute',
|
||||
interface='admin')
|
||||
self.assertEqual(self.TEST_COMPUTE_ADMIN, data.url)
|
||||
self.assertEqual(v3_compute, data.url)
|
||||
|
||||
v2_data = data.get_versioned_data(s, version='2.0')
|
||||
self.assertEqual(v2_compute, v2_data.url)
|
||||
|
|
@ -592,17 +620,25 @@ class CommonIdentityTests(object):
|
|||
href=self.TEST_VOLUME.versions['v2'].discovery.public,
|
||||
id='v2.0', status='SUPPORTED')
|
||||
|
||||
# We should only try to fetch the versioned discovery url once
|
||||
resps = [{'json': disc}, {'status_code': 500}]
|
||||
self.requests_mock.get(
|
||||
self.TEST_VOLUME.versions['v3'].discovery.public, resps)
|
||||
|
||||
body = 'SUCCESS'
|
||||
self.stub_url('GET', ['path'], text=body)
|
||||
|
||||
a = self.create_auth_plugin()
|
||||
s = session.Session(auth=a)
|
||||
|
||||
# volume endpoint ends in v3, we should not make an API call
|
||||
endpoint = a.get_endpoint(session=s,
|
||||
service_type='volumev3',
|
||||
interface='public',
|
||||
version='3.0')
|
||||
self.assertEqual(self.TEST_VOLUME.catalog.public, endpoint)
|
||||
|
||||
resps = [{'json': disc}, {'status_code': 500}]
|
||||
|
||||
# We should only try to fetch the versioned discovery url once
|
||||
self.requests_mock.get(
|
||||
self.TEST_VOLUME.versions['v3'].discovery.public, resps)
|
||||
|
||||
data = a.get_endpoint_data(session=s,
|
||||
service_type='volumev3',
|
||||
interface='public')
|
||||
|
|
@ -615,11 +651,10 @@ class CommonIdentityTests(object):
|
|||
self.assertEqual(self.TEST_VOLUME.versions['v3'].service.public,
|
||||
v3_data.url)
|
||||
self.assertEqual(self.TEST_VOLUME.catalog.public, v3_data.catalog_url)
|
||||
# TODO(mordred) fix in next patch - updating mock urls exposes bug
|
||||
# self.assertEqual((3, 0), v3_data.min_microversion)
|
||||
# self.assertEqual((3, 20), v3_data.max_microversion)
|
||||
# self.assertEqual(self.TEST_VOLUME.versions['v3'].service.public,
|
||||
# v3_data.service_url)
|
||||
self.assertEqual((3, 0), v3_data.min_microversion)
|
||||
self.assertEqual((3, 20), v3_data.max_microversion)
|
||||
self.assertEqual(self.TEST_VOLUME.versions['v3'].service.public,
|
||||
v3_data.service_url)
|
||||
|
||||
# Because of the v3 optimization before, requesting v2 should now go
|
||||
# find the unversioned endpoint
|
||||
|
|
@ -638,6 +673,73 @@ class CommonIdentityTests(object):
|
|||
self.assertEqual(None, v2_data.min_microversion)
|
||||
self.assertEqual(None, v2_data.max_microversion)
|
||||
|
||||
def test_get_versioned_data_volume_project_id_unversioned_first(self):
|
||||
|
||||
disc = fixture.DiscoveryList(v2=False, v3=False)
|
||||
|
||||
# The version discovery dict will not have a project_id
|
||||
disc.add_nova_microversion(
|
||||
href=self.TEST_VOLUME.versions['v3'].discovery.public,
|
||||
id='v3.0', status='CURRENT',
|
||||
min_version='3.0', version='3.20')
|
||||
|
||||
# Adding a v2 version to a service named volumev3 is not
|
||||
# an error. The service itself is cinder and has more than
|
||||
# one major version.
|
||||
disc.add_nova_microversion(
|
||||
href=self.TEST_VOLUME.versions['v2'].discovery.public,
|
||||
id='v2.0', status='SUPPORTED')
|
||||
|
||||
body = 'SUCCESS'
|
||||
self.stub_url('GET', ['path'], text=body)
|
||||
|
||||
a = self.create_auth_plugin()
|
||||
s = session.Session(auth=a)
|
||||
|
||||
# cinder endpoint ends in v3, we should not make an API call
|
||||
endpoint = a.get_endpoint(session=s,
|
||||
service_type='volumev3',
|
||||
interface='public',
|
||||
version='3.0')
|
||||
self.assertEqual(self.TEST_VOLUME.catalog.public, endpoint)
|
||||
|
||||
resps = [{'json': disc}, {'status_code': 500}]
|
||||
|
||||
# We should only try to fetch the unversioned non-project_id url once
|
||||
self.requests_mock.get(self.TEST_VOLUME.unversioned.public, resps)
|
||||
|
||||
# Fetch v2.0 first - since that doesn't match endpoint optimization,
|
||||
# it should fetch the unversioned endpoint
|
||||
v2_data = s.get_endpoint_data(service_type='volumev3',
|
||||
interface='public',
|
||||
version='2.0',
|
||||
project_id=self.project_id)
|
||||
|
||||
# Even though we never requested volumev2 from the catalog, we should
|
||||
# wind up re-constructing it via version discovery and re-appending
|
||||
# the project_id to the URL
|
||||
self.assertEqual(self.TEST_VOLUME.versions['v2'].service.public,
|
||||
v2_data.url)
|
||||
self.assertEqual(self.TEST_VOLUME.versions['v2'].service.public,
|
||||
v2_data.service_url)
|
||||
self.assertEqual(self.TEST_VOLUME.catalog.public, v2_data.catalog_url)
|
||||
self.assertEqual(None, v2_data.min_microversion)
|
||||
self.assertEqual(None, v2_data.max_microversion)
|
||||
|
||||
# Since we fetched from the unversioned endpoint to satisfy the
|
||||
# request for v2, we should have all the relevant data cached in the
|
||||
# discovery object - and should not fetch anything new.
|
||||
v3_data = v2_data.get_versioned_data(
|
||||
s, version='3.0', project_id=self.project_id)
|
||||
|
||||
self.assertEqual(self.TEST_VOLUME.versions['v3'].service.public,
|
||||
v3_data.url)
|
||||
self.assertEqual(self.TEST_VOLUME.catalog.public, v3_data.catalog_url)
|
||||
self.assertEqual((3, 0), v3_data.min_microversion)
|
||||
self.assertEqual((3, 20), v3_data.max_microversion)
|
||||
self.assertEqual(self.TEST_VOLUME.versions['v3'].service.public,
|
||||
v3_data.service_url)
|
||||
|
||||
def test_asking_for_auth_endpoint_ignores_checks(self):
|
||||
a = self.create_auth_plugin()
|
||||
s = session.Session(auth=a)
|
||||
|
|
|
|||
Loading…
Reference in New Issue