From 8260169fa72655705d4c86640c08e4262538bb09 Mon Sep 17 00:00:00 2001
From: Accela Zhao <accelazh@gmail.com>
Date: Tue, 2 Feb 2016 23:19:45 -0800
Subject: [PATCH] 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
---
 cinder/consistencygroup/api.py                | 23 ++++++++-------
 .../unit/api/contrib/test_cgsnapshots.py      | 29 +++++++++++++++++++
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/cinder/consistencygroup/api.py b/cinder/consistencygroup/api.py
index 171d43ed2..c10a1a0b4 100644
--- 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 6206a3a50..659c92594 100644
--- 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(
-- 
2.45.2