openstack
/
nova
Code Issues Proposed changes

[placement] Don't use floats in microversion handling 

When microversioned request handling was added to placement some
comparisons were added that treat the microversions as floats. This
will break as soon as the right hand side of the version chnages
from one digit to two. A float of 1.1 and 1.10 is the same. A
version string of '1.1' and '1.10' is not.

This patch changes the internals of version handling to always use
Version objects which represent the versions using a name tuple
(major=1, minor=10). It also changes the externals to expect versions
as strings ('1.10'). Where floats were present before, these have
been changed to strings. Where tuples have been accepted, these are
still accepted, but strings are as well.

This is an internal only change, users shouldn't notice any
difference as their versions (in the openstack-api-version header)
were always strings and we haven't made it to version 1.10, sorry
'1.10', yet.

If developers try to version_handler with something other than a
version string, test runs will fail very early (at import time).

Change-Id: Ic2b655ac4c75c6104eddecd174f193413a0764d2
Closes-Bug: #1675143
This commit is contained in:
Chris Dent 2017-03-23 12:33:00 +00:00
parent ab44e75fb3
commit b2b366aa4a
3 changed files with 51 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
nova/api/openstack/placement/handlers/resource_class.py
View File

@ -65,7 +65,7 @@ def _serialize_resource_classes(environ, rcs):
@wsgi_wrapper.PlacementWsgify
@microversion.version_handler(1.2)
@microversion.version_handler('1.2')
@util.require_content('application/json')
def create_resource_class(req):
"""POST to create a resource class.
@ -97,7 +97,7 @@ def create_resource_class(req):
@wsgi_wrapper.PlacementWsgify
@microversion.version_handler(1.2)
@microversion.version_handler('1.2')
def delete_resource_class(req):
"""DELETE to destroy a single resource class.
@ -123,7 +123,7 @@ def delete_resource_class(req):
@wsgi_wrapper.PlacementWsgify
@microversion.version_handler(1.2)
@microversion.version_handler('1.2')
@util.check_accept('application/json')
def get_resource_class(req):
"""Get a single resource class.
@ -144,7 +144,7 @@ def get_resource_class(req):
@wsgi_wrapper.PlacementWsgify
@microversion.version_handler(1.2)
@microversion.version_handler('1.2')
@util.check_accept('application/json')
def list_resource_classes(req):
"""GET a list of resource classes.
@ -164,7 +164,7 @@ def list_resource_classes(req):
@wsgi_wrapper.PlacementWsgify
@microversion.version_handler(1.2)
@microversion.version_handler('1.2')
@util.require_content('application/json')
def update_resource_class(req):
"""PUT to update a single resource class.

23
nova/api/openstack/placement/microversion.py
View File

@ -87,6 +87,10 @@ def raise_http_status_code_if_not_version(req, status_code, min_version,
:raises: HTTP status code if the specified microversion does not match
:raises: KeyError if status_code is not a valid HTTP status code
"""
if not isinstance(min_version, tuple):
min_version = parse_version_string(min_version)
if max_version and not isinstance(max_version, tuple):
max_version = parse_version_string(max_version)
want_version = req.environ[MICROVERSION_ENVIRON]
if not want_version.matches(min_version, max_version):
raise webob.exc.status_map[status_code]
@ -143,9 +147,6 @@ class Version(collections.namedtuple('Version', 'major minor')):
def __str__(self):
return '%s.%s' % (self.major, self.minor)
def __float__(self):
return float(self.__str__())
@property
def max_version(self):
if not self.MAX_VERSION:
@ -214,7 +215,7 @@ def _fully_qualified_name(obj):
return name
def _find_method(f, version_float):
def _find_method(f, version):
"""Look in VERSIONED_METHODS for method with right name matching version.
If no match is found raise a 404.
@ -224,7 +225,7 @@ def _find_method(f, version_float):
# just in case.
method_list = VERSIONED_METHODS.get(qualified_name, [])
for min_version, max_version, func in method_list:
if min_version <= version_float <= max_version:
if min_version <= version <= max_version:
return func
raise webob.exc.HTTPNotFound()
@ -246,18 +247,18 @@ def version_handler(min_ver, max_ver=None):
maximum version allowed for the decorated method.
"""
def decorator(f):
min_version_float = float(min_ver)
min_version = parse_version_string(min_ver)
if max_ver:
max_version_float = float(max_ver)
max_version = parse_version_string(max_ver)
else:
max_version_float = float(max_version_string())
max_version = parse_version_string(max_version_string())
qualified_name = _fully_qualified_name(f)
VERSIONED_METHODS[qualified_name].append(
(min_version_float, max_version_float, f))
(min_version, max_version, f))
def decorated_func(req, *args, **kwargs):
version_float = float(req.environ[MICROVERSION_ENVIRON])
return _find_method(f, version_float)(req, *args, **kwargs)
version = req.environ[MICROVERSION_ENVIRON]
return _find_method(f, version)(req, *args, **kwargs)
# Sort highest min version to beginning of list.
VERSIONED_METHODS[qualified_name].sort(key=lambda x: x[0],

39
nova/tests/unit/api/openstack/placement/test_microversion.py
View File

@ -37,17 +37,32 @@ class TestMicroversionDecoration(test.NoDBTestCase):
self.assertEqual(0, len(microversion.VERSIONED_METHODS))
fully_qualified_method = microversion._fully_qualified_name(
handler)
microversion.version_handler('1.0', '1.9')(handler)
microversion.version_handler('1.1', '1.10')(handler)
microversion.version_handler('2.0')(handler)
methods_data = microversion.VERSIONED_METHODS[fully_qualified_method]
stored_method_data = methods_data[-1]
self.assertEqual(2, len(methods_data))
self.assertEqual(1.0, stored_method_data[0])
self.assertEqual(1.9, stored_method_data[1])
self.assertEqual(microversion.Version(1, 1), stored_method_data[0])
self.assertEqual(microversion.Version(1, 10), stored_method_data[1])
self.assertEqual(handler, stored_method_data[2])
self.assertEqual(2.0, methods_data[0][0])
self.assertEqual(microversion.Version(2, 0), methods_data[0][0])
def test_version_handler_float_exception(self):
self.assertRaises(AttributeError,
microversion.version_handler(1.1),
handler)
def test_version_handler_nan_exception(self):
self.assertRaises(TypeError,
microversion.version_handler('cow'),
handler)
def test_version_handler_tuple_exception(self):
self.assertRaises(AttributeError,
microversion.version_handler((1, 1)),
handler)
class TestMicroversionIntersection(test.NoDBTestCase):
@ -121,9 +136,23 @@ class TestMicroversionUtility(test.NoDBTestCase):
microversion.raise_http_status_code_if_not_version,
self.req, 405, (1, 5))
def test_raise_keyerror_out_of_date_version(self):
def test_raise_405_out_of_date_version_max(self):
version_obj = microversion.parse_version_string('1.4')
self.req.environ['placement.microversion'] = version_obj
self.assertRaises(webob.exc.HTTPMethodNotAllowed,
microversion.raise_http_status_code_if_not_version,
self.req, 405, (1, 2), '1.3')
def test_raise_keyerror_out_of_date_version_tuple(self):
version_obj = microversion.parse_version_string('1.4')
self.req.environ['placement.microversion'] = version_obj
self.assertRaises(KeyError,
microversion.raise_http_status_code_if_not_version,
self.req, 999, (1, 5))
def test_raise_keyerror_out_of_date_version_string(self):
version_obj = microversion.parse_version_string('1.4')
self.req.environ['placement.microversion'] = version_obj
self.assertRaises(KeyError,
microversion.raise_http_status_code_if_not_version,
self.req, 999, '1.5')