Merge "db: Remove weird error handling code"

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
+        data.save(context.session)
     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"