Don't create cgsnapshot if cg is empty

If we create cgsnapshot from an empty consistencygroup, it fails
with "Consistency group is empty. No cgsnapshot will be created".
However, a cgsnapshot object is still created in DB.

This is because in cinder/consistencygroup/api.py::_create_cgsnapshot,
we create cgsnapshot object before we test whether the corresponding
consistencygroup is empy.

This patch fixes this problem by moving the creation of cgsnapshot
object to after the test of whether consistencygroup is empty.

Change-Id: I09500ab1cf19b28591668a3fed6834d1a73ad176
Closes-Bug: #1535979

diff --git a/cinder/consistencygroup/api.py b/cinder/consistencygroup/api.py
index 171d43ed2226310c873dde594e40d009a366c943..c10a1a0b4a91e026c877b0a0ae5856c186e9bf04 100644 (file)
--- a/cinder/consistencygroup/api.py
+++ b/cinder/consistencygroup/api.py
@@ -713,6 +713,15 @@ class API(base.Base):
 
     def _create_cgsnapshot(self, context,
                            group, name, description):
+        volumes = self.db.volume_get_all_by_group(
+            context.elevated(),
+            group.id)
+
+        if not volumes:
+            msg = _("Consistency group is empty. No cgsnapshot "
+                    "will be created.")
+            raise exception.InvalidConsistencyGroup(reason=msg)
+
         options = {'consistencygroup_id': group.id,
                    'user_id': context.user_id,
                    'project_id': context.project_id,
@@ -720,20 +729,13 @@ class API(base.Base):
                    'name': name,
                    'description': description}
 
+        cgsnapshot = None
+        cgsnapshot_id = None
         try:
             cgsnapshot = objects.CGSnapshot(context, **options)
             cgsnapshot.create()
             cgsnapshot_id = cgsnapshot.id
 
-            volumes = self.db.volume_get_all_by_group(
-                context.elevated(),
-                cgsnapshot.consistencygroup_id)
-
-            if not volumes:
-                msg = _("Consistency group is empty. No cgsnapshot "
-                        "will be created.")
-                raise exception.InvalidConsistencyGroup(reason=msg)
-
             snap_name = cgsnapshot.name
             snap_desc = cgsnapshot.description
             self.volume_api.create_snapshots_in_db(
@@ -742,7 +744,8 @@ class API(base.Base):
         except Exception:
             with excutils.save_and_reraise_exception():
                 try:
-                    cgsnapshot.destroy()
+                    if cgsnapshot:
+                        cgsnapshot.destroy()
                 finally:
                     LOG.error(_LE("Error occurred when creating cgsnapshot"
                                   " %s."), cgsnapshot_id)

diff --git a/cinder/tests/unit/api/contrib/test_cgsnapshots.py b/cinder/tests/unit/api/contrib/test_cgsnapshots.py
index 6206a3a50b3c64059da712096d1cb72d465286c2..659c92594eb602d368838685e29ca1a590bdd264 100644 (file)
--- a/cinder/tests/unit/api/contrib/test_cgsnapshots.py
+++ b/cinder/tests/unit/api/contrib/test_cgsnapshots.py
@@ -396,6 +396,35 @@ class CgsnapshotsAPITestCase(test.TestCase):
                          res_dict['itemNotFound']['message'])
         consistencygroup.destroy()
 
+    @mock.patch.object(objects.CGSnapshot, 'create')
+    def test_create_cgsnapshot_from_empty_consistencygroup(
+            self,
+            mock_cgsnapshot_create):
+        consistencygroup = utils.create_consistencygroup(self.context)
+
+        body = {"cgsnapshot": {"name": "cg1",
+                               "description":
+                               "CG Snapshot 1",
+                               "consistencygroup_id": consistencygroup.id}}
+
+        req = webob.Request.blank('/v2/fake/cgsnapshots')
+        req.method = 'POST'
+        req.headers['Content-Type'] = 'application/json'
+        req.body = jsonutils.dump_as_bytes(body)
+        res = req.get_response(fakes.wsgi_app())
+        res_dict = jsonutils.loads(res.body)
+
+        self.assertEqual(400, res.status_int)
+        self.assertEqual(400, res_dict['badRequest']['code'])
+        self.assertEqual('Invalid ConsistencyGroup: Consistency group is '
+                         'empty. No cgsnapshot will be created.',
+                         res_dict['badRequest']['message'])
+
+        # If failed to create cgsnapshot, its DB object should not be created
+        self.assertFalse(mock_cgsnapshot_create.called)
+
+        consistencygroup.destroy()
+
     def test_delete_cgsnapshot_available(self):
         consistencygroup = utils.create_consistencygroup(self.context)
         volume_id = utils.create_volume(