From c85ae69ee977fdea8cdef6aab0e8e563d2e0a803 Mon Sep 17 00:00:00 2001 From: Tetsuro Nakamura Date: Fri, 14 Sep 2018 13:57:49 +0900 Subject: [PATCH] Fix aggregate members in nested alloc candidates When placement picks up allocation candidates, the aggregates of nested providers were assumed as the same as root providers. This means that the `GET /allocation_candidates API` ignored the aggregates on the nested providers. This could result in the lack of allocation candidates when an aggregate is on a nested provider but the aggregate is not on its root provider and the aggregate is specified in the API by the `member_of` query parameter. This patch fixes the bug changing it to consider the aggregates not only on root rps but also on the nested rp itself and adds a release note for this. The document which explains the whole constraint of `member_of` and other query parameters with nested providers, will be submitted in a follow up patch. Change-Id: I9a11a577174f85a1b47a9e895bb25cadd90bd2ea Closes-Bug: #1792503 --- api-ref/source/parameters.yaml | 29 +++++++++++++++---- placement/objects/resource_provider.py | 9 ++++-- .../allocation-candidates-bug-1792503.yaml | 22 ++++---------- ...ug-1792503-member-of-5c10df94caf3bd08.yaml | 11 +++++++ 4 files changed, 47 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/bug-1792503-member-of-5c10df94caf3bd08.yaml diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 9c76f4b10..0362a3bc5 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -70,8 +70,8 @@ member_of: &member_of description: > 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 be associated with at least one of the - aggregates identified by uuid:: + returned resource providers must directly be associated with at least one + of the aggregates identified by uuid:: member_of=5e08ea53-c4c6-448e-9334-ac4953de3cfa member_of=in:42896e0d-205d-4fe3-bd1e-100924931787,5e08ea53-c4c6-448e-9334-ac4953de3cfa @@ -87,6 +87,25 @@ member_of: &member_of min_version: 1.3 member_of_1_21: <<: *member_of + description: > + A string representing an aggregate uuid; or the prefix ``in:`` followed by + a comma-separated list of strings representing aggregate uuids. The + resource providers in the allocation request in the response must directly + or via the root provider be associated with the aggregate or aggregates + identified by uuid:: + + member_of=5e08ea53-c4c6-448e-9334-ac4953de3cfa + member_of=in:42896e0d-205d-4fe3-bd1e-100924931787,5e08ea53-c4c6-448e-9334-ac4953de3cfa + + **Starting from microversion 1.24** specifying multiple ``member_of`` query + string parameters is possible. Multiple ``member_of`` parameters will + result in filtering providers that are directly or via root provider + associated with aggregates listed in all of the ``member_of`` query string + values. For example, to get the providers that are associated with + aggregate A as well as associated with any of aggregates B or C, the user + could issue the following query:: + + member_of=AGGA_UUID&member_of=in:AGGB_UUID,AGGC_UUID min_version: 1.21 member_of_granular: type: string @@ -95,9 +114,9 @@ member_of_granular: description: > 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 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 + 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 diff --git a/placement/objects/resource_provider.py b/placement/objects/resource_provider.py index 10b2eb151..7367c4d95 100644 --- a/placement/objects/resource_provider.py +++ b/placement/objects/resource_provider.py @@ -3240,14 +3240,19 @@ def _get_trees_matching_all(ctx, resources, required_traits, forbidden_traits, # If 'member_of' has values, do a separate lookup to identify the # resource providers that meet the member_of constraints. if member_of: + involved_rps = set(p[0] for p in provs_with_inv) | trees_with_inv rps_in_aggs = _provider_ids_matching_aggregates(ctx, member_of, - rp_ids=trees_with_inv) + rp_ids=involved_rps) if not rps_in_aggs: # Short-circuit. The user either asked for a non-existing # aggregate or there were no resource providers that matched # the requirements... return [] - provs_with_inv = set(p for p in provs_with_inv if p[1] in rps_in_aggs) + # If its root p[1] is in the specified aggregate, let's take that + # resource provider p[0], otherwise the resource provider p[0] + # itself should be in the aggregate + provs_with_inv = set( + p for p in provs_with_inv if set([p[1], p[0]]) & rps_in_aggs) if (not required_traits and not forbidden_traits) or ( any(sharing.values())): diff --git a/placement/tests/functional/gabbits/allocation-candidates-bug-1792503.yaml b/placement/tests/functional/gabbits/allocation-candidates-bug-1792503.yaml index e1a33e6cc..7be4fceb4 100644 --- a/placement/tests/functional/gabbits/allocation-candidates-bug-1792503.yaml +++ b/placement/tests/functional/gabbits/allocation-candidates-bug-1792503.yaml @@ -45,14 +45,8 @@ tests: response_json_paths: # Aggregate C is *NOT* on the root, so we should get only NUMA1_1 # here that is only the rp in aggregate C. - # --------------------- - # Bug#1792503: It lacks allocation candidates when an aggregate on the - # nested rp but not on the root rp has been specified in - # the `member_of` query parameter. - # --------------------- - # $.allocation_requests.`len`: 1 - # $.allocation_requests..allocations["$ENVIRON['NUMA1_1_UUID']"].resources.VCPU: 1 - $.allocation_requests.`len`: 0 + $.allocation_requests.`len`: 1 + $.allocation_requests..allocations["$ENVIRON['NUMA1_1_UUID']"].resources.VCPU: 1 - name: get allocation candidates with shared storage GET: /allocation_candidates?resources=VCPU:1,DISK_GB:1000 @@ -107,12 +101,6 @@ tests: # [ # (numa1-1, ss2), # ] - # --------------------- - # Bug#1792503: It lacks allocation candidates when an aggregate on the - # nested rp but not on the root rp has been specified in - # the `member_of` query parameter. - # --------------------- - # $.allocation_requests.`len`: 1 - # $.allocation_requests..allocations["$ENVIRON['NUMA1_1_UUID']"].resources.VCPU: 1 - # $.allocation_requests..allocations["$ENVIRON['SS2_UUID']"].resources.DISK_GB: 1000 - $.allocation_requests.`len`: 0 + $.allocation_requests.`len`: 1 + $.allocation_requests..allocations["$ENVIRON['NUMA1_1_UUID']"].resources.VCPU: 1 + $.allocation_requests..allocations["$ENVIRON['SS2_UUID']"].resources.DISK_GB: 1000 diff --git a/releasenotes/notes/bug-1792503-member-of-5c10df94caf3bd08.yaml b/releasenotes/notes/bug-1792503-member-of-5c10df94caf3bd08.yaml new file mode 100644 index 000000000..320d1d940 --- /dev/null +++ b/releasenotes/notes/bug-1792503-member-of-5c10df94caf3bd08.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Previously, when an aggregate was specified by the ``member_of`` + query parameter in the ``GET /allocation_candidates`` operation, + the non-root providers in the aggregate were excluded unless their + root provider was also in the aggregate. With this release, the + non-root providers directly associated with the aggregate are also + considered. See the `Bug#1792503`_ for details. + + .. _Bug#1792503: https://bugs.launchpad.net/nova/+bug/1792503