diff --git a/placement/objects/allocation_candidate.py b/placement/objects/allocation_candidate.py index 9e5f961e7..4e078f230 100644 --- a/placement/objects/allocation_candidate.py +++ b/placement/objects/allocation_candidate.py @@ -188,16 +188,17 @@ class AllocationCandidates(object): alloc_request_objs = alloc_request_objs[:limit] # Limit summaries to only those mentioned in the allocation reqs. kept_summary_objs = [] - alloc_req_rp_uuids = set() - # Extract resource provider uuids from the resource requests. + alloc_req_root_uuids = set() + # Extract root resource provider uuids from the resource requests. for aro in alloc_request_objs: for arr in aro.resource_requests: - alloc_req_rp_uuids.add(arr.resource_provider.uuid) + alloc_req_root_uuids.add( + arr.resource_provider.root_provider_uuid) for summary in summary_objs: - rp_uuid = summary.resource_provider.uuid + rp_root_uuid = summary.resource_provider.root_provider_uuid # Skip a summary if we are limiting and haven't selected an # allocation request that uses the resource provider. - if rp_uuid not in alloc_req_rp_uuids: + if rp_root_uuid not in alloc_req_root_uuids: continue kept_summary_objs.append(summary) summary_objs = kept_summary_objs diff --git a/placement/tests/functional/gabbits/allocation-candidates.yaml b/placement/tests/functional/gabbits/allocation-candidates.yaml index 5d95d1634..be0787902 100644 --- a/placement/tests/functional/gabbits/allocation-candidates.yaml +++ b/placement/tests/functional/gabbits/allocation-candidates.yaml @@ -427,6 +427,19 @@ tests: $.allocation_requests..allocations["$ENVIRON['PF2_1_UUID']"].resources.SRIOV_NET_VF: 4 $.allocation_requests..allocations["$ENVIRON['PF2_2_UUID']"].resources.SRIOV_NET_VF: 4 +- name: get allocation candidates nested limit + desc: confirm provider summaries are complete, fixes story/2005859 + GET: /allocation_candidates?resources=VCPU:1,SRIOV_NET_VF:4&limit=1 + status: 200 + request_headers: + openstack-api-version: placement 1.29 + response_json_paths: + $.allocation_requests.`len`: 1 + $.allocation_requests[0].allocations.`len`: 2 + # We expect all the providers that share roots with the allocations. + # In this case it the compute node, its two numa nodes and its two pfs. + $.provider_summaries.`len`: 5 + # Make sure that old microversions can return combinations where # sharing providers are involved - name: get allocation candidates shared and nested old microversion diff --git a/placement/tests/unit/objects/test_allocation_candidate.py b/placement/tests/unit/objects/test_allocation_candidate.py index 7f6c9abb4..c4e78806b 100644 --- a/placement/tests/unit/objects/test_allocation_candidate.py +++ b/placement/tests/unit/objects/test_allocation_candidate.py @@ -18,28 +18,34 @@ from placement.tests.unit.objects import base class TestAllocationCandidatesNoDB(base.TestCase): def test_limit_results(self): - # UUIDs don't have to be real UUIDs to test the logic + # Results are limited based on their root provider uuid, not uuid. + # For a more "real" test of this functionality, one that exercises + # nested providers, see the 'get allocation candidates nested limit' + # test in the 'allocation-candidates.yaml' gabbit. aro_in = [ mock.Mock( resource_requests=[ - mock.Mock(resource_provider=mock.Mock(uuid=uuid)) + mock.Mock(resource_provider=mock.Mock( + root_provider_uuid=uuid)) for uuid in (1, 0, 4, 8)]), mock.Mock( resource_requests=[ - mock.Mock(resource_provider=mock.Mock(uuid=uuid)) + mock.Mock(resource_provider=mock.Mock( + root_provider_uuid=uuid)) for uuid in (4, 8, 5)]), mock.Mock( resource_requests=[ - mock.Mock(resource_provider=mock.Mock(uuid=uuid)) + mock.Mock(resource_provider=mock.Mock( + root_provider_uuid=uuid)) for uuid in (1, 7, 6, 4, 8, 5)]), ] - sum1 = mock.Mock(resource_provider=mock.Mock(uuid=1)) - sum0 = mock.Mock(resource_provider=mock.Mock(uuid=0)) - sum4 = mock.Mock(resource_provider=mock.Mock(uuid=4)) - sum8 = mock.Mock(resource_provider=mock.Mock(uuid=8)) - sum5 = mock.Mock(resource_provider=mock.Mock(uuid=5)) - sum7 = mock.Mock(resource_provider=mock.Mock(uuid=7)) - sum6 = mock.Mock(resource_provider=mock.Mock(uuid=6)) + sum1 = mock.Mock(resource_provider=mock.Mock(root_provider_uuid=1)) + sum0 = mock.Mock(resource_provider=mock.Mock(root_provider_uuid=0)) + sum4 = mock.Mock(resource_provider=mock.Mock(root_provider_uuid=4)) + sum8 = mock.Mock(resource_provider=mock.Mock(root_provider_uuid=8)) + sum5 = mock.Mock(resource_provider=mock.Mock(root_provider_uuid=5)) + sum7 = mock.Mock(resource_provider=mock.Mock(root_provider_uuid=7)) + sum6 = mock.Mock(resource_provider=mock.Mock(root_provider_uuid=6)) sum_in = [sum1, sum0, sum4, sum8, sum5, sum7, sum6] aro, sum = allocation_candidate.AllocationCandidates._limit_results( self.context, aro_in, sum_in, 2) diff --git a/releasenotes/notes/limit-nested-allocation-candidates-0886e569d15ad951.yaml b/releasenotes/notes/limit-nested-allocation-candidates-0886e569d15ad951.yaml new file mode 100644 index 000000000..8d01ae9a4 --- /dev/null +++ b/releasenotes/notes/limit-nested-allocation-candidates-0886e569d15ad951.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Limiting nested resource providers with the ``limit=N`` query parameter + when calling ``GET /allocation_candidates`` could result in incomplete + provider summaries. This is now fixed so that all resource providers that + are in the same trees as any provider mentioned in the limited allocation + requests are shown in the provider summaries collection. For more + information see `story/2005859`_. + + .. _story/2005859: https://storyboard.openstack.org/#!/story/2005859