diff --git a/doc/source/contributor/placement.rst b/doc/source/contributor/placement.rst index 0a9bc38bfe77..7884c457e165 100644 --- a/doc/source/contributor/placement.rst +++ b/doc/source/contributor/placement.rst @@ -252,6 +252,24 @@ environment. It can be retrieved as follows:: changes in a patch that is separate from and prior to the HTTP API change. +If a handler needs to return an error response, with the advent of `link to +spec once it merges`_, it is possible to include a code in the JSON error +response. This can be used to distinguish different errors with the same HTTP +response status code (a common case is a generation conflict versus an +inventory in use conflict). Error codes are simple namespaced strings (e.g., +``placement.inventory.inuse``) for which symbols are maintained in +``nova.api.openstack.placement.errors``. Adding a symbol to a response is done +by using the ``comment`` kwarg to a WebOb exception, like this:: + + except exception.InventoryInUse as exc: + raise webob.exc.HTTPConflict( + _('update conflict: %(error)s') % {'error': exc}, + comment=errors.INVENTORY_INUSE) + +Code that adds newly raised exceptions should include an error code. Find +additional guidelines on use in the docs for +``nova.api.openstack.placement.errors``. + Testing of handler code is described in the next section. Testing diff --git a/nova/api/openstack/placement/errors.py b/nova/api/openstack/placement/errors.py new file mode 100644 index 000000000000..41be89d2a80c --- /dev/null +++ b/nova/api/openstack/placement/errors.py @@ -0,0 +1,42 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +"""Error code symbols to be used in structured JSON error responses. + +These are strings to be used in the 'code' attribute, as described by +the API guideline on `errors`_. + +There must be only one instance of any string value and it should have +only one associated constant SYMBOL. + +In a WSGI handler (representing the sole handler for an HTTP method and +URI) each error condition should get a separate error code. Reusing an +error code in a different handler is not just acceptable, but useful. + +For example 'placement.inventory.inuse' is meaningful and correct in both +``PUT /resource_providers/{uuid}/inventories`` and ``DELETE`` on the same +URI. + +.. _errors: http://specs.openstack.org/openstack/api-wg/guidelines/errors.html +""" + +# NOTE(cdent): This is the simplest thing that can possibly work, for now. +# If it turns out we want to automate this, or put different resources in +# different files, or otherwise change things, that's fine. The only thing +# that needs to be maintained as the same are the strings that API end +# users use. How they are created is completely fungible. + + +# Do not change the string values. Once set, they are set. +# Do not reuse string values. There should be only one symbol for any +# value. +DEFAULT = 'placement.undefined_code' +INVENTORY_INUSE = 'placement.inventory.inuse' diff --git a/nova/api/openstack/placement/handlers/inventory.py b/nova/api/openstack/placement/handlers/inventory.py index 7e83d5e0b9d0..4445302eba9b 100644 --- a/nova/api/openstack/placement/handlers/inventory.py +++ b/nova/api/openstack/placement/handlers/inventory.py @@ -18,6 +18,7 @@ from oslo_serialization import jsonutils from oslo_utils import encodeutils import webob +from nova.api.openstack.placement import errors from nova.api.openstack.placement import exception from nova.api.openstack.placement import microversion from nova.api.openstack.placement.objects import resource_provider as rp_obj @@ -317,10 +318,13 @@ def set_inventories(req): '%(rp_uuid)s: %(error)s') % {'rp_uuid': resource_provider.uuid, 'error': exc}) except (exception.ConcurrentUpdateDetected, - exception.InventoryInUse, db_exc.DBDuplicateEntry) as exc: raise webob.exc.HTTPConflict( _('update conflict: %(error)s') % {'error': exc}) + except exception.InventoryInUse as exc: + raise webob.exc.HTTPConflict( + _('update conflict: %(error)s') % {'error': exc}, + comment=errors.INVENTORY_INUSE) except exception.InvalidInventoryCapacity as exc: raise webob.exc.HTTPBadRequest( _('Unable to update inventory for resource provider ' diff --git a/nova/api/openstack/placement/microversion.py b/nova/api/openstack/placement/microversion.py index 79956f993d44..1f8391db1190 100644 --- a/nova/api/openstack/placement/microversion.py +++ b/nova/api/openstack/placement/microversion.py @@ -63,6 +63,7 @@ VERSIONS = [ # GET /allocation_candidates '1.22', # Support forbidden traits in the required parameter of # GET /resource_providers and GET /allocation_candidates + '1.23', # Add support for error codes in error response JSON ] diff --git a/nova/api/openstack/placement/rest_api_version_history.rst b/nova/api/openstack/placement/rest_api_version_history.rst index 2644e37339d7..c12c994dece6 100644 --- a/nova/api/openstack/placement/rest_api_version_history.rst +++ b/nova/api/openstack/placement/rest_api_version_history.rst @@ -270,3 +270,12 @@ Add support for expressing traits which are forbidden when filtering trait is a properly formatted trait in the existing ``required`` parameter, prefixed by a ``!``. For example ``required=!STORAGE_DISK_SSD`` asks that the results not include any resource providers that provide solid state disk. + +1.23 Include code attribute in JSON error responses +--------------------------------------------------- + +JSON formatted error responses gain a new attribute, ``code``, with a value +that identifies the type of this error. This can be used to distinguish errors +that are different but use the same HTTP status code. Any error response which +does not specifically define a code will have the code +``placement.undefined_code``. diff --git a/nova/api/openstack/placement/util.py b/nova/api/openstack/placement/util.py index 81658c79f945..834896cd31b3 100644 --- a/nova/api/openstack/placement/util.py +++ b/nova/api/openstack/placement/util.py @@ -21,12 +21,16 @@ from oslo_utils import timeutils from oslo_utils import uuidutils import webob +from nova.api.openstack.placement import errors from nova.api.openstack.placement import lib as placement_lib # NOTE(cdent): avoid cyclical import conflict between util and # microversion import nova.api.openstack.placement.microversion from nova.i18n import _ +# Error code handling constants +ENV_ERROR_CODE = 'placement.error_code' +ERROR_CODE_MICROVERSION = (1, 23) # Querystring-related constants _QS_RESOURCES = 'resources' @@ -101,6 +105,9 @@ def json_error_formatter(body, status, title, environ): Follows API-WG guidelines at http://specs.openstack.org/openstack/api-wg/guidelines/errors.html """ + # Shortcut to microversion module, to avoid wraps below. + microversion = nova.api.openstack.placement.microversion + # Clear out the html that webob sneaks in. body = webob.exc.strip_tags(body) # Get status code out of status message. webob's error formatter @@ -111,6 +118,13 @@ def json_error_formatter(body, status, title, environ): 'title': title, 'detail': body } + + # Version may not be set if we have experienced an error before it + # is set. + want_version = environ.get(microversion.MICROVERSION_ENVIRON) + if want_version and want_version.matches(ERROR_CODE_MICROVERSION): + error_dict['code'] = environ.get(ENV_ERROR_CODE, errors.DEFAULT) + # If the request id middleware has had a chance to add an id, # put it in the error response. if request_id.ENV_REQUEST_ID in environ: @@ -119,7 +133,6 @@ def json_error_formatter(body, status, title, environ): # When there is a no microversion in the environment and a 406, # microversion parsing failed so we need to include microversion # min and max information in the error response. - microversion = nova.api.openstack.placement.microversion if status_code == 406 and microversion.MICROVERSION_ENVIRON not in environ: error_dict['max_version'] = microversion.max_version_string() error_dict['min_version'] = microversion.min_version_string() diff --git a/nova/api/openstack/placement/wsgi_wrapper.py b/nova/api/openstack/placement/wsgi_wrapper.py index 4aa8b789ba5f..fcb6551d3e4f 100644 --- a/nova/api/openstack/placement/wsgi_wrapper.py +++ b/nova/api/openstack/placement/wsgi_wrapper.py @@ -30,4 +30,9 @@ class PlacementWsgify(wsgify): except webob.exc.HTTPException as exc: LOG.debug("Placement API returning an error response: %s", exc) exc.json_formatter = util.json_error_formatter + # The exception itself is not passed to json_error_formatter + # but environ is, so set the environ. + if exc.comment: + req.environ[util.ENV_ERROR_CODE] = exc.comment + exc.comment = None raise diff --git a/nova/tests/functional/api/openstack/placement/gabbits/basic-http.yaml b/nova/tests/functional/api/openstack/placement/gabbits/basic-http.yaml index 244369bfdc3d..162301e90141 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/basic-http.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/basic-http.yaml @@ -26,6 +26,14 @@ tests: response_json_paths: $.errors[0].request_id: /req-[a-fA-F0-9-]+/ +- name: error message has default code 1.23 + GET: /barnabas + status: 404 + request_headers: + openstack-api-version: placement 1.23 + response_json_paths: + $.errors[0].code: placement.undefined_code + - name: 404 at no resource provider GET: /resource_providers/fd0dd55c-6330-463b-876c-31c54e95cb95 status: 404 diff --git a/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml b/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml index d832cdd1f0a4..b90344ad11ed 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.22 +- name: latest microversion is 1.23 GET: / request_headers: openstack-api-version: placement latest response_headers: vary: /openstack-api-version/ - openstack-api-version: placement 1.22 + openstack-api-version: placement 1.23 - name: other accept header bad version GET: / diff --git a/nova/tests/functional/api/openstack/placement/gabbits/with-allocations.yaml b/nova/tests/functional/api/openstack/placement/gabbits/with-allocations.yaml index d19fb453e033..c1c903a2228f 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/with-allocations.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/with-allocations.yaml @@ -31,6 +31,19 @@ tests: response_strings: - "Unable to delete resource provider $ENVIRON['RP_UUID']: Resource provider has allocations." +- name: fail to change inventory via put 1.23 + PUT: /resource_providers/$ENVIRON['RP_UUID']/inventories + request_headers: + accept: application/json + content-type: application/json + openstack-api-version: placement 1.23 + data: + resource_provider_generation: 5 + inventories: {} + status: 409 + response_json_paths: + $.errors[0].code: placement.inventory.inuse + - name: fail to delete all inventory DELETE: /resource_providers/$ENVIRON['RP_UUID']/inventories request_headers: diff --git a/releasenotes/notes/placement-error-code-fcbbf5d45560984e.yaml b/releasenotes/notes/placement-error-code-fcbbf5d45560984e.yaml new file mode 100644 index 000000000000..9d114f4eda0f --- /dev/null +++ b/releasenotes/notes/placement-error-code-fcbbf5d45560984e.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + In microversion 1.23 of the placement service, JSON formatted error + responses gain a new attribute, ``code``, with a value that identifies the + type of this error. This can be used to distinguish errors that are + different but use the same HTTP status code. Any error response which does + not specifically define a code will have the code + ``placement.undefined_code``.