From 7f79758dd994675a0b90c35ae035d0c182290b83 Mon Sep 17 00:00:00 2001 From: Vladyslav Drok Date: Fri, 27 Apr 2018 19:45:55 +0300 Subject: [PATCH] Placement: allow to set reserved value equal to total for inventory This is needed for ironic use case, when during cleaning, resources are reserved by ironic itself. Cyborg will also benefit from this during FPGA programming. blueprint: allow-reserved-equal-total-inventory Change-Id: I037d9b8c1bb554c3437434cc9a57ddb630dd62f0 --- nova/api/openstack/placement/exception.py | 6 ++ .../openstack/placement/handlers/inventory.py | 32 +++++++ nova/api/openstack/placement/microversion.py | 2 + .../placement/objects/resource_provider.py | 8 -- .../placement/rest_api_version_history.rst | 6 ++ .../placement/db/test_resource_provider.py | 27 ------ .../placement/gabbits/inventory.yaml | 85 +++++++++++++++++-- .../placement/gabbits/microversion.yaml | 4 +- ...qual-total-inventory-fe93584dd28c460d.yaml | 6 ++ 9 files changed, 134 insertions(+), 42 deletions(-) create mode 100644 releasenotes/notes/allow-reserved-equal-total-inventory-fe93584dd28c460d.yaml diff --git a/nova/api/openstack/placement/exception.py b/nova/api/openstack/placement/exception.py index d5a4461ef106..ae17d5470f67 100644 --- a/nova/api/openstack/placement/exception.py +++ b/nova/api/openstack/placement/exception.py @@ -99,6 +99,12 @@ class InvalidInventoryCapacity(InvalidInventory): "The reserved value is greater than or equal to total.") +class InvalidInventoryCapacityReservedCanBeTotal(InvalidInventoryCapacity): + msg_fmt = _("Invalid inventory for '%(resource_class)s' on " + "resource provider '%(resource_provider)s'. " + "The reserved value is greater than total.") + + # An exception with this name is used on both sides of the placement/ # nova interaction. class InventoryInUse(InvalidInventory): diff --git a/nova/api/openstack/placement/handlers/inventory.py b/nova/api/openstack/placement/handlers/inventory.py index 3fed8ac42ca6..4e6aea60b8bd 100644 --- a/nova/api/openstack/placement/handlers/inventory.py +++ b/nova/api/openstack/placement/handlers/inventory.py @@ -12,6 +12,7 @@ """Inventory handlers for Placement API.""" import copy +import operator from oslo_db import exception as db_exc from oslo_serialization import jsonutils @@ -147,6 +148,31 @@ def _serialize_inventories(inventories, generation): 'inventories': inventories_dict}, last_modified) +def _validate_inventory_capacity(version, inventories): + """Validate inventory capacity. + + :param version: request microversion. + :param inventories: Inventory or InventoryList to validate capacities of. + :raises: exception.InvalidInventoryCapacityReservedCanBeTotal if request + microversion is 1.26 or higher and any inventory has capacity < 0. + :raises: exception.InvalidInventoryCapacity if request + microversion is lower than 1.26 and any inventory has capacity <= 0. + """ + if not version.matches((1, 26)): + op = operator.le + exc_class = exception.InvalidInventoryCapacity + else: + op = operator.lt + exc_class = exception.InvalidInventoryCapacityReservedCanBeTotal + if isinstance(inventories, rp_obj.Inventory): + inventories = rp_obj.InventoryList(objects=[inventories]) + for inventory in inventories: + if op(inventory.capacity, 0): + raise exc_class( + resource_class=inventory.resource_class, + resource_provider=inventory.resource_provider.uuid) + + @wsgi_wrapper.PlacementWsgify @util.require_content('application/json') def create_inventory(req): @@ -168,6 +194,8 @@ def create_inventory(req): **data) try: + _validate_inventory_capacity( + req.environ[microversion.MICROVERSION_ENVIRON], inventory) resource_provider.add_inventory(inventory) except (exception.ConcurrentUpdateDetected, db_exc.DBDuplicateEntry) as exc: @@ -305,6 +333,8 @@ def set_inventories(req): inventories = rp_obj.InventoryList(objects=inv_list) try: + _validate_inventory_capacity( + req.environ[microversion.MICROVERSION_ENVIRON], inventories) resource_provider.set_inventory(inventories) except exception.ResourceClassNotFound as exc: raise webob.exc.HTTPBadRequest( @@ -401,6 +431,8 @@ def update_inventory(req): **data) try: + _validate_inventory_capacity( + req.environ[microversion.MICROVERSION_ENVIRON], inventory) resource_provider.update_inventory(inventory) except (exception.ConcurrentUpdateDetected, db_exc.DBDuplicateEntry) as exc: diff --git a/nova/api/openstack/placement/microversion.py b/nova/api/openstack/placement/microversion.py index 5d2f588a58f3..e34ce0b7b4ce 100644 --- a/nova/api/openstack/placement/microversion.py +++ b/nova/api/openstack/placement/microversion.py @@ -68,6 +68,8 @@ VERSIONS = [ # GET /resource_providers '1.25', # Adds support for granular resource requests via numbered # querystring groups in GET /allocation_candidates + '1.26', # Add ability to specify inventory with reserved value equal to + # total. ] diff --git a/nova/api/openstack/placement/objects/resource_provider.py b/nova/api/openstack/placement/objects/resource_provider.py index 784a6de242de..8cbb1587bfd6 100644 --- a/nova/api/openstack/placement/objects/resource_provider.py +++ b/nova/api/openstack/placement/objects/resource_provider.py @@ -201,10 +201,6 @@ def _add_inventory_to_provider(ctx, rp, inv_list, to_add): for rc_id in to_add: rc_str = _RC_CACHE.string_from_id(rc_id) inv_record = inv_list.find(rc_str) - if inv_record.capacity <= 0: - raise exception.InvalidInventoryCapacity( - resource_class=rc_str, - resource_provider=rp.uuid) ins_stmt = _INV_TBL.insert().values( resource_provider_id=rp.id, resource_class_id=rc_id, @@ -232,10 +228,6 @@ def _update_inventory_for_provider(ctx, rp, inv_list, to_update): for rc_id in to_update: rc_str = _RC_CACHE.string_from_id(rc_id) inv_record = inv_list.find(rc_str) - if inv_record.capacity <= 0: - raise exception.InvalidInventoryCapacity( - resource_class=rc_str, - resource_provider=rp.uuid) allocation_query = sa.select( [func.sum(_ALLOC_TBL.c.used).label('usage')]).\ where(sa.and_( diff --git a/nova/api/openstack/placement/rest_api_version_history.rst b/nova/api/openstack/placement/rest_api_version_history.rst index 16bde5befbb6..2673b8a9f20f 100644 --- a/nova/api/openstack/placement/rest_api_version_history.rst +++ b/nova/api/openstack/placement/rest_api_version_history.rst @@ -323,3 +323,9 @@ The semantic of the (unnumbered) ``resources``, ``required``, and ``member_of`` query parameters is unchanged: the resources, traits, and aggregate associations specified thereby may be satisfied by any provider in the same non-sharing tree or associated via the specified aggregate(s). + +1.26 Allow inventories to have reserved value equal to total +------------------------------------------------------------ + +Starting with this version, it is allowed to set the reserved value of the +resource provider inventory to be equal to total. diff --git a/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py b/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py index b49a3736c0cc..feba1e037b5c 100644 --- a/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py +++ b/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py @@ -584,22 +584,6 @@ class ResourceProviderTestCase(tb.PlacementDbBaseTestCase): self.assertEqual(1, len(new_inv_list)) self.assertEqual(2048, new_inv_list[0].total) - # fail when inventory bad - disk_inv = rp_obj.Inventory( - resource_provider=rp, - resource_class=fields.ResourceClass.DISK_GB, - total=2048, - reserved=2048) - disk_inv.obj_set_defaults() - error = self.assertRaises(exception.InvalidInventoryCapacity, - rp.update_inventory, disk_inv) - self.assertIn("Invalid inventory for '%s'" - % fields.ResourceClass.DISK_GB, str(error)) - self.assertIn("on resource provider '%s'." % rp.uuid, str(error)) - - # generation has not bumped - self.assertEqual(saved_generation, rp.generation) - # delete inventory rp.delete_inventory(fields.ResourceClass.DISK_GB) @@ -693,17 +677,6 @@ class ResourceProviderTestCase(tb.PlacementDbBaseTestCase): mock_log.warning.assert_called_once_with( mock.ANY, {'uuid': rp.uuid, 'resource': 'DISK_GB'}) - def test_add_invalid_inventory(self): - rp = self._create_provider(uuidsentinel.rp_name) - error = self.assertRaises( - exception.InvalidInventoryCapacity, - tb.add_inventory, rp, fields.ResourceClass.DISK_GB, 1024, - reserved=2048) - self.assertIn("Invalid inventory for '%s'" - % fields.ResourceClass.DISK_GB, str(error)) - self.assertIn("on resource provider '%s'." - % rp.uuid, str(error)) - def test_add_allocation_increments_generation(self): rp = self._create_provider(name='foo') tb.add_inventory(rp, DISK_INVENTORY['resource_class'], diff --git a/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml b/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml index 5ab3a8390113..732f8a32ca79 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml @@ -649,9 +649,84 @@ tests: status: 400 response_strings: - Unable to update inventory + - greater than or equal to total response_json_paths: $.errors[0].title: Bad Request +- name: put all inventory zero capacity old microversion + PUT: $LAST_URL + request_headers: + content-type: application/json + data: + resource_provider_generation: 6 + inventories: + IPV4_ADDRESS: + total: 253 + reserved: 253 + status: 400 + response_strings: + - Unable to update inventory + - greater than or equal to total + response_json_paths: + $.errors[0].title: Bad Request + +- name: put inventory with reserved equal to total + PUT: $LAST_URL + request_headers: + content-type: application/json + openstack-api-version: placement 1.26 + data: + resource_provider_generation: 6 + inventories: + IPV4_ADDRESS: + total: 253 + reserved: 253 + status: 200 + +- name: put all inventory bad capacity in new microversion + PUT: $LAST_URL + request_headers: + content-type: application/json + openstack-api-version: placement 1.26 + data: + resource_provider_generation: 7 + inventories: + IPV4_ADDRESS: + total: 253 + reserved: 512 + status: 400 + response_strings: + - Unable to update inventory + - greater than total + response_json_paths: + $.errors[0].title: Bad Request + +- name: put one inventory zero capacity old microversion + PUT: /resource_providers/$ENVIRON['RP_UUID']/inventories/IPV4_ADDRESS + request_headers: + content-type: application/json + data: + resource_provider_generation: 7 + total: 253 + reserved: 253 + status: 400 + response_strings: + - Unable to update inventory + - greater than or equal to total + response_json_paths: + $.errors[0].title: Bad Request + +- name: put one inventory with reserved equal to total new microversion + PUT: $LAST_URL + request_headers: + content-type: application/json + openstack-api-version: placement 1.26 + data: + resource_provider_generation: 7 + total: 512 + reserved: 512 + status: 200 + - name: delete all inventory bad generation PUT: /resource_providers/$ENVIRON['RP_UUID']/inventories request_headers: @@ -680,7 +755,7 @@ tests: - name: get inventories after deletions GET: /resource_providers/$ENVIRON['RP_UUID']/inventories response_json_paths: - $.resource_provider_generation: 8 + $.resource_provider_generation: 10 $.inventories: {} - name: post an inventory again @@ -699,7 +774,7 @@ tests: response_headers: location: $SCHEME://$NETLOC/resource_providers/$ENVIRON['RP_UUID']/inventories/DISK_GB response_json_paths: - $.resource_provider_generation: 9 + $.resource_provider_generation: 11 $.total: 2048 $.reserved: 512 @@ -709,17 +784,17 @@ tests: content-type: application/json openstack-api-version: placement 1.4 data: - resource_provider_generation: 9 + resource_provider_generation: 11 inventories: {} response_json_paths: - $.resource_provider_generation: 10 + $.resource_provider_generation: 12 $.inventories: {} status: 200 - name: get generation after deletion GET: /resource_providers/$ENVIRON['RP_UUID']/inventories response_json_paths: - $.resource_provider_generation: 10 + $.resource_provider_generation: 12 $.inventories: {} - name: delete inventories earlier version diff --git a/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml b/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml index 7970eb9a7ce4..1ffd631f5cda 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml @@ -39,13 +39,13 @@ tests: response_json_paths: $.errors[0].title: Not Acceptable -- name: latest microversion is 1.25 +- name: latest microversion is 1.26 GET: / request_headers: openstack-api-version: placement latest response_headers: vary: /openstack-api-version/ - openstack-api-version: placement 1.25 + openstack-api-version: placement 1.26 - name: other accept header bad version GET: / diff --git a/releasenotes/notes/allow-reserved-equal-total-inventory-fe93584dd28c460d.yaml b/releasenotes/notes/allow-reserved-equal-total-inventory-fe93584dd28c460d.yaml new file mode 100644 index 000000000000..ecbeee8d4d40 --- /dev/null +++ b/releasenotes/notes/allow-reserved-equal-total-inventory-fe93584dd28c460d.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Introduces new placement API version ``1.26``. Starting with this version + it is allowed to define resource provider inventories with reserved value + equal to total.