diff --git a/nova/api/openstack/placement/lib.py b/nova/api/openstack/placement/lib.py index c3108983a7b9..73f73d8c00e9 100644 --- a/nova/api/openstack/placement/lib.py +++ b/nova/api/openstack/placement/lib.py @@ -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 [] diff --git a/nova/api/openstack/placement/util.py b/nova/api/openstack/placement/util.py index f9db5e034907..81658c79f945 100644 --- a/nova/api/openstack/placement/util.py +++ b/nova/api/openstack/placement/util.py @@ -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)] diff --git a/nova/tests/functional/api/openstack/placement/gabbits/allocation-candidates.yaml b/nova/tests/functional/api/openstack/placement/gabbits/allocation-candidates.yaml index ec405b6fb013..a177329f7d82 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/allocation-candidates.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/allocation-candidates.yaml @@ -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 diff --git a/nova/tests/unit/api/openstack/placement/test_util.py b/nova/tests/unit/api/openstack/placement/test_util.py index 23f583da5175..aa9f3ff4c681 100644 --- a/nova/tests/unit/api/openstack/placement/test_util.py +++ b/nova/tests/unit/api/openstack/placement/test_util.py @@ -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):