Merge "Revert "Proper error handling by _ensure_resource_provider"" into stable/ocata
This commit is contained in:
commit
c135d81fc1
|
@ -22,7 +22,7 @@
|
||||||
"host_ip": "1.1.1.1",
|
"host_ip": "1.1.1.1",
|
||||||
"free_disk_gb": 1028,
|
"free_disk_gb": 1028,
|
||||||
"free_ram_mb": 7680,
|
"free_ram_mb": 7680,
|
||||||
"hypervisor_hostname": "host1",
|
"hypervisor_hostname": "fake-mini",
|
||||||
"hypervisor_type": "fake",
|
"hypervisor_type": "fake",
|
||||||
"hypervisor_version": 1000,
|
"hypervisor_version": 1000,
|
||||||
"id": 2,
|
"id": 2,
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
{
|
{
|
||||||
"hypervisors": [
|
"hypervisors": [
|
||||||
{
|
{
|
||||||
"hypervisor_hostname": "host1",
|
"hypervisor_hostname": "fake-mini",
|
||||||
"id": 2,
|
"id": 2,
|
||||||
"state": "up",
|
"state": "up",
|
||||||
"status": "enabled"
|
"status": "enabled"
|
||||||
|
|
|
@ -2114,14 +2114,6 @@ class ResourceProviderInUse(NovaException):
|
||||||
msg_fmt = _("Resource provider has allocations.")
|
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):
|
class InventoryWithResourceClassNotFound(NotFound):
|
||||||
msg_fmt = _("No inventory of class %(resource_class)s found.")
|
msg_fmt = _("No inventory of class %(resource_class)s found.")
|
||||||
|
|
||||||
|
|
|
@ -301,10 +301,10 @@ class SchedulerReportClient(object):
|
||||||
"""Queries the placement API for a resource provider record with the
|
"""Queries the placement API for a resource provider record with the
|
||||||
supplied UUID.
|
supplied UUID.
|
||||||
|
|
||||||
|
Returns an `objects.ResourceProvider` object if found or None if no
|
||||||
|
such resource provider could be found.
|
||||||
|
|
||||||
:param uuid: UUID identifier for the resource provider to look up
|
:param uuid: UUID identifier for the resource provider to look up
|
||||||
:return: An `objects.ResourceProvider` object if found or None if no
|
|
||||||
such resource provider could be found.
|
|
||||||
:raise: ResourceProviderRetrievalFailed on error.
|
|
||||||
"""
|
"""
|
||||||
resp = self.get("/resource_providers/%s" % uuid)
|
resp = self.get("/resource_providers/%s" % uuid)
|
||||||
if resp.status_code == 200:
|
if resp.status_code == 200:
|
||||||
|
@ -326,18 +326,16 @@ class SchedulerReportClient(object):
|
||||||
'err_text': resp.text,
|
'err_text': resp.text,
|
||||||
}
|
}
|
||||||
LOG.error(msg, args)
|
LOG.error(msg, args)
|
||||||
raise exception.ResourceProviderRetrievalFailed(uuid=uuid)
|
|
||||||
|
|
||||||
@safe_connect
|
@safe_connect
|
||||||
def _create_resource_provider(self, uuid, name):
|
def _create_resource_provider(self, uuid, name):
|
||||||
"""Calls the placement API to create a new resource provider record.
|
"""Calls the placement API to create a new resource provider record.
|
||||||
|
|
||||||
|
Returns an `objects.ResourceProvider` object representing the
|
||||||
|
newly-created resource provider object.
|
||||||
|
|
||||||
:param uuid: UUID of the new resource provider
|
:param uuid: UUID of the new resource provider
|
||||||
:param name: Name of the resource provider
|
:param name: Name of the resource provider
|
||||||
:return: An `objects.ResourceProvider` object representing the
|
|
||||||
newly-created resource provider object.
|
|
||||||
:raise: ResourceProviderCreationFailed or
|
|
||||||
ResourceProviderRetrievalFailed on error.
|
|
||||||
"""
|
"""
|
||||||
url = "/resource_providers"
|
url = "/resource_providers"
|
||||||
payload = {
|
payload = {
|
||||||
|
@ -355,10 +353,7 @@ class SchedulerReportClient(object):
|
||||||
name=name,
|
name=name,
|
||||||
generation=0,
|
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
|
# Another thread concurrently created a resource provider with the
|
||||||
# same UUID. Log a warning and then just return the resource
|
# same UUID. Log a warning and then just return the resource
|
||||||
# provider object from _get_resource_provider()
|
# provider object from _get_resource_provider()
|
||||||
|
@ -368,17 +363,16 @@ class SchedulerReportClient(object):
|
||||||
msg = msg.format(uuid)
|
msg = msg.format(uuid)
|
||||||
LOG.info(msg)
|
LOG.info(msg)
|
||||||
return self._get_resource_provider(uuid)
|
return self._get_resource_provider(uuid)
|
||||||
|
else:
|
||||||
# A provider with the same *name* already exists, or some other error.
|
msg = _LE("Failed to create resource provider record in "
|
||||||
msg = _LE("Failed to create resource provider record in placement API "
|
"placement API for UUID %(uuid)s. "
|
||||||
"for UUID %(uuid)s. Got %(status_code)d: %(err_text)s.")
|
"Got %(status_code)d: %(err_text)s.")
|
||||||
args = {
|
args = {
|
||||||
'uuid': uuid,
|
'uuid': uuid,
|
||||||
'status_code': resp.status_code,
|
'status_code': resp.status_code,
|
||||||
'err_text': resp.text,
|
'err_text': resp.text,
|
||||||
}
|
}
|
||||||
LOG.error(msg, args)
|
LOG.error(msg, args)
|
||||||
raise exception.ResourceProviderCreationFailed(name=name)
|
|
||||||
|
|
||||||
def _ensure_resource_provider(self, uuid, name=None):
|
def _ensure_resource_provider(self, uuid, name=None):
|
||||||
"""Ensures that the placement API has a record of a resource provider
|
"""Ensures that the placement API has a record of a resource provider
|
||||||
|
@ -389,11 +383,7 @@ class SchedulerReportClient(object):
|
||||||
The found or created resource provider object is returned from this
|
The found or created resource provider object is returned from this
|
||||||
method. If the resource provider object for the supplied uuid was not
|
method. If the resource provider object for the supplied uuid was not
|
||||||
found and the resource provider record could not be created in the
|
found and the resource provider record could not be created in the
|
||||||
placement API, an exception is raised.
|
placement API, we return None.
|
||||||
|
|
||||||
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 uuid: UUID identifier for the resource provider to ensure exists
|
||||||
:param name: Optional name for the resource provider if the record
|
:param name: Optional name for the resource provider if the record
|
||||||
|
@ -416,8 +406,10 @@ class SchedulerReportClient(object):
|
||||||
|
|
||||||
rp = self._get_resource_provider(uuid)
|
rp = self._get_resource_provider(uuid)
|
||||||
if rp is None:
|
if rp is None:
|
||||||
rp = self._create_resource_provider(uuid, name or uuid)
|
name = name or uuid
|
||||||
|
rp = self._create_resource_provider(uuid, name)
|
||||||
|
if rp is None:
|
||||||
|
return
|
||||||
msg = "Grabbing aggregate associations for resource provider %s"
|
msg = "Grabbing aggregate associations for resource provider %s"
|
||||||
LOG.debug(msg, uuid)
|
LOG.debug(msg, uuid)
|
||||||
aggs = self._get_provider_aggregates(uuid)
|
aggs = self._get_provider_aggregates(uuid)
|
||||||
|
|
|
@ -22,7 +22,7 @@
|
||||||
"host_ip": "%(ip)s",
|
"host_ip": "%(ip)s",
|
||||||
"free_disk_gb": 1028,
|
"free_disk_gb": 1028,
|
||||||
"free_ram_mb": 7680,
|
"free_ram_mb": 7680,
|
||||||
"hypervisor_hostname": "host1",
|
"hypervisor_hostname": "fake-mini",
|
||||||
"hypervisor_type": "fake",
|
"hypervisor_type": "fake",
|
||||||
"hypervisor_version": 1000,
|
"hypervisor_version": 1000,
|
||||||
"id": %(hypervisor_id)s,
|
"id": %(hypervisor_id)s,
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
{
|
{
|
||||||
"hypervisors": [
|
"hypervisors": [
|
||||||
{
|
{
|
||||||
"hypervisor_hostname": "host1",
|
"hypervisor_hostname": "fake-mini",
|
||||||
"id": 2,
|
"id": 2,
|
||||||
"state": "up",
|
"state": "up",
|
||||||
"status": "enabled"
|
"status": "enabled"
|
||||||
|
|
|
@ -18,7 +18,6 @@ import mock
|
||||||
from nova.cells import utils as cells_utils
|
from nova.cells import utils as cells_utils
|
||||||
from nova import objects
|
from nova import objects
|
||||||
from nova.tests.functional.api_sample_tests import api_sample_base
|
from nova.tests.functional.api_sample_tests import api_sample_base
|
||||||
from nova.virt import fake
|
|
||||||
|
|
||||||
|
|
||||||
class HypervisorsSampleJsonTests(api_sample_base.ApiSampleTestBaseV21):
|
class HypervisorsSampleJsonTests(api_sample_base.ApiSampleTestBaseV21):
|
||||||
|
@ -156,10 +155,7 @@ class HypervisorsSampleJson233Tests(api_sample_base.ApiSampleTestBaseV21):
|
||||||
self.api.microversion = self.microversion
|
self.api.microversion = self.microversion
|
||||||
# Start a new compute service to fake a record with hypervisor id=2
|
# Start a new compute service to fake a record with hypervisor id=2
|
||||||
# for pagination test.
|
# for pagination test.
|
||||||
host = 'host1'
|
self.start_service('compute', host='host1')
|
||||||
fake.set_nodes([host])
|
|
||||||
self.addCleanup(fake.restore_nodes)
|
|
||||||
self.start_service('compute', host=host)
|
|
||||||
|
|
||||||
def test_hypervisors_list(self):
|
def test_hypervisors_list(self):
|
||||||
response = self._do_get('os-hypervisors?limit=1&marker=1')
|
response = self._do_get('os-hypervisors?limit=1&marker=1')
|
||||||
|
|
|
@ -238,19 +238,16 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
||||||
'_get_provider_aggregates')
|
'_get_provider_aggregates')
|
||||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||||
'_get_resource_provider')
|
'_get_resource_provider')
|
||||||
def test_ensure_resource_provider_create_fail(self, get_rp_mock,
|
def test_ensure_resource_provider_create_none(self, get_rp_mock,
|
||||||
get_agg_mock, create_rp_mock):
|
get_agg_mock, create_rp_mock):
|
||||||
# No resource provider exists in the client's cache, and
|
# No resource provider exists in the client's cache, and
|
||||||
# _create_provider raises, indicating there was an error with the
|
# _create_provider returns None, indicating there was an error with the
|
||||||
# create call. Ensure we don't populate the resource provider cache
|
# create call. Ensure we don't populate the resource provider cache
|
||||||
# with a None value.
|
# with a None value.
|
||||||
get_rp_mock.return_value = None
|
get_rp_mock.return_value = None
|
||||||
create_rp_mock.side_effect = exception.ResourceProviderCreationFailed(
|
create_rp_mock.return_value = None
|
||||||
name=uuids.compute_node)
|
|
||||||
|
|
||||||
self.assertRaises(
|
self.client._ensure_resource_provider(uuids.compute_node)
|
||||||
exception.ResourceProviderCreationFailed,
|
|
||||||
self.client._ensure_resource_provider, uuids.compute_node)
|
|
||||||
|
|
||||||
get_rp_mock.assert_called_once_with(uuids.compute_node)
|
get_rp_mock.assert_called_once_with(uuids.compute_node)
|
||||||
create_rp_mock.assert_called_once_with(uuids.compute_node,
|
create_rp_mock.assert_called_once_with(uuids.compute_node,
|
||||||
|
@ -394,9 +391,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
||||||
self.ks_sess_mock.get.return_value = resp_mock
|
self.ks_sess_mock.get.return_value = resp_mock
|
||||||
|
|
||||||
uuid = uuids.compute_node
|
uuid = uuids.compute_node
|
||||||
self.assertRaises(
|
result = self.client._get_resource_provider(uuid)
|
||||||
exception.ResourceProviderRetrievalFailed,
|
|
||||||
self.client._get_resource_provider, uuid)
|
|
||||||
|
|
||||||
expected_url = '/resource_providers/' + uuid
|
expected_url = '/resource_providers/' + uuid
|
||||||
self.ks_sess_mock.get.assert_called_once_with(expected_url,
|
self.ks_sess_mock.get.assert_called_once_with(expected_url,
|
||||||
|
@ -405,6 +400,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
||||||
# A 503 Service Unavailable should trigger an error logged and
|
# A 503 Service Unavailable should trigger an error logged and
|
||||||
# return None from _get_resource_provider()
|
# return None from _get_resource_provider()
|
||||||
self.assertTrue(logging_mock.called)
|
self.assertTrue(logging_mock.called)
|
||||||
|
self.assertIsNone(result)
|
||||||
|
|
||||||
def test_create_resource_provider(self):
|
def test_create_resource_provider(self):
|
||||||
# Ensure _create_resource_provider() returns a ResourceProvider object
|
# Ensure _create_resource_provider() returns a ResourceProvider object
|
||||||
|
@ -445,9 +441,8 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
||||||
# record.
|
# record.
|
||||||
uuid = uuids.compute_node
|
uuid = uuids.compute_node
|
||||||
name = 'computehost'
|
name = 'computehost'
|
||||||
self.ks_sess_mock.post.return_value = mock.Mock(
|
resp_mock = mock.Mock(status_code=409)
|
||||||
status_code=409,
|
self.ks_sess_mock.post.return_value = resp_mock
|
||||||
text='not a name conflict')
|
|
||||||
|
|
||||||
get_rp_mock.return_value = mock.sentinel.get_rp
|
get_rp_mock.return_value = mock.sentinel.get_rp
|
||||||
|
|
||||||
|
@ -465,18 +460,6 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
||||||
raise_exc=False)
|
raise_exc=False)
|
||||||
self.assertEqual(mock.sentinel.get_rp, result)
|
self.assertEqual(mock.sentinel.get_rp, result)
|
||||||
|
|
||||||
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')
|
@mock.patch.object(report.LOG, 'error')
|
||||||
def test_create_resource_provider_error(self, logging_mock):
|
def test_create_resource_provider_error(self, logging_mock):
|
||||||
# Ensure _create_resource_provider() sets the error flag when trying to
|
# Ensure _create_resource_provider() sets the error flag when trying to
|
||||||
|
@ -487,9 +470,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
||||||
resp_mock = mock.Mock(status_code=503)
|
resp_mock = mock.Mock(status_code=503)
|
||||||
self.ks_sess_mock.post.return_value = resp_mock
|
self.ks_sess_mock.post.return_value = resp_mock
|
||||||
|
|
||||||
self.assertRaises(
|
result = self.client._create_resource_provider(uuid, name)
|
||||||
exception.ResourceProviderCreationFailed,
|
|
||||||
self.client._create_resource_provider, uuid, name)
|
|
||||||
|
|
||||||
expected_payload = {
|
expected_payload = {
|
||||||
'uuid': uuid,
|
'uuid': uuid,
|
||||||
|
@ -504,6 +485,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
||||||
# A 503 Service Unavailable should log an error and
|
# A 503 Service Unavailable should log an error and
|
||||||
# _create_resource_provider() should return None
|
# _create_resource_provider() should return None
|
||||||
self.assertTrue(logging_mock.called)
|
self.assertTrue(logging_mock.called)
|
||||||
|
self.assertFalse(result)
|
||||||
|
|
||||||
|
|
||||||
class TestAggregates(SchedulerReportClientTestCase):
|
class TestAggregates(SchedulerReportClientTestCase):
|
||||||
|
|
Loading…
Reference in New Issue