openstack/nova
Code Issues Proposed changes

Merge "Tie requester_id to RequestGroup suffix"

Browse Source
This commit is contained in:
Zuul
2019-12-11 01:13:44 +00:00
committed by Gerrit Code Review
parent 5a3ef39539 2e3985c303
commit 2f31c8d1b9
3 changed files with 94 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
nova/exception.py
View File

@@ -2478,3 +2478,7 @@ class GetPMEMNamespaceFailed(NovaException):
class VPMEMCleanupFailed(NovaException):
msg_fmt = _("Failed to clean up the vpmem backend device %(dev)s: "
"%(error)s")
class RequestGroupSuffixConflict(NovaException):
msg_fmt = _("Duplicate request group suffix %(suffix)s!")

59
nova/scheduler/utils.py
View File

@@ -78,6 +78,11 @@ class ResourceRequest(object):
This does *not* yet handle ``member_of[$S]``.
The string suffix is used as the RequestGroup.requester_id to
facilitate mapping of requests to allocation candidates using the
``mappings`` piece of the response added in Placement microversion 1.34
https://docs.openstack.org/placement/train/specs/train/implemented/placement-resource-provider-request-group-mapping-in-allocation-candidates.html # noqa
For image metadata, traits are extracted from the ``traits_required``
property, if present.
@@ -294,7 +299,9 @@ class ResourceRequest(object):
def get_request_group(self, ident):
if ident not in self._rg_by_id:
rq_grp = objects.RequestGroup(use_same_provider=bool(ident))
rq_grp = objects.RequestGroup(
use_same_provider=bool(ident),
requester_id=ident)
self._rg_by_id[ident] = rq_grp
return self._rg_by_id[ident]
@@ -304,24 +311,46 @@ class ResourceRequest(object):
The groups coming from the flavor can have arbitrary suffixes; those
are guaranteed to be unique within the flavor.
A group coming from "outside" (ports, device profiles) must be given a
suffix that is unique in combination with suffixes from the flavor.
A group coming from "outside" (ports, device profiles) must be
associated with a requester_id, such as a port UUID. We use this
requester_id as the group suffix (but ensure that it is unique in
combination with suffixes from the flavor).
.. todo:: Tie suffixes to RequestGroup.requester_id
Groups coming from "outside" are not allowed to be no-ops. That is,
they must provide resources and/or required/forbidden traits/aggregates
:param request_group: the RequestGroup to be added
:param request_group: the RequestGroup to be added.
:raise: ValueError if request_group has no requester_id, or if it
provides no resources or (required/forbidden) traits or aggregates.
:raise: RequestGroupSuffixConflict if request_group.requester_id
already exists in this ResourceRequest.
"""
# Generate a unique suffix by peeling out all the suffixes that are
# integers (respond to int()) and adding 1 to the highest one.
max_ident = 0
for ident in self._rg_by_id:
try:
max_ident = max(int(ident), max_ident)
except (TypeError, ValueError):
# Non-numeric or None (unsuffixed)
continue
# NOTE(efried): Deliberately check False-ness rather than None-ness
# here, since both would result in the unsuffixed request group being
# used, and that's bad.
if not request_group.requester_id:
# NOTE(efried): An "outside" RequestGroup is created by a
# programmatic agent and that agent is responsible for guaranteeing
# the presence of a unique requester_id. This is in contrast to
# flavor extra_specs where a human is responsible for the group
# suffix.
raise ValueError(
_('Missing requester_id in RequestGroup! This is probably a '
'programmer error. %s') % request_group)
self._rg_by_id[max_ident + 1] = request_group
if request_group.is_empty():
# NOTE(efried): It is up to the calling code to enforce a nonempty
# RequestGroup with suitable logic and exceptions.
raise ValueError(
_('Refusing to add no-op RequestGroup with requester_id=%s. '
'This is a probably a programmer error.') %
request_group.requester_id)
if request_group.requester_id in self._rg_by_id:
raise exception.RequestGroupSuffixConflict(
suffix=request_group.requester_id)
self._rg_by_id[request_group.requester_id] = request_group
def _add_resource(self, groupid, rclass, amount):
self.get_request_group(groupid).add_resource(rclass, amount)

66
nova/tests/unit/scheduler/test_utils.py
View File

@@ -310,12 +310,14 @@ class TestUtils(TestUtilsBase):
}
)
expected_resources._rg_by_id['1'] = objects.RequestGroup(
requester_id='1',
resources={
'VGPU': 1,
'VGPU_DISPLAY_HEAD': 2,
}
)
expected_resources._rg_by_id['3'] = objects.RequestGroup(
requester_id='3',
resources={
'VCPU': 2,
},
@@ -325,11 +327,13 @@ class TestUtils(TestUtilsBase):
}
)
expected_resources._rg_by_id['24'] = objects.RequestGroup(
requester_id='24',
resources={
'SRIOV_NET_VF': 2,
},
)
expected_resources._rg_by_id['42'] = objects.RequestGroup(
requester_id='42',
resources={
'SRIOV_NET_VF': 1,
}
@@ -675,16 +679,40 @@ class TestUtils(TestUtilsBase):
root_gb=10,
ephemeral_gb=5,
swap=0)
rg1 = objects.RequestGroup(resources={'CUSTOM_FOO': 1})
rg1 = objects.RequestGroup(
resources={'CUSTOM_FOO': 1}, requester_id='The-first-group')
# Leave requester_id out to trigger ValueError
rg2 = objects.RequestGroup(required_traits={'CUSTOM_BAR'})
reqspec = objects.RequestSpec(flavor=flavor,
requested_resources=[rg1, rg2])
self.assertRaises(
ValueError,
utils.resources_from_request_spec,
self.context, reqspec, self.mock_host_manager)
# Set conflicting requester_id
rg2.requester_id = 'The-first-group'
self.assertRaises(
exception.RequestGroupSuffixConflict,
utils.resources_from_request_spec,
self.context, reqspec, self.mock_host_manager)
# Good path: nonempty non-conflicting requester_id
rg2.requester_id = 'The-second-group'
req = utils.resources_from_request_spec(
self.context, reqspec, self.mock_host_manager)
self.assertEqual({'MEMORY_MB': 1024, 'DISK_GB': 15, 'VCPU': 1},
req.get_request_group(None).resources)
self.assertIs(rg1, req.get_request_group(1))
self.assertIs(rg2, req.get_request_group(2))
self.assertIs(rg1, req.get_request_group('The-first-group'))
self.assertIs(rg2, req.get_request_group('The-second-group'))
# Make sure those ended up as suffixes correctly
qs = req.to_querystring()
self.assertIn('resourcesThe-first-group=CUSTOM_FOO%3A1', qs)
self.assertIn('requiredThe-second-group=CUSTOM_BAR', qs)
def test_resources_from_request_spec_requested_resources_unfilled(self):
flavor = objects.Flavor(
@@ -876,6 +904,7 @@ class TestUtils(TestUtilsBase):
},
)
expected._rg_by_id['1'] = objects.RequestGroup(
requester_id='1',
resources={
'SRIOV_NET_VF': 1,
'IPV4_ADDRESS': 1,
@@ -888,6 +917,7 @@ class TestUtils(TestUtilsBase):
},
)
expected._rg_by_id['2'] = objects.RequestGroup(
requester_id='2',
resources={
'SRIOV_NET_VF': 1,
'IPV4_ADDRESS': 2,
@@ -898,6 +928,7 @@ class TestUtils(TestUtilsBase):
}
)
expected._rg_by_id['3'] = objects.RequestGroup(
requester_id='3',
resources={
'DISK_GB': 5,
}
@@ -1093,30 +1124,25 @@ class TestUtils(TestUtilsBase):
vcpus=1, memory_mb=1024, root_gb=10, ephemeral_gb=5, swap=0)
rs = objects.RequestSpec(flavor=flavor, is_bfv=False)
req = utils.ResourceRequest(rs)
rg1 = objects.RequestGroup()
rg1 = objects.RequestGroup(requester_id='foo',
required_traits={'CUSTOM_FOO'})
req._add_request_group(rg1)
rg2 = objects.RequestGroup()
rg2 = objects.RequestGroup(requester_id='bar',
forbidden_traits={'CUSTOM_BAR'})
req._add_request_group(rg2)
self.assertIs(rg1, req.get_request_group(1))
self.assertIs(rg2, req.get_request_group(2))
self.assertIs(rg1, req.get_request_group('foo'))
self.assertIs(rg2, req.get_request_group('bar'))
def test_resource_request_add_group_inserts_the_group_implicit_group(self):
def test_empty_groups_forbidden(self):
"""Not allowed to add premade RequestGroup without resources/traits/
aggregates.
"""
flavor = objects.Flavor(
vcpus=1, memory_mb=1024, root_gb=10, ephemeral_gb=5, swap=0)
rs = objects.RequestSpec(flavor=flavor, is_bfv=False)
req = utils.ResourceRequest(rs)
# this implicitly creates the specified group
unnumbered_rg = req.get_request_group(42)
rg1 = objects.RequestGroup()
req._add_request_group(rg1)
rg2 = objects.RequestGroup()
req._add_request_group(rg2)
self.assertIs(rg1, req.get_request_group(43))
self.assertIs(rg2, req.get_request_group(44))
self.assertIs(unnumbered_rg, req.get_request_group(42))
rg = objects.RequestGroup(requester_id='foo')
self.assertRaises(ValueError, req._add_request_group, rg)
def test_claim_resources_on_destination_no_source_allocations(self):
"""Tests the negative scenario where the instance does not have