openstack
/
nova
Code Issues Proposed changes

placement: raise exc when resource class not found 

The ResourceProvider.set_inventory() method was not raising NotFound
when an Inventory object in the supplied InventoryList object was using
a resource class that did not exist in the resource class cache. This
patch fixes that by having the string_from_id() and id_from_string()
methods of the resource class cache raise ResourceClassNotFound instead
of returning None. We raise specific ResourceClassNotFound and
InventoryWithResourceClassNotFound exception subclasses to differentiate
in the inventory API handler between an unknown resource class and a
known resource class that doesn't exist in a given resource provider's
inventory.

We return a 400 Not Found when InventoryWithResourceClassNotFound is
raised on updating a single inventory, but return a 409 Conflict when
InventoryWithResourceClassNotFound is raised on updating a set of
inventory records since in the latter scenario, it represents a race
condition that frankly should not occur.

Change-Id: I350b02dcdbaa9d30d885cd085f60daa6b53a7745
This commit is contained in:
Jay Pipes 2016-10-23 10:58:25 +02:00
parent 419cd6a8bc
commit 69bac2f99c
8 changed files with 135 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
nova/api/openstack/placement/handlers/inventory.py
View File

@ -340,6 +340,19 @@ def set_inventories(req):
try:
resource_provider.set_inventory(inventories)
except exception.ResourceClassNotFound as exc:
raise webob.exc.HTTPBadRequest(
_('Unknown resource class in inventory for resource provider '
'%(rp_uuid)s: %(error)s') % {'rp_uuid': resource_provider.uuid,
'error': exc},
json_formatter=util.json_error_formatter)
except exception.InventoryWithResourceClassNotFound as exc:
raise webob.exc.HTTPConflict(
_('Race condition detected when setting inventory. No inventory '
'record with resource class for resource provider '
'%(rp_uuid)s: %(error)s') % {'rp_uuid': resource_provider.uuid,
'error': exc},
json_formatter=util.json_error_formatter)
except (exception.ConcurrentUpdateDetected,
exception.InventoryInUse,
db_exc.DBDuplicateEntry) as exc:
@ -392,6 +405,12 @@ def update_inventory(req):
raise webob.exc.HTTPConflict(
_('update conflict: %(error)s') % {'error': exc},
json_formatter=util.json_error_formatter)
except exception.InventoryWithResourceClassNotFound as exc:
raise webob.exc.HTTPBadRequest(
_('No inventory record with resource class for resource provider '
'%(rp_uuid)s: %(error)s') % {'rp_uuid': resource_provider.uuid,
'error': exc},
json_formatter=util.json_error_formatter)
except exception.InvalidInventoryCapacity as exc:
raise webob.exc.HTTPBadRequest(
_('Unable to update inventory for resource provider '

13
nova/db/sqlalchemy/resource_class_cache.py
View File

@ -15,6 +15,7 @@ import sqlalchemy as sa
from nova.db.sqlalchemy import api as db_api
from nova.db.sqlalchemy import api_models as models
from nova import exception
from nova.objects import fields
_RC_TBL = models.ResourceClass.__table__
@ -78,6 +79,8 @@ class ResourceClassCache(object):
:returns integer identifier for the resource class, or None, if no such
resource class was found in the list of standard resource
classes or the resource_classes database table.
:raises `exception.ResourceClassNotFound` if rc_str cannot be found in
either the standard classes or the DB.
"""
if rc_str in self.id_cache:
return self.id_cache[rc_str]
@ -88,7 +91,9 @@ class ResourceClassCache(object):
else:
# Otherwise, check the database table
_refresh_from_db(self.ctx, self)
return self.id_cache.get(rc_str)
if rc_str in self.id_cache:
return self.id_cache[rc_str]
raise exception.ResourceClassNotFound(resource_class=rc_str)
def string_from_id(self, rc_id):
"""The reverse of the id_from_string() method. Given a supplied numeric
@ -102,6 +107,8 @@ class ResourceClassCache(object):
:returns string identifier for the resource class, or None, if no such
resource class was found in the list of standard resource
classes or the resource_classes database table.
:raises `exception.ResourceClassNotFound` if rc_id cannot be found in
either the standard classes or the DB.
"""
if rc_id in self.str_cache:
return self.str_cache[rc_id]
@ -112,4 +119,6 @@ class ResourceClassCache(object):
except IndexError:
# Otherwise, check the database table
_refresh_from_db(self.ctx, self)
return self.str_cache.get(rc_id)
if rc_id in self.str_cache:
return self.str_cache[rc_id]
raise exception.ResourceClassNotFound(resource_class=rc_id)

8
nova/exception.py
View File

@ -2114,10 +2114,18 @@ class ConcurrentUpdateDetected(NovaException):
"Please retry your update")
class ResourceClassNotFound(NotFound):
msg_fmt = _("No such resource class %(resource_class)s.")
class ResourceProviderInUse(NovaException):
msg_fmt = _("Resource provider has allocations.")
class InventoryWithResourceClassNotFound(NotFound):
msg_fmt = _("No inventory of class %(resource_class)s found.")
class InvalidInventory(Invalid):
msg_fmt = _("Inventory for '%(resource_class)s' on "
"resource provider '%(resource_provider)s' invalid.")

42
nova/objects/resource_provider.py
View File

@ -159,8 +159,8 @@ def _update_inventory_for_provider(conn, rp, inv_list, to_update):
allocation_ratio=inv_record.allocation_ratio)
res = conn.execute(upd_stmt)
if not res.rowcount:
raise exception.NotFound(
'No inventory of class %s found for update' % rc_str)
raise exception.InventoryWithResourceClassNotFound(
resource_class=rc_str)
return exceeded
@ -191,12 +191,13 @@ def _increment_provider_generation(conn, rp):
@db_api.api_context_manager.writer
def _add_inventory(context, rp, inventory):
"""Add one Inventory that wasn't already on the provider."""
"""Add one Inventory that wasn't already on the provider.
:raises `exception.ResourceClassNotFound` if inventory.resource_class
cannot be found in either the standard classes or the DB.
"""
_ensure_rc_cache(context)
rc_id = _RC_CACHE.id_from_string(inventory.resource_class)
if rc_id is None:
raise exception.NotFound("No such resource class '%s'" %
inventory.resource_class)
inv_list = InventoryList(objects=[inventory])
conn = context.session.connection()
with conn.begin():
@ -207,12 +208,13 @@ def _add_inventory(context, rp, inventory):
@db_api.api_context_manager.writer
def _update_inventory(context, rp, inventory):
"""Update an inventory already on the provider."""
"""Update an inventory already on the provider.
:raises `exception.ResourceClassNotFound` if inventory.resource_class
cannot be found in either the standard classes or the DB.
"""
_ensure_rc_cache(context)
rc_id = _RC_CACHE.id_from_string(inventory.resource_class)
if rc_id is None:
raise exception.NotFound("No such resource class '%s'" %
inventory.resource_class)
inv_list = InventoryList(objects=[inventory])
conn = context.session.connection()
with conn.begin():
@ -224,7 +226,11 @@ def _update_inventory(context, rp, inventory):
@db_api.api_context_manager.writer
def _delete_inventory(context, rp, resource_class):
"""Delete up to one Inventory of the given resource_class string."""
"""Delete up to one Inventory of the given resource_class string.
:raises `exception.ResourceClassNotFound` if resource_class
cannot be found in either the standard classes or the DB.
"""
_ensure_rc_cache(context)
conn = context.session.connection()
rc_id = _RC_CACHE.id_from_string(resource_class)
@ -251,6 +257,9 @@ def _set_inventory(context, rp, inv_list):
the same resource provider's view of its inventory or allocations
in between the time when this object was originally read
and the call to set the inventory.
:raises `exception.ResourceClassNotFound` if any resource class in any
inventory in inv_list cannot be found in either the standard
classes or the DB.
"""
_ensure_rc_cache(context)
conn = context.session.connection()
@ -853,16 +862,19 @@ class AllocationList(base.ObjectListBase, base.NovaObject):
We must check that there is capacity for each allocation.
If there is not we roll back the entire set.
:raises `exception.ResourceClassNotFound` if any resource class in any
allocation in allocs cannot be found in either the standard
classes or the DB.
"""
_ensure_rc_cache(context)
conn = context.session.connection()
# Short-circuit out if there are any allocations with string
# resource class names that don't exist.
# resource class names that don't exist this will raise a
# ResourceClassNotFound exception.
for alloc in allocs:
if _RC_CACHE.id_from_string(alloc.resource_class) is None:
raise exception.NotFound("No such resource class '%s'" %
alloc.resource_class)
_RC_CACHE.id_from_string(alloc.resource_class)
# Before writing any allocation records, we check that the submitted
# allocations do not cause any inventory capacity to be exceeded for

2
nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml
View File

@ -144,7 +144,7 @@ tests:
COWS: 12
status: 400
response_strings:
- No such resource class 'COWS'
- No such resource class COWS
- name: delete allocation
DELETE: /allocations/599ffd2d-526a-4b2e-8683-f13ad25f9958

26
nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml
View File

@ -107,6 +107,17 @@ tests:
response_strings:
- resource provider generation conflict
- name: modify inventory no such resource class in inventory
PUT: /resource_providers/$ENVIRON['RP_UUID']/inventories/MEMORY_MB
request_headers:
content-type: application/json
data:
resource_provider_generation: 2
total: 2048
status: 400
response_strings:
- No inventory record with resource class
- name: modify inventory invalid data
desc: This should 400 because reserved is greater than total
PUT: $LAST_URL
@ -163,7 +174,7 @@ tests:
total: 2048
status: 400
response_strings:
- No such resource class 'NO_CLASS_14'
- No such resource class NO_CLASS_14
- name: post inventory duplicated resource class
desc: DISK_GB was already created above
@ -302,6 +313,19 @@ tests:
response_strings:
- resource provider generation conflict
- name: put all inventory unknown resource class
PUT: /resource_providers/$ENVIRON['RP_UUID']/inventories
request_headers:
content-type: application/json
data:
resource_provider_generation: 6
inventories:
HOUSE:
total: 253
status: 400
response_strings:
- Unknown resource class in inventory
# NOTE(cdent): The generation is 6 now, based on the activity at
# the start of this file.
- name: put all inventory bad capacity

20
nova/tests/functional/db/test_resource_class_cache.py
View File

@ -13,6 +13,7 @@
import mock
from nova.db.sqlalchemy import resource_class_cache as rc_cache
from nova import exception
from nova import test
from nova.tests import fixtures
@ -49,9 +50,12 @@ class TestResourceClassCache(test.TestCase):
"""
cache = rc_cache.ResourceClassCache(self.context)
# Haven't added anything to the DB yet, so should return None
self.assertIsNone(cache.string_from_id(1001))
self.assertIsNone(cache.id_from_string("IRON_NFV"))
# Haven't added anything to the DB yet, so should raise
# ResourceClassNotFound
self.assertRaises(exception.ResourceClassNotFound,
cache.string_from_id, 1001)
self.assertRaises(exception.ResourceClassNotFound,
cache.id_from_string, "IRON_NFV")
# Now add to the database and verify appropriate results...
with self.context.session.connection() as conn:
@ -69,3 +73,13 @@ class TestResourceClassCache(test.TestCase):
self.assertEqual('IRON_NFV', cache.string_from_id(1001))
self.assertEqual(1001, cache.id_from_string('IRON_NFV'))
self.assertFalse(sel_mock.called)
def test_rc_cache_miss(self):
"""Test that we raise ResourceClassNotFound if an unknown resource
class ID or string is searched for.
"""
cache = rc_cache.ResourceClassCache(self.context)
self.assertRaises(exception.ResourceClassNotFound,
cache.string_from_id, 99999999)
self.assertRaises(exception.ResourceClassNotFound,
cache.id_from_string, 'UNKNOWN')

28
nova/tests/functional/db/test_resource_provider.py
View File

@ -200,6 +200,32 @@ class ResourceProviderTestCase(ResourceProviderBaseCase):
self.context, resource_provider.uuid))
self.assertEqual(33, reloaded_inventories[0].total)
def test_set_inventory_unknown_resource_class(self):
"""Test attempting to set inventory to an unknown resource class raises
an exception.
"""
rp = objects.ResourceProvider(
context=self.context,
uuid=uuidsentinel.rp_uuid,
name='compute-host',
)
rp.create()
inv = objects.Inventory(
resource_provider=rp,
resource_class='UNKNOWN',
total=1024,
reserved=15,
min_unit=10,
max_unit=100,
step_size=10,
allocation_ratio=1.0,
)
inv_list = objects.InventoryList(objects=[inv])
self.assertRaises(exception.ResourceClassNotFound,
rp.set_inventory, inv_list)
@mock.patch('nova.objects.resource_provider.LOG')
def test_set_inventory_over_capacity(self, mock_log):
rp = objects.ResourceProvider(context=self.context,
@ -419,7 +445,7 @@ class ResourceProviderTestCase(ResourceProviderBaseCase):
disk_inv.obj_set_defaults()
error = self.assertRaises(exception.NotFound, rp.update_inventory,
disk_inv)
self.assertIn('No inventory of class DISK_GB found for update',
self.assertIn('No inventory of class DISK_GB found',
str(error))
@mock.patch('nova.objects.resource_provider.LOG')