From 09db0ba7cc275e0282226ddb9a6a259a2b179316 Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Fri, 9 Sep 2016 10:22:15 -0400 Subject: [PATCH] [placement] make PUT inventory consistent with GET The format of accepted request payload for PUT /resource-providers/{uuid}/inventories was different from the response payload describing inventories. It was using an anonymous dict approach instead of a dict keyed by resource class identifier. This patch changes the PUT implementation to accept a dict keyed by resource class identifier. Change-Id: I8770b36b11e10975e595f70084c9f06ee5b085a0 --- .../openstack/placement/handlers/inventory.py | 41 ++++++++++------ nova/scheduler/client/report.py | 49 ++++++++++--------- .../placement/gabbits/allocations.yaml | 8 +-- .../placement/gabbits/inventory.yaml | 10 ++-- .../unit/scheduler/client/test_report.py | 49 ++++++++++--------- 5 files changed, 87 insertions(+), 70 deletions(-) diff --git a/nova/api/openstack/placement/handlers/inventory.py b/nova/api/openstack/placement/handlers/inventory.py index 472f1d50c430..76941730eca5 100644 --- a/nova/api/openstack/placement/handlers/inventory.py +++ b/nova/api/openstack/placement/handlers/inventory.py @@ -22,7 +22,7 @@ from nova.api.openstack.placement import util from nova import exception from nova import objects - +RESOURCE_CLASS_IDENTIFIER = "^[A-Z0-9_]+$" BASE_INVENTORY_SCHEMA = { "type": "object", "properties": { @@ -57,10 +57,12 @@ BASE_INVENTORY_SCHEMA = { POST_INVENTORY_SCHEMA = copy.deepcopy(BASE_INVENTORY_SCHEMA) POST_INVENTORY_SCHEMA['properties']['resource_class'] = { "type": "string", - "pattern": "^[A-Z0-9_]+$" + "pattern": RESOURCE_CLASS_IDENTIFIER, } POST_INVENTORY_SCHEMA['required'].append('resource_class') POST_INVENTORY_SCHEMA['required'].remove('resource_provider_generation') +PUT_INVENTORY_RECORD_SCHEMA = copy.deepcopy(BASE_INVENTORY_SCHEMA) +PUT_INVENTORY_RECORD_SCHEMA['required'].remove('resource_provider_generation') PUT_INVENTORY_SCHEMA = { "type": "object", "properties": { @@ -68,8 +70,10 @@ PUT_INVENTORY_SCHEMA = { "type": "integer" }, "inventories": { - "type": "array", - "items": POST_INVENTORY_SCHEMA + "type": "object", + "patternProperties": { + RESOURCE_CLASS_IDENTIFIER: PUT_INVENTORY_RECORD_SCHEMA, + } } }, "required": [ @@ -130,17 +134,17 @@ def _extract_inventories(body, schema): """Extract and validate multiple inventories from JSON body.""" data = _extract_json(body, schema) - inventories = [] - for raw_inventory in data['inventories']: + inventories = {} + for res_class, raw_inventory in data['inventories'].items(): inventory_data = copy.copy(INVENTORY_DEFAULTS) inventory_data.update(raw_inventory) - inventories.append(inventory_data) + inventories[res_class] = inventory_data data['inventories'] = inventories return data -def _make_inventory_object(resource_provider, **data): +def _make_inventory_object(resource_provider, resource_class, **data): """Single place to catch malformed Inventories.""" # TODO(cdent): Some of the validation checks that are done here # could be done via JSONschema (using, for example, "minimum": @@ -148,11 +152,12 @@ def _make_inventory_object(resource_provider, **data): # duplication or decoupling so leaving it as this for now. try: inventory = objects.Inventory( - resource_provider=resource_provider, **data) + resource_provider=resource_provider, + resource_class=resource_class, **data) except (ValueError, TypeError) as exc: raise webob.exc.HTTPBadRequest( 'Bad inventory %s for resource provider %s: %s' - % (data['resource_class'], resource_provider.uuid, exc), + % (resource_class, resource_provider.uuid, exc), json_formatter=util.json_error_formatter) return inventory @@ -210,8 +215,11 @@ def create_inventory(req): resource_provider = objects.ResourceProvider.get_by_uuid( context, uuid) data = _extract_inventory(req.body, POST_INVENTORY_SCHEMA) + resource_class = data.pop('resource_class') - inventory = _make_inventory_object(resource_provider, **data) + inventory = _make_inventory_object(resource_provider, + resource_class, + **data) try: resource_provider.add_inventory(inventory) @@ -228,7 +236,7 @@ def create_inventory(req): response = req.response response.location = util.inventory_url( - req.environ, resource_provider, data['resource_class']) + req.environ, resource_provider, resource_class) return _send_inventory(response, resource_provider, inventory, status=201) @@ -335,9 +343,9 @@ def set_inventories(req): json_formatter=util.json_error_formatter) inv_list = [] - for inventory_data in data['inventories']: + for res_class, inventory_data in data['inventories'].items(): inventory = _make_inventory_object( - resource_provider, **inventory_data) + resource_provider, res_class, **inventory_data) inv_list.append(inventory) inventories = objects.InventoryList(objects=inv_list) @@ -383,8 +391,9 @@ def update_inventory(req): 'resource provider generation conflict', json_formatter=util.json_error_formatter) - data['resource_class'] = resource_class - inventory = _make_inventory_object(resource_provider, **data) + inventory = _make_inventory_object(resource_provider, + resource_class, + **data) try: resource_provider.update_inventory(inventory) diff --git a/nova/scheduler/client/report.py b/nova/scheduler/client/report.py index 739c43b288f6..4a13f7ec6c5e 100644 --- a/nova/scheduler/client/report.py +++ b/nova/scheduler/client/report.py @@ -207,29 +207,32 @@ class SchedulerReportClient(object): return rp def _compute_node_inventory(self, compute_node): - inventories = [ - {'resource_class': 'VCPU', - 'total': compute_node.vcpus, - 'reserved': 0, - 'min_unit': 1, - 'max_unit': 1, - 'step_size': 1, - 'allocation_ratio': compute_node.cpu_allocation_ratio}, - {'resource_class': 'MEMORY_MB', - 'total': compute_node.memory_mb, - 'reserved': CONF.reserved_host_memory_mb, - 'min_unit': 1, - 'max_unit': 1, - 'step_size': 1, - 'allocation_ratio': compute_node.ram_allocation_ratio}, - {'resource_class': 'DISK_GB', - 'total': compute_node.local_gb, - 'reserved': CONF.reserved_host_disk_mb * 1024, - 'min_unit': 1, - 'max_unit': 1, - 'step_size': 1, - 'allocation_ratio': compute_node.disk_allocation_ratio}, - ] + inventories = { + 'VCPU': { + 'total': compute_node.vcpus, + 'reserved': 0, + 'min_unit': 1, + 'max_unit': 1, + 'step_size': 1, + 'allocation_ratio': compute_node.cpu_allocation_ratio, + }, + 'MEMORY_MB': { + 'total': compute_node.memory_mb, + 'reserved': CONF.reserved_host_memory_mb, + 'min_unit': 1, + 'max_unit': 1, + 'step_size': 1, + 'allocation_ratio': compute_node.ram_allocation_ratio, + }, + 'DISK_GB': { + 'total': compute_node.local_gb, + 'reserved': CONF.reserved_host_disk_mb * 1024, + 'min_unit': 1, + 'max_unit': 1, + 'step_size': 1, + 'allocation_ratio': compute_node.disk_allocation_ratio, + }, + } generation = self._resource_providers[compute_node.uuid].generation data = { 'resource_provider_generation': generation, diff --git a/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml b/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml index 7ba6ef18d316..f5b0512408d0 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml @@ -266,9 +266,9 @@ tests: # of other changes resource_provider_generation: 0 inventories: - - resource_class: VCPU + VCPU: total: 32 - - resource_class: DISK_GB + DISK_GB: total: 10 - name: set inventory on rp2 @@ -280,9 +280,9 @@ tests: # of other changes resource_provider_generation: 0 inventories: - - resource_class: VCPU + VCPU: total: 16 - - resource_class: DISK_GB + DISK_GB: total: 20 status: 200 diff --git a/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml b/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml index e594c3f3705a..06937bb5b03b 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/inventory.yaml @@ -160,6 +160,8 @@ tests: resource_class: NO_CLASS_14 total: 2048 status: 400 + response_strings: + - Bad inventory NO_CLASS_14 for resource provider $ENVIRON['RP_UUID'] - name: post inventory duplicated resource class desc: DISK_GB was already created above @@ -258,9 +260,9 @@ tests: data: resource_provider_generation: 0 inventories: - - resource_class: IPV4_ADDRESS + IPV4_ADDRESS: total: 253 - - resource_class: DISK_GB + DISK_GB: total: 1024 status: 200 response_json_paths: @@ -287,7 +289,7 @@ tests: data: resource_provider_generation: 99 inventories: - - resource_class: IPV4_ADDRESS + IPV4_ADDRESS: total: 253 status: 409 response_strings: @@ -302,7 +304,7 @@ tests: data: resource_provider_generation: 6 inventories: - - resource_class: IPV4_ADDRESS + IPV4_ADDRESS: total: 253 reserved: 512 status: 400 diff --git a/nova/tests/unit/scheduler/client/test_report.py b/nova/tests/unit/scheduler/client/test_report.py index 1ea59c6c4d60..c33130e592e1 100644 --- a/nova/tests/unit/scheduler/client/test_report.py +++ b/nova/tests/unit/scheduler/client/test_report.py @@ -371,29 +371,32 @@ class SchedulerReportClientTestCase(test.NoDBTestCase): result = self.client._compute_node_inventory(compute_node) - expected_inventories = [ - {'resource_class': 'VCPU', - 'total': compute_node.vcpus, - 'reserved': 0, - 'min_unit': 1, - 'max_unit': 1, - 'step_size': 1, - 'allocation_ratio': compute_node.cpu_allocation_ratio}, - {'resource_class': 'MEMORY_MB', - 'total': compute_node.memory_mb, - 'reserved': CONF.reserved_host_memory_mb, - 'min_unit': 1, - 'max_unit': 1, - 'step_size': 1, - 'allocation_ratio': compute_node.ram_allocation_ratio}, - {'resource_class': 'DISK_GB', - 'total': compute_node.local_gb, - 'reserved': CONF.reserved_host_disk_mb * 1024, - 'min_unit': 1, - 'max_unit': 1, - 'step_size': 1, - 'allocation_ratio': compute_node.disk_allocation_ratio}, - ] + expected_inventories = { + 'VCPU': { + 'total': compute_node.vcpus, + 'reserved': 0, + 'min_unit': 1, + 'max_unit': 1, + 'step_size': 1, + 'allocation_ratio': compute_node.cpu_allocation_ratio, + }, + 'MEMORY_MB': { + 'total': compute_node.memory_mb, + 'reserved': CONF.reserved_host_memory_mb, + 'min_unit': 1, + 'max_unit': 1, + 'step_size': 1, + 'allocation_ratio': compute_node.ram_allocation_ratio, + }, + 'DISK_GB': { + 'total': compute_node.local_gb, + 'reserved': CONF.reserved_host_disk_mb * 1024, + 'min_unit': 1, + 'max_unit': 1, + 'step_size': 1, + 'allocation_ratio': compute_node.disk_allocation_ratio, + }, + } expected = { 'resource_provider_generation': rp.generation, 'inventories': expected_inventories,