From 5e95c807d1d0ca23c4042d6df54c0cdda72b0caa Mon Sep 17 00:00:00 2001 From: Tetsuro Nakamura Date: Thu, 23 Nov 2017 07:56:12 +0900 Subject: [PATCH] Add aggregates check in allocation candidates Allocation candidates api returned some wrong combinations of non-sharing resource provider and shared resource provider, ignoring aggregates. This patch adds aggregate check when constructing shared allocation request being aware of non-sharing resource in _shared_allocation_request_resources. Change-Id: Ib45fde56706a861df0fc048bbec8a568fd26d85d Closes-Bug:#1732707 --- nova/objects/resource_provider.py | 64 +++++++++++++++++-- .../db/test_allocation_candidates.py | 25 +------- 2 files changed, 59 insertions(+), 30 deletions(-) diff --git a/nova/objects/resource_provider.py b/nova/objects/resource_provider.py index 20b08b23b60d..8d49c1538a06 100644 --- a/nova/objects/resource_provider.py +++ b/nova/objects/resource_provider.py @@ -2852,6 +2852,31 @@ def _get_provider_ids_matching_all(ctx, resources, required_traits): return [r[0] for r in ctx.session.execute(sel)] +@db_api.api_context_manager.reader +def _provider_aggregates(ctx, rp_ids): + """Given a list of resource provider internal IDs, returns a dict, + keyed by those provider IDs, of sets of aggregate ids associated + with that provider. + + :raises: ValueError when rp_ids is empty. + + :param ctx: nova.context.RequestContext object + :param rp_ids: list of resource provider IDs + """ + if not rp_ids: + raise ValueError(_("Expected rp_ids to be a list of resource provider " + "internal IDs, but got an empty list.")) + + rpat = sa.alias(_RP_AGG_TBL, name='rpat') + sel = sa.select([rpat.c.resource_provider_id, + rpat.c.aggregate_id]) + sel = sel.where(rpat.c.resource_provider_id.in_(rp_ids)) + res = collections.defaultdict(set) + for r in ctx.session.execute(sel): + res[r[0]].add(r[1]) + return res + + def _build_provider_summaries(context, usages, prov_traits): """Given a list of dicts of usage information and a map of providers to their associated string traits, returns a dict, keyed by resource provider @@ -2910,13 +2935,27 @@ def _build_provider_summaries(context, usages, prov_traits): return summaries -def _shared_allocation_request_resources(ctx, requested_resources, sharing, - summaries): +def _aggregates_associated_with_providers(a, b, prov_aggs): + """quickly check if the two rps are in the same aggregates + + :param a: resource provider ID for first provider + :param b: resource provider ID for second provider + :param prov_aggs: a dict keyed by resource provider IDs, of sets + of aggregate ids associated with that provider + """ + a_aggs = prov_aggs[a] + b_aggs = prov_aggs[b] + return a_aggs & b_aggs + + +def _shared_allocation_request_resources(ctx, ns_rp_id, requested_resources, + sharing, summaries, prov_aggs): """Returns a dict, keyed by resource class ID, of lists of AllocationRequestResource objects that represent resources that are provided by a sharing provider. :param ctx: nova.context.RequestContext object + :param ns_rp_id: an internal ID of a non-sharing resource provider :param requested_resources: dict, keyed by resource class ID, of amounts being requested for that resource class :param sharing: dict, keyed by resource class ID, of lists of resource @@ -2925,10 +2964,16 @@ def _shared_allocation_request_resources(ctx, requested_resources, sharing, :param summaries: dict, keyed by resource provider ID, of ProviderSummary objects containing usage and trait information for resource providers involved in the overall request + :param prov_aggs: dict, keyed by resource provider ID, of sets of + aggregate ids associated with that provider. """ res_requests = collections.defaultdict(list) for rc_id in sharing: for rp_id in sharing[rc_id]: + aggs_in_both = _aggregates_associated_with_providers( + ns_rp_id, rp_id, prov_aggs) + if not aggs_in_both: + continue summary = summaries[rp_id] rp_uuid = summary.resource_provider.uuid res_req = AllocationRequestResource( @@ -3069,10 +3114,9 @@ def _alloc_candidates_with_shared(ctx, requested_resources, required_traits, # having allocation requests with duplicate sets of resource providers. alloc_prov_ids = [] - # Build a dict, keyed by resource class ID, of AllocationRequestResource - # objects that represent each resource provider for a shared resource - sharing_resource_requests = _shared_allocation_request_resources( - ctx, requested_resources, sharing, summaries) + # Get a dict, keyed by resource provider internal ID, of sets of aggregate + # ids that provider has associated with it + prov_aggregates = _provider_aggregates(ctx, all_rp_ids) for ns_rp_id in ns_rp_ids: if ns_rp_id not in summaries: @@ -3146,6 +3190,14 @@ def _alloc_candidates_with_shared(ctx, requested_resources, required_traits, ) for rc_id, amount in requested_resources.items() if rc_id in ns_resources ] + + # Build a dict, keyed by resource class ID, of lists of + # AllocationRequestResource objects that represent each + # resource provider for a shared resource + sharing_resource_requests = _shared_allocation_request_resources( + ctx, ns_rp_id, requested_resources, + sharing, summaries, prov_aggregates) + # A list of lists of AllocationRequestResource objects for each type of # shared resource class shared_request_groups = [ diff --git a/nova/tests/functional/db/test_allocation_candidates.py b/nova/tests/functional/db/test_allocation_candidates.py index 591d5420e01b..2aed6bbf8a49 100644 --- a/nova/tests/functional/db/test_allocation_candidates.py +++ b/nova/tests/functional/db/test_allocation_candidates.py @@ -1235,9 +1235,7 @@ class AllocationCandidatesTestCase(ProviderDBBase): } )] ) - # NOTE(gibi): We expect two candidates one with cn1, ss1 and ss3 - # and one with cn2, ss1 and ss2 but we get two invalid combinations as - # well. This is reported in bug 1732707 + expected = [ [('cn1', fields.ResourceClass.VCPU, 2), ('ss1', fields.ResourceClass.DISK_GB, 1500), @@ -1245,14 +1243,6 @@ class AllocationCandidatesTestCase(ProviderDBBase): [('cn2', fields.ResourceClass.VCPU, 2), ('ss1', fields.ResourceClass.DISK_GB, 1500), ('ss2', fields.ResourceClass.IPV4_ADDRESS, 2)], - # 1) cn1 and ss2 are not connected so this is not valid - [('cn1', fields.ResourceClass.VCPU, 2), - ('ss1', fields.ResourceClass.DISK_GB, 1500), - ('ss2', fields.ResourceClass.IPV4_ADDRESS, 2)], - # 2) cn2 and ss3 are not connected so this is not valid - [('cn2', fields.ResourceClass.VCPU, 2), - ('ss1', fields.ResourceClass.DISK_GB, 1500), - ('ss3', fields.ResourceClass.IPV4_ADDRESS, 2)], ] self._validate_allocation_requests(expected, alloc_cands) @@ -1324,11 +1314,6 @@ class AllocationCandidatesTestCase(ProviderDBBase): # [('cn1', fields.ResourceClass.VCPU, 1), # ('cn1', fields.ResourceClass.MEMORY_MB, 64), # ('ss1', fields.ResourceClass.DISK_GB, 1500)], - # The next invalid candidate is mixing not connected RPs as - # reported in bug #1732707 - [('cn2', fields.ResourceClass.VCPU, 1), - ('ss1', fields.ResourceClass.DISK_GB, 1500), - ('ss2_1', fields.ResourceClass.MEMORY_MB, 64)], ] self._validate_allocation_requests(expected, alloc_cands) @@ -1378,14 +1363,6 @@ class AllocationCandidatesTestCase(ProviderDBBase): # [('cn1', fields.ResourceClass.VCPU, 1), # ('cn1', fields.ResourceClass.MEMORY_MB, 64), # ('ss1_2', fields.ResourceClass.DISK_GB, 1500)], - # The rest are invalid candidates mixing not connected RPs as - # reported in bug #1732707 - [('cn2', fields.ResourceClass.VCPU, 1), - ('ss1_1', fields.ResourceClass.DISK_GB, 1500), - ('ss2_1', fields.ResourceClass.MEMORY_MB, 64)], - [('cn2', fields.ResourceClass.VCPU, 1), - ('ss1_2', fields.ResourceClass.DISK_GB, 1500), - ('ss2_1', fields.ResourceClass.MEMORY_MB, 64)], ] self._validate_allocation_requests(expected, alloc_cands)