From 368b6d9293102696d7b208119e444127f638e0f1 Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Fri, 13 Apr 2018 15:43:03 -0400 Subject: [PATCH] support multiple member_of qparams Adds a new placement API microversion that supports specifying multiple member_of parameters to the GET /resource_providers and GET /allocation_candidates API endpoints. When multiple member_of parameters are found, they are passed down to the ResourceProviderList.get_by_filters() method as a list. Items in this list are lists of aggregate UUIDs. The list of member_of items is evaluated so that resource providers matching ALL of the member_of constraints are returned. When a member_of item contains multiple UUIDs, we look up resource providers that have *any* of those aggregate UUIDs associated with them. Change-Id: Ib4f1955f06f2159dfb221f3d2bc8ff7bfce71ee2 blueprint: alloc-candidates-member-of --- .../handlers/allocation_candidate.py | 6 +- .../placement/handlers/resource_provider.py | 11 +- nova/api/openstack/placement/microversion.py | 2 + .../placement/objects/resource_provider.py | 133 ++++++++++++++---- .../placement/rest_api_version_history.rst | 12 ++ nova/api/openstack/placement/util.py | 65 +++++++-- .../placement/db/test_resource_provider.py | 12 +- .../allocation-candidates-member-of.yaml | 33 ++++- .../placement/gabbits/microversion.yaml | 4 +- .../gabbits/resource-provider-aggregates.yaml | 63 ++++++++- .../unit/api/openstack/placement/test_util.py | 117 +++++++++++++-- placement-api-ref/source/parameters.yaml | 9 ++ .../multi-member-of-4f9518a96652c0c6.yaml | 20 +++ 13 files changed, 416 insertions(+), 71 deletions(-) create mode 100644 releasenotes/notes/multi-member-of-4f9518a96652c0c6.yaml diff --git a/nova/api/openstack/placement/handlers/allocation_candidate.py b/nova/api/openstack/placement/handlers/allocation_candidate.py index ed7fbac5d975..a7c29f87b413 100644 --- a/nova/api/openstack/placement/handlers/allocation_candidate.py +++ b/nova/api/openstack/placement/handlers/allocation_candidate.py @@ -220,11 +220,7 @@ def list_allocation_candidates(req): get_schema = schema.GET_SCHEMA_1_16 util.validate_query_params(req, get_schema) - # Control whether we handle forbidden traits. - allow_forbidden = want_version.matches((1, 22)) - - requests = util.parse_qs_request_groups( - req.GET, allow_forbidden=allow_forbidden) + requests = util.parse_qs_request_groups(req) limit = req.GET.getall('limit') # JSONschema has already confirmed that limit has the form # of an integer. diff --git a/nova/api/openstack/placement/handlers/resource_provider.py b/nova/api/openstack/placement/handlers/resource_provider.py index 57deb0e7bf0c..c80e91fd82eb 100644 --- a/nova/api/openstack/placement/handlers/resource_provider.py +++ b/nova/api/openstack/placement/handlers/resource_provider.py @@ -196,13 +196,16 @@ def list_resource_providers(req): util.validate_query_params(req, schema) filters = {} - qpkeys = ('uuid', 'name', 'member_of', 'in_tree', 'resources', 'required') + # 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) + + qpkeys = ('uuid', 'name', 'in_tree', 'resources', 'required') for attr in qpkeys: if attr in req.GET: value = req.GET[attr] - if attr == 'member_of': - value = util.normalize_member_of_qs_param(value) - elif attr == 'resources': + if attr == 'resources': value = util.normalize_resources_qs_param(value) elif attr == 'required': value = util.normalize_traits_qs_param( diff --git a/nova/api/openstack/placement/microversion.py b/nova/api/openstack/placement/microversion.py index 0a835097d924..ed93d3e53163 100644 --- a/nova/api/openstack/placement/microversion.py +++ b/nova/api/openstack/placement/microversion.py @@ -64,6 +64,8 @@ VERSIONS = [ '1.22', # Support forbidden traits in the required parameter of # GET /resource_providers and GET /allocation_candidates '1.23', # Add support for error codes in error response JSON + '1.24', # Support multiple ?member_of= queryparams on + # GET /resource_providers ] diff --git a/nova/api/openstack/placement/objects/resource_provider.py b/nova/api/openstack/placement/objects/resource_provider.py index 6bd877cf4408..dc398e6e2f1e 100644 --- a/nova/api/openstack/placement/objects/resource_provider.py +++ b/nova/api/openstack/placement/objects/resource_provider.py @@ -653,6 +653,84 @@ def _provider_ids_from_uuid(context, uuid): return ProviderIds(**dict(res)) +def _provider_ids_matching_aggregates(context, member_of): + """Given a list of lists of aggregate UUIDs, return the internal IDs of all + resource providers associated with the aggregates. + + :param member_of: A list containing lists of aggregate UUIDs. Each item in + the outer list is to be AND'd together. If that item contains multiple + values, they are OR'd together. + + For example, if member_of is:: + + [ + ['agg1'], + ['agg2', 'agg3'], + ] + + we will return all the resource providers that are + associated with agg1 as well as either (agg2 or agg3) + + :returns: A list of internal resource provider IDs having all required + aggregate associations + """ + # Given a request for the following: + # + # member_of = [ + # [agg1], + # [agg2], + # [agg3, agg4] + # ] + # + # we need to produce the following SQL expression: + # + # SELECT + # rp.id + # FROM resource_providers AS rp + # JOIN resource_provider_aggregates AS rpa1 + # ON rp.id = rpa1.resource_provider_id + # AND rpa1.aggregate_id IN ($AGG1_ID) + # JOIN resource_provider_aggregates AS rpa2 + # ON rp.id = rpa2.resource_provider_id + # AND rpa2.aggregate_id IN ($AGG2_ID) + # JOIN resource_provider_aggregates AS rpa3 + # ON rp.id = rpa3.resource_provider_id + # AND rpa3.aggregate_id IN ($AGG3_ID, $AGG4_ID) + + # First things first, get a map of all the aggregate UUID to internal + # aggregate IDs + agg_uuids = set() + for members in member_of: + for member in members: + agg_uuids.add(member) + agg_tbl = sa.alias(_AGG_TBL, name='aggs') + agg_sel = sa.select([agg_tbl.c.uuid, agg_tbl.c.id]) + agg_sel = agg_sel.where(agg_tbl.c.uuid.in_(agg_uuids)) + agg_uuid_map = { + r[0]: r[1] for r in context.session.execute(agg_sel).fetchall() + } + + rp_tbl = sa.alias(_RP_TBL, name='rp') + join_chain = rp_tbl + + for x, members in enumerate(member_of): + rpa_tbl = sa.alias(_RP_AGG_TBL, name='rpa%d' % x) + + agg_ids = [agg_uuid_map[member] for member in members + if member in agg_uuid_map] + if not agg_ids: + # This member_of list contains only non-existent aggregate UUIDs + # and therefore we will always return 0 results, so short-circuit + return [] + + join_cond = sa.and_( + rp_tbl.c.id == rpa_tbl.c.resource_provider_id, + rpa_tbl.c.aggregate_id.in_(agg_ids)) + join_chain = sa.join(join_chain, rpa_tbl, join_cond) + sel = sa.select([rp_tbl.c.id]).select_from(join_chain) + return [r[0] for r in context.session.execute(sel).fetchall()] + + @db_api.api_context_manager.writer def _delete_rp_record(context, _id): return context.session.query(models.ResourceProvider).\ @@ -1388,16 +1466,16 @@ def _get_all_with_shared(ctx, resources, member_of=None): )) join_chain = sharing_join - # If 'member_of' has values join with the PlacementAggregates to - # get those resource providers that are associated with any of the - # list of aggregate uuids provided with 'member_of'. + # If 'member_of' has values, do a separate lookup to identify the + # resource providers that meet the member_of constraints. if member_of: - member_join = sa.join(join_chain, _RP_AGG_TBL, - _RP_AGG_TBL.c.resource_provider_id == rpt.c.id) - agg_join = sa.join(member_join, _AGG_TBL, sa.and_( - _AGG_TBL.c.id == _RP_AGG_TBL.c.aggregate_id, - _AGG_TBL.c.uuid.in_(member_of))) - join_chain = agg_join + rps_in_aggs = _provider_ids_matching_aggregates(ctx, member_of) + 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 [] + where_conds.append(rpt.c.id.in_(rps_in_aggs)) sel = sel.select_from(join_chain) sel = sel.where(sa.and_(*where_conds)) @@ -1497,17 +1575,16 @@ class ResourceProviderList(base.ObjectListBase, base.VersionedObject): rp.c.root_provider_id == root_id) query = query.where(where_cond) - # If 'member_of' has values join with the PlacementAggregates to - # get those resource providers that are associated with any of the - # list of aggregate uuids provided with 'member_of'. + # If 'member_of' has values, do a separate lookup to identify the + # resource providers that meet the member_of constraints. if member_of: - join_statement = sa.join(_AGG_TBL, _RP_AGG_TBL, sa.and_( - _AGG_TBL.c.id == _RP_AGG_TBL.c.aggregate_id, - _AGG_TBL.c.uuid.in_(member_of))) - resource_provider_id = _RP_AGG_TBL.c.resource_provider_id - rps_in_aggregates = sa.select( - [resource_provider_id]).select_from(join_statement) - query = query.where(rp.c.id.in_(rps_in_aggregates)) + rps_in_aggs = _provider_ids_matching_aggregates(context, member_of) + 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 [] + query = query.where(rp.c.id.in_(rps_in_aggs)) # If 'required' has values, add a filter to limit results to providers # possessing *all* of the listed traits. @@ -2933,16 +3010,16 @@ def _get_provider_ids_matching(ctx, resources, required_traits, ) where_conds.append(usage_cond) - # If 'member_of' has values join with the PlacementAggregates to - # get those resource providers that are associated with any of the - # list of aggregate uuids provided with 'member_of'. + # If 'member_of' has values, do a separate lookup to identify the + # resource providers that meet the member_of constraints. if member_of: - member_join = sa.join(join_chain, _RP_AGG_TBL, - _RP_AGG_TBL.c.resource_provider_id == rpt.c.id) - agg_join = sa.join(member_join, _AGG_TBL, sa.and_( - _AGG_TBL.c.id == _RP_AGG_TBL.c.aggregate_id, - _AGG_TBL.c.uuid.in_(member_of))) - join_chain = agg_join + rps_in_aggs = _provider_ids_matching_aggregates(ctx, member_of) + 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 [] + where_conds.append(rpt.c.id.in_(rps_in_aggs)) sel = sel.select_from(join_chain) sel = sel.where(sa.and_(*where_conds)) diff --git a/nova/api/openstack/placement/rest_api_version_history.rst b/nova/api/openstack/placement/rest_api_version_history.rst index 1840da2fcf7f..e73fa468debd 100644 --- a/nova/api/openstack/placement/rest_api_version_history.rst +++ b/nova/api/openstack/placement/rest_api_version_history.rst @@ -280,3 +280,15 @@ that identifies the type of this error. This can be used to distinguish errors that are different but use the same HTTP status code. Any error response which does not specifically define a code will have the code ``placement.undefined_code``. + +1.24 Support multiple ?member_of queryparams +-------------------------------------------- + +Add support for specifying multiple ``member_of`` query parameters to the ``GET +/resource_providers`` API. When multiple ``member_of`` query parameters are +found, they are AND'd together in the final query. For example, issuing a +request for ``GET /resource_providers?member_of=agg1&member_of=agg2`` means get +the resource providers that are associated with BOTH agg1 and agg2. Issuing a +request for ``GET /resource_providers?member_of=in:agg1,agg2&member_of=agg3`` +means get the resource providers that are associated with agg3 and are also +associated with *any of* (agg1, agg2). diff --git a/nova/api/openstack/placement/util.py b/nova/api/openstack/placement/util.py index 46271e849262..9c1d6ec05d44 100644 --- a/nova/api/openstack/placement/util.py +++ b/nova/api/openstack/placement/util.py @@ -347,17 +347,39 @@ def normalize_traits_qs_param(val, allow_forbidden=False): return ret -def normalize_member_of_qs_param(value): - """We need to handle member_of as a special case to always make its value a - list, either by accepting the single value, or if it starts with 'in:' - splitting on ','. +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. - NOTE(cdent): This will all change when we start using - JSONSchema validation of query params. + :param req: webob.Request object + :return: A list containing sets of UUIDs of 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 val parameter is not in the + expected format. + """ + microversion = nova.api.openstack.placement.microversion + want_version = req.environ[microversion.MICROVERSION_ENVIRON] + multi_member_of = want_version.matches((1, 24)) + 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 = [] + for value in req.GET.getall('member_of' + suffix): + values.append(normalize_member_of_qs_param(value)) + return values + + +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. :param value: A member_of query parameter of either a single UUID, or a - comma-separated string of one or more UUIDs, prefixed with - the "in:" operator. + comma-separated string of two or more UUIDs, prefixed with + the "in:" operator :return: A set of UUIDs :raises `webob.exc.HTTPBadRequest` if the value parameter is not in the expected format. @@ -374,12 +396,12 @@ def normalize_member_of_qs_param(value): for aggr_uuid in value: if not uuidutils.is_uuid_like(aggr_uuid): msg = _("Invalid query string parameters: Expected 'member_of' " - "parameter to contain valid UUID(s). Got: %s") % value + "parameter to contain valid UUID(s). Got: %s") % aggr_uuid raise webob.exc.HTTPBadRequest(msg) return value -def parse_qs_request_groups(qsdict, allow_forbidden=False): +def parse_qs_request_groups(req): """Parse numbered resources, traits, and member_of groupings out of a querystring dict. @@ -455,12 +477,15 @@ def parse_qs_request_groups(qsdict, allow_forbidden=False): ), ] - :param qsdict: The MultiDict representing the querystring on a GET. - :param allow_forbidden: If True, parse for forbidden traits. + :param req: webob.Request object :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. """ + microversion = nova.api.openstack.placement.microversion + want_version = req.environ[microversion.MICROVERSION_ENVIRON] + # Control whether we handle forbidden traits. + allow_forbidden = want_version.matches((1, 22)) # Temporary dict of the form: { suffix: RequestGroup } by_suffix = {} @@ -470,21 +495,31 @@ def parse_qs_request_groups(qsdict, allow_forbidden=False): by_suffix[suffix] = rq_grp return by_suffix[suffix] - for key, val in qsdict.items(): + for key, val in req.GET.items(): match = _QS_KEY_PATTERN.match(key) if not match: continue # `prefix` is 'resources', 'required', or 'member_of' # `suffix` is an integer string, or None prefix, suffix = match.groups() - request_group = get_request_group(suffix or '') + suffix = suffix or '' + request_group = get_request_group(suffix) 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, allow_forbidden=allow_forbidden) elif prefix == _QS_MEMBER_OF: - request_group.member_of = normalize_member_of_qs_param(val) + # special handling of member_of qparam since we allow multiple + # member_of params at microversion 1.24. + # NOTE(jaypipes): Yes, this is inefficient to do this when there + # are multiple member_of query parameters, but we do this so we can + # error out if someone passes an "orphaned" member_of request + # group. + # TODO(jaypipes): Do validation of query parameters using + # JSONSchema + request_group.member_of = normalize_member_of_qs_params( + req, suffix) # Ensure any group with 'required' or 'member_of' also has 'resources'. orphans = [('required%s' % suff) for suff, group in by_suffix.items() diff --git a/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py b/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py index 09c9f8576973..3c82f4b67bee 100644 --- a/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py +++ b/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py @@ -373,7 +373,7 @@ class ResourceProviderTestCase(ResourceProviderBaseCase): rps = rp_obj.ResourceProviderList.get_all_by_filters( self.ctx, filters={ - 'member_of': [uuidsentinel.agg], + 'member_of': [[uuidsentinel.agg]], 'in_tree': uuidsentinel.grandchild_rp, } ) @@ -1105,7 +1105,7 @@ class ResourceProviderListTestCase(ResourceProviderBaseCase): rp.set_aggregates(aggregate_uuids) resource_providers = rp_obj.ResourceProviderList.get_all_by_filters( - self.ctx, filters={'member_of': [uuidsentinel.agg_a]}) + self.ctx, filters={'member_of': [[uuidsentinel.agg_a]]}) self.assertEqual(2, len(resource_providers)) names = [_rp.name for _rp in resource_providers] @@ -1116,24 +1116,24 @@ class ResourceProviderListTestCase(ResourceProviderBaseCase): resource_providers = rp_obj.ResourceProviderList.get_all_by_filters( self.ctx, filters={'member_of': - [uuidsentinel.agg_a, uuidsentinel.agg_b]}) + [[uuidsentinel.agg_a, uuidsentinel.agg_b]]}) self.assertEqual(2, len(resource_providers)) resource_providers = rp_obj.ResourceProviderList.get_all_by_filters( self.ctx, filters={'member_of': - [uuidsentinel.agg_a, uuidsentinel.agg_b], + [[uuidsentinel.agg_a, uuidsentinel.agg_b]], 'name': u'rp_name_1'}) self.assertEqual(1, len(resource_providers)) resource_providers = rp_obj.ResourceProviderList.get_all_by_filters( self.ctx, filters={'member_of': - [uuidsentinel.agg_a, uuidsentinel.agg_b], + [[uuidsentinel.agg_a, uuidsentinel.agg_b]], 'name': u'barnabas'}) self.assertEqual(0, len(resource_providers)) resource_providers = rp_obj.ResourceProviderList.get_all_by_filters( self.ctx, filters={'member_of': - [uuidsentinel.agg_1, uuidsentinel.agg_2]}) + [[uuidsentinel.agg_1, uuidsentinel.agg_2]]}) self.assertEqual(0, len(resource_providers)) def test_get_all_by_required(self): diff --git a/nova/tests/functional/api/openstack/placement/gabbits/allocation-candidates-member-of.yaml b/nova/tests/functional/api/openstack/placement/gabbits/allocation-candidates-member-of.yaml index 92d5e3d86486..22b6ce6f8e62 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/allocation-candidates-member-of.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/allocation-candidates-member-of.yaml @@ -8,7 +8,7 @@ defaults: x-auth-token: admin content-type: application/json accept: application/json - openstack-api-version: placement 1.21 + openstack-api-version: placement 1.24 tests: @@ -108,3 +108,34 @@ tests: response_json_paths: $.allocation_requests.`len`: 2 status: 200 + +- name: verify microversion fail for multiple member_of params + GET: /allocation_candidates?resources=VCPU:1,MEMORY_MB:1024,DISK_GB:100&member_of=$ENVIRON['AGGA_UUID']&member_of=$ENVIRON['AGGB_UUID'] + request_headers: + openstack-api-version: placement 1.23 + status: 400 + response_strings: + - 'Multiple member_of parameters are not supported' + response_json_paths: + $.errors[0].title: Bad Request + +- name: verify that no RP is associated with BOTH aggA and aggB + GET: /allocation_candidates?resources=VCPU:1,MEMORY_MB:1024,DISK_GB:100&member_of=$ENVIRON['AGGA_UUID']&member_of=$ENVIRON['AGGB_UUID'] + status: 200 + response_json_paths: + $.allocation_requests.`len`: 0 + +- name: associate the second compute node with aggA and aggB + PUT: /resource_providers/$ENVIRON['CN2_UUID']/aggregates + data: + aggregates: + - $ENVIRON['AGGA_UUID'] + - $ENVIRON['AGGB_UUID'] + resource_provider_generation: $HISTORY['associate the second compute node with aggB'].$RESPONSE['$.resource_provider_generation'] + status: 200 + +- name: verify that second RP is associated with BOTH aggA and aggB + GET: /allocation_candidates?resources=VCPU:1,MEMORY_MB:1024,DISK_GB:100&member_of=$ENVIRON['AGGA_UUID']&member_of=$ENVIRON['AGGB_UUID'] + status: 200 + response_json_paths: + $.allocation_requests.`len`: 1 diff --git a/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml b/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml index b90344ad11ed..e9fb2d25b2a1 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml @@ -39,13 +39,13 @@ tests: response_json_paths: $.errors[0].title: Not Acceptable -- name: latest microversion is 1.23 +- name: latest microversion is 1.24 GET: / request_headers: openstack-api-version: placement latest response_headers: vary: /openstack-api-version/ - openstack-api-version: placement 1.23 + openstack-api-version: placement 1.24 - name: other accept header bad version GET: / diff --git a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-aggregates.yaml b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-aggregates.yaml index dbcad3e5977f..5041be68b2ee 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-aggregates.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-aggregates.yaml @@ -53,7 +53,7 @@ tests: GET: '/resource_providers?member_of=not+a+uuid' status: 400 response_strings: - - Expected 'member_of' parameter to contain valid UUID(s). + - "Expected 'member_of' parameter to contain valid UUID(s)." response_json_paths: $.errors[0].title: Bad Request @@ -118,3 +118,64 @@ tests: - 'Invalid query string parameters' response_json_paths: $.errors[0].title: Bad Request + +- name: error trying multiple member_of params prior correct microversion + GET: '/resource_providers?member_of=83a3d69d-8920-48e2-8914-cadfd8fa2f91&member_of=99652f11-9f77-46b9-80b7-4b1989be9f8c' + request_headers: + openstack-api-version: placement 1.23 + status: 400 + response_strings: + - 'Multiple member_of parameters are not supported' + response_json_paths: + $.errors[0].title: Bad Request + +- name: multiple member_of params with no results + GET: '/resource_providers?member_of=83a3d69d-8920-48e2-8914-cadfd8fa2f91&member_of=99652f11-9f77-46b9-80b7-4b1989be9f8c' + status: 200 + response_json_paths: + # No provider is associated with both aggregates + resource_providers: [] + +- name: associate two aggregates with rp2 + PUT: /resource_providers/5202c48f-c960-4eec-bde3-89c4f22a17b9/aggregates + data: + aggregates: + - 99652f11-9f77-46b9-80b7-4b1989be9f8c + - 83a3d69d-8920-48e2-8914-cadfd8fa2f91 + resource_provider_generation: 2 + status: 200 + +- name: multiple member_of params AND together to result in one provider + GET: '/resource_providers?member_of=83a3d69d-8920-48e2-8914-cadfd8fa2f91&member_of=99652f11-9f77-46b9-80b7-4b1989be9f8c' + status: 200 + response_json_paths: + # One provider is now associated with both aggregates + $.resource_providers.`len`: 1 + $.resource_providers[0].uuid: 5202c48f-c960-4eec-bde3-89c4f22a17b9 + +- name: associate two aggregates to rp1, one of which overlaps with rp2 + PUT: /resource_providers/893337e9-1e55-49f0-bcfe-6a2f16fbf2f7/aggregates + data: + aggregates: + - 282d469e-29e2-4a8a-8f2e-31b3202b696a + - 83a3d69d-8920-48e2-8914-cadfd8fa2f91 + resource_provider_generation: 2 + status: 200 + +- name: two AND'd member_ofs with one OR'd member_of + GET: '/resource_providers?member_of=83a3d69d-8920-48e2-8914-cadfd8fa2f91&member_of=in:99652f11-9f77-46b9-80b7-4b1989be9f8c,282d469e-29e2-4a8a-8f2e-31b3202b696a' + status: 200 + response_json_paths: + # Both rp1 and rp2 returned because both are associated with agg 83a3d69d + # and each is associated with either agg 99652f11 or agg 282s469e + $.resource_providers.`len`: 2 + $.resource_providers[0].uuid: /5202c48f-c960-4eec-bde3-89c4f22a17b9|893337e9-1e55-49f0-bcfe-6a2f16fbf2f7/ + $.resource_providers[1].uuid: /5202c48f-c960-4eec-bde3-89c4f22a17b9|893337e9-1e55-49f0-bcfe-6a2f16fbf2f7/ + +- name: two AND'd member_ofs using same agg UUID + GET: '/resource_providers?member_of=282d469e-29e2-4a8a-8f2e-31b3202b696a&member_of=282d469e-29e2-4a8a-8f2e-31b3202b696a' + status: 200 + response_json_paths: + # 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/ diff --git a/nova/tests/unit/api/openstack/placement/test_util.py b/nova/tests/unit/api/openstack/placement/test_util.py index aa9f3ff4c681..1586ef06813a 100644 --- a/nova/tests/unit/api/openstack/placement/test_util.py +++ b/nova/tests/unit/api/openstack/placement/test_util.py @@ -23,7 +23,6 @@ from oslo_utils import timeutils import webob import six -import six.moves.urllib.parse as urlparse from nova.api.openstack.placement import lib as pl from nova.api.openstack.placement import microversion @@ -427,15 +426,21 @@ class TestNormalizeTraitsQsParam(test.NoDBTestCase): util.normalize_traits_qs_param, fmt % traits) -class TestParseQsResourcesAndTraits(test.NoDBTestCase): +class TestParseQsRequestGroups(test.NoDBTestCase): @staticmethod - def do_parse(qstring, allow_forbidden=False): + def do_parse(qstring, version=(1, 18)): """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)), allow_forbidden=allow_forbidden) + req = webob.Request.blank('?' + qstring) + mv_parsed = microversion_parse.Version(*version) + mv_parsed.max_version = microversion_parse.parse_version_string( + microversion.max_version_string()) + mv_parsed.min_version = microversion_parse.parse_version_string( + microversion.min_version_string()) + req.environ['placement.microversion'] = mv_parsed + return util.parse_qs_request_groups(req) def assertRequestGroupsEqual(self, expected, observed): self.assertEqual(len(expected), len(observed)) @@ -464,6 +469,59 @@ class TestParseQsResourcesAndTraits(test.NoDBTestCase): ] self.assertRequestGroupsEqual(expected, self.do_parse(qs)) + def test_member_of_single_agg(self): + """Unnumbered resources with one member_of query param.""" + agg1_uuid = uuidsentinel.agg1 + qs = ('resources=VCPU:2,MEMORY_MB:2048' + '&member_of=%s' % agg1_uuid) + expected = [ + pl.RequestGroup( + use_same_provider=False, + resources={ + 'VCPU': 2, + 'MEMORY_MB': 2048, + }, + member_of=[ + set([agg1_uuid]) + ] + ), + ] + self.assertRequestGroupsEqual(expected, self.do_parse(qs)) + + def test_member_of_multiple_aggs_prior_microversion(self): + """Unnumbered resources with multiple member_of query params before the + supported microversion should raise a 400. + """ + agg1_uuid = uuidsentinel.agg1 + agg2_uuid = uuidsentinel.agg2 + qs = ('resources=VCPU:2,MEMORY_MB:2048' + '&member_of=%s' + '&member_of=%s' % (agg1_uuid, agg2_uuid)) + self.assertRaises(webob.exc.HTTPBadRequest, self.do_parse, qs) + + def test_member_of_multiple_aggs(self): + """Unnumbered resources with multiple member_of query params.""" + agg1_uuid = uuidsentinel.agg1 + agg2_uuid = uuidsentinel.agg2 + qs = ('resources=VCPU:2,MEMORY_MB:2048' + '&member_of=%s' + '&member_of=%s' % (agg1_uuid, agg2_uuid)) + expected = [ + pl.RequestGroup( + use_same_provider=False, + resources={ + 'VCPU': 2, + 'MEMORY_MB': 2048, + }, + member_of=[ + set([agg1_uuid]), + set([agg2_uuid]) + ] + ), + ] + self.assertRequestGroupsEqual( + expected, self.do_parse(qs, version=(1, 24))) + def test_unnumbered_resources_only(self): """Validate the bit that can be used for 1.10 and earlier.""" qs = 'resources=VCPU:2,MEMORY_MB:2048,DISK_GB:5,CUSTOM_MAGIC:123' @@ -566,6 +624,40 @@ class TestParseQsResourcesAndTraits(test.NoDBTestCase): ] self.assertRequestGroupsEqual(expected, self.do_parse(qs)) + def test_member_of_multiple_aggs_numbered(self): + """Numbered resources with multiple member_of query params.""" + agg1_uuid = uuidsentinel.agg1 + agg2_uuid = uuidsentinel.agg2 + agg3_uuid = uuidsentinel.agg3 + agg4_uuid = uuidsentinel.agg4 + qs = ('resources1=VCPU:2' + '&member_of1=%s' + '&member_of1=%s' + '&resources2=VCPU:2' + '&member_of2=in:%s,%s' % ( + agg1_uuid, agg2_uuid, agg3_uuid, agg4_uuid)) + expected = [ + pl.RequestGroup( + resources={ + 'VCPU': 2, + }, + member_of=[ + set([agg1_uuid]), + set([agg2_uuid]) + ] + ), + pl.RequestGroup( + resources={ + 'VCPU': 2, + }, + member_of=[ + set([agg3_uuid, agg4_uuid]), + ] + ), + ] + self.assertRequestGroupsEqual( + expected, self.do_parse(qs, version=(1, 24))) + def test_400_malformed_resources(self): # Somewhat duplicates TestNormalizeResourceQsParam.test_400*. qs = ('resources=VCPU:0,MEMORY_MB:4096,DISK_GB:10' @@ -617,6 +709,13 @@ class TestParseQsResourcesAndTraits(test.NoDBTestCase): '&resources3=CUSTOM_MAGIC:123') self.assertRaises(webob.exc.HTTPBadRequest, self.do_parse, qs) + def test_400_member_of_no_resources_numbered(self): + agg1_uuid = uuidsentinel.agg1 + qs = ('resources=VCPU:7,MEMORY_MB:4096,DISK_GB:10' + '&required=HW_CPU_X86_VMX,CUSTOM_MEM_FLASH,STORAGE_DISK_SSD' + '&member_of2=%s' % agg1_uuid) + 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. @@ -645,7 +744,7 @@ class TestParseQsResourcesAndTraits(test.NoDBTestCase): 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)) + expected_forbidden, self.do_parse(qs, version=(1, 22))) def test_forbidden_conflict(self): qs = ('resources=VCPU:2,MEMORY_MB:2048' @@ -656,7 +755,7 @@ class TestParseQsResourcesAndTraits(test.NoDBTestCase): 'in the following traits keys: required: (CUSTOM_PHYSNET1)') exc = self.assertRaises(webob.exc.HTTPBadRequest, self.do_parse, qs, - allow_forbidden=True) + version=(1, 22)) self.assertEqual(expected_message, six.text_type(exc)) def test_forbidden_two_groups(self): @@ -684,7 +783,7 @@ class TestParseQsResourcesAndTraits(test.NoDBTestCase): ] self.assertRequestGroupsEqual( - expected, self.do_parse(qs, allow_forbidden=True)) + expected, self.do_parse(qs, version=(1, 22))) def test_forbidden_separate_groups_no_conflict(self): qs = ('resources1=CUSTOM_MAGIC:1&required1=CUSTOM_PHYSNET1' @@ -711,7 +810,7 @@ class TestParseQsResourcesAndTraits(test.NoDBTestCase): ] self.assertRequestGroupsEqual( - expected, self.do_parse(qs, allow_forbidden=True)) + expected, self.do_parse(qs, version=(1, 22))) class TestPickLastModified(test.NoDBTestCase): diff --git a/placement-api-ref/source/parameters.yaml b/placement-api-ref/source/parameters.yaml index c7f91c7ea20a..df64ad91dc94 100644 --- a/placement-api-ref/source/parameters.yaml +++ b/placement-api-ref/source/parameters.yaml @@ -74,6 +74,15 @@ member_of: &member_of 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 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.3 member_of_1_21: <<: *member_of diff --git a/releasenotes/notes/multi-member-of-4f9518a96652c0c6.yaml b/releasenotes/notes/multi-member-of-4f9518a96652c0c6.yaml new file mode 100644 index 000000000000..7bf29cb5e45d --- /dev/null +++ b/releasenotes/notes/multi-member-of-4f9518a96652c0c6.yaml @@ -0,0 +1,20 @@ +--- +features: + - | + A new 1.24 placement API microversion adds the ability to specify multiple + `member_of` query parameters for the `GET /resource_providers` and `GET + allocation_candidates` endpoints. + When multiple `member_of` query parameters are received, the placement + service will return resource providers that match all of the requested + aggregate memberships. The `member_of=in:` format is still + supported and continues to indicate an IN() operation for aggregate + membership. Some examples for using the new functionality: + Get all providers that are associated with BOTH agg1 and agg2: + ?member_of=agg1&member_of=agg2 + Get all providers that are associated with agg1 OR agg2: + ?member_of=in:agg1,agg2 + Get all providers that are associated with agg1 and ANY OF (agg2, agg3): + ?member_of=agg1&member_of=in:agg2,agg3 + Get all providers that are associated with ANY OF (agg1, agg2) AND are also + associated with ANY OF (agg3, agg4): + ?member_of=in:agg1,agg2&member_of=in:agg3,agg4