From f87af91d90f8f1322e92f9bc4d198a83162bb118 Mon Sep 17 00:00:00 2001 From: Pushkar Umaranikar Date: Mon, 17 Oct 2016 20:42:43 +0000 Subject: [PATCH] Placement api: 404 response do not indicate what was not found Add informative messages to the 404 error response from nova/objects/resource_provider.py For the method _update_inventory_in_db() from resource_provider.py, gabbi test validation is not added since it is not being used and from the placement API you create an inventory but then call set_inventory or add_inventory on a resource_provider. Similar change, i.e. adding informative messages to 404 error can be done for api tests from inventory.yaml and usage.yaml It includes adding informative logging in code for 404 exceptions and adding gabbi tests for those. This can be done in follow up patch. Change-Id: If53f84ac5f7521e9926b97bdcce3cf77ec5b4ffd Closes-Bug: #1634115 --- .../openstack/placement/handlers/allocation.py | 15 +++++++++++++-- .../placement/handlers/resource_provider.py | 7 +++++-- nova/objects/resource_provider.py | 4 +++- .../openstack/placement/gabbits/allocations.yaml | 2 ++ .../placement/gabbits/resource-provider.yaml | 12 ++++++++++++ 5 files changed, 35 insertions(+), 5 deletions(-) diff --git a/nova/api/openstack/placement/handlers/allocation.py b/nova/api/openstack/placement/handlers/allocation.py index d7e5bc4ea200..474677651ee2 100644 --- a/nova/api/openstack/placement/handlers/allocation.py +++ b/nova/api/openstack/placement/handlers/allocation.py @@ -286,12 +286,23 @@ def delete_allocations(req): allocations = objects.AllocationList.get_all_by_consumer_id( context, consumer_uuid) - if not allocations: + if allocations: + try: + allocations.delete_all() + # NOTE(pumaranikar): Following NotFound exception added in the case + # when allocation is deleted from allocations list by some other + # activity. In that case, delete_all() will throw a NotFound exception. + except exception.NotFound as exc: + raise webob.exc.HTPPNotFound( + _("Allocation for consumer with id %(id)s not found." + "error: %(error)s") % + {'id': consumer_uuid, 'error': exc}, + json_formatter=util.json_error_formatter) + else: raise webob.exc.HTTPNotFound( _("No allocations for consumer '%(consumer_uuid)s'") % {'consumer_uuid': consumer_uuid}, json_formatter=util.json_error_formatter) - allocations.delete_all() LOG.debug("Successfully deleted allocations %s", allocations) req.response.status = 204 diff --git a/nova/api/openstack/placement/handlers/resource_provider.py b/nova/api/openstack/placement/handlers/resource_provider.py index b24fb9a06341..271c8f57a333 100644 --- a/nova/api/openstack/placement/handlers/resource_provider.py +++ b/nova/api/openstack/placement/handlers/resource_provider.py @@ -114,15 +114,18 @@ def delete_resource_provider(req): uuid = util.wsgi_path_item(req.environ, 'uuid') context = req.environ['placement.context'] # The containing application will catch a not found here. - resource_provider = objects.ResourceProvider.get_by_uuid( - context, uuid) try: + resource_provider = objects.ResourceProvider.get_by_uuid( + context, uuid) resource_provider.destroy() except exception.ResourceProviderInUse as exc: raise webob.exc.HTTPConflict( _('Unable to delete resource provider %(rp_uuid)s: %(error)s') % {'rp_uuid': uuid, 'error': exc}, json_formatter=util.json_error_formatter) + except exception.NotFound as exc: + raise webob.exc.HTTPNotFound( + _("No resource provider with uuid %s found for delete") % uuid) req.response.status = 204 req.response.content_type = None return req.response diff --git a/nova/objects/resource_provider.py b/nova/objects/resource_provider.py index 9ba4d1906dd2..640e70c9d9de 100644 --- a/nova/objects/resource_provider.py +++ b/nova/objects/resource_provider.py @@ -396,7 +396,9 @@ class ResourceProvider(base.NovaObject): result = context.session.query(models.ResourceProvider).filter_by( uuid=uuid).first() if not result: - raise exception.NotFound() + raise exception.NotFound( + 'No resource provider with uuid %s found' + % uuid) return result diff --git a/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml b/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml index adc9c3108f2a..dd02f7af5c75 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml @@ -153,6 +153,8 @@ tests: - name: delete allocation again DELETE: /allocations/599ffd2d-526a-4b2e-8683-f13ad25f9958 status: 404 + response_strings: + - No allocations for consumer '599ffd2d-526a-4b2e-8683-f13ad25f9958' - name: delete allocation of unknown consumer id DELETE: /allocations/da78521f-bf7e-4e6e-9901-3f79bd94d55d diff --git a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml index 3c3687401d51..50d9c40de516 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml @@ -86,6 +86,12 @@ tests: response_json_paths: $.uuid: $ENVIRON['RP_UUID'] +- name: get non-existing resource provider + GET: /resource_providers/d67370b5-4dc0-470d-a4fa-85e8e89abc6c + status: 404 + response_strings: + - No resource provider with uuid d67370b5-4dc0-470d-a4fa-85e8e89abc6c found + - name: list one resource providers GET: /resource_providers response_json_paths: @@ -213,6 +219,12 @@ tests: GET: /resource_providers/random_sauce status: 404 +- name: delete non-existing resource provider + DELETE: /resource_providers/d67370b5-4dc0-470d-a4fa-85e8e89abc6c + status: 404 + response_strings: + - No resource provider with uuid d67370b5-4dc0-470d-a4fa-85e8e89abc6c found for delete + - name: post resource provider no uuid POST: /resource_providers request_headers: