Improve user error messages for duplicate objects

The error handling for duplicate records was not great, specifically
health monitors would return a 500 or a 409 with a bad error message.
This patch improves the errors the user gets back when creating a
duplicate entity.

Co-Authored-By: Jacky Hu <hudayou@hotmail.com>
Change-Id: I03ec527b42d67427541121e59e4433699e04aab8

octavia/api/v2/controllers/health_monitor.py

@@ -129,9 +129,8 @@ class HealthMonitorController(base.BaseController):
         try:
             return self.repositories.health_monitor.create(
                 lock_session, **hm_dict)
-        except odb_exceptions.DBDuplicateEntry as de:
-            if ['id'] == de.columns:
-                raise exceptions.IDAlreadyExists()
+        except odb_exceptions.DBDuplicateEntry:
+            raise exceptions.DuplicateHealthMonitor()
         except odb_exceptions.DBError:
             # TODO(blogan): will have to do separate validation protocol
             # before creation or update since the exception messages

octavia/api/v2/controllers/l7policy.py

@@ -107,9 +107,8 @@ class L7PolicyController(base.BaseController):
         try:
             return self.repositories.l7policy.create(lock_session,
                                                      **l7policy_dict)
-        except odb_exceptions.DBDuplicateEntry as de:
-            if ['id'] == de.columns:
-                raise exceptions.IDAlreadyExists()
+        except odb_exceptions.DBDuplicateEntry:
+            raise exceptions.IDAlreadyExists()
         except odb_exceptions.DBError:
             # TODO(blogan): will have to do separate validation protocol
             # before creation or update since the exception messages

octavia/api/v2/controllers/l7rule.py

@@ -116,10 +116,9 @@ class L7RuleController(base.BaseController):
     def _validate_create_l7rule(self, lock_session, l7rule_dict):
         try:
             return self.repositories.l7rule.create(lock_session, **l7rule_dict)
-        except odb_exceptions.DBDuplicateEntry as de:
-            if ['id'] == de.columns:
-                raise exceptions.IDAlreadyExists()
-        except odb_exceptions.DBError as de:
+        except odb_exceptions.DBDuplicateEntry:
+            raise exceptions.IDAlreadyExists()
+        except odb_exceptions.DBError:
             # TODO(blogan): will have to do separate validation protocol
             # before creation or update since the exception messages
             # do not give any information as to what constraint failed

octavia/api/v2/controllers/pool.py

@@ -114,9 +114,8 @@ class PoolsController(base.BaseController):
             return self.repositories.create_pool_on_load_balancer(
                 lock_session, pool_dict,
                 listener_id=listener_id)
-        except odb_exceptions.DBDuplicateEntry as de:
-            if ['id'] == de.columns:
-                raise exceptions.IDAlreadyExists()
+        except odb_exceptions.DBDuplicateEntry:
+            raise exceptions.IDAlreadyExists()
         except odb_exceptions.DBError:
             # TODO(blogan): will have to do separate validation protocol
             # before creation or update since the exception messages

octavia/db/models.py

@@ -218,6 +218,11 @@ class HealthMonitor(base_models.BASE, base_models.IdMixin,
 
     __v2_wsme__ = health_monitor.HealthMonitorResponse
 
+    __table_args__ = (
+        sa.UniqueConstraint('pool_id',
+                            name='uq_health_monitor_pool'),
+    )
+
     type = sa.Column(
         sa.String(36),
         sa.ForeignKey("health_monitor_type.name",

octavia/tests/functional/api/v2/test_health_monitor.py

@@ -746,12 +746,6 @@ class TestHealthMonitor(base.BaseAPITest):
             hm_op_status=constants.OFFLINE)
 
     def test_duplicate_create(self):
-        # TODO(rm_work): I am fairly certain this is the same issue as we see
-        # in test_repositories.py where PySqlite commits too early and can't
-        # roll back, causing things to get out of whack. This runs fine solo.
-        # It would be useful to test this *in reality* and see if it breaks.
-        self.skipTest("PySqlite transaction handling is broken. We can unskip"
-                      "this when `test_sqlite_transactions_broken` fails.")
         self.create_health_monitor(
             self.pool_id, constants.HEALTH_MONITOR_HTTP, 1, 1, 1, 1)
         self.set_lb_status(self.lb_id)

releasenotes/notes/fix-error-messages-ec817a66249e6666.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Improves error messages returned to the user, such as errors for attempting
+    to add a second health monitor to a pool.