openstack/openstacksdk
Code Issues Proposed changes

Pass default_microversion to adapter constructor

Browse Source 
Based on discussion with the API-SIG, we determined that if someone sets
*_api_version or OS_*_API_VERSION to a value that looks like a
microversion, we should pass it to default_microversion in addition to
using it for major version discovery.

Update the docs to reflect the microversion story (and to remove shade
references)

Also, throw an error on missing support for default microversion

We've given the user the ability to set a default microversion. However,
that microversion might not be available on the cloud in question. Throw
an error in such cases.

Depends-On: https://review.openstack.org/568640
Change-Id: Iad7194dc9e933660c0d0df9130c51d505dda50cb
This commit is contained in:
Monty Taylor 2018-03-30 08:49:07 -05:00
parent dade89b51f
commit 264c802ad8
No known key found for this signature in database
GPG Key ID: 7BAE94BC7141A594
9 changed files with 290 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

85
doc/source/user/microversions.rst
View File

@ -2,50 +2,74 @@
Microversions
=============
As shade rolls out support for consuming microversions, it will do so on a
call by call basis as needed. Just like with major versions, shade should have
logic to handle each microversion for a given REST call it makes, with the
following rules in mind:
As openstacksdk rolls out support for consuming microversions, it will do so
on a call by call basis as needed. Just like with major versions, openstacksdk
should have logic to handle each microversion for a given REST call it makes,
with the following rules in mind:
* If an activity shade performs can be done differently or more efficiently
with a new microversion, the support should be added to openstack.cloud.
* If an activity openstack performs can be done differently or more efficiently
with a new microversion, the support should be added to openstack.cloud and
to the appropriate Proxy class.
* shade should always attempt to use the latest microversion it is aware of
for a given call, unless a microversion removes important data.
* openstacksdk should always attempt to use the latest microversion it is aware
of for a given call, unless a microversion removes important data.
* Microversion selection should under no circumstances be exposed to the user,
except in the case of missing feature error messages.
* Microversion selection should under no circumstances be exposed to the user
in python API calls in the Resource layer or the openstack.cloud layer.
* Microversion selection is exposed to the user in the REST layer via the
``microversion`` argument to each REST call.
* A user of the REST layer may set the default microversion by setting
``{service_type}_default_microversion`` in clouds.yaml or
``OS_{service_type|upper}_DEFAULT_MICROVERSION`` environment variable.
.. note::
Setting the default microversion in any circumstance other than when using
the REST layer is highly discouraged. Both of the higher layers in
openstacksdk provide data normalization as well as logic about which REST
call to make. Setting the default microversion could change the behavior
of the service in question in such a way that openstacksdk does not
understand. If there is a feature of a service that needs a microversion
and it is not already transparently exposed in openstacksdk, please file
a bug.
* If a feature is only exposed for a given microversion and cannot be simulated
for older clouds without that microversion, it is ok to add it to shade but
for older clouds without that microversion, it is ok to add it, but
a clear error message should be given to the user that the given feature is
not available on their cloud. (A message such as "This cloud only supports
not available on their cloud. (A message such as "This cloud supports
a maximum microversion of XXX for service YYY and this feature only exists
on clouds with microversion ZZZ. Please contact your cloud provider for
information about when this feature might be available")
* When adding a feature to shade that only exists behind a new microversion,
* When adding a feature that only exists behind a new microversion,
every effort should be made to figure out how to provide the same
functionality if at all possible, even if doing so is inefficient. If an
inefficient workaround is employed, a warning should be provided to the
user. (the user's workaround to skip the inefficient behavior would be to
stop using that shade API call)
stop using that openstacksdk API call) An example of this is the nova
"get me a network" feature. The logic of "get me a network" can be done
client-side, albeit less efficiently. Adding support for the
"get me a network" feature via nova microversion should also add support for
doing the client-side workaround.
* If shade is aware of logic for more than one microversion, it should always
attempt to use the latest version available for the service for that call.
* Objects returned from shade should always go through normalization and thus
should always conform to shade's documented data model and should never look
different to the shade user regardless of the microversion used for the REST
* If openstacksdk is aware of logic for more than one microversion, it should
always attempt to use the latest version available for the service for that
call.
* Objects returned from openstacksdk should always go through normalization and
thus should always conform to openstacksdk's documented data model. The
objects should never look different to the user regardless of the
microversion used for the REST call.
* If a microversion adds new fields to an object, those fields should be
added to shade's data model contract for that object and the data should
either be filled in by performing additional REST calls if the data is
added to openstacksdk's data model contract for that object and the data
should either be filled in by performing additional REST calls if the data is
available that way, or the field should have a default value of None which
the user can be expected to test for when attempting to use the new value.
* If a microversion removes fields from an object that are part of shade's
* If a microversion removes fields from an object that are part of the
existing data model contract, care should be taken to not use the new
microversion for that call unless forced to by lack of availablity of the
old microversion on the cloud in question. In the case where an old
@ -54,21 +78,22 @@ following rules in mind:
field and document for the user that on some clouds the value may not exist.
* If a microversion removes a field and the outcome is particularly intractable
and impossible to work around without fundamentally breaking shade's users,
and impossible to work around without fundamentally breaking users,
an issue should be raised with the service team in question. Hopefully a
resolution can be found during the period while clouds still have the old
microversion.
* As new calls or objects are added to shade, it is important to check in with
* As new calls or objects are added, it is important to check in with
the service team in question on the expected stability of the object. If
there are known changes expected in the future, even if they may be a few
years off, shade should take care to not add committments to its data model
for those fields/features. It is ok for shade to not have something.
years off, openstacksdk should take care to not add committments to its data
model for those fields/features. It is ok for openstacksdk to not have
something.
..note::
shade does not currently have any sort of "experimental" opt-in API that
would allow a shade to expose things to a user that may not be supportable
under shade's normal compatibility contract. If a conflict arises in the
openstacksdk does not currently have any sort of "experimental" opt-in API
that would allow exposing things to a user that may not be supportable
under the normal compatibility contract. If a conflict arises in the
future where there is a strong desire for a feature but also a lack of
certainty about its stability over time, an experimental API may want to
be explored ... but concrete use cases should arise before such a thing

2
lower-constraints.txt
View File

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

2
openstack/config/_util.py
View File

@ -51,7 +51,9 @@ class VersionRequest(object):
version=None,
min_api_version=None,
max_api_version=None,
default_microversion=None,
):
self.version = version
self.min_api_version = min_api_version
self.max_api_version = max_api_version
self.default_microversion = default_microversion

68
openstack/config/cloud_region.py
View File

@ -16,6 +16,7 @@ import copy
import warnings
from keystoneauth1 import adapter
from keystoneauth1 import discover
import keystoneauth1.exceptions.catalog
from keystoneauth1 import session as ks_session
import os_service_types
@ -225,6 +226,9 @@ class CloudRegion(object):
def get_api_version(self, service_type):
return self._get_config('api_version', service_type)
def get_default_microversion(self, service_type):
return self._get_config('default_microversion', service_type)
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
@ -324,6 +328,9 @@ class CloudRegion(object):
If version is not set and we don't have a configured version, default
to latest.
If version is set, contains a '.', and default_microversion is not
set, also pass it as a default microversion.
"""
version_request = _util.VersionRequest()
if version == 'latest':
@ -339,6 +346,22 @@ class CloudRegion(object):
version_request.max_api_version = 'latest'
else:
version_request.version = version
default_microversion = self.get_default_microversion(service_key)
if not default_microversion and version and '.' in version:
# Some services historically had a .0 in their normal api version.
# Neutron springs to mind with version "2.0". If a user has "2.0"
# set in a variable or config file just because history, we don't
# need to send any microversion headers.
if version.split('.')[1] != "0":
default_microversion = version
# If we're inferring a microversion, don't pass the whole
# string in as api_version, since that tells keystoneauth
# we're looking for a major api version.
version_request.version = version[0]
version_request.default_microversion = default_microversion
return version_request
def get_session_client(
@ -359,7 +382,7 @@ class CloudRegion(object):
"""
version_request = self._get_version_request(service_key, version)
return constructor(
client = constructor(
session=self.get_session(),
service_type=self.get_service_type(service_key),
service_name=self.get_service_name(service_key),
@ -369,7 +392,50 @@ class CloudRegion(object):
min_version=version_request.min_api_version,
max_version=version_request.max_api_version,
endpoint_override=self.get_endpoint(service_key),
default_microversion=version_request.default_microversion,
**kwargs)
if version_request.default_microversion:
default_microversion = version_request.default_microversion
info = client.get_endpoint_data()
if not discover.version_between(
info.min_microversion,
info.max_microversion,
default_microversion
):
if self.get_default_microversion(service_key):
raise exceptions.ConfigException(
"A default microversion for service {service_type} of"
" {default_microversion} was requested, but the cloud"
" only supports a minimum of {min_microversion} and"
" a maximum of {max_microversion}.".format(
service_type=service_key,
default_microversion=default_microversion,
min_microversion=discover.version_to_string(
info.min_microversion),
max_microversion=discover.version_to_string(
info.max_microversion)))
else:
raise exceptions.ConfigException(
"A default microversion for service {service_type} of"
" {default_microversion} was requested, but the cloud"
" only supports a maximum of"
" only supports a minimum of {min_microversion} and"
" a maximum of {max_microversion}. The default"
" microversion was set because a microversion"
" formatted version string, '{api_version}', was"
" passed for the api_version of the service. If it"
" was not intended to set a default microversion"
" please remove anything other than an integer major"
" version from the version setting for"
" the service.".format(
service_type=service_key,
api_version=self.get_api_version(service_key),
default_microversion=default_microversion,
min_microversion=discover.version_to_string(
info.min_microversion),
max_microversion=discover.version_to_string(
info.max_microversion)))
return client
def get_session_endpoint(
self, service_key, min_version=None, max_version=None):

19
openstack/tests/unit/base.py
View File

@ -457,6 +457,17 @@ class TestCase(base.TestCase):
status_code=300,
text=open(discovery_fixture, 'r').read())
def get_nova_discovery_mock_dict(
self,
compute_version_json='compute-version.json',
compute_discovery_url='https://compute.example.com/v2.1/'):
discovery_fixture = os.path.join(
self.fixtures_directory, compute_version_json)
return dict(
method='GET',
uri=compute_discovery_url,
text=open(discovery_fixture, 'r').read())
def get_designate_discovery_mock_dict(self):
discovery_fixture = os.path.join(
self.fixtures_directory, "dns.json")
@ -469,6 +480,14 @@ class TestCase(base.TestCase):
return dict(method='GET', uri="https://bare-metal.example.com/",
text=open(discovery_fixture, 'r').read())
def use_compute_discovery(
self, compute_version_json='compute-version.json',
compute_discovery_url='https://compute.example.com/v2.1/'):
self.__do_register_uris([
self.get_nova_discovery_mock_dict(
compute_version_json, compute_discovery_url),
])
def use_glance(
self, image_version_json='image-version.json',
image_discovery_url='https://image.example.com/'):

30
openstack/tests/unit/fixtures/compute-version.json Normal file
View File

@ -0,0 +1,30 @@
{
"versions": [
{
"status": "SUPPORTED",
"updated": "2011-01-21T11:33:21Z",
"links": [
{
"href": "https://compute.example.com/v2/",
"rel": "self"
}
],
"min_version": "",
"version": "",
"id": "v2.0"
},
{
"status": "CURRENT",
"updated": "2013-07-23T11:33:21Z",
"links": [
{
"href": "https://compute.example.com/v2.1/",
"rel": "self"
}
],
"min_version": "2.10",
"version": "2.53",
"id": "v2.1"
}
]
}

108
openstack/tests/unit/test_microversions.py Normal file
View File

@ -0,0 +1,108 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from openstack import exceptions
from openstack.tests import fakes
from openstack.tests.unit import base
class TestMicroversions(base.TestCase):
def setUp(self):
super(TestMicroversions, self).setUp()
self.use_compute_discovery()
def test_get_bad_inferred_max_microversion(self):
self.cloud.config.config['compute_api_version'] = '2.61'
self.assertRaises(
exceptions.ConfigException,
self.cloud.get_server, 'doesNotExist',
)
self.assert_calls()
def test_get_bad_default_max_microversion(self):
self.cloud.config.config['compute_default_microversion'] = '2.61'
self.assertRaises(
exceptions.ConfigException,
self.cloud.get_server, 'doesNotExist',
)
self.assert_calls()
def test_get_bad_inferred_min_microversion(self):
self.cloud.config.config['compute_api_version'] = '2.7'
self.assertRaises(
exceptions.ConfigException,
self.cloud.get_server, 'doesNotExist',
)
self.assert_calls()
def test_get_bad_default_min_microversion(self):
self.cloud.config.config['compute_default_microversion'] = '2.7'
self.assertRaises(
exceptions.ConfigException,
self.cloud.get_server, 'doesNotExist',
)
self.assert_calls()
def test_inferred_default_microversion(self):
self.cloud.config.config['compute_api_version'] = '2.42'
server1 = fakes.make_fake_server('123', 'mickey')
server2 = fakes.make_fake_server('345', 'mouse')
self.register_uris([
dict(method='GET',
uri=self.get_mock_url(
'compute', 'public', append=['servers', 'detail']),
request_headers={'OpenStack-API-Version': 'compute 2.42'},
json={'servers': [server1, server2]}),
])
r = self.cloud.get_server('mickey', bare=True)
self.assertIsNotNone(r)
self.assertEqual(server1['name'], r['name'])
self.assert_calls()
def test_default_microversion(self):
self.cloud.config.config['compute_default_microversion'] = '2.42'
server1 = fakes.make_fake_server('123', 'mickey')
server2 = fakes.make_fake_server('345', 'mouse')
self.register_uris([
dict(method='GET',
uri=self.get_mock_url(
'compute', 'public', append=['servers', 'detail']),
request_headers={'OpenStack-API-Version': 'compute 2.42'},
json={'servers': [server1, server2]}),
])
r = self.cloud.get_server('mickey', bare=True)
self.assertIsNotNone(r)
self.assertEqual(server1['name'], r['name'])
self.assert_calls()

7
releasenotes/notes/default-microversion-b2401727cb591002.yaml Normal file
View File

@ -0,0 +1,7 @@
---
features:
- |
Versions set in config via ``*_api_version`` or ``OS_*_API_VERSION``
that have a ``.`` in them will be also passed as the default microversion
to the Adapter constructor. An additional config option,
``*_default_microversion`` has been added to support being more explicit.

2
requirements.txt
View File

@ -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.6.0 # Apache-2.0
keystoneauth1>=3.7.0 # Apache-2.0
deprecation>=1.0 # Apache-2.0
munch>=2.1.0 # MIT