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

@@ -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)

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(