Merge "Proper error handling by _ensure_resource_provider" into stable/pike

This commit is contained in:
Zuul 2018-02-28 17:54:03 +00:00 committed by Gerrit Code Review
commit 75e1f0d77e
11 changed files with 86 additions and 44 deletions

View File

@ -22,7 +22,7 @@
"host_ip": "1.1.1.1",
"free_disk_gb": 1028,
"free_ram_mb": 7680,
"hypervisor_hostname": "fake-mini",
"hypervisor_hostname": "host1",
"hypervisor_type": "fake",
"hypervisor_version": 1000,
"id": 2,

View File

@ -1,7 +1,7 @@
{
"hypervisors": [
{
"hypervisor_hostname": "fake-mini",
"hypervisor_hostname": "host1",
"id": 2,
"state": "up",
"status": "enabled"

View File

@ -22,7 +22,7 @@
"host_ip": "1.1.1.1",
"free_disk_gb": 1028,
"free_ram_mb": 7680,
"hypervisor_hostname": "fake-mini",
"hypervisor_hostname": "host2",
"hypervisor_type": "fake",
"hypervisor_version": 1000,
"id": "1bb62a04-c576-402c-8147-9e89757a09e3",

View File

@ -2066,6 +2066,14 @@ class ResourceProviderInUse(NovaException):
msg_fmt = _("Resource provider has allocations.")
class ResourceProviderRetrievalFailed(NovaException):
msg_fmt = _("Failed to get resource provider with UUID %(uuid)s")
class ResourceProviderCreationFailed(NovaException):
msg_fmt = _("Failed to create resource provider %(name)s")
class InventoryWithResourceClassNotFound(NotFound):
msg_fmt = _("No inventory of class %(resource_class)s found.")

View File

@ -389,10 +389,10 @@ class SchedulerReportClient(object):
"""Queries the placement API for a resource provider record with the
supplied UUID.
Returns a dict of resource provider information if found or None if no
such resource provider could be found.
:param uuid: UUID identifier for the resource provider to look up
:return: A dict of resource provider information if found or None if no
such resource provider could be found.
:raise: ResourceProviderRetrievalFailed on error.
"""
resp = self.get("/resource_providers/%s" % uuid)
if resp.status_code == 200:
@ -412,16 +412,18 @@ class SchedulerReportClient(object):
'placement_req_id': placement_req_id,
}
LOG.error(msg, args)
raise exception.ResourceProviderRetrievalFailed(uuid=uuid)
@safe_connect
def _create_resource_provider(self, uuid, name):
"""Calls the placement API to create a new resource provider record.
Returns a dict of resource provider information object representing
the newly-created resource provider.
:param uuid: UUID of the new resource provider
:param name: Name of the resource provider
:return: A dict of resource provider information object representing
the newly-created resource provider.
:raise: ResourceProviderCreationFailed or
ResourceProviderRetrievalFailed on error.
"""
url = "/resource_providers"
payload = {
@ -445,7 +447,10 @@ class SchedulerReportClient(object):
name=name,
generation=0,
)
elif resp.status_code == 409:
# TODO(efried): Push error codes from placement, and use 'em.
name_conflict = 'Conflicting resource provider name:'
if resp.status_code == 409 and name_conflict not in resp.text:
# Another thread concurrently created a resource provider with the
# same UUID. Log a warning and then just return the resource
# provider object from _get_resource_provider()
@ -458,17 +463,19 @@ class SchedulerReportClient(object):
}
LOG.info(msg, args)
return self._get_resource_provider(uuid)
else:
msg = _LE("[%(placement_req_id)s] Failed to create resource "
"provider record in placement API for UUID %(uuid)s. "
"Got %(status_code)d: %(err_text)s.")
args = {
'uuid': uuid,
'status_code': resp.status_code,
'err_text': resp.text,
'placement_req_id': placement_req_id,
}
LOG.error(msg, args)
# A provider with the same *name* already exists, or some other error.
msg = ("[%(placement_req_id)s] Failed to create resource provider "
"record in placement API for UUID %(uuid)s. Got "
"%(status_code)d: %(err_text)s.")
args = {
'uuid': uuid,
'status_code': resp.status_code,
'err_text': resp.text,
'placement_req_id': placement_req_id,
}
LOG.error(msg, args)
raise exception.ResourceProviderCreationFailed(name=name)
def _ensure_resource_provider(self, uuid, name=None):
"""Ensures that the placement API has a record of a resource provider
@ -479,7 +486,11 @@ class SchedulerReportClient(object):
The found or created resource provider object is returned from this
method. If the resource provider object for the supplied uuid was not
found and the resource provider record could not be created in the
placement API, we return None.
placement API, an exception is raised.
If this method returns successfully, callers are assured both that
the placement API contains a record of the provider and the local cache
of resource provider information contains a record of the provider.
:param uuid: UUID identifier for the resource provider to ensure exists
:param name: Optional name for the resource provider if the record
@ -502,10 +513,8 @@ class SchedulerReportClient(object):
rp = self._get_resource_provider(uuid)
if rp is None:
name = name or uuid
rp = self._create_resource_provider(uuid, name)
if rp is None:
return
rp = self._create_resource_provider(uuid, name or uuid)
msg = "Grabbing aggregate associations for resource provider %s"
LOG.debug(msg, uuid)
aggs = self._get_provider_aggregates(uuid)

View File

@ -22,7 +22,7 @@
"host_ip": "%(ip)s",
"free_disk_gb": 1028,
"free_ram_mb": 7680,
"hypervisor_hostname": "fake-mini",
"hypervisor_hostname": "host1",
"hypervisor_type": "fake",
"hypervisor_version": 1000,
"id": %(hypervisor_id)s,

View File

@ -1,7 +1,7 @@
{
"hypervisors": [
{
"hypervisor_hostname": "fake-mini",
"hypervisor_hostname": "host1",
"id": 2,
"state": "up",
"status": "enabled"

View File

@ -22,7 +22,7 @@
"host_ip": "%(ip)s",
"free_disk_gb": 1028,
"free_ram_mb": 7680,
"hypervisor_hostname": "fake-mini",
"hypervisor_hostname": "host2",
"hypervisor_type": "fake",
"hypervisor_version": 1000,
"id": "%(hypervisor_id)s",

View File

@ -18,6 +18,7 @@ import mock
from nova.cells import utils as cells_utils
from nova import objects
from nova.tests.functional.api_sample_tests import api_sample_base
from nova.virt import fake
class HypervisorsSampleJsonTests(api_sample_base.ApiSampleTestBaseV21):
@ -155,7 +156,10 @@ class HypervisorsSampleJson233Tests(api_sample_base.ApiSampleTestBaseV21):
self.api.microversion = self.microversion
# Start a new compute service to fake a record with hypervisor id=2
# for pagination test.
self.start_service('compute', host='host1')
host = 'host1'
fake.set_nodes([host])
self.addCleanup(fake.restore_nodes)
self.start_service('compute', host=host)
def test_hypervisors_list(self):
response = self._do_get('os-hypervisors?limit=1&marker=1')
@ -200,7 +204,10 @@ class HypervisorsSampleJson253Tests(HypervisorsSampleJson228Tests):
def test_hypervisors_detail(self):
# Start another compute service to get a 2nd compute for paging tests.
service_2 = self.start_service('compute', host='host2').service_ref
host = 'host2'
fake.set_nodes([host])
self.addCleanup(fake.restore_nodes)
service_2 = self.start_service('compute', host=host).service_ref
compute_node_2 = service_2.compute_node
marker = self.compute_node_1.uuid
subs = {

View File

@ -37,6 +37,8 @@ class TestListServersIpFilter(test.TestCase):
# the image fake backend needed for image discovery
nova.tests.unit.image.fake.stub_out_image_service(self)
self.useFixture(nova_fixtures.PlacementFixture())
self.start_service('conductor')
# Use the chance scheduler to bypass filtering and just pick the single
# compute host that we have.
@ -46,7 +48,6 @@ class TestListServersIpFilter(test.TestCase):
self.start_service('consoleauth')
self.useFixture(cast_as_call.CastAsCall(self))
self.useFixture(nova_fixtures.PlacementFixture())
self.image_id = self.api.get_images()[0]['id']
self.flavor_id = self.api.get_flavors()[0]['id']

View File

@ -1019,16 +1019,19 @@ class TestProviderOperations(SchedulerReportClientTestCase):
'_get_provider_aggregates')
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
'_get_resource_provider')
def test_ensure_resource_provider_create_none(self, get_rp_mock,
def test_ensure_resource_provider_create_fail(self, get_rp_mock,
get_agg_mock, create_rp_mock):
# No resource provider exists in the client's cache, and
# _create_provider returns None, indicating there was an error with the
# _create_provider raises, indicating there was an error with the
# create call. Ensure we don't populate the resource provider cache
# with a None value.
get_rp_mock.return_value = None
create_rp_mock.return_value = None
create_rp_mock.side_effect = exception.ResourceProviderCreationFailed(
name=uuids.compute_node)
self.client._ensure_resource_provider(uuids.compute_node)
self.assertRaises(
exception.ResourceProviderCreationFailed,
self.client._ensure_resource_provider, uuids.compute_node)
get_rp_mock.assert_called_once_with(uuids.compute_node)
create_rp_mock.assert_called_once_with(uuids.compute_node,
@ -1165,7 +1168,9 @@ class TestProviderOperations(SchedulerReportClientTestCase):
'openstack-request-id': uuids.request_id}
uuid = uuids.compute_node
result = self.client._get_resource_provider(uuid)
self.assertRaises(
exception.ResourceProviderRetrievalFailed,
self.client._get_resource_provider, uuid)
expected_url = '/resource_providers/' + uuid
self.ks_sess_mock.get.assert_called_once_with(expected_url,
@ -1177,7 +1182,6 @@ class TestProviderOperations(SchedulerReportClientTestCase):
self.assertTrue(logging_mock.called)
self.assertEqual(uuids.request_id,
logging_mock.call_args[0][1]['placement_req_id'])
self.assertIsNone(result)
def test_create_resource_provider(self):
# Ensure _create_resource_provider() returns a dict of resource
@ -1219,10 +1223,10 @@ class TestProviderOperations(SchedulerReportClientTestCase):
# record.
uuid = uuids.compute_node
name = 'computehost'
resp_mock = mock.Mock(status_code=409)
self.ks_sess_mock.post.return_value = resp_mock
self.ks_sess_mock.post.return_value.headers = {
'openstack-request-id': uuids.request_id}
self.ks_sess_mock.post.return_value = mock.Mock(
status_code=409,
headers={'openstack-request-id': uuids.request_id},
text='not a name conflict')
get_rp_mock.return_value = mock.sentinel.get_rp
@ -1244,6 +1248,18 @@ class TestProviderOperations(SchedulerReportClientTestCase):
self.assertEqual(uuids.request_id,
logging_mock.call_args[0][1]['placement_req_id'])
def test_create_resource_provider_name_conflict(self):
# When the API call to create the resource provider fails 409 with a
# name conflict, we raise an exception.
self.ks_sess_mock.post.return_value = mock.Mock(
status_code=409,
text='<stuff>Conflicting resource provider name: foo already '
'exists.</stuff>')
self.assertRaises(
exception.ResourceProviderCreationFailed,
self.client._create_resource_provider, uuids.compute_node, 'foo')
@mock.patch.object(report.LOG, 'error')
def test_create_resource_provider_error(self, logging_mock):
# Ensure _create_resource_provider() sets the error flag when trying to
@ -1256,7 +1272,9 @@ class TestProviderOperations(SchedulerReportClientTestCase):
self.ks_sess_mock.post.return_value.headers = {
'x-openstack-request-id': uuids.request_id}
result = self.client._create_resource_provider(uuid, name)
self.assertRaises(
exception.ResourceProviderCreationFailed,
self.client._create_resource_provider, uuid, name)
expected_payload = {
'uuid': uuid,
@ -1274,7 +1292,6 @@ class TestProviderOperations(SchedulerReportClientTestCase):
self.assertTrue(logging_mock.called)
self.assertEqual(uuids.request_id,
logging_mock.call_args[0][1]['placement_req_id'])
self.assertFalse(result)
class TestAggregates(SchedulerReportClientTestCase):