openstack
/
nova
Code Issues Proposed changes

[placement] Parse forbidden traits in query strings 

Add support to parse_qs_request_groups to optionally process the
"required" parameter for forbidden traits, expressed as the
normal trait form prefixed with "!". The parsing removes "!"
prefixed traits from the required_traits attribute on a
RequestGroup and puts them (without the "!") in a new
forbidden_traits attribute.

The optionality allows the caller to control the processing
based on the microversion. Later code will add that to
the list resource providers and list allocation canidates
handler code, in the same microversion.

This allows declaring forbidden handling only at the API layer.
Subsequent patches will turn on forbidden handling in the object/
data layer but no forbidden traits will reach that layer until
the microversion turns on allow_forbidden in the API.

Partially implements blueprint placement-forbidden-traits
Change-Id: Icc9dc2631a0eca9e998b9ce8706c7b6920e0cb69
This commit is contained in:
Chris Dent 2018-03-27 09:52:00 +01:00
parent 4c1a1be88a
commit 8bde4042da
4 changed files with 162 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
nova/api/openstack/placement/lib.py
View File

@ -18,7 +18,7 @@ common library that both placement and its consumers can require."""
class RequestGroup(object):
def __init__(self, use_same_provider=True, resources=None,
required_traits=None, member_of=None):
required_traits=None, forbidden_traits=None, member_of=None):
"""Create a grouping of resource and trait requests.
:param use_same_provider:
@ -28,9 +28,11 @@ class RequestGroup(object):
in any resource provider in the same tree, or a sharing provider.
:param resources: A dict of { resource_class: amount, ... }
:param required_traits: A set of { trait_name, ... }
:param forbidden_traits: A set of { trait_name, ... }
:param member_of: A list of [ aggregate_UUID, ... ]
"""
self.use_same_provider = use_same_provider
self.resources = resources or {}
self.required_traits = required_traits or set()
self.forbidden_traits = forbidden_traits or set()
self.member_of = member_of or []

62
nova/api/openstack/placement/util.py
View File

@ -292,7 +292,17 @@ def normalize_resources_qs_param(qs):
return result
def normalize_traits_qs_param(val):
def valid_trait(trait, allow_forbidden):
"""Return True if the provided trait is the expected form.
When allow_forbidden is True, then a leading '!' is acceptable.
"""
if trait.startswith('!') and not allow_forbidden:
return False
return True
def normalize_traits_qs_param(val, allow_forbidden=False):
"""Parse a traits query string parameter value.
Note that this method doesn't know or care about the query parameter key,
@ -304,15 +314,22 @@ def normalize_traits_qs_param(val):
:param val: A traits query parameter value: a comma-separated string of
trait names.
:param allow_forbidden: If True, accept forbidden traits (that is, traits
prefixed by '!') as a valid form when notifying
the caller that the provided value is not properly
formed.
:return: A set of trait names.
:raises `webob.exc.HTTPBadRequest` if the val parameter is not in the
expected format.
"""
ret = set(substr.strip() for substr in val.split(','))
if not all(trait for trait in ret):
expected_form = 'HW_CPU_X86_VMX,CUSTOM_MAGIC'
if allow_forbidden:
expected_form = 'HW_CPU_X86_VMX,!CUSTOM_MAGIC'
if not all(trait and valid_trait(trait, allow_forbidden) for trait in ret):
msg = _("Invalid query string parameters: Expected 'required' "
"parameter value of the form: HW_CPU_X86_VMX,CUSTOM_MAGIC. "
"Got: %s") % val
"parameter value of the form: %(form)s. "
"Got: %(val)s") % {'form': expected_form, 'val': val}
raise webob.exc.HTTPBadRequest(msg)
return ret
@ -346,7 +363,7 @@ def normalize_member_of_qs_param(val):
return ret
def parse_qs_request_groups(qsdict):
def parse_qs_request_groups(qsdict, allow_forbidden=False):
"""Parse numbered resources, traits, and member_of groupings out of a
querystring dict.
@ -365,6 +382,12 @@ def parse_qs_request_groups(qsdict):
suffix, the RequestGroup.use_same_provider attribute is False; for the
numbered groups it is True.
If a trait in the required parameter is prefixed with ``!`` this
indicates that that trait must not be present on the resource
providers in the group. That is, the trait is forbidden. Forbidden traits
are only processed if ``allow_forbidden`` is True. This allows the
caller to control processing based on microversion handling.
The return is a list of these RequestGroup instances.
As an example, if qsdict represents the query string:
@ -375,7 +398,7 @@ def parse_qs_request_groups(qsdict):
&resources1=SRIOV_NET_VF:2
&required1=CUSTOM_PHYSNET_PUBLIC,CUSTOM_SWITCH_A
&resources2=SRIOV_NET_VF:1
&required2=CUSTOM_PHYSNET_PRIVATE
&required2=!CUSTOM_PHYSNET_PUBLIC
...the return value will be:
@ -410,13 +433,14 @@ def parse_qs_request_groups(qsdict):
resources={
"SRIOV_NET_VF": 1,
},
required_traits=[
"CUSTOM_PHYSNET_PRIVATE",
forbidden_traits=[
"CUSTOM_PHYSNET_PUBLIC",
],
),
]
:param qsdict: The MultiDict representing the querystring on a GET.
:param allow_forbidden: If True, parse for forbidden traits.
:return: A list of RequestGroup instances.
:raises `webob.exc.HTTPBadRequest` if any value is malformed, or if a
trait list is given without corresponding resources.
@ -441,7 +465,8 @@ def parse_qs_request_groups(qsdict):
if prefix == _QS_RESOURCES:
request_group.resources = normalize_resources_qs_param(val)
elif prefix == _QS_REQUIRED:
request_group.required_traits = normalize_traits_qs_param(val)
request_group.required_traits = normalize_traits_qs_param(
val, allow_forbidden=allow_forbidden)
elif prefix == _QS_MEMBER_OF:
request_group.member_of = normalize_member_of_qs_param(val)
@ -460,6 +485,25 @@ def parse_qs_request_groups(qsdict):
' values: %s')
raise webob.exc.HTTPBadRequest(msg % ', '.join(orphans))
# Make adjustments for forbidden traits by stripping forbidden out
# of required.
if allow_forbidden:
conflicting_traits = []
for suff, group in by_suffix.items():
forbidden = [trait for trait in group.required_traits
if trait.startswith('!')]
group.required_traits = (group.required_traits - set(forbidden))
group.forbidden_traits = set([trait.lstrip('!') for trait in
forbidden])
conflicts = group.forbidden_traits & group.required_traits
if conflicts:
conflicting_traits.append('required%s: (%s)'
% (suff, ', '.join(conflicts)))
if conflicting_traits:
msg = _('Conflicting required and forbidden traits found in the '
'following traits keys: %s')
raise webob.exc.HTTPBadRequest(msg % ', '.join(conflicting_traits))
# NOTE(efried): The sorting is not necessary for the API, but it makes
# testing easier.
return [by_suffix[suff] for suff in sorted(by_suffix)]

8
nova/tests/functional/api/openstack/placement/gabbits/allocation-candidates.yaml
View File

@ -213,6 +213,14 @@ tests:
response_strings:
- "Invalid query string parameters: Expected 'required' parameter value of the form: HW_CPU_X86_VMX,CUSTOM_MAGIC."
- name: get allocation candidates with forbidden trait pre-forbidden
GET: /allocation_candidates?resources=VCPU:1,MEMORY_MB:1024,DISK_GB:100&required=!CUSTOM_MAGIC
status: 400
request_headers:
openstack-api-version: placement 1.17
response_strings:
- "Invalid query string parameters: Expected 'required' parameter value of the form: HW_CPU_X86_VMX,CUSTOM_MAGIC."
- name: get allocation candidates with required trait
GET: /allocation_candidates?resources=VCPU:1,MEMORY_MB:1024,DISK_GB:100&required=HW_CPU_X86_SSE
status: 200

100
nova/tests/unit/api/openstack/placement/test_util.py
View File

@ -430,12 +430,12 @@ class TestNormalizeTraitsQsParam(test.NoDBTestCase):
class TestParseQsResourcesAndTraits(test.NoDBTestCase):
@staticmethod
def do_parse(qstring):
def do_parse(qstring, allow_forbidden=False):
"""Converts a querystring to a MultiDict, mimicking request.GET, and
runs parse_qs_request_groups on it.
"""
return util.parse_qs_request_groups(webob.multidict.MultiDict(
urlparse.parse_qsl(qstring)))
urlparse.parse_qsl(qstring)), allow_forbidden=allow_forbidden)
def assertRequestGroupsEqual(self, expected, observed):
self.assertEqual(len(expected), len(observed))
@ -617,6 +617,102 @@ class TestParseQsResourcesAndTraits(test.NoDBTestCase):
'&resources3=CUSTOM_MAGIC:123')
self.assertRaises(webob.exc.HTTPBadRequest, self.do_parse, qs)
def test_forbidden_one_group(self):
"""When forbidden are allowed this will parse, but otherwise will
indicate an invalid trait.
"""
qs = ('resources=VCPU:2,MEMORY_MB:2048'
'&required=CUSTOM_PHYSNET1,!CUSTOM_SWITCH_BIG')
expected_forbidden = [
pl.RequestGroup(
use_same_provider=False,
resources={
'VCPU': 2,
'MEMORY_MB': 2048,
},
required_traits={
'CUSTOM_PHYSNET1',
},
forbidden_traits={
'CUSTOM_SWITCH_BIG',
}
),
]
expected_message = (
"Invalid query string parameters: Expected 'required' parameter "
"value of the form: HW_CPU_X86_VMX,CUSTOM_MAGIC. Got: "
"CUSTOM_PHYSNET1,!CUSTOM_SWITCH_BIG")
exc = self.assertRaises(webob.exc.HTTPBadRequest, self.do_parse, qs)
self.assertEqual(expected_message, six.text_type(exc))
self.assertRequestGroupsEqual(
expected_forbidden, self.do_parse(qs, allow_forbidden=True))
def test_forbidden_conflict(self):
qs = ('resources=VCPU:2,MEMORY_MB:2048'
'&required=CUSTOM_PHYSNET1,!CUSTOM_PHYSNET1')
expected_message = (
'Conflicting required and forbidden traits found '
'in the following traits keys: required: (CUSTOM_PHYSNET1)')
exc = self.assertRaises(webob.exc.HTTPBadRequest, self.do_parse, qs,
allow_forbidden=True)
self.assertEqual(expected_message, six.text_type(exc))
def test_forbidden_two_groups(self):
qs = ('resources=VCPU:2,MEMORY_MB:2048&resources1=CUSTOM_MAGIC:1'
'&required1=CUSTOM_PHYSNET1,!CUSTOM_PHYSNET2')
expected = [
pl.RequestGroup(
use_same_provider=False,
resources={
'VCPU': 2,
'MEMORY_MB': 2048,
},
),
pl.RequestGroup(
resources={
'CUSTOM_MAGIC': 1,
},
required_traits={
'CUSTOM_PHYSNET1',
},
forbidden_traits={
'CUSTOM_PHYSNET2',
}
),
]
self.assertRequestGroupsEqual(
expected, self.do_parse(qs, allow_forbidden=True))
def test_forbidden_separate_groups_no_conflict(self):
qs = ('resources1=CUSTOM_MAGIC:1&required1=CUSTOM_PHYSNET1'
'&resources2=CUSTOM_MAGIC:1&required2=!CUSTOM_PHYSNET1')
expected = [
pl.RequestGroup(
use_same_provider=True,
resources={
'CUSTOM_MAGIC': 1,
},
required_traits={
'CUSTOM_PHYSNET1',
}
),
pl.RequestGroup(
use_same_provider=True,
resources={
'CUSTOM_MAGIC': 1,
},
forbidden_traits={
'CUSTOM_PHYSNET1',
}
),
]
self.assertRequestGroupsEqual(
expected, self.do_parse(qs, allow_forbidden=True))
class TestPickLastModified(test.NoDBTestCase):