openstack
/
nova
Code Issues Proposed changes

Report client: Real get_allocs_for_consumer 

In preparation for reshaper work, implement a superior method to
retrieve allocations for a consumer. The new get_allocs_for_consumer:
- Uses the microversion that returns consumer generations (1.28).
- Doesn't hide error conditions:
  - If the request returns non-200, instead of returning {}, it raises a
    new ConsumerAllocationRetrievalFailed exception.
  - If we fail to communicate with the placement API, instead of
    returning None, it raises (a subclass of) ksa ClientException.
- Returns the entire payload rather than just the 'allocations' dict.

The existing get_allocations_for_consumer is refactored to behave
compatibly (except it logs warnings for the previously-silently-hidden
error conditions). In a subsequent patch, we should rework all callers
of this method to use the new one, and get rid of the old one.

Change-Id: I0e9a804ae7717252175f7fe409223f5eb8f50013
blueprint: reshape-provider-tree
This commit is contained in:
Eric Fried 2018-07-21 14:28:29 -05:00
parent f534495a42
commit 176d1d90fd
6 changed files with 117 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
nova/cmd/manage.py
View File

@ -31,6 +31,7 @@ import traceback
from dateutil import parser as dateutil_parser
import decorator
from keystoneauth1 import exceptions as ks_exc
import netaddr
from oslo_config import cfg
from oslo_db import exception as db_exc
@ -1804,8 +1805,15 @@ class PlacementCommands(object):
output(_('Instance %s is not on a host.') % instance.uuid)
return
allocations = placement.get_allocations_for_consumer(
ctxt, instance.uuid, include_generation=True)
try:
allocations = placement.get_allocs_for_consumer(
ctxt, instance.uuid)
except (ks_exc.ClientException,
exception.ConsumerAllocationRetrievalFailed) as e:
output(_("Allocation retrieval failed: %s") % e)
# TODO(mriedem): Fail fast here, since we can't talk to placement.
allocations = None
# get_allocations_for_consumer uses safe_connect which will
# return None if we can't communicate with Placement, and the
# response can have an empty {'allocations': {}} response if
@ -1831,10 +1839,12 @@ class PlacementCommands(object):
# provider allocations.
allocations['project_id'] = instance.project_id
allocations['user_id'] = instance.user_id
# We use 1.28 for PUT /allocations/{consumer_id} to mirror
# the body structure from get_allocations_for_consumer.
resp = placement.put('/allocations/%s' % instance.uuid,
allocations, version='1.28')
# We use CONSUMER_GENERATION_VERSION for PUT
# /allocations/{consumer_id} to mirror the body structure from
# get_allocs_for_consumer.
resp = placement.put(
'/allocations/%s' % instance.uuid,
allocations, version=report.CONSUMER_GENERATION_VERSION)
if resp:
output(_('Successfully updated allocations for '
'instance %s.') % instance.uuid)

5
nova/exception.py
View File

@ -2373,3 +2373,8 @@ class NoResourceClass(NovaException):
class ResourceProviderAllocationRetrievalFailed(NovaException):
msg_fmt = _("Failed to retrieve allocations for resource provider "
"%(rp_uuid)s: %(error)s")
class ConsumerAllocationRetrievalFailed(NovaException):
msg_fmt = _("Failed to retrieve allocations for consumer "
"%(consumer_uuid)s: %(error)s")

82
nova/scheduler/client/report.py
View File

@ -1536,32 +1536,74 @@ class SchedulerReportClient(object):
if not success:
raise exception.ResourceProviderSyncFailed()
@safe_connect
def get_allocations_for_consumer(self, context, consumer,
include_generation=False):
# TODO(efried): Cut users of this method over to get_allocs_for_consumer
def get_allocations_for_consumer(self, context, consumer):
"""Legacy method for allocation retrieval.
Callers should move to using get_allocs_for_consumer, which handles
errors properly and returns the entire payload.
:param context: The nova.context.RequestContext auth context
:param consumer: UUID of the consumer resource
:returns: A dict of the form:
{
$RP_UUID: {
"generation": $RP_GEN,
"resources": {
$RESOURCE_CLASS: $AMOUNT
...
},
},
...
}
"""
try:
return self.get_allocs_for_consumer(
context, consumer)['allocations']
except ks_exc.ClientException as e:
LOG.warning("Failed to get allocations for consumer %(consumer)s: "
"%(error)s", {'consumer': consumer, 'error': e})
# Because this is what @safe_connect did
return None
except exception.ConsumerAllocationRetrievalFailed as e:
LOG.warning(e)
# Because this is how we used to treat non-200
return {}
def get_allocs_for_consumer(self, context, consumer):
"""Makes a GET /allocations/{consumer} call to Placement.
:param context: The nova.context.RequestContext auth context
:param consumer: UUID of the consumer resource
:param include_generation: True if the response should be the
full allocations response including ``consumer_generation`` (new
in microversion 1.28), False if only the "allocations" dict from
the response body should be returned.
:returns: dict, see ``include_generation`` for details on format;
returns None if unable to connect to Placement (see safe_connect)
:return: Dict of the form:
{ "allocations": {
$RP_UUID: {
"generation": $RP_GEN,
"resources": {
$RESOURCE_CLASS: $AMOUNT
...
},
},
...
},
"consumer_generation": $CONSUMER_GEN,
"project_id": $PROJ_ID,
"user_id": $USER_ID,
}
:raises: keystoneauth1.exceptions.base.ClientException on failure to
communicate with the placement API
:raises: ConsumerAllocationRetrievalFailed if the placement API call
fails
"""
url = '/allocations/%s' % consumer
resp = self.get(
url, version=CONSUMER_GENERATION_VERSION,
global_request_id=context.global_id)
resp = self.get('/allocations/%s' % consumer,
version=CONSUMER_GENERATION_VERSION,
global_request_id=context.global_id)
if not resp:
return {}
else:
# TODO(efried): refactor all callers to accept the whole response
# so we can get rid of this condition
if include_generation:
return resp.json()
return resp.json()['allocations']
# TODO(efried): Use code/title/detail to make a better exception
raise exception.ConsumerAllocationRetrievalFailed(
consumer_uuid=consumer, error=resp.text)
return resp.json()
def get_allocations_for_consumer_by_provider(self, context, rp_uuid,
consumer):

6
nova/tests/functional/test_report_client.py
View File

@ -234,12 +234,6 @@ class SchedulerReportClientTests(SchedulerReportClientTestBase):
vcpu_data = usage_data[res_class]
self.assertEqual(2, vcpu_data)
# Check that we can get allocations for the consumer with
# the generation.
allocations = self.client.get_allocations_for_consumer(
self.context, self.instance.uuid, include_generation=True)
self.assertIn('consumer_generation', allocations)
# Delete allocations with our instance
self.client.update_instance_allocation(
self.context, self.compute_node, self.instance, -1)

30
nova/tests/unit/scheduler/client/test_report.py
View File

@ -3428,6 +3428,36 @@ class TestAllocations(SchedulerReportClientTestCase):
'/resource_providers/rpuuid/allocations',
global_request_id=self.context.global_id)
@mock.patch("nova.scheduler.client.report.SchedulerReportClient.get")
def test_get_allocs_for_consumer(self, mock_get):
mock_get.return_value = fake_requests.FakeResponse(
200, content=jsonutils.dumps({'foo': 'bar'}))
ret = self.client.get_allocs_for_consumer(self.context, 'consumer')
self.assertEqual({'foo': 'bar'}, ret)
mock_get.assert_called_once_with(
'/allocations/consumer', version='1.28',
global_request_id=self.context.global_id)
@mock.patch("nova.scheduler.client.report.SchedulerReportClient.get")
def test_get_allocs_for_consumer_fail(self, mock_get):
mock_get.return_value = fake_requests.FakeResponse(400, content='err')
self.assertRaises(exception.ConsumerAllocationRetrievalFailed,
self.client.get_allocs_for_consumer,
self.context, 'consumer')
mock_get.assert_called_once_with(
'/allocations/consumer', version='1.28',
global_request_id=self.context.global_id)
@mock.patch("nova.scheduler.client.report.SchedulerReportClient.get")
def test_get_allocs_for_consumer_safe_connect_fail(self, mock_get):
mock_get.side_effect = ks_exc.EndpointNotFound()
self.assertRaises(ks_exc.ClientException,
self.client.get_allocs_for_consumer,
self.context, 'consumer')
mock_get.assert_called_once_with(
'/allocations/consumer', version='1.28',
global_request_id=self.context.global_id)
class TestResourceClass(SchedulerReportClientTestCase):
def setUp(self):

10
nova/tests/unit/test_nova_manage.py
View File

@ -2538,7 +2538,7 @@ class TestNovaManagePlacement(test.NoDBTestCase):
project_id='fake-project', user_id='fake-user')]),
objects.InstanceList()))
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
'get_allocations_for_consumer')
'get_allocs_for_consumer')
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename',
new_callable=mock.NonCallableMock) # assert not called
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.put',
@ -2571,8 +2571,7 @@ class TestNovaManagePlacement(test.NoDBTestCase):
self.assertEqual(0, self.cli.heal_allocations(verbose=True))
self.assertIn('Processed 1 instances.', self.output.getvalue())
mock_get_allocs.assert_called_once_with(
test.MatchType(context.RequestContext), uuidsentinel.instance,
include_generation=True)
test.MatchType(context.RequestContext), uuidsentinel.instance)
expected_put_data = mock_get_allocs.return_value
expected_put_data['project_id'] = 'fake-project'
expected_put_data['user_id'] = 'fake-user'
@ -2591,7 +2590,7 @@ class TestNovaManagePlacement(test.NoDBTestCase):
task_state=None, project_id='fake-project',
user_id='fake-user')]))
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
'get_allocations_for_consumer')
'get_allocs_for_consumer')
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.put',
return_value=fake_requests.FakeResponse(
409, content='Inventory and/or allocations changed while '
@ -2623,8 +2622,7 @@ class TestNovaManagePlacement(test.NoDBTestCase):
self.assertIn(
'Inventory and/or allocations changed', self.output.getvalue())
mock_get_allocs.assert_called_once_with(
test.MatchType(context.RequestContext), uuidsentinel.instance,
include_generation=True)
test.MatchType(context.RequestContext), uuidsentinel.instance)
expected_put_data = mock_get_allocs.return_value
expected_put_data['project_id'] = 'fake-project'
expected_put_data['user_id'] = 'fake-user'