Ensure resource classes correctly
Previously, SchedulerReportClient.set_inventory_for_provider used this
logic to pre-create custom resource classes found in the input
inventory.
list(map(self._ensure_resource_class,
(rc_name for rc_name in inv_data
if rc_name not in fields.ResourceClass.STANDARD)))
...where _ensure_resource_class would always attempt to create the
resource class it was given, and raise an exception if that failed for
any reason. Note in particular that attempting to create an
already-extant "standard" resource class will fail just the same as
attempting to create any nonexistent resource class that doesn't conform
to the schema (i.e. beginning with "CUSTOM_").
The problem is that this relies on the local system's notion of the set
of standard resource classes. If the placement service is running newer
code, standard resource classes may have been added that the compute
service doesn't know about yet.
This change set solves the problem by only attempting to create resource
classes with the 'CUSTOM_' prefix.
self._ensure_resource_class (which worked on a single resource class) is
replaced with self._ensure_resource_classes (plural) which accepts a
list of *all* the desired resource classes (including the standard
ones), filters down to the CUSTOM_* ones, and attempts to create the
remainder.
Note that if placement ever decides to start allowing creation of
resource classes with prefixes other than CUSTOM_, it will have to do so
under a new microversion, so we can't future-proof this by e.g.
prefetching all the known resource classes from placement and attempting
to create any not found in that list.
In the process of doing this rework, obsolete code paths relying on
pre-1.7 placement microversions are removed.
Change-Id: I600ed262e1b5d11facbf461e28291193665280cf
Closes-Bug: #1746615
This commit is contained in:
Eric Fried
parent
ecfbf1fcdf
commit
02a1551f64
|
|
@ -1044,9 +1044,7 @@ class SchedulerReportClient(object):
|
|||
parent_provider_uuid=parent_provider_uuid)
|
||||
|
||||
# Auto-create custom resource classes coming from a virt driver
|
||||
for rc_name in inv_data:
|
||||
if rc_name not in fields.ResourceClass.STANDARD:
|
||||
self._ensure_resource_class(context, rc_name)
|
||||
self._ensure_resource_classes(context, set(inv_data))
|
||||
|
||||
if inv_data:
|
||||
self._update_inventory(context, rp_uuid, inv_data)
|
||||
|
|
@ -1201,34 +1199,38 @@ class SchedulerReportClient(object):
|
|||
raise exception.ResourceProviderUpdateFailed(url=url, error=resp.text)
|
||||
|
||||
@safe_connect
|
||||
def _ensure_resource_class(self, context, name):
|
||||
"""Make sure a custom resource class exists.
|
||||
|
||||
PUT the resource class using microversion 1.7.
|
||||
|
||||
Returns the name of the resource class if it was successfully
|
||||
created or already exists. Otherwise None.
|
||||
def _ensure_resource_classes(self, context, names):
|
||||
"""Make sure resource classes exist.
|
||||
|
||||
:param context: The security context
|
||||
:param name: String name of the resource class to check/create.
|
||||
:raises: `exception.InvalidResourceClass` upon error.
|
||||
:param names: Iterable of string names of the resource classes to
|
||||
check/create. Must not be None.
|
||||
:raises: exception.InvalidResourceClass if an attempt is made to create
|
||||
an invalid resource class.
|
||||
"""
|
||||
# no payload on the put request
|
||||
response = self.put("/resource_classes/%s" % name, None, version="1.7",
|
||||
global_request_id=context.global_id)
|
||||
if 200 <= response.status_code < 300:
|
||||
return name
|
||||
else:
|
||||
msg = ("Failed to ensure resource class record with placement API "
|
||||
"for resource class %(rc_name)s. Got %(status_code)d: "
|
||||
"%(err_text)s.")
|
||||
args = {
|
||||
'rc_name': name,
|
||||
'status_code': response.status_code,
|
||||
'err_text': response.text,
|
||||
}
|
||||
LOG.error(msg, args)
|
||||
raise exception.InvalidResourceClass(resource_class=name)
|
||||
# Placement API version that supports PUT /resource_classes/CUSTOM_*
|
||||
# to create (or validate the existence of) a consumer-specified
|
||||
# resource class.
|
||||
version = '1.7'
|
||||
to_ensure = set(n for n in names
|
||||
if n.startswith(fields.ResourceClass.CUSTOM_NAMESPACE))
|
||||
|
||||
for name in to_ensure:
|
||||
# no payload on the put request
|
||||
resp = self.put(
|
||||
"/resource_classes/%s" % name, None, version=version,
|
||||
global_request_id=context.global_id)
|
||||
if not resp:
|
||||
msg = ("Failed to ensure resource class record with placement "
|
||||
"API for resource class %(rc_name)s. Got "
|
||||
"%(status_code)d: %(err_text)s.")
|
||||
args = {
|
||||
'rc_name': name,
|
||||
'status_code': resp.status_code,
|
||||
'err_text': resp.text,
|
||||
}
|
||||
LOG.error(msg, args)
|
||||
raise exception.InvalidResourceClass(resource_class=name)
|
||||
|
||||
def update_compute_node(self, context, compute_node):
|
||||
"""Creates or updates stats for the supplied compute node.
|
||||
|
|
|
|||
|
|
@ -202,7 +202,7 @@ class SchedulerReportClientTests(test.TestCase):
|
|||
# Try setting some invalid inventory and make sure the report
|
||||
# client raises the expected error.
|
||||
inv_data = {
|
||||
'BAD_FOO': {
|
||||
'CUSTOM_BOGU$_CLA$$': {
|
||||
'total': 100,
|
||||
'reserved': 0,
|
||||
'min_unit': 1,
|
||||
|
|
@ -274,18 +274,8 @@ class SchedulerReportClientTests(test.TestCase):
|
|||
}
|
||||
with interceptor.RequestsInterceptor(app=self.app, url=self.url):
|
||||
self.client.update_compute_node(self.context, self.compute_node)
|
||||
# Simulate that our locally-running code has an outdated notion of
|
||||
# standard resource classes.
|
||||
with mock.patch.object(fields.ResourceClass, 'STANDARD',
|
||||
('VCPU', 'MEMORY_MB', 'DISK_GB')):
|
||||
# TODO(efried): Once bug #1746615 is fixed, this will no longer
|
||||
# raise, and can be replaced with:
|
||||
# self.client.set_inventory_for_provider(
|
||||
# self.context, self.compute_uuid, self.compute_name, inv)
|
||||
self.assertRaises(
|
||||
exception.InvalidResourceClass,
|
||||
self.client.set_inventory_for_provider,
|
||||
self.context, self.compute_uuid, self.compute_name, inv)
|
||||
self.client.set_inventory_for_provider(
|
||||
self.context, self.compute_uuid, self.compute_name, inv)
|
||||
|
||||
@mock.patch('keystoneauth1.session.Session.get_endpoint',
|
||||
return_value='http://localhost:80/placement')
|
||||
|
|
|
|||
|
|
@ -3059,7 +3059,7 @@ There was a conflict when trying to complete your request.
|
|||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_delete_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_class')
|
||||
'_ensure_resource_classes')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_provider')
|
||||
def test_set_inventory_for_provider_no_custom(self, mock_erp, mock_erc,
|
||||
|
|
@ -3106,7 +3106,8 @@ There was a conflict when trying to complete your request.
|
|||
parent_provider_uuid=None,
|
||||
)
|
||||
# No custom resource classes to ensure...
|
||||
self.assertFalse(mock_erc.called)
|
||||
mock_erc.assert_called_once_with(self.context,
|
||||
set(['VCPU', 'MEMORY_MB', 'DISK_GB']))
|
||||
mock_upd.assert_called_once_with(
|
||||
self.context,
|
||||
mock.sentinel.rp_uuid,
|
||||
|
|
@ -3119,7 +3120,7 @@ There was a conflict when trying to complete your request.
|
|||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_delete_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_class')
|
||||
'_ensure_resource_classes')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_provider')
|
||||
def test_set_inventory_for_provider_no_inv(self, mock_erp, mock_erc,
|
||||
|
|
@ -3140,7 +3141,7 @@ There was a conflict when trying to complete your request.
|
|||
mock.sentinel.rp_name,
|
||||
parent_provider_uuid=None,
|
||||
)
|
||||
self.assertFalse(mock_erc.called)
|
||||
mock_erc.assert_called_once_with(self.context, set())
|
||||
self.assertFalse(mock_upd.called)
|
||||
mock_del.assert_called_once_with(self.context, mock.sentinel.rp_uuid)
|
||||
|
||||
|
|
@ -3149,7 +3150,7 @@ There was a conflict when trying to complete your request.
|
|||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_delete_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_class')
|
||||
'_ensure_resource_classes')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_provider')
|
||||
def test_set_inventory_for_provider_with_custom(self, mock_erp,
|
||||
|
|
@ -3205,7 +3206,9 @@ There was a conflict when trying to complete your request.
|
|||
mock.sentinel.rp_name,
|
||||
parent_provider_uuid=None,
|
||||
)
|
||||
mock_erc.assert_called_once_with(self.context, 'CUSTOM_IRON_SILVER')
|
||||
mock_erc.assert_called_once_with(
|
||||
self.context,
|
||||
set(['VCPU', 'MEMORY_MB', 'DISK_GB', 'CUSTOM_IRON_SILVER']))
|
||||
mock_upd.assert_called_once_with(
|
||||
self.context,
|
||||
mock.sentinel.rp_uuid,
|
||||
|
|
@ -3216,7 +3219,7 @@ There was a conflict when trying to complete your request.
|
|||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_delete_inventory', new=mock.Mock())
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_class', new=mock.Mock())
|
||||
'_ensure_resource_classes', new=mock.Mock())
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_provider')
|
||||
def test_set_inventory_for_provider_with_parent(self, mock_erp):
|
||||
|
|
@ -3526,3 +3529,39 @@ class TestAllocations(SchedulerReportClientTestCase):
|
|||
# With a 409, only the error should be called
|
||||
self.assertEqual(0, mock_log.info.call_count)
|
||||
self.assertEqual(1, mock_log.error.call_count)
|
||||
|
||||
|
||||
class TestResourceClass(SchedulerReportClientTestCase):
|
||||
def setUp(self):
|
||||
super(TestResourceClass, self).setUp()
|
||||
_put_patch = mock.patch(
|
||||
"nova.scheduler.client.report.SchedulerReportClient.put")
|
||||
self.addCleanup(_put_patch.stop)
|
||||
self.mock_put = _put_patch.start()
|
||||
|
||||
def test_ensure_resource_classes(self):
|
||||
rcs = ['VCPU', 'CUSTOM_FOO', 'MEMORY_MB', 'CUSTOM_BAR']
|
||||
self.client._ensure_resource_classes(self.context, rcs)
|
||||
self.mock_put.assert_has_calls([
|
||||
mock.call('/resource_classes/%s' % rc, None, version='1.7',
|
||||
global_request_id=self.context.global_id)
|
||||
for rc in ('CUSTOM_FOO', 'CUSTOM_BAR')
|
||||
], any_order=True)
|
||||
|
||||
def test_ensure_resource_classes_none(self):
|
||||
for empty in ([], (), set(), {}):
|
||||
self.client._ensure_resource_classes(self.context, empty)
|
||||
self.mock_put.assert_not_called()
|
||||
|
||||
def test_ensure_resource_classes_put_fail(self):
|
||||
resp = requests.Response()
|
||||
resp.status_code = 503
|
||||
self.mock_put.return_value = resp
|
||||
rcs = ['VCPU', 'MEMORY_MB', 'CUSTOM_BAD']
|
||||
self.assertRaises(
|
||||
exception.InvalidResourceClass,
|
||||
self.client._ensure_resource_classes, self.context, rcs)
|
||||
# Only called with the "bad" one
|
||||
self.mock_put.assert_called_once_with(
|
||||
'/resource_classes/CUSTOM_BAD', None, version='1.7',
|
||||
global_request_id=self.context.global_id)
|
||||
|
|
|
|||
Loading…
Reference in New Issue