Merge "Add any-traits support for listing resource providers"

This commit is contained in:
Zuul 2022-02-22 16:27:17 +00:00 committed by Gerrit Code Review
commit 4b43b80f11
8 changed files with 405 additions and 72 deletions

View File

@ -239,7 +239,8 @@ def list_resource_providers(req):
util.normalize_member_of_qs_params(req))
if 'required' in req.GET:
filters['required'] = util.normalize_traits_qs_params(req)
filters['required_traits'], filters['forbidden_traits'] = (
util.normalize_traits_qs_params(req))
qpkeys = ('uuid', 'name', 'in_tree', 'resources')
for attr in qpkeys:

View File

@ -113,8 +113,11 @@ class RequestGroup(object):
request_group.resources = util.normalize_resources_qs_param(
val)
elif prefix == _QS_REQUIRED:
# TODO(gibi): switch this to normalize_traits_qs_params when
# the data model can handle nested required_traits structure
# as part of the any-traits feature
request_group.required_traits = (
util.normalize_traits_qs_params(req, suffix))
util.normalize_traits_qs_params_legacy(req, suffix))
elif prefix == _QS_MEMBER_OF:
# special handling of member_of qparam since we allow multiple
# member_of params at microversion 1.24.
@ -392,9 +395,14 @@ class RequestWideParams(object):
raise webob.exc.HTTPBadRequest(
"Query parameter 'root_required' may be specified only "
"once.", comment=errors.ILLEGAL_DUPLICATE_QUERYPARAM)
# NOTE(gibi): root_required does not support any-traits so here
# we continue using the old query parsing function that does not
# accept the `in:` prefix and that always returns a flat trait
# list
anchor_required_traits, anchor_forbidden_traits, conflicts = (
_fix_one_forbidden(util.normalize_traits_qs_param(
root_required[0], allow_forbidden=True)))
_fix_one_forbidden(
util.normalize_traits_qs_param_to_legacy_value(
root_required[0], allow_forbidden=True)))
if conflicts:
raise webob.exc.HTTPBadRequest(
'Conflicting required and forbidden traits found in '

View File

@ -921,7 +921,8 @@ def _get_all_by_filters_from_db(context, filters):
# 'MEMORY_MB': 1024
# },
# 'in_tree': <uuid>,
# 'required': [<trait_name>, ...]
# 'required_traits': [{<trait_name>, ...}, {...}]
# 'forbidden_traits': {<trait_name>, ...}
# }
if not filters:
filters = {}
@ -933,11 +934,8 @@ def _get_all_by_filters_from_db(context, filters):
uuid = filters.pop('uuid', None)
member_of = filters.pop('member_of', [])
forbidden_aggs = filters.pop('forbidden_aggs', [])
required = set(filters.pop('required', []))
forbidden = set([trait for trait in required
if trait.startswith('!')])
required = required - forbidden
forbidden = set([trait.lstrip('!') for trait in forbidden])
required_traits = filters.pop('required_traits', [])
forbidden_traits = filters.pop('forbidden_traits', {})
resources = filters.pop('resources', {})
in_tree = filters.pop('in_tree', None)
@ -983,15 +981,25 @@ def _get_all_by_filters_from_db(context, filters):
return []
root_id = tree_ids.root_id
query = query.where(rp.c.root_provider_id == root_id)
if required:
trait_map = trait_obj.ids_from_names(context, required)
trait_rps = res_ctx.provider_ids_matching_required_traits(
context, trait_map.values())
if not trait_rps:
if required_traits:
# translate trait names to trait internal IDs while keeping the nested
# structure
required_traits = [
{
context.trait_cache.id_from_string(trait)
for trait in any_traits
}
for any_traits in required_traits
]
rps_with_matching_traits = (
res_ctx.provider_ids_matching_required_traits(
context, required_traits))
if not rps_with_matching_traits:
return []
query = query.where(rp.c.id.in_(trait_rps))
if forbidden:
trait_map = trait_obj.ids_from_names(context, forbidden)
query = query.where(rp.c.id.in_(rps_with_matching_traits))
if forbidden_traits:
trait_map = trait_obj.ids_from_names(context, forbidden_traits)
trait_rps = res_ctx.get_provider_ids_having_any_trait(
context, trait_map.values())
if trait_rps:
@ -1026,14 +1034,14 @@ def get_all_by_filters(context, filters=None):
empty list.
:param context: `placement.context.RequestContext` that may be used to
grab a DB connection.
:param filters: Can be `name`, `uuid`, `member_of`, `in_tree` or
`resources` where `member_of` is a list of list of
aggregate UUIDs, `in_tree` is a UUID of a resource
provider that we can use to find the root provider ID
of the tree of providers to filter results by and
`resources` is a dict of amounts keyed by resource
classes.
grab a DB connection.
:param filters: Can be `name`, `uuid`, `member_of`, `in_tree`,
`required_traits`, `forbidden_traits`, or `resources` where
`member_of` is a list of list of aggregate UUIDs, `required_traits` is
a list of set of trait names, `forbidden_traits` is a set of trait
names, `in_tree` is a UUID of a resource provider that we can use to
find the root provider ID of the tree of providers to filter results
by and `resources` is a dict of amounts keyed by resource classes.
:type filters: dict
"""
resource_providers = _get_all_by_filters_from_db(context, filters)

View File

@ -1050,13 +1050,16 @@ class ResourceProviderListTestCase(tb.PlacementDbBaseTestCase):
tb.set_traits(rp, *traits)
# Three rps (1, 2, 3) should have CUSTOM_TRAIT_A
filters = {'required': ['CUSTOM_TRAIT_A']}
filters = {'required_traits': [{'CUSTOM_TRAIT_A'}]}
expected_rps = ['rp_1', 'rp_2', 'rp_3']
self._run_get_all_by_filters(expected_rps, filters=filters)
# One rp (rp 1) if we forbid CUSTOM_TRAIT_B, with a single trait of
# CUSTOM_TRAIT_A
filters = {'required': ['CUSTOM_TRAIT_A', '!CUSTOM_TRAIT_B']}
filters = {
'required_traits': [{'CUSTOM_TRAIT_A'}],
'forbidden_traits': {'CUSTOM_TRAIT_B'},
}
expected_rps = ['rp_1']
custom_a_rps = self._run_get_all_by_filters(expected_rps,
filters=filters)
@ -1067,6 +1070,22 @@ class ResourceProviderListTestCase(tb.PlacementDbBaseTestCase):
self.assertEqual(1, len(traits))
self.assertEqual('CUSTOM_TRAIT_A', traits[0].name)
# (A or B) and not C
filters = {
'required_traits': [{'CUSTOM_TRAIT_A', 'CUSTOM_TRAIT_B'}],
'forbidden_traits': {'CUSTOM_TRAIT_C'},
}
expected_rps = ['rp_1', 'rp_2']
self._run_get_all_by_filters(expected_rps, filters=filters)
# A and (B or C)
filters = {
'required_traits': [
{'CUSTOM_TRAIT_A'}, {'CUSTOM_TRAIT_B', 'CUSTOM_TRAIT_C'}],
}
expected_rps = ['rp_2', 'rp_3']
self._run_get_all_by_filters(expected_rps, filters=filters)
class TestResourceProviderAggregates(tb.PlacementDbBaseTestCase):
def test_set_and_get_new_aggregates(self):

View File

@ -15,7 +15,7 @@ tests:
openstack-api-version: placement 1.38
status: 400
response_strings:
- "No such trait(s): in:CUSTOM_FOO"
- "The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' only supported since microversion 1.39."
- name: the 'in:' trait query is not supported yet in named request group
GET: /allocation_candidates?requiredX=in:CUSTOM_FOO,HW_CPU_X86_MMX&resourcesX=VCPU:1
@ -23,7 +23,7 @@ tests:
openstack-api-version: placement 1.38
status: 400
response_strings:
- "No such trait(s): in:CUSTOM_FOO"
- "The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' only supported since microversion 1.39."
- name: the second required field overwrites the first
# The fixture has one RP for each trait but no RP for both traits.

View File

@ -15,7 +15,7 @@ tests:
openstack-api-version: placement 1.38
status: 400
response_strings:
- "No such trait(s): in:CUSTOM_FOO"
- "The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' only supported since microversion 1.39."
- name: the second required field overwrites the first
# The fixture has one RP for each trait but no RP for both traits.

View File

@ -395,14 +395,16 @@ class TestNormalizeResourceQsParam(testtools.TestCase):
)
class TestNormalizeTraitsQsParam(testtools.TestCase):
class TestNormalizeTraitsQsParamLegacy(testtools.TestCase):
def test_one(self):
trait = 'HW_CPU_X86_VMX'
# Various whitespace permutations
for fmt in ('%s', ' %s', '%s ', ' %s ', ' %s '):
self.assertEqual(set([trait]),
util.normalize_traits_qs_param(fmt % trait))
self.assertEqual(
set([trait]),
util.normalize_traits_qs_param_to_legacy_value(fmt % trait)
)
def test_multiple(self):
traits = (
@ -414,12 +416,15 @@ class TestNormalizeTraitsQsParam(testtools.TestCase):
)
self.assertEqual(
set(traits),
util.normalize_traits_qs_param('%s, %s,%s , %s , %s ' % traits))
util.normalize_traits_qs_param_to_legacy_value(
'%s, %s,%s , %s , %s ' % traits)
)
def test_400_all_empty(self):
for qs in ('', ' ', ' ', ',', ' , , '):
self.assertRaises(
webob.exc.HTTPBadRequest, util.normalize_traits_qs_param, qs)
webob.exc.HTTPBadRequest,
util.normalize_traits_qs_param_to_legacy_value, qs)
def test_400_some_empty(self):
traits = (
@ -428,8 +433,107 @@ class TestNormalizeTraitsQsParam(testtools.TestCase):
'STORAGE_DISK_SSD',
)
for fmt in ('%s,,%s,%s', ',%s,%s,%s', '%s,%s,%s,', ' %s , %s , , %s'):
self.assertRaises(webob.exc.HTTPBadRequest,
util.normalize_traits_qs_param, fmt % traits)
self.assertRaises(
webob.exc.HTTPBadRequest,
util.normalize_traits_qs_param_to_legacy_value, fmt % traits)
class TestNormalizeTraitsQsParam(testtools.TestCase):
def test_one(self):
trait = 'HW_CPU_X86_VMX'
# Various whitespace permutations
for fmt in ('%s', ' %s', '%s ', ' %s ', ' %s '):
self.assertEqual(
([{trait}], set()),
util.normalize_traits_qs_param(fmt % trait)
)
def test_multiple(self):
traits = (
'HW_CPU_X86_VMX',
'HW_GPU_API_DIRECT3D_V12_0',
'HW_NIC_OFFLOAD_RX',
'CUSTOM_GOLD',
'STORAGE_DISK_SSD',
)
self.assertEqual(
([{trait} for trait in traits], set()),
util.normalize_traits_qs_param(
'%s, %s,%s , %s , %s ' % traits)
)
def test_400_all_empty(self):
for qs in ('', ' ', ' ', ',', ' , , '):
self.assertRaises(
webob.exc.HTTPBadRequest,
util.normalize_traits_qs_param, qs)
def test_400_some_empty(self):
traits = (
'HW_NIC_OFFLOAD_RX',
'CUSTOM_GOLD',
'STORAGE_DISK_SSD',
)
for fmt in (
'%s,,%s,%s',
',%s,%s,%s',
'%s,%s,%s,',
' %s , %s , , %s',
'!,%s,%s,%s',
):
self.assertRaises(
webob.exc.HTTPBadRequest,
util.normalize_traits_qs_param,
fmt % traits,
allow_forbidden=True,
)
def test_multiple_with_forbidden(self):
traits = (
'!HW_CPU_X86_VMX',
'HW_GPU_API_DIRECT3D_V12_0',
'!HW_NIC_OFFLOAD_RX',
'CUSTOM_GOLD',
'!STORAGE_DISK_SSD',
)
self.assertRaises(
webob.exc.HTTPBadRequest,
util.normalize_traits_qs_param,
'%s, %s,%s , %s , %s ' % traits,
allow_forbidden=False)
self.assertEqual(
(
[{'HW_GPU_API_DIRECT3D_V12_0'}, {'CUSTOM_GOLD'}],
{'HW_CPU_X86_VMX', 'HW_NIC_OFFLOAD_RX', 'STORAGE_DISK_SSD'}),
util.normalize_traits_qs_param(
'%s, %s,%s , %s , %s ' % traits, allow_forbidden=True)
)
def test_any_traits(self):
param = 'in:T1 ,T2 , T3'
self.assertRaises(
webob.exc.HTTPBadRequest,
util.normalize_traits_qs_param,
param,
allow_any_traits=False
)
self.assertEqual(
([{'T1', 'T2', 'T3'}], set()),
util.normalize_traits_qs_param(param, allow_any_traits=True)
)
def test_any_traits_not_mix_with_forbidden(self):
param = 'in:T1 ,!T2 , T3'
self.assertRaises(
webob.exc.HTTPBadRequest,
util.normalize_traits_qs_param,
param,
allow_forbidden=True,
allow_any_traits=True,
)
class TestNormalizeTraitsQsParams(testtools.TestCase):
@ -451,13 +555,15 @@ class TestNormalizeTraitsQsParams(testtools.TestCase):
def test_suffix(self):
req = self._get_req('required=!BAZ&requiredX=FOO,BAR', (1, 38))
traits = util.normalize_traits_qs_params(req, suffix='')
required, forbidden = util.normalize_traits_qs_params(req, suffix='')
self.assertEqual({'!BAZ'}, traits)
self.assertEqual([], required)
self.assertEqual({'BAZ'}, forbidden)
traits = util.normalize_traits_qs_params(req, suffix='X')
required, forbidden = util.normalize_traits_qs_params(req, suffix='X')
self.assertEqual({'FOO', 'BAR'}, traits)
self.assertEqual([{'FOO'}, {'BAR'}], required)
self.assertEqual(set(), forbidden)
def test_allow_forbidden_1_21(self):
req = self._get_req('required=!BAZ', (1, 21))
@ -478,16 +584,67 @@ class TestNormalizeTraitsQsParams(testtools.TestCase):
def test_allow_forbidden_1_22(self):
req = self._get_req('required=!BAZ', (1, 22))
traits = util.normalize_traits_qs_params(req, suffix='')
required, forbidden = util.normalize_traits_qs_params(req, suffix='')
self.assertEqual({'!BAZ'}, traits)
self.assertEqual([], required)
self.assertEqual({'BAZ'}, forbidden)
def test_repeated_param_1_38(self):
req = self._get_req('required=FOO,!BAR&required=BAZ', (1, 38))
traits = util.normalize_traits_qs_params(req, suffix='')
required, forbidden = util.normalize_traits_qs_params(req, suffix='')
self.assertEqual({'BAZ'}, traits)
self.assertEqual([{'BAZ'}], required)
self.assertEqual(set(), forbidden)
def test_allow_any_traits_1_38(self):
req = self._get_req('required=in:FOO,BAZ', (1, 38))
ex = self.assertRaises(
webob.exc.HTTPBadRequest,
util.normalize_traits_qs_params,
req,
suffix='',
)
self.assertIn(
"Invalid query string parameters: "
"The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' only supported "
"since microversion 1.39. Got: in:FOO,BAZ",
str(ex),
)
# TODO(gibi): remove the mock when microversion 1.39 is fully added
@mock.patch(
'placement.microversion.max_version_string',
new=mock.Mock(return_value='1.39'))
def test_allow_any_traits_1_39(self):
req = self._get_req('required=in:FOO,BAZ', (1, 39))
required, forbidden = util.normalize_traits_qs_params(req, suffix='')
self.assertEqual([{'FOO', 'BAZ'}], required)
self.assertEqual(set(), forbidden)
# TODO(gibi): remove the mock when microversion 1.39 is fully added
@mock.patch(
'placement.microversion.max_version_string',
new=mock.Mock(return_value='1.39'))
def test_repeated_param_1_39(self):
req = self._get_req(
'required=in:T1,T2'
'&required=T3,!T4'
'&required=in:T5,T6'
'&required=!T7,T8',
(1, 39)
)
required, forbidden = util.normalize_traits_qs_params(req, suffix='')
self.assertEqual(
[{'T1', 'T2'}, {'T3'}, {'T5', 'T6'}, {'T8'}],
required)
self.assertEqual({'T4', 'T7'}, forbidden)
class TestParseQsRequestGroups(testtools.TestCase):

View File

@ -12,6 +12,7 @@
"""Utility methods for placement API."""
import functools
import itertools
import jsonschema
from oslo_log import log as logging
@ -295,18 +296,9 @@ def normalize_resources_qs_param(qs):
return result
def valid_trait(trait, allow_forbidden):
"""Return True if the provided trait is the expected form.
When allow_forbidden is True, then a leading '!' is acceptable.
"""
if trait.startswith('!') and not allow_forbidden:
return False
return True
def normalize_traits_qs_param(val, allow_forbidden=False):
"""Parse a traits query string parameter value.
def normalize_traits_qs_param_to_legacy_value(val, allow_forbidden=False):
"""Parse a traits query string parameter value into the legacy return
format.
Note that this method doesn't know or care about the query parameter key,
which may currently be of the form `required`, `required123`, etc., but
@ -315,29 +307,38 @@ def normalize_traits_qs_param(val, allow_forbidden=False):
This method currently does no format validation of trait strings, other
than to ensure they're not zero-length.
This method only accepts query parameter value without 'in:' prefix support
:param val: A traits query parameter value: a comma-separated string of
trait names.
:param allow_forbidden: If True, accept forbidden traits (that is, traits
prefixed by '!') as a valid form when notifying
the caller that the provided value is not properly
formed.
:return: A set of trait names.
:return: A set of trait names or trait names prefixed with '!'
:raises `webob.exc.HTTPBadRequest` if the val parameter is not in the
expected format.
"""
ret = set(substr.strip() for substr in val.split(','))
expected_form = 'HW_CPU_X86_VMX,CUSTOM_MAGIC'
if allow_forbidden:
expected_form = 'HW_CPU_X86_VMX,!CUSTOM_MAGIC'
if not all(trait and valid_trait(trait, allow_forbidden) for trait in ret):
msg = ("Invalid query string parameters: Expected 'required' "
"parameter value of the form: %(form)s. "
"Got: %(val)s") % {'form': expected_form, 'val': val}
raise webob.exc.HTTPBadRequest(msg)
return ret
# let's parse the query string to the new internal format
required, forbidden = normalize_traits_qs_param(val, allow_forbidden)
# then reformat that structure to the old format
legacy_traits = set()
for any_traits in required:
# a legacy request does not have any-trait support so every internal
# set expressing OR relationship should exactly contain one trait
assert len(any_traits) == 1
legacy_traits.add(list(any_traits)[0])
for forbidden_trait in forbidden:
legacy_traits.add('!' + forbidden_trait)
return legacy_traits
def normalize_traits_qs_params(req, suffix=''):
# TODO(gibi): remove this once the allocation candidate code path also
# supports nested required_traits structure.
def normalize_traits_qs_params_legacy(req, suffix=''):
"""Given a webob.Request object, validate and collect required querystring
parameters.
@ -359,11 +360,150 @@ def normalize_traits_qs_params(req, suffix=''):
# NOTE(gibi): This means if the same query param is repeated then only
# the last one will be considered
for value in req.GET.getall('required' + suffix):
traits = normalize_traits_qs_param(value, allow_forbidden)
traits = normalize_traits_qs_param_to_legacy_value(
value, allow_forbidden)
return traits
def normalize_traits_qs_param(
val, allow_forbidden=False, allow_any_traits=False
):
"""Parse a traits query string parameter value.
Note that this method doesn't know or care about the query parameter key,
which may currently be of the form `required`, `required123`, etc., but
which may someday also include `preferred`, etc.
:param val: A traits query parameter value: either a comma-separated string
of trait names including trait names with ! prefix, or a string with
'in:' prefix and of comma-separated list of trait names. The 'in:'
prefixed string does not support trait names with ! prefix
:param allow_forbidden:
If True, accept forbidden traits (that is, traits prefixed by '!') as a
valid form.
:param allow_any_traits: if True, accept the 'in:' prefixed format.
:return: a two tuple where:
The first item is a list of set of traits. Each set of traits
represents a set of required traits in an OR relationship, while
different sets in the list represent required traits in an AND
relationship.
The second item is a set of forbidden traits.
:raises `webob.exc.HTTPBadRequest` if the val parameter is not in the
expected format.
"""
if val.startswith('in:'):
if not allow_any_traits:
msg = (
f"Invalid query string parameters: "
f"The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' only supported "
f"since microversion 1.39. Got: {val}")
raise webob.exc.HTTPBadRequest(msg)
any_traits = set(substr.strip() for substr in val[3:].split(','))
if not all(trait for trait in any_traits):
msg = (
f"Invalid query string parameters: Expected 'required' "
f"parameter value of the form: "
f"in:HW_CPU_X86_VMX,CUSTOM_MAGIC. Got an empty trait in: "
f"{val}")
raise webob.exc.HTTPBadRequest(msg)
if any(trait.startswith('!') for trait in any_traits):
msg = (
f"Invalid query string parameters: "
f"The format 'in:HW_CPU_X86_VMX,CUSTOM_MAGIC' does not "
f"support forbidden traits. Got: {val}")
raise webob.exc.HTTPBadRequest(msg)
# the in: prefix means all the traits are in a single OR relationship
# so we return [{every trait after the in: prefix}]
return [any_traits], set()
else:
all_traits = [substr.strip() for substr in val.split(',')]
# NOTE(gibi): lstrip will remove any number of consecutive '!'
# characters from the beginning of the trait name. This means !!!!!FOO
# is parsed as FOO. This is not a documented behavior of the API but
# this is a bug that decided not to be fixed outside a microversion
# bump. See
# https://review.opendev.org/c/openstack/placement/+/826491/7/placement/util.py#426
forbidden_traits = {
trait.lstrip('!') for trait in all_traits if trait.startswith('!')}
if not all(
trait
for trait in itertools.chain(forbidden_traits, all_traits)
):
expected_form = 'HW_CPU_X86_VMX,!CUSTOM_MAGIC'
if not allow_forbidden:
expected_form = 'HW_CPU_X86_VMX,CUSTOM_MAGIC'
msg = (
f"Invalid query string parameters: Expected 'required' "
f"parameter value of the form: {expected_form}. "
f"Got an empty trait in: {val}")
raise webob.exc.HTTPBadRequest(msg)
# NOTE(gibi): we need to wrap each required trait into a one element
# set of traits to keep the format of [{}, {}...] where each set of
# traits represent OR relationship
required_traits = [
{trait} for trait in all_traits if not trait.startswith('!')]
if forbidden_traits and not allow_forbidden:
msg = (
f"Invalid query string parameters: Expected 'required' "
f"parameter value of the form: HW_CPU_X86_VMX,CUSTOM_MAGIC. "
f"Got: {val}")
raise webob.exc.HTTPBadRequest(msg)
return required_traits, forbidden_traits
def normalize_traits_qs_params(req, suffix=''):
"""Given a webob.Request object, validate and collect required querystring
parameters.
We begin supporting forbidden traits in microversion 1.22.
We begin supporting any-traits and repeating the required param in
microversion 1.39.
:param req: a webob.Request object to read the params from
:param suffix: the string suffix of the request group to read from the
request. If empty then the unnamed request group is processed.
:returns: a two tuple where:
The first item is a list of set of traits. Each set of traits
represents a set of required traits in an OR relationship, while
different sets in the list represent required traits in an AND
relationship.
The second item is a set of forbidden traits.
:raises webob.exc.HTTPBadRequest: if the format of the query param is not
valid
"""
want_version = req.environ[placement.microversion.MICROVERSION_ENVIRON]
allow_forbidden = want_version.matches((1, 22))
allow_any_traits = want_version.matches((1, 39))
required_traits = []
forbidden_traits = set()
values = req.GET.getall('required' + suffix)
if not allow_any_traits:
# to keep the behavior of <= 1.38 we need to make sure that if
# the query param is repeated we only consider the last one from the
# request
values = values[-1:]
for value in values:
rts, fts = normalize_traits_qs_param(
value, allow_forbidden, allow_any_traits)
required_traits += rts
forbidden_traits |= fts
return required_traits, forbidden_traits
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