From c40eb2951d5cf24589ea357a11aa252978636020 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Thu, 23 Aug 2018 21:07:36 -0500 Subject: [PATCH] Add support for ironic single-version responses The ironic payload looks like: {'id': 'v1', 'links': [{"href": "https://bare-metal.example.com/v1/", "rel": "self"}]} This does not have version info in it, nor min/max ranges for microversion discovery. We can't really get any useful information from this document, but we can at least not fail when trying to deal with it. This should then be upwards-compatible with ironic adding version discovery information to the document that is returned. Change-Id: I47e0f9b295c24ef168f4a033faf573b953025d4c --- keystoneauth1/discover.py | 27 +++++++- keystoneauth1/tests/unit/test_discovery.py | 66 +++++++++++++++++++ ...ironic-microversions-a69bf92ab21f0cf5.yaml | 5 ++ 3 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/ironic-microversions-a69bf92ab21f0cf5.yaml diff --git a/keystoneauth1/discover.py b/keystoneauth1/discover.py index b8c768c6..00b9752a 100644 --- a/keystoneauth1/discover.py +++ b/keystoneauth1/discover.py @@ -119,6 +119,28 @@ def get_version_data(session, url, authenticated=None): except KeyError: pass + # Older Ironic does not actually return a discovery document for the + # single version discovery endpoint, which confuses the single-version + # fallback logic. While there are no known other services returning + # min/max ranges using headers instead of body, this is done in a + # non-Ironic specific manner just in case. + # The existence of this support should not be an indication to any + # OpenStack services that they should ADD this. + if 'id' in body_resp: + body_resp['status'] = Status.CURRENT + for header in resp.headers: + # We lose the case-insensitive quality here + header = header.lower() + if not header.startswith('x-openstack'): + continue + # Once the body starts having these values, stop overriding + # with the header values + if header.endswith('api-minimum-version'): + body_resp.setdefault('min_version', resp.headers[header]) + if header.endswith('api-maximum-version'): + body_resp.setdefault('version', resp.headers[header]) + return [body_resp] + # Otherwise if we query an endpoint like /v2.0 then we will get back # just the one available version. try: @@ -742,7 +764,7 @@ class Discover(object): # so that they know what version they got. We can return the first # entry from version_data, because the user hasn't requested anything # different. - if no_version and url: + if no_version and url and len(version_data) > 0: return version_data[0] # We couldn't find a match. @@ -1121,6 +1143,9 @@ class EndpointData(object): " and {max_version}".format( min_version=version_to_string(min_version), max_version=version_to_string(max_version))) + else: + raise exceptions.DiscoveryFailure( + "No version data found remotely at all") self.min_microversion = discovered_data['min_microversion'] self.max_microversion = discovered_data['max_microversion'] diff --git a/keystoneauth1/tests/unit/test_discovery.py b/keystoneauth1/tests/unit/test_discovery.py index cb035139..b0cdd9c0 100644 --- a/keystoneauth1/tests/unit/test_discovery.py +++ b/keystoneauth1/tests/unit/test_discovery.py @@ -560,6 +560,72 @@ class VersionDataTests(utils.TestCase): self.assertTrue(mock.called_once) + def test_version_data_ironic_microversions(self): + """Validate detection of Ironic microversion ranges.""" + ironic_url = 'https://bare-metal.example.com/v1/' + self.requests_mock.get( + ironic_url, status_code=200, + json={ + 'id': 'v1', + 'links': [{ + "href": ironic_url, + "rel": "self"}], + 'version': '1.40', + 'min_version': '1.10', + }, + # Keep headers so we can verify that body trumps headers + headers={ + 'X-OpenStack-Ironic-API-Minimum-Version': '1.3', + 'X-OpenStack-Ironic-API-Maximum-Version': '1.21', + }) + + self.assertEqual( + [ + { + 'collection': None, + 'version': (1, 0), + 'url': ironic_url, + 'status': discover.Status.CURRENT, + 'raw_status': discover.Status.CURRENT, + 'min_microversion': (1, 10), + 'max_microversion': (1, 40), + 'next_min_version': None, + 'not_before': None, + }, + ], + discover.Discover(self.session, ironic_url).version_data()) + + def test_version_data_legacy_ironic_microversions(self): + """Validate detection of legacy Ironic microversion ranges.""" + ironic_url = 'https://bare-metal.example.com/v1/' + self.requests_mock.get( + ironic_url, status_code=200, + json={ + 'id': 'v1', + 'links': [{ + "href": ironic_url, + "rel": "self"}]}, + headers={ + 'X-OpenStack-Ironic-API-Minimum-Version': '1.3', + 'X-OpenStack-Ironic-API-Maximum-Version': '1.21', + }) + + self.assertEqual( + [ + { + 'collection': None, + 'version': (1, 0), + 'url': ironic_url, + 'status': discover.Status.CURRENT, + 'raw_status': discover.Status.CURRENT, + 'min_microversion': (1, 3), + 'max_microversion': (1, 21), + 'next_min_version': None, + 'not_before': None, + }, + ], + discover.Discover(self.session, ironic_url).version_data()) + def test_version_data_microversions(self): """Validate [min_|max_]version conversion to {min|max}_microversion.""" def setup_mock(versions_in): diff --git a/releasenotes/notes/ironic-microversions-a69bf92ab21f0cf5.yaml b/releasenotes/notes/ironic-microversions-a69bf92ab21f0cf5.yaml new file mode 100644 index 00000000..bb27acec --- /dev/null +++ b/releasenotes/notes/ironic-microversions-a69bf92ab21f0cf5.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixed support for detecting microversion ranges on older Ironic + installations.