diff --git a/nova/api/openstack/placement/handlers/inventory.py b/nova/api/openstack/placement/handlers/inventory.py index 2a445cc05b..814475e50a 100644 --- a/nova/api/openstack/placement/handlers/inventory.py +++ b/nova/api/openstack/placement/handlers/inventory.py @@ -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 ' diff --git a/nova/db/sqlalchemy/resource_class_cache.py b/nova/db/sqlalchemy/resource_class_cache.py index 97304311fe..e1ce13da68 100644 --- a/nova/db/sqlalchemy/resource_class_cache.py +++ b/nova/db/sqlalchemy/resource_class_cache.py @@ -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) diff --git a/nova/exception.py b/nova/exception.py index c9e5212055..e56d2ad6ee 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -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.") diff --git a/nova/objects/resource_provider.py b/nova/objects/resource_provider.py index 7d0f460907..778f0910c7 100644 --- a/nova/objects/resource_provider.py +++ b/nova/objects/resource_provider.py @@ -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 diff --git a/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml b/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml index 13e392d75e..cf769a683e 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml @@ -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 diff --git a/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml b/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml index 358474dc3e..237a22a8aa 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml @@ -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 diff --git a/nova/tests/functional/db/test_resource_class_cache.py b/nova/tests/functional/db/test_resource_class_cache.py index aae9984d17..5c9858a266 100644 --- a/nova/tests/functional/db/test_resource_class_cache.py +++ b/nova/tests/functional/db/test_resource_class_cache.py @@ -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') diff --git a/nova/tests/functional/db/test_resource_provider.py b/nova/tests/functional/db/test_resource_provider.py index 5215df3d67..614d6f8223 100644 --- a/nova/tests/functional/db/test_resource_provider.py +++ b/nova/tests/functional/db/test_resource_provider.py @@ -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')