diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 425807481..dadb2ec82 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -102,6 +102,36 @@ member_of: &member_of any of aggregates B or C, the user could issue the following query:: member_of=AGGA_UUID&member_of=in:AGGB_UUID,AGGC_UUID + + **Starting from microversion 1.32** specifying forbidden aggregates is + supported in the ``member_of`` query string parameter. Forbidden aggregates + are prefixed with a ``!``. This negative expression can also be used in + multiple ``member_of`` parameters:: + + member_of=AGGA_UUID&member_of=!AGGB_UUID + + would translate logically to "Candidate resource providers must be in aggA + and *not* in AGGB. + + We do NOT support ``!`` on the values within ``in:``, but we support + ``!in:``. Both of the following two example queries return candidate + resource providers that are NOT in AGGA, AGGB, or AGGC:: + + member_of=!in:AGGA_UUID,AGGB_UUID,AGGC_UUID + member_of=!AGGA_UUID&member_of=!AGGB_UUID&member_of=!AGGC_UUID + + We do not check if the same aggregate uuid is in both positive and negative + expression to raise 400 BadRequest. We still return 200 for such cases. + For example:: + + member_of=AGGA_UUID&member_of=!AGGA_UUID + + would return empty ``allocation_requests`` and ``provider_summaries``, + while:: + + member_of=in:AGGA_UUID,AGGB_UUID&member_of=!AGGA_UUID + + would return resource providers that are NOT in AGGA but in AGGB. min_version: 1.3 member_of_1_21: <<: *member_of @@ -124,6 +154,35 @@ member_of_1_21: could issue the following query:: member_of=AGGA_UUID&member_of=in:AGGB_UUID,AGGC_UUID + + **Starting from microversion 1.32** specifying forbidden aggregates is + supported in the ``member_of`` query string parameter. Forbidden aggregates + are prefixed with a ``!``. This negative expression can also be used in + multiple ``member_of`` parameters:: + + member_of=AGGA_UUID&member_of=!AGGB_UUID + + would translate logically to "Candidate resource providers must be in AGGA + and *not* in AGGB. + + We do NOT support ``!`` on the values within ``in:``, but we support + ``!in:``. Both of the following two example queries return candidate + resource providers that are NOT in AGGA, AGGB, or AGGC:: + + member_of=!in:AGGA_UUID,AGGB_UUID,AGGC_UUID + member_of=!AGGA_UUID&member_of=!AGGB_UUID&member_of=!AGGC_UUID + + We do not check if the same aggregate uuid is in both positive and negative + expression to return 400 BadRequest. We still return 200 for such cases. + For example:: + member_of=AGGA_UUID&member_of=!AGGA_UUID + + would return empty ``allocation_requests`` and ``provider_summaries``, + while:: + + member_of=in:AGGA_UUID,AGGB_UUID&member_of=!AGGA_UUID + + would return resource providers that are NOT in AGGA but in AGGB. min_version: 1.21 member_of_granular: type: string @@ -133,14 +192,22 @@ member_of_granular: A string representing an aggregate uuid; or the prefix ``in:`` followed by a comma-separated list of strings representing aggregate uuids. The returned resource providers must directly be associated with at least one - of the aggregates identified by uuid. The parameter key is ``member_ofN``, - where ``N`` represents a positive integer suffix corresponding with a - ``resourcesN`` parameter. The value format is the same as for the - (unnumbered) ``member_of`` parameter; but all of the resources and traits - specified in a numbered grouping will always be satisfied by the same - resource provider. Separate groupings - numbered or unnumbered - may or may - not be satisfied by the same provider, depending on the value of the - ``group_policy`` parameter. + of the aggregates identified by uuid. + + **Starting from microversion 1.32** specifying forbidden aggregates is + supported. Forbidden aggregates are expressed with a ``!`` prefix; or the + prefix ``!in:`` followed by a comma-separated list of strings representing + aggregate uuids. The returned resource providers must not directly be + associated with any of the aggregates identified by uuid. + + The parameter key is ``member_ofN``, where ``N`` represents a positive + integer suffix corresponding with a ``resourcesN`` parameter. The value + format is the same as for the (unnumbered) ``member_of`` parameter; but + all of the resources and traits specified in a numbered grouping will + always be satisfied by the same resource provider. + Separate groupings - numbered or unnumbered - may or may not be satisfied + by the same provider, depending on the value of the ``group_policy`` + parameter. It is an error to specify a ``member_ofN`` parameter without a corresponding ``resourcesN`` parameter with the same suffix. diff --git a/placement/handlers/resource_provider.py b/placement/handlers/resource_provider.py index c2a6c9ba5..a6da43cc1 100644 --- a/placement/handlers/resource_provider.py +++ b/placement/handlers/resource_provider.py @@ -222,7 +222,8 @@ def list_resource_providers(req): # special handling of member_of qparam since we allow multiple member_of # params at microversion 1.24. if 'member_of' in req.GET: - filters['member_of'] = util.normalize_member_of_qs_params(req) + filters['member_of'], filters['forbidden_aggs'] = ( + util.normalize_member_of_qs_params(req)) qpkeys = ('uuid', 'name', 'in_tree', 'resources', 'required') for attr in qpkeys: diff --git a/placement/lib.py b/placement/lib.py index aa7dc3fd5..8c7b8f3bc 100644 --- a/placement/lib.py +++ b/placement/lib.py @@ -35,7 +35,7 @@ _QS_KEY_PATTERN = re.compile( class RequestGroup(object): def __init__(self, use_same_provider=True, resources=None, required_traits=None, forbidden_traits=None, member_of=None, - in_tree=None): + in_tree=None, forbidden_aggs=None): """Create a grouping of resource and trait requests. :param use_same_provider: @@ -57,6 +57,7 @@ class RequestGroup(object): self.forbidden_traits = forbidden_traits or set() self.member_of = member_of or [] self.in_tree = in_tree + self.forbidden_aggs = forbidden_aggs or set() def __str__(self): ret = 'RequestGroup(use_same_provider=%s' % str(self.use_same_provider) @@ -101,8 +102,8 @@ class RequestGroup(object): # request group. # TODO(jaypipes): Do validation of query parameters using # JSONSchema - request_group.member_of = util.normalize_member_of_qs_params( - req, suffix) + request_group.member_of, request_group.forbidden_aggs = ( + util.normalize_member_of_qs_params(req, suffix)) elif prefix == _QS_IN_TREE: request_group.in_tree = util.normalize_in_tree_qs_params( val) @@ -119,7 +120,8 @@ class RequestGroup(object): 'Found the following orphaned traits keys: %s') raise webob.exc.HTTPBadRequest(msg % ', '.join(orphans)) orphans = [('member_of%s' % suff) for suff, group in by_suffix.items() - if group.member_of and not group.resources] + if not group.resources and ( + group.member_of or group.forbidden_aggs)] if orphans: msg = ('All member_of parameters must be associated with ' 'resources. Found the following orphaned member_of ' diff --git a/placement/microversion.py b/placement/microversion.py index 588222586..4c33a28eb 100644 --- a/placement/microversion.py +++ b/placement/microversion.py @@ -79,6 +79,8 @@ VERSIONS = [ # inventories and allocations. '1.31', # Add in_tree and in_tree queryparam on # `GET /allocation_candidates` API + '1.32', # Support negative member_of queryparams on + # `GET /resource_providers` and `GET /allocation_candidates` ] diff --git a/placement/objects/allocation_candidate.py b/placement/objects/allocation_candidate.py index d18bdb854..db9f7f14b 100644 --- a/placement/objects/allocation_candidate.py +++ b/placement/objects/allocation_candidate.py @@ -121,6 +121,8 @@ class AllocationCandidates(object): trait_map.update(trait_obj.ids_from_names(context, traits)) member_of = request.member_of + forbidden_aggs = request.forbidden_aggs + tree_root_id = None if request.in_tree: tree_ids = rp_obj.provider_ids_from_uuid(context, request.in_tree) @@ -132,9 +134,6 @@ class AllocationCandidates(object): LOG.debug("getting allocation candidates in the same tree " "with the root provider %s", tree_ids.root_uuid) - # TODO(tetsuro): get the forbidden aggregates from the request - forbidden_aggs = [] - any_sharing = any(sharing_providers.values()) if not request.use_same_provider and (has_trees or any_sharing): # TODO(jaypipes): The check/callout to handle trees goes here. diff --git a/placement/rest_api_version_history.rst b/placement/rest_api_version_history.rst index 4b8c27c95..9e26bf952 100644 --- a/placement/rest_api_version_history.rst +++ b/placement/rest_api_version_history.rst @@ -566,3 +566,40 @@ and ``DISK_GB`` resources from ``sharing1`` might look like:: ?resources=VCPU:1&in_tree= &resources1=VGPU:1&in_tree1= &resources2=DISK_GB:100&in_tree2= + + +Train +----- + +1.32 - Support forbidden aggregates +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. versionadded:: Train + +Add support for forbidden aggregates in ``member_of`` queryparam +in ``GET /resource_providers`` and ``GET /allocation_candidates``. +Forbidden aggregates are prefixed with a ``!``. + +This negative expression can also be used in multiple ``member_of`` +parameters:: + + ?member_of=in:,&member_of=&member_of=! + +would translate logically to + +"Candidate resource providers must be at least one of agg1 or agg2, +definitely in agg3 and definitely *not* in agg4." + +We do NOT support ``!`` within the ``in:`` list:: + + ?member_of=in:,,! + +but we support ``!in:`` prefix:: + + ?member_of=!in:,, + +which is equivalent to:: + + ?member_of=!&member_of=!&member_of=!`` + +where candidate resource providers must not be in agg1, agg2, or agg3. diff --git a/placement/tests/functional/fixtures/gabbits.py b/placement/tests/functional/fixtures/gabbits.py index d067874da..24607dd3a 100644 --- a/placement/tests/functional/fixtures/gabbits.py +++ b/placement/tests/functional/fixtures/gabbits.py @@ -361,8 +361,8 @@ class NUMAAggregateFixture(APIFixture): class NonSharedStorageFixture(APIFixture): - """An APIFixture that has two compute nodes with local storage that do not - use shared storage. + """An APIFixture that has three compute nodes with local storage that do + not use shared storage. """ def start_fixture(self): super(NonSharedStorageFixture, self).start_fixture() @@ -376,12 +376,14 @@ class NonSharedStorageFixture(APIFixture): cn1 = tb.create_provider(self.context, 'cn1') cn2 = tb.create_provider(self.context, 'cn2') + cn3 = tb.create_provider(self.context, 'cn3') os.environ['CN1_UUID'] = cn1.uuid os.environ['CN2_UUID'] = cn2.uuid + os.environ['CN3_UUID'] = cn3.uuid # Populate compute node inventory for VCPU, RAM and DISK - for cn in (cn1, cn2): + for cn in (cn1, cn2, cn3): tb.add_inventory(cn, 'VCPU', 24) tb.add_inventory(cn, 'MEMORY_MB', 128 * 1024) tb.add_inventory(cn, 'DISK_GB', 2000) diff --git a/placement/tests/functional/gabbits/allocation-candidates-bug-1792503.yaml b/placement/tests/functional/gabbits/allocation-candidates-bug-1792503.yaml index 7be4fceb4..62aceffb1 100644 --- a/placement/tests/functional/gabbits/allocation-candidates-bug-1792503.yaml +++ b/placement/tests/functional/gabbits/allocation-candidates-bug-1792503.yaml @@ -7,7 +7,7 @@ defaults: request_headers: x-auth-token: admin accept: application/json - openstack-api-version: placement 1.29 + openstack-api-version: placement 1.32 tests: @@ -104,3 +104,71 @@ tests: $.allocation_requests.`len`: 1 $.allocation_requests..allocations["$ENVIRON['NUMA1_1_UUID']"].resources.VCPU: 1 $.allocation_requests..allocations["$ENVIRON['SS2_UUID']"].resources.DISK_GB: 1000 + +# Tests for negative aggregate membership from microversion 1.32. +# The negative aggregate feature had not yet been implemented when bug1792503 +# was reported, but we include the tests here to make sure that it is +# consistent with the positive aggregate strategy with nested providers above. +- name: get allocation candidates with shared storage without aggregate A + GET: /allocation_candidates?resources=VCPU:1,DISK_GB:1000&member_of=!$ENVIRON['AGGA_UUID'] + response_json_paths: + # Aggregate A is on the root rps (both cn1 and cn2) so it spans on the + # whole tree. We have no allocation requests here. + $.allocation_requests.`len`: 0 + +- name: get allocation candidates with shared storage without aggregate B + GET: /allocation_candidates?resources=VCPU:1,DISK_GB:1000&member_of=!$ENVIRON['AGGB_UUID'] + response_json_paths: + # Aggregate B is on the root of cn2 and it spans on the whole tree + # including rps of NUMA2_1 and NUMA2_2 so we exclude them. + # As a result, there should be 4 allocation candidates: + # [ + # (numa1-1, ss1), (numa1-2, ss1), + # (numa1-1, ss2), (numa1-2, ss2), + # ] + $.allocation_requests.`len`: 4 + $.allocation_requests..allocations["$ENVIRON['NUMA1_1_UUID']"].resources.VCPU: [1, 1] + $.allocation_requests..allocations["$ENVIRON['NUMA1_2_UUID']"].resources.VCPU: [1, 1] + $.allocation_requests..allocations["$ENVIRON['SS1_UUID']"].resources.DISK_GB: [1000, 1000] + $.allocation_requests..allocations["$ENVIRON['SS2_UUID']"].resources.DISK_GB: [1000, 1000] + +- name: get allocation candidates with shared storage without aggregate C + GET: /allocation_candidates?resources=VCPU:1,DISK_GB:1000&member_of=!$ENVIRON['AGGC_UUID'] + response_json_paths: + # Aggregate C is *NOT* on the root. We should exclude NUMA1_1 and SS2, + # but we should get NUMA1_2 + # [ + # (numa1-2, ss1), (numa2-1, ss1), (numa2-2, ss1) + # ] + $.allocation_requests.`len`: 3 + $.allocation_requests..allocations["$ENVIRON['NUMA1_2_UUID']"].resources.VCPU: 1 + $.allocation_requests..allocations["$ENVIRON['NUMA2_1_UUID']"].resources.VCPU: 1 + $.allocation_requests..allocations["$ENVIRON['NUMA2_2_UUID']"].resources.VCPU: 1 + $.allocation_requests..allocations["$ENVIRON['SS1_UUID']"].resources.DISK_GB: [1000, 1000, 1000] + +- name: get allocation candidates with shared storage in (aggA or aggB) and (not aggC) + GET: /allocation_candidates?resources=VCPU:1,DISK_GB:1000&member_of=in:$ENVIRON['AGGA_UUID'],$ENVIRON['AGGB_UUID']&member_of=!$ENVIRON['AGGC_UUID'] + response_json_paths: + # Aggregate C is *NOT* on the root. We should exclude NUMA1_1 and SS2, + # but we should get NUMA1_2 + # [ + # (numa1-2, ss1), (numa2-1, ss1), (numa2-2, ss1) + # ] + $.allocation_requests.`len`: 3 + $.allocation_requests..allocations["$ENVIRON['NUMA1_2_UUID']"].resources.VCPU: 1 + $.allocation_requests..allocations["$ENVIRON['NUMA2_1_UUID']"].resources.VCPU: 1 + $.allocation_requests..allocations["$ENVIRON['NUMA2_2_UUID']"].resources.VCPU: 1 + $.allocation_requests..allocations["$ENVIRON['SS1_UUID']"].resources.DISK_GB: [1000, 1000, 1000] + +- name: get allocation candidates with shared storage neither in aggB nor in aggC but in aggA + GET: /allocation_candidates?resources=VCPU:1,DISK_GB:1000&member_of=$ENVIRON['AGGA_UUID']&member_of=!in:$ENVIRON['AGGB_UUID'],$ENVIRON['AGGC_UUID'] + response_json_paths: + # Aggregate B is on the root. We should exclude all the rps on CN2 + # Aggregate C is *NOT* on the root. We should exclude NUMA1_1 and SS2, + # but we should get NUMA1_1 + # [ + # (numa1-1, ss1) + # ] + $.allocation_requests.`len`: 1 + $.allocation_requests..allocations["$ENVIRON['NUMA1_2_UUID']"].resources.VCPU: 1 + $.allocation_requests..allocations["$ENVIRON['SS1_UUID']"].resources.DISK_GB: 1000 diff --git a/placement/tests/functional/gabbits/allocation-candidates-member-of.yaml b/placement/tests/functional/gabbits/allocation-candidates-member-of.yaml index 22b6ce6f8..a5a58a02d 100644 --- a/placement/tests/functional/gabbits/allocation-candidates-member-of.yaml +++ b/placement/tests/functional/gabbits/allocation-candidates-member-of.yaml @@ -31,7 +31,7 @@ tests: GET: /allocation_candidates?resources=VCPU:1,MEMORY_MB:1024,DISK_GB:100&member_of=$ENVIRON['AGGA_UUID'],$ENVIRON['AGGB_UUID'] status: 400 response_strings: - - Multiple values for 'member_of' must be prefixed with the 'in:' keyword + - Multiple values for 'member_of' must be prefixed with the 'in:' or '!in:' keyword using the valid microversion. - name: get allocation candidates multiple member_of with 'in:' but invalid values GET: /allocation_candidates?resources=VCPU:1,MEMORY_MB:1024,DISK_GB:100&member_of=in:$ENVIRON['AGGA_UUID'],INVALID_UUID @@ -139,3 +139,115 @@ tests: status: 200 response_json_paths: $.allocation_requests.`len`: 1 + +# Tests for negative aggregate membership from microversion 1.32 +# Now the aggregation map is as below +# { +# CN1: [AGGA, AGGC], +# CN2: [AGGA, AGGB], +# CN3: [] +# } +- name: negative agg error on old microversion with ! prefix + GET: /allocation_candidates?resources=VCPU:1&member_of=!$ENVIRON['AGGA_UUID'] + status: 400 + request_headers: + openstack-api-version: placement 1.31 + response_strings: + - "Forbidden member_of parameters are not supported in the specified microversion" + +- name: negative agg error on old microversion with !in prefix + GET: /allocation_candidates?resources=VCPU:1&member_of=!in:$ENVIRON['AGGA_UUID'] + status: 400 + request_headers: + openstack-api-version: placement 1.31 + response_strings: + - "Forbidden member_of parameters are not supported in the specified microversion" + +- name: negative agg error on orphaned queryparam + GET: /allocation_candidates?member_of=!$ENVIRON['AGGA_UUID'] + status: 400 + request_headers: + openstack-api-version: placement 1.32 + response_strings: + - "All member_of parameters must be associated with resources" + +- name: negative agg error on invalid agg + GET: /allocation_candidates?resources=VCPU:1&member_of=!(^o^) + status: 400 + request_headers: + openstack-api-version: placement 1.32 + response_strings: + - "Invalid query string parameters: Expected 'member_of' parameter to contain valid UUID(s)." + +- name: negative agg error on invalid usage of in prefix + GET: /allocation_candidates?resources=VCPU:1&member_of=in:$ENVIRON['AGGA_UUID'],!$ENVIRON['AGGB_UUID'] + status: 400 + request_headers: + openstack-api-version: placement 1.32 + response_strings: + - "Invalid query string parameters: Expected 'member_of' parameter to contain valid UUID(s)." + +- name: negative agg + GET: /allocation_candidates?resources=VCPU:1&member_of=!$ENVIRON['AGGC_UUID'] + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + # CN1 is excluded + $.allocation_requests.`len`: 2 + $.provider_summaries.`len`: 2 + $.allocation_requests..allocations["$ENVIRON['CN2_UUID']"].resources.VCPU: 1 + $.allocation_requests..allocations["$ENVIRON['CN3_UUID']"].resources.VCPU: 1 + +- name: negative agg multiple + GET: /allocation_candidates?resources=VCPU:1&member_of=!in:$ENVIRON['AGGB_UUID'],$ENVIRON['AGGC_UUID'] + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + # Both CN1 and CN2 are excluded + $.allocation_requests.`len`: 1 + $.provider_summaries.`len`: 1 + $.allocation_requests..allocations["$ENVIRON['CN3_UUID']"].resources.VCPU: 1 + +- name: negative agg with positive agg + GET: /allocation_candidates?resources=VCPU:1&member_of=!$ENVIRON['AGGB_UUID']&member_of=$ENVIRON['AGGC_UUID'] + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + # Only CN1 is returned + $.allocation_requests.`len`: 1 + $.provider_summaries.`len`: 1 + $.allocation_requests..allocations["$ENVIRON['CN1_UUID']"].resources.VCPU: 1 + +- name: negative agg multiple with positive agg + GET: /allocation_candidates?resources=VCPU:1&member_of=!in:$ENVIRON['AGGB_UUID'],$ENVIRON['AGGC_UUID']&member_of=$ENVIRON['AGGA_UUID'] + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + # no rp is returned + $.allocation_requests.`len`: 0 + $.provider_summaries.`len`: 0 + +# This request is equivalent to the one in "negative agg with positive agg" +- name: negative agg with the same agg on positive get rp + GET: /allocation_candidates?resources=VCPU:1&member_of=!$ENVIRON['AGGB_UUID']&member_of=in:$ENVIRON['AGGB_UUID'],$ENVIRON['AGGC_UUID'] + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + $.allocation_requests.`len`: 1 + $.provider_summaries.`len`: 1 + $.allocation_requests..allocations["$ENVIRON['CN1_UUID']"].resources.VCPU: 1 + +- name: negative agg with the same agg on positive no rp + GET: /allocation_candidates?resources=VCPU:1&member_of=!$ENVIRON['AGGB_UUID']&member_of=$ENVIRON['AGGB_UUID'] + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + # no rp is returned + $.allocation_requests.`len`: 0 + $.provider_summaries.`len`: 0 diff --git a/placement/tests/functional/gabbits/microversion.yaml b/placement/tests/functional/gabbits/microversion.yaml index 01ee3096d..28aac7f51 100644 --- a/placement/tests/functional/gabbits/microversion.yaml +++ b/placement/tests/functional/gabbits/microversion.yaml @@ -41,13 +41,13 @@ tests: response_json_paths: $.errors[0].title: Not Acceptable -- name: latest microversion is 1.31 +- name: latest microversion is 1.32 GET: / request_headers: openstack-api-version: placement latest response_headers: vary: /openstack-api-version/ - openstack-api-version: placement 1.31 + openstack-api-version: placement 1.32 - name: other accept header bad version GET: / diff --git a/placement/tests/functional/gabbits/resource-provider-aggregates.yaml b/placement/tests/functional/gabbits/resource-provider-aggregates.yaml index 5041be68b..206f22ced 100644 --- a/placement/tests/functional/gabbits/resource-provider-aggregates.yaml +++ b/placement/tests/functional/gabbits/resource-provider-aggregates.yaml @@ -26,6 +26,13 @@ tests: uuid: 5202c48f-c960-4eec-bde3-89c4f22a17b9 status: 200 +- name: post new provider 3 + POST: /resource_providers + data: + name: rp_3 + uuid: 0621521c-ad3a-4f9c-9b72-2933788fab19 + status: 200 + - name: get by aggregates no result GET: '/resource_providers?member_of=in:83a3d69d-8920-48e2-8914-cadfd8fa2f91' response_json_paths: @@ -179,3 +186,114 @@ tests: # Only rp2 returned since it's the only one associated with the duplicated agg $.resource_providers.`len`: 1 $.resource_providers[0].uuid: /893337e9-1e55-49f0-bcfe-6a2f16fbf2f7/ + +# Tests for negative aggregate membership from microversion 1.32 +# Now the aggregation map is as below +# { +# 893337e9-1e55-49f0-bcfe-6a2f16fbf2f7 (rp_1): +# [83a3d69d-8920-48e2-8914-cadfd8fa2f91, 282d469e-29e2-4a8a-8f2e-31b3202b696a] +# 5202c48f-c960-4eec-bde3-89c4f22a17b9 (rp_2) +# [83a3d69d-8920-48e2-8914-cadfd8fa2f91, 99652f11-9f77-46b9-80b7-4b1989be9f8c] +# 0621521c-ad3a-4f9c-9b72-2933788fab19 (rp_3): +# [] +# } +- name: negative agg error on old microversion with ! prefix + GET: /resource_providers?member_of=!282d469e-29e2-4a8a-8f2e-31b3202b696a + status: 400 + request_headers: + openstack-api-version: placement 1.31 + response_strings: + - "Forbidden member_of parameters are not supported in the specified microversion" + +- name: negative agg error on old microversion with !in prefix + GET: /allocation_candidates?resources=VCPU:1&member_of=!in:282d469e-29e2-4a8a-8f2e-31b3202b696a + status: 400 + request_headers: + openstack-api-version: placement 1.31 + response_strings: + - "Forbidden member_of parameters are not supported in the specified microversion" + +- name: negative agg error on invalid agg + GET: /resource_providers?member_of=!(^o^) + status: 400 + request_headers: + openstack-api-version: placement 1.32 + response_strings: + - "Invalid query string parameters: Expected 'member_of' parameter to contain valid UUID(s)." + +- name: negative agg error on invalid usage of in prefix + GET: /resource_providers?resources=VCPU:1&member_of=in:99652f11-9f77-46b9-80b7-4b1989be9f8c,!282d469e-29e2-4a8a-8f2e-31b3202b696a + status: 400 + request_headers: + openstack-api-version: placement 1.32 + response_strings: + - "Invalid query string parameters: Expected 'member_of' parameter to contain valid UUID(s)." + +- name: negative agg + GET: /resource_providers?member_of=!282d469e-29e2-4a8a-8f2e-31b3202b696a + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + # rp_2 is excluded + $.resource_providers.`len`: 2 + $.resource_providers[0].uuid: /5202c48f-c960-4eec-bde3-89c4f22a17b9|0621521c-ad3a-4f9c-9b72-2933788fab19/ + $.resource_providers[1].uuid: /5202c48f-c960-4eec-bde3-89c4f22a17b9|0621521c-ad3a-4f9c-9b72-2933788fab19/ + +- name: negative agg multiple + GET: /resource_providers?member_of=!282d469e-29e2-4a8a-8f2e-31b3202b696a&member_of=!99652f11-9f77-46b9-80b7-4b1989be9f8c + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + # Both rp_1 and rp_2 are excluded + $.resource_providers.`len`: 1 + $.resource_providers[0].uuid: 0621521c-ad3a-4f9c-9b72-2933788fab19 + +- name: negative agg with in prefix + GET: /resource_providers?member_of=!in:282d469e-29e2-4a8a-8f2e-31b3202b696a,99652f11-9f77-46b9-80b7-4b1989be9f8c + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + # The same results as above + $.resource_providers.`len`: 1 + $.resource_providers[0].uuid: 0621521c-ad3a-4f9c-9b72-2933788fab19 + +- name: negative agg with positive agg + GET: /resource_providers?member_of=!282d469e-29e2-4a8a-8f2e-31b3202b696a&member_of=83a3d69d-8920-48e2-8914-cadfd8fa2f91 + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + # only rp_2 is returned + $.resource_providers.`len`: 1 + $.resource_providers[0].uuid: 5202c48f-c960-4eec-bde3-89c4f22a17b9 + +- name: negative agg multiple with positive agg + GET: /resource_providers?member_of=!in:282d469e-29e2-4a8a-8f2e-31b3202b696a,83a3d69d-8920-48e2-8914-cadfd8fa2f91&member_of=99652f11-9f77-46b9-80b7-4b1989be9f8c + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + # no rp is returned + $.resource_providers.`len`: 0 + +# This request is equivalent to the one in "negative agg with positive agg" +- name: negative agg with the same agg on positive get rp + GET: /resource_providers?member_of=!282d469e-29e2-4a8a-8f2e-31b3202b696a&member_of=in:83a3d69d-8920-48e2-8914-cadfd8fa2f91,282d469e-29e2-4a8a-8f2e-31b3202b696a + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + $.resource_providers.`len`: 1 + $.resource_providers[0].uuid: 5202c48f-c960-4eec-bde3-89c4f22a17b9 + +- name: negative agg with the same agg on positive no rp + GET: /resource_providers?member_of=!282d469e-29e2-4a8a-8f2e-31b3202b696a&member_of=282d469e-29e2-4a8a-8f2e-31b3202b696a + status: 200 + request_headers: + openstack-api-version: placement 1.32 + response_json_paths: + # no rp is returned + $.resource_providers.`len`: 0 diff --git a/placement/tests/unit/test_util.py b/placement/tests/unit/test_util.py index 2ac1f58f9..7d9b7b78a 100644 --- a/placement/tests/unit/test_util.py +++ b/placement/tests/unit/test_util.py @@ -667,6 +667,85 @@ class TestParseQsRequestGroups(testtools.TestCase): self.assertRequestGroupsEqual( expected, self.do_parse(qs, version=(1, 24))) + def test_member_of_forbidden_aggs(self): + agg1_uuid = uuidsentinel.agg1 + agg2_uuid = uuidsentinel.agg2 + agg3_uuid = uuidsentinel.agg3 + agg4_uuid = uuidsentinel.agg4 + qs = ('resources=VCPU:2' + '&member_of=%s' + '&member_of=%s' + '&member_of=!%s' + '&member_of=!%s' % ( + agg1_uuid, agg2_uuid, agg3_uuid, agg4_uuid)) + expected = [ + pl.RequestGroup( + use_same_provider=False, + resources={ + 'VCPU': 2, + }, + member_of=[ + set([agg1_uuid]), + set([agg2_uuid]), + ], + forbidden_aggs=set( + [agg3_uuid, agg4_uuid] + ), + ), + ] + self.assertRequestGroupsEqual( + expected, self.do_parse(qs, version=(1, 32))) + + def test_member_of_multiple_forbidden_aggs(self): + agg1_uuid = uuidsentinel.agg1 + agg2_uuid = uuidsentinel.agg2 + agg3_uuid = uuidsentinel.agg3 + qs = ('resources=VCPU:2' + '&member_of=!in:%s,%s,%s' % ( + agg1_uuid, agg2_uuid, agg3_uuid)) + expected = [ + pl.RequestGroup( + use_same_provider=False, + resources={ + 'VCPU': 2, + }, + forbidden_aggs=set( + [agg1_uuid, agg2_uuid, agg3_uuid] + ), + ), + ] + self.assertRequestGroupsEqual( + expected, self.do_parse(qs, version=(1, 32))) + + def test_member_of_forbidden_aggs_prior_microversion(self): + agg1_uuid = uuidsentinel.agg1 + agg2_uuid = uuidsentinel.agg2 + qs = ('resources=VCPU:2' + '&member_of=!%s' + '&member_of=!%s' % (agg1_uuid, agg2_uuid)) + self.assertRaises( + webob.exc.HTTPBadRequest, self.do_parse, qs, version=(1, 31)) + + qs = ('resources=VCPU:2' + '&member_of=!in:%s,%s' % (agg1_uuid, agg2_uuid)) + self.assertRaises( + webob.exc.HTTPBadRequest, self.do_parse, qs, version=(1, 31)) + + def test_member_of_forbidden_aggs_invalid_usage(self): + agg1_uuid = uuidsentinel.agg1 + agg2_uuid = uuidsentinel.agg2 + qs = ('resources=VCPU:2' + '&member_of=in:%s,!%s' % (agg1_uuid, agg2_uuid)) + self.assertRaises( + webob.exc.HTTPBadRequest, self.do_parse, qs, version=(1, 32)) + + agg1_uuid = uuidsentinel.agg1 + agg2_uuid = uuidsentinel.agg2 + qs = ('resources=VCPU:2' + '&member_of=!%s,!%s' % (agg1_uuid, agg2_uuid)) + self.assertRaises( + webob.exc.HTTPBadRequest, self.do_parse, qs, version=(1, 32)) + def test_400_malformed_resources(self): # Somewhat duplicates TestNormalizeResourceQsParam.test_400*. qs = ('resources=VCPU:0,MEMORY_MB:4096,DISK_GB:10' diff --git a/placement/util.py b/placement/util.py index 32c63956f..98e7c5327 100644 --- a/placement/util.py +++ b/placement/util.py @@ -340,54 +340,82 @@ def normalize_traits_qs_param(val, allow_forbidden=False): def normalize_member_of_qs_params(req, suffix=''): """Given a webob.Request object, validate that the member_of querystring parameters are correct. We begin supporting multiple member_of params in - microversion 1.24. + microversion 1.24 and forbidden aggregates in microversion 1.32. :param req: webob.Request object - :return: A list containing sets of UUIDs of aggregates to filter on + :return: A tuple of + required_aggs: A list containing sets of UUIDs of required + aggregates to filter on + forbidden_aggs: A set of UUIDs of forbidden aggregates to filter on :raises `webob.exc.HTTPBadRequest` if the microversion requested is <1.24 and the request contains multiple member_of querystring params + :raises `webob.exc.HTTPBadRequest` if the microversion requested is <1.32 + and the request contains forbidden format of member_of querystring + params with '!' prefix :raises `webob.exc.HTTPBadRequest` if the val parameter is not in the expected format. """ want_version = req.environ[placement.microversion.MICROVERSION_ENVIRON] multi_member_of = want_version.matches((1, 24)) + allow_forbidden = want_version.matches((1, 32)) if not multi_member_of and len(req.GET.getall('member_of' + suffix)) > 1: raise webob.exc.HTTPBadRequest( 'Multiple member_of%s parameters are not supported' % suffix) - values = [] + required_aggs = [] + forbidden_aggs = set() for value in req.GET.getall('member_of' + suffix): - values.append(normalize_member_of_qs_param(value)) - return values + required, forbidden = normalize_member_of_qs_param(value) + if required: + required_aggs.append(required) + if forbidden: + if not allow_forbidden: + raise webob.exc.HTTPBadRequest( + 'Forbidden member_of%s parameters are not supported ' + 'in the specified microversion' % suffix) + forbidden_aggs |= forbidden + return required_aggs, forbidden_aggs def normalize_member_of_qs_param(value): """Parse a member_of query string parameter value. - Valid values are either a single UUID, or the prefix 'in:' followed by two - or more comma-separated UUIDs. + Valid values are one of either + - a single UUID + - the prefix '!' followed by a single UUID + - the prefix 'in:' or '!in:' followed by two or more + comma-separated UUIDs. - :param value: A member_of query parameter of either a single UUID, or a - comma-separated string of two or more UUIDs, prefixed with - the "in:" operator - :return: A set of UUIDs + :param value: A member_of query parameter + :return: A tuple of: + required: A set of aggregate UUIDs at least one of which is required + forbidden: A set of aggregate UUIDs all of which are forbidden :raises `webob.exc.HTTPBadRequest` if the value parameter is not in the expected format. """ - if "," in value and not value.startswith("in:"): + if "," in value and not ( + value.startswith("in:") or value.startswith("!in:")): msg = ("Multiple values for 'member_of' must be prefixed with the " - "'in:' keyword. Got: %s") % value + "'in:' or '!in:' keyword using the valid microversion. " + "Got: %s") % value raise webob.exc.HTTPBadRequest(msg) - if value.startswith('in:'): - value = set(value[3:].split(',')) + + required = forbidden = set() + if value.startswith('!in:'): + forbidden = set(value[4:].split(',')) + elif value.startswith('!'): + forbidden = set([value[1:]]) + elif value.startswith('in:'): + required = set(value[3:].split(',')) else: - value = set([value]) + required = set([value]) + # Make sure the values are actually UUIDs. - for aggr_uuid in value: + for aggr_uuid in (required | forbidden): if not uuidutils.is_uuid_like(aggr_uuid): msg = ("Invalid query string parameters: Expected 'member_of' " "parameter to contain valid UUID(s). Got: %s") % aggr_uuid raise webob.exc.HTTPBadRequest(msg) - return value + return required, forbidden def normalize_in_tree_qs_params(value): diff --git a/releasenotes/notes/negative-aggregate-membership-1dde3cbe27c69279.yaml b/releasenotes/notes/negative-aggregate-membership-1dde3cbe27c69279.yaml new file mode 100644 index 000000000..69e1b51b6 --- /dev/null +++ b/releasenotes/notes/negative-aggregate-membership-1dde3cbe27c69279.yaml @@ -0,0 +1,31 @@ +--- +features: + - | + Add support for forbidden aggregates in ``member_of`` queryparam + in ``GET /resource_providers`` and ``GET /allocation_candidates``. + Forbidden aggregates are prefixed with a ``!``. + + This negative expression can also be used in multiple ``member_of`` + parameters:: + + ?member_of=in:,&member_of=&member_of=! + + would translate logically to + + "Candidate resource providers must be at least one of agg1 or agg2, + definitely in agg3 and definitely *not* in agg4." + + We do NOT support ``!`` within the ``in:`` list:: + + ?member_of=in:,,! + + but we support ``!in:`` prefix:: + + ?member_of=!in:,, + + which is equivalent to:: + + ?member_of=!&member_of=!&member_of=! + + where candidate resource providers must not be in agg1, agg2, or agg3. +