Fix volume_create()/snapshot_create() DB methods

volume_create() and snapshot_create() DB API methods create
entities and return information about them by calling
_volume_get() and snapshot_get() methods. There is no need
to create a new transaction to call the corresponding 'getter'.

Unfortunately, fixing this breaks two existing tests, which rely
on the current behaviour of these DB API methods: both volume_create()
and snapshot_create() raise exceptions, nevertheless new entities are
created.

Blueprint: db-session-cleanup

Change-Id: Idecac70893ec280c149f9227f86a6959d70c15fe

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 83fc5c10b54656f9245395e6fc4294233c1195a3..749b36eda906570b1241d331ded3683ce7f03a54 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -1006,7 +1006,7 @@ def volume_create(context, values):
     with session.begin():
         volume_ref.save(session=session)
 
-    return _volume_get(context, values['id'], session=session)
+        return _volume_get(context, values['id'], session=session)
 
 
 @require_admin_context
@@ -1330,7 +1330,7 @@ def snapshot_create(context, values):
     with session.begin():
         snapshot_ref.save(session=session)
 
-    return _snapshot_get(context, values['id'], session=session)
+        return _snapshot_get(context, values['id'], session=session)
 
 
 @require_admin_context

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index 198bf89571855648f9fc53ed29d10b26c63b0e5d..e2cfa0e4eede926880a124626a65dcf2524b192a 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -1139,48 +1139,45 @@ class VolumeTestCase(test.TestCase):
         self.assertEquals(snap['display_name'], 'test update name')
 
     def test_volume_get_active_by_window(self):
+        ctx = context.get_admin_context(read_deleted="yes")
+
         # Find all all volumes valid within a timeframe window.
-        try:  # Not in window
-            db.volume_create(
-                self.context,
-                {
-                    'id': 1,
-                    'host': 'devstack',
-                    'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
-                    'deleted': True, 'status': 'deleted',
-                    'deleted_at': datetime.datetime(1, 2, 1, 1, 1, 1),
-                }
-            )
-        except exception.VolumeNotFound:
-            pass
 
-        try:  # In - deleted in window
-            db.volume_create(
-                self.context,
-                {
-                    'id': 2,
-                    'host': 'devstack',
-                    'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
-                    'deleted': True, 'status': 'deleted',
-                    'deleted_at': datetime.datetime(1, 3, 10, 1, 1, 1),
-                }
-            )
-        except exception.VolumeNotFound:
-            pass
+        # Not in window
+        db.volume_create(
+            ctx,
+            {
+                'id': 1,
+                'host': 'devstack',
+                'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+                'deleted': True, 'status': 'deleted',
+                'deleted_at': datetime.datetime(1, 2, 1, 1, 1, 1),
+            }
+        )
 
-        try:  # In - deleted after window
-            db.volume_create(
-                self.context,
-                {
-                    'id': 3,
-                    'host': 'devstack',
-                    'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
-                    'deleted': True, 'status': 'deleted',
-                    'deleted_at': datetime.datetime(1, 5, 1, 1, 1, 1),
-                }
-            )
-        except exception.VolumeNotFound:
-            pass
+        # In - deleted in window
+        db.volume_create(
+            ctx,
+            {
+                'id': 2,
+                'host': 'devstack',
+                'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+                'deleted': True, 'status': 'deleted',
+                'deleted_at': datetime.datetime(1, 3, 10, 1, 1, 1),
+            }
+        )
+
+        # In - deleted after window
+        db.volume_create(
+            ctx,
+            {
+                'id': 3,
+                'host': 'devstack',
+                'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+                'deleted': True, 'status': 'deleted',
+                'deleted_at': datetime.datetime(1, 5, 1, 1, 1, 1),
+            }
+        )
 
         # In - created in window
         db.volume_create(
@@ -1212,53 +1209,49 @@ class VolumeTestCase(test.TestCase):
         self.assertEqual(volumes[2].id, u'4')
 
     def test_snapshot_get_active_by_window(self):
+        ctx = context.get_admin_context(read_deleted="yes")
+
         # Find all all snapshots valid within a timeframe window.
         vol = db.volume_create(self.context, {'id': 1})
 
-        try:  # Not in window
-            db.snapshot_create(
-                self.context,
-                {
-                    'id': 1,
-                    'host': 'devstack',
-                    'volume_id': 1,
-                    'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
-                    'deleted': True, 'status': 'deleted',
-                    'deleted_at': datetime.datetime(1, 2, 1, 1, 1, 1),
-                }
-            )
-        except exception.SnapshotNotFound:
-            pass
+        # Not in window
+        db.snapshot_create(
+            ctx,
+            {
+                'id': 1,
+                'host': 'devstack',
+                'volume_id': 1,
+                'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+                'deleted': True, 'status': 'deleted',
+                'deleted_at': datetime.datetime(1, 2, 1, 1, 1, 1),
+            }
+        )
 
-        try:  # In - deleted in window
-            db.snapshot_create(
-                self.context,
-                {
-                    'id': 2,
-                    'host': 'devstack',
-                    'volume_id': 1,
-                    'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
-                    'deleted': True, 'status': 'deleted',
-                    'deleted_at': datetime.datetime(1, 3, 10, 1, 1, 1),
-                }
-            )
-        except exception.SnapshotNotFound:
-            pass
+        # In - deleted in window
+        db.snapshot_create(
+            ctx,
+            {
+                'id': 2,
+                'host': 'devstack',
+                'volume_id': 1,
+                'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+                'deleted': True, 'status': 'deleted',
+                'deleted_at': datetime.datetime(1, 3, 10, 1, 1, 1),
+            }
+        )
 
-        try:  # In - deleted after window
-            db.snapshot_create(
-                self.context,
-                {
-                    'id': 3,
-                    'host': 'devstack',
-                    'volume_id': 1,
-                    'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
-                    'deleted': True, 'status': 'deleted',
-                    'deleted_at': datetime.datetime(1, 5, 1, 1, 1, 1),
-                }
-            )
-        except exception.SnapshotNotFound:
-            pass
+        # In - deleted after window
+        db.snapshot_create(
+            ctx,
+            {
+                'id': 3,
+                'host': 'devstack',
+                'volume_id': 1,
+                'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+                'deleted': True, 'status': 'deleted',
+                'deleted_at': datetime.datetime(1, 5, 1, 1, 1, 1),
+            }
+        )
 
         # In - created in window
         db.snapshot_create(