From 863f0ce0b543631cf13b3d5d6491659931e2697e Mon Sep 17 00:00:00 2001
From: Chris Dent <cdent@anticdent.org>
Date: Mon, 29 May 2017 17:27:22 +0000
Subject: [PATCH] [placement] Disambiguate resource provider conflict message

When creating a resource provider, if the uuid or name (or both)
of the resource provider is already being used a 409 response is
returned. Prior to this change the error message always mentioned
the name of the resource provider, even if it was not the cause of
the duplication. That's misleading and unfriendly.

The new code will list the field and the value of the field that
was duplicated. From looking at the olso_db code that creates a
DBDuplicateEntry it appears that sometimes, but not always, the
exception could list both fields, so allowance for that is made
in the message output and the related gabbi tests.

Because this changes the error message, but not the error response
code, this does not violate the api interoperability guidelines[1],
so I'm willing to say this doesn't require a microversion, and thus
does not require a spec.

[1] http://specs.openstack.org/openstack/api-wg/guidelines/api_interoperability.html
If we do think this is a violation, then the guideline should be
made more explicit.

Change-Id: Ibafbfd8302977a2b4e9250fbb5ada283b69d3c25
Closes-Bug: #1693349
---
 .../placement/handlers/resource_provider.py   |  9 +++-
 .../resource-provider-duplication.yaml        | 48 +++++++++++++++++++
 .../placement/gabbits/resource-provider.yaml  |  6 ++-
 3 files changed, 59 insertions(+), 4 deletions(-)
 create mode 100644 nova/tests/functional/api/openstack/placement/gabbits/resource-provider-duplication.yaml

diff --git a/nova/api/openstack/placement/handlers/resource_provider.py b/nova/api/openstack/placement/handlers/resource_provider.py
index 1eef16f4a6..d9261f1af1 100644
--- a/nova/api/openstack/placement/handlers/resource_provider.py
+++ b/nova/api/openstack/placement/handlers/resource_provider.py
@@ -190,9 +190,14 @@ def create_resource_provider(req):
             context, name=data['name'], uuid=uuid)
         resource_provider.create()
     except db_exc.DBDuplicateEntry as exc:
+        # Whether exc.columns has one or two entries (in the event
+        # of both fields being duplicates) appears to be database
+        # dependent, so going with the complete solution here.
+        duplicate = ', '.join(['%s: %s' % (column, data[column])
+                          for column in exc.columns])
         raise webob.exc.HTTPConflict(
-            _('Conflicting resource provider %(name)s already exists.') %
-            {'name': data['name']})
+            _('Conflicting resource provider %(duplicate)s already exists.') %
+            {'duplicate': duplicate})
     except exception.ObjectActionError as exc:
         raise webob.exc.HTTPBadRequest(
             _('Unable to create resource provider %(rp_uuid)s: %(error)s') %
diff --git a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-duplication.yaml b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-duplication.yaml
new file mode 100644
index 0000000000..9eb60fd9f4
--- /dev/null
+++ b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-duplication.yaml
@@ -0,0 +1,48 @@
+# Verify different error messages was attempting to create a
+# resource provider with a duplicated name or UUID.
+
+fixtures:
+    - APIFixture
+
+defaults:
+    request_headers:
+        x-auth-token: admin
+        accept: application/json
+        content-type: application/json
+
+tests:
+- name: post new resource provider
+  POST: /resource_providers
+  data:
+      name: shared disk
+      uuid: $ENVIRON['RP_UUID']
+  status: 201
+
+- name: same uuid different name
+  POST: /resource_providers
+  data:
+      name: shared disk X
+      uuid: $ENVIRON['RP_UUID']
+  status: 409
+  response_strings:
+      - "Conflicting resource provider uuid: $ENVIRON['RP_UUID']"
+
+- name: same name different uuid
+  POST: /resource_providers
+  data:
+      name: shared disk
+      uuid: 2c2059d8-005c-4f5c-82b1-b1701b1a29b7
+  status: 409
+  response_strings:
+      - 'Conflicting resource provider name: shared disk'
+
+# On this one, don't test for which field was a duplicate because
+# that depends on how the database reports columns.
+- name: same name same uuid
+  POST: /resource_providers
+  data:
+      name: $ENVIRON['RP_NAME']
+      uuid: $ENVIRON['RP_UUID']
+  status: 409
+  response_strings:
+      - Conflicting resource provider
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 9ad2de113c..1ae19be47b 100644
--- a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml
+++ b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml
@@ -45,6 +45,8 @@ tests:
   response_forbidden_headers:
       - content-type
 
+# On this one, don't test for which field was a duplicate because
+# that depends on how the database reports columns.
 - name: try to create same all again
   POST: /resource_providers
   request_headers:
@@ -54,7 +56,7 @@ tests:
       uuid: $ENVIRON['RP_UUID']
   status: 409
   response_strings:
-      - Conflicting resource provider $ENVIRON['RP_NAME'] already exists
+      - Conflicting resource provider
   response_json_paths:
       $.errors[0].title: Conflict
 
@@ -67,7 +69,7 @@ tests:
       uuid: ada30fb5-566d-4fe1-b43b-28a9e988790c
   status: 409
   response_strings:
-      - Conflicting resource provider $ENVIRON['RP_NAME'] already exists
+      - "Conflicting resource provider name: $ENVIRON['RP_NAME'] already exists"
   response_json_paths:
       $.errors[0].title: Conflict