db: Remove weird error handling code

The 'driver_initiator_data_insert_by_key' DB API is exceptional in that
it attempts to create a DB entry and returns a boolean value if there's
a duplicate error rather than raise an exception. This means we need to
do some special handling. Avoid the need for this special handling by
raising an exception like everyone else and do the mapping to boolean
values in the sole caller.

Change-Id: I25472f592dbdb487fbbb376cb92ee0dda76b677a
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

cinder/db/api.py

@@ -1784,9 +1784,6 @@ def driver_initiator_data_insert_by_key(context, initiator,
     """Updates DriverInitiatorData entry.
 
     Sets the value for the specified key within the namespace.
-
-    If the entry already exists return False, if it inserted successfully
-    return True.
     """
     return IMPL.driver_initiator_data_insert_by_key(context,
                                                     initiator,

cinder/db/sqlalchemy/api.py

@@ -8287,9 +8287,8 @@ def cleanup_expired_messages(context):
 ###############################
 
 
-# NOTE: We don't need a transaction context manager decorator since we're using
-# the context manager directly inside
 @require_context
+@main_context_manager.writer
 def driver_initiator_data_insert_by_key(
     context,
     initiator,
@@ -8303,14 +8302,14 @@ def driver_initiator_data_insert_by_key(
     data.key = key
     data.value = value
     try:
-        # NOTE: We use a context manager since the decorator pattern requires
-        # we raise an exception or succeed, while we want to return a boolean.
-        # The context manager allows us to do manual cleanup here.
-        with main_context_manager.writer.savepoint.using(context):
         data.save(context.session)
-        return True
     except db_exc.DBDuplicateEntry:
-        return False
+        raise exception.DriverInitiatorDataExists(
+            initiator=initiator,
+            namespace=namespace,
+            key=key,
+        )
+    return data
 
 
 @require_context

cinder/exception.py

@@ -1085,3 +1085,10 @@ class CinderAcceleratorError(CinderException):
 class SnapshotLimitReached(CinderException):
     message = _("Exceeded the configured limit of "
                 "%(set_limit)s snapshots per volume.")
+
+
+class DriverInitiatorDataExists(Duplicate):
+    message = _(
+        "Driver initiator data for initiator '%(initiator)s' and backend "
+        "'%(namespace)s' with key '%(key)s' already exists."
+    )

cinder/tests/unit/test_db_api.py

@@ -3354,22 +3354,35 @@ class DBAPIDriverInitiatorDataTestCase(BaseTest):
     initiator = 'iqn.1993-08.org.debian:01:222'
     namespace = 'test_ns'
 
-    def _test_insert(self, key, value, expected_result=True):
-        result = db.driver_initiator_data_insert_by_key(
-            self.ctxt, self.initiator, self.namespace, key, value)
-        self.assertEqual(expected_result, result)
+    def test_insert(self):
+        key = 'key1'
+        value = 'foo'
 
-        data = db.driver_initiator_data_get(self.ctxt, self.initiator,
-                                            self.namespace)
+        db.driver_initiator_data_insert_by_key(
+            self.ctxt, self.initiator, self.namespace, key, value,
+        )
+        data = db.driver_initiator_data_get(
+            self.ctxt, self.initiator, self.namespace,
+        )
         self.assertEqual(data[0].key, key)
         self.assertEqual(data[0].value, value)
 
-    def test_insert(self):
-        self._test_insert('key1', 'foo')
-
     def test_insert_already_exists(self):
-        self._test_insert('key2', 'bar')
-        self._test_insert('key2', 'bar', expected_result=False)
+        key = 'key1'
+        value = 'foo'
+
+        db.driver_initiator_data_insert_by_key(
+            self.ctxt, self.initiator, self.namespace, key, value,
+        )
+        self.assertRaises(
+            exception.DriverInitiatorDataExists,
+            db.driver_initiator_data_insert_by_key,
+            self.ctxt,
+            self.initiator,
+            self.namespace,
+            key,
+            value,
+        )
 
 
 @ddt.ddt

cinder/volume/driver_utils.py

@@ -54,13 +54,16 @@ class VolumeDriverUtils(object):
         if saved successfully return True.
         """
         try:
-            return self._db.driver_initiator_data_insert_by_key(
+            self._db.driver_initiator_data_insert_by_key(
                 self._get_context(ctxt),
                 initiator,
                 self._data_namespace,
                 key,
                 value
             )
+            return True
+        except exception.DriverInitiatorDataExists:
+            return False
         except exception.CinderException:
             LOG.exception("Failed to insert initiator data for"
                           " initiator %(initiator)s and backend"