Provide framework for setting placement error codes

The API-sig has a guideline[1] for including error codes in error
responses to help distinguish errors with the same status code
from one another. This change provides a simplest-thing-that-could-
possibly-work solution to make that go.

This solution comes about after a few different constraints and attempts:

* We would prefer to go on using the existing webob.exc exceptions, not
  make subclasses.
* We already have a special wrapper around our wsgi apps to deal with
  setting the json_error_formatter.
* Though webob allows custom Request and Response objects, it uses the
  default Response object as the parent of the HTTP exceptions.
* The Response object accepts kwargs, but only if they can be associated
  with known attributes on the class. Since we can't subclass...
* The json_error_formatter method is not passed the raw exception, but
  it does get the current WSGI environ
* The webob.exc classes take a 'comment' kwarg that is not used, but
  is also not passed to the json_error_formatter.

Therefore, when we raise an exception, we can set 'comment' to a code
and then assign that comment to a well known field in the environ and if
that environ is set in json_error_formatter, we can set 'code' in the
output.

This is done in a new microversion, 1.23. Every response gets a default
code 'placement.undefined_code' from 1.23 on. Future development will
add specific codes where required. This change adds a stub code for
inventory in use when doing a PUT to .../inventories but the name
may need improvement.

[1] http://specs.openstack.org/openstack/api-wg/guidelines/errors.html

Implements blueprint placement-api-error-handling

Change-Id: I9a833aa35d474caa35e640bbad6c436a3b16ac5e

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

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'

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 '

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
 ]
 
 

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``.

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()

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

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

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: /

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:

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``.