Remove API races from consistency groups

There is a potential window of opportunity where races can happen in the
API on consistency group related actions, this patch removes those
windows of opportunity using compare-and-swap for DB updates.

Races have been removed in following actions:

- create
- delete
- update
- create_cgsnapshot
- delete_cgsnapshot

Specs: https://review.openstack.org/232599/

Implements: blueprint cinder-volume-active-active-support
Change-Id: I67aec4cd8bcf2f7e09473a8d296aa383fe85ad23

cinder/api/contrib/cgsnapshots.py

@@ -16,6 +16,7 @@
 """The cgsnapshots api."""
 
 from oslo_log import log as logging
+import six
 import webob
 from webob import exc
 
@@ -67,9 +68,8 @@ class CgsnapshotsController(wsgi.Controller):
             self.cgsnapshot_api.delete_cgsnapshot(context, cgsnapshot)
         except exception.CgSnapshotNotFound as error:
             raise exc.HTTPNotFound(explanation=error.msg)
-        except exception.InvalidCgSnapshot:
+        except exception.InvalidCgSnapshot as e:
-            msg = _("Invalid cgsnapshot")
+            raise exc.HTTPBadRequest(explanation=six.text_type(e))
-            raise exc.HTTPBadRequest(explanation=msg)
         except Exception:
             msg = _("Failed cgsnapshot")
             raise exc.HTTPBadRequest(explanation=msg)

cinder/consistencygroup/api.py

@@ -25,6 +25,7 @@ from oslo_log import log as logging
 from oslo_utils import excutils
 from oslo_utils import timeutils
 
+from cinder import db
 from cinder.db import base
 from cinder import exception
 from cinder.i18n import _, _LE, _LW
@@ -172,32 +173,6 @@ class API(base.Base):
     def create_from_src(self, context, name, description=None,
                         cgsnapshot_id=None, source_cgid=None):
         check_policy(context, 'create')
-        cgsnapshot = None
-        orig_cg = None
-        if cgsnapshot_id:
-            try:
-                cgsnapshot = objects.CGSnapshot.get_by_id(context,
-                                                          cgsnapshot_id)
-            except exception.CgSnapshotNotFound:
-                with excutils.save_and_reraise_exception():
-                    LOG.error(_LE("CG snapshot %(cgsnap)s not found when "
-                                  "creating consistency group %(cg)s from "
-                                  "source."),
-                              {'cg': name, 'cgsnap': cgsnapshot_id})
-            else:
-                orig_cg = cgsnapshot.consistencygroup
-
-        source_cg = None
-        if source_cgid:
-            try:
-                source_cg = objects.ConsistencyGroup.get_by_id(context,
-                                                               source_cgid)
-            except exception.ConsistencyGroupNotFound:
-                with excutils.save_and_reraise_exception():
-                    LOG.error(_LE("Source CG %(source_cg)s not found when "
-                                  "creating consistency group %(cg)s from "
-                                  "source."),
-                              {'cg': name, 'source_cg': source_cgid})
 
         kwargs = {
             'user_id': context.user_id,
@@ -209,20 +184,21 @@ class API(base.Base):
             'source_cgid': source_cgid,
         }
 
-        if orig_cg:
-            kwargs['volume_type_id'] = orig_cg.volume_type_id
-            kwargs['availability_zone'] = orig_cg.availability_zone
-            kwargs['host'] = orig_cg.host
-
-        if source_cg:
-            kwargs['volume_type_id'] = source_cg.volume_type_id
-            kwargs['availability_zone'] = source_cg.availability_zone
-            kwargs['host'] = source_cg.host
-
         group = None
         try:
             group = objects.ConsistencyGroup(context=context, **kwargs)
-            group.create()
+            group.create(cg_snap_id=cgsnapshot_id, cg_id=source_cgid)
+        except exception.ConsistencyGroupNotFound:
+            with excutils.save_and_reraise_exception():
+                LOG.error(_LE("Source CG %(source_cg)s not found when "
+                              "creating consistency group %(cg)s from "
+                              "source."),
+                          {'cg': name, 'source_cg': source_cgid})
+        except exception.CgSnapshotNotFound:
+            with excutils.save_and_reraise_exception():
+                LOG.error(_LE("CG snapshot %(cgsnap)s not found when creating "
+                              "consistency group %(cg)s from source."),
+                          {'cg': name, 'cgsnap': cgsnapshot_id})
         except Exception:
             with excutils.save_and_reraise_exception():
                 LOG.error(_LE("Error occurred when creating consistency group"
@@ -237,15 +213,16 @@ class API(base.Base):
             LOG.error(msg)
             raise exception.InvalidConsistencyGroup(reason=msg)
 
-        if cgsnapshot:
+        if cgsnapshot_id:
-            self._create_cg_from_cgsnapshot(context, group, cgsnapshot)
+            self._create_cg_from_cgsnapshot(context, group, cgsnapshot_id)
-        elif source_cg:
+        elif source_cgid:
-            self._create_cg_from_source_cg(context, group, source_cg)
+            self._create_cg_from_source_cg(context, group, source_cgid)
 
         return group
 
-    def _create_cg_from_cgsnapshot(self, context, group, cgsnapshot):
+    def _create_cg_from_cgsnapshot(self, context, group, cgsnapshot_id):
         try:
+            cgsnapshot = objects.CGSnapshot.get_by_id(context, cgsnapshot_id)
             snapshots = objects.SnapshotList.get_all_for_cgsnapshot(
                 context, cgsnapshot.id)
 
@@ -305,8 +282,10 @@ class API(base.Base):
         self.volume_rpcapi.create_consistencygroup_from_src(
             context, group, cgsnapshot)
 
-    def _create_cg_from_source_cg(self, context, group, source_cg):
+    def _create_cg_from_source_cg(self, context, group, source_cgid):
         try:
+            source_cg = objects.ConsistencyGroup.get_by_id(context,
+                                                           source_cgid)
             source_vols = self.db.volume_get_all_by_group(context,
                                                           source_cg.id)
 
@@ -448,59 +427,43 @@ class API(base.Base):
 
             return
 
-        if not force and group.status not in (
+        if force:
-                [c_fields.ConsistencyGroupStatus.AVAILABLE,
+            expected = {}
-                 c_fields.ConsistencyGroupStatus.ERROR]):
+        else:
-            msg = _("Consistency group status must be available or error, "
+            expected = {'status': (c_fields.ConsistencyGroupStatus.AVAILABLE,
-                    "but current status is: %s") % group.status
+                                   c_fields.ConsistencyGroupStatus.ERROR)}
+        filters = [~db.cg_has_cgsnapshot_filter(),
+                   ~db.cg_has_volumes_filter(attached_or_with_snapshots=force),
+                   ~db.cg_creating_from_src(cg_id=group.id)]
+        values = {'status': c_fields.ConsistencyGroupStatus.DELETING}
+
+        if not group.conditional_update(values, expected, filters):
+            if force:
+                reason = _('Consistency group must not have attached volumes, '
+                           'volumes with snapshots, or dependent cgsnapshots')
+            else:
+                reason = _('Consistency group status must be available or '
+                           'error and must not have volumes or dependent '
+                           'cgsnapshots')
+            msg = (_('Cannot delete consistency group %(id)s. %(reason)s, and '
+                     'it cannot be the source for an ongoing CG or CG '
+                     'Snapshot creation.')
+                   % {'id': group.id, 'reason': reason})
             raise exception.InvalidConsistencyGroup(reason=msg)
-
-        cgsnapshots = objects.CGSnapshotList.get_all_by_group(
-            context.elevated(), group.id)
-        if cgsnapshots:
-            msg = _("Consistency group %s still has dependent "
-                    "cgsnapshots.") % group.id
-            LOG.error(msg)
-            raise exception.InvalidConsistencyGroup(reason=msg)
-
-        volumes = self.db.volume_get_all_by_group(context.elevated(),
-                                                  group.id)
-
-        if volumes and not force:
-            msg = _("Consistency group %s still contains volumes. "
-                    "The force flag is required to delete it.") % group.id
-            LOG.error(msg)
-            raise exception.InvalidConsistencyGroup(reason=msg)
-
-        for volume in volumes:
-            if volume['attach_status'] == "attached":
-                msg = _("Volume in consistency group %s is attached. "
-                        "Need to detach first.") % group.id
-                LOG.error(msg)
-                raise exception.InvalidConsistencyGroup(reason=msg)
-
-            snapshots = objects.SnapshotList.get_all_for_volume(context,
-                                                                volume['id'])
-            if snapshots:
-                msg = _("Volume in consistency group still has "
-                        "dependent snapshots.")
-                LOG.error(msg)
-                raise exception.InvalidConsistencyGroup(reason=msg)
-
-        group.status = c_fields.ConsistencyGroupStatus.DELETING
-        group.terminated_at = timeutils.utcnow()
-        group.save()
-
         self.volume_rpcapi.delete_consistencygroup(context, group)
 
+    def _check_update(self, group, name, description, add_volumes,
+                      remove_volumes):
+        if not (name or description or add_volumes or remove_volumes):
+            msg = (_("Cannot update consistency group %(group_id)s "
+                     "because no valid name, description, add_volumes, "
+                     "or remove_volumes were provided.") %
+                   {'group_id': group.id})
+            raise exception.InvalidConsistencyGroup(reason=msg)
+
     def update(self, context, group, name, description,
                add_volumes, remove_volumes):
         """Update consistency group."""
-        if group.status != c_fields.ConsistencyGroupStatus.AVAILABLE:
-            msg = _("Consistency group status must be available, "
-                    "but current status is: %s.") % group.status
-            raise exception.InvalidConsistencyGroup(reason=msg)
-
         add_volumes_list = []
         remove_volumes_list = []
         if add_volumes:
@@ -519,33 +482,16 @@ class API(base.Base):
                     "list.") % invalid_uuids
             raise exception.InvalidVolume(reason=msg)
 
-        volumes = self.db.volume_get_all_by_group(context, group.id)
-
         # Validate name.
-        if not name or name == group.name:
+        if name == group.name:
             name = None
 
         # Validate description.
-        if not description or description == group.description:
+        if description == group.description:
             description = None
 
-        # Validate volumes in add_volumes and remove_volumes.
+        self._check_update(group, name, description, add_volumes,
-        add_volumes_new = ""
+                           remove_volumes)
-        remove_volumes_new = ""
-        if add_volumes_list:
-            add_volumes_new = self._validate_add_volumes(
-                context, volumes, add_volumes_list, group)
-        if remove_volumes_list:
-            remove_volumes_new = self._validate_remove_volumes(
-                volumes, remove_volumes_list, group)
-
-        if (not name and not description and not add_volumes_new and
-                not remove_volumes_new):
-            msg = (_("Cannot update consistency group %(group_id)s "
-                     "because no valid name, description, add_volumes, "
-                     "or remove_volumes were provided.") %
-                   {'group_id': group.id})
-            raise exception.InvalidConsistencyGroup(reason=msg)
 
         fields = {'updated_at': timeutils.utcnow()}
 
@@ -555,14 +501,43 @@ class API(base.Base):
             fields['name'] = name
         if description:
             fields['description'] = description
-        if not add_volumes_new and not remove_volumes_new:
-            # Only update name or description. Set status to available.
-            fields['status'] = 'available'
-        else:
-            fields['status'] = 'updating'
 
-        group.update(fields)
+        # NOTE(geguileo): We will use the updating status in the CG as a lock
-        group.save()
+        # mechanism to prevent volume add/remove races with other API, while we
+        # figure out if we really need to add or remove volumes.
+        if add_volumes or remove_volumes:
+            fields['status'] = c_fields.ConsistencyGroupStatus.UPDATING
+
+            # We cannot modify the members of this CG if the CG is being used
+            # to create another CG or a CGsnapshot is being created
+            filters = [~db.cg_creating_from_src(cg_id=group.id),
+                       ~db.cgsnapshot_creating_from_src()]
+        else:
+            filters = []
+
+        expected = {'status': c_fields.ConsistencyGroupStatus.AVAILABLE}
+        if not group.conditional_update(fields, expected, filters):
+            msg = _("Cannot update consistency group %s, status must be "
+                    "available, and it cannot be the source for an ongoing "
+                    "CG or CG Snapshot creation.") % group.id
+            raise exception.InvalidConsistencyGroup(reason=msg)
+
+        # Now the CG is "locked" for updating
+        try:
+            # Validate volumes in add_volumes and remove_volumes.
+            add_volumes_new = self._validate_add_volumes(
+                context, group.volumes, add_volumes_list, group)
+            remove_volumes_new = self._validate_remove_volumes(
+                group.volumes, remove_volumes_list, group)
+
+            self._check_update(group, name, description, add_volumes_new,
+                               remove_volumes_new)
+        except Exception:
+            # If we have an error on the volume_lists we must return status to
+            # available as we were doing before removing API races
+            with excutils.save_and_reraise_exception():
+                group.status = c_fields.ConsistencyGroupStatus.AVAILABLE
+                group.save()
 
         # Do an RPC call only if the update request includes
         # adding/removing volumes. add_volumes_new and remove_volumes_new
@@ -573,9 +548,16 @@ class API(base.Base):
                 context, group,
                 add_volumes=add_volumes_new,
                 remove_volumes=remove_volumes_new)
+        # If there are no new volumes to add or remove and we had changed
+        # the status to updating, turn it back to available
+        elif group.status == c_fields.ConsistencyGroupStatus.UPDATING:
+            group.status = c_fields.ConsistencyGroupStatus.AVAILABLE
+            group.save()
 
     def _validate_remove_volumes(self, volumes, remove_volumes_list, group):
         # Validate volumes in remove_volumes.
+        if not remove_volumes_list:
+            return None
         remove_volumes_new = ""
         for volume in volumes:
             if volume['id'] in remove_volumes_list:
@@ -606,6 +588,8 @@ class API(base.Base):
         return remove_volumes_new
 
     def _validate_add_volumes(self, context, volumes, add_volumes_list, group):
+        if not add_volumes_list:
+            return None
         add_volumes_new = ""
         for volume in volumes:
             if volume['id'] in add_volumes_list:
@@ -715,19 +699,6 @@ class API(base.Base):
         return groups
 
     def create_cgsnapshot(self, context, group, name, description):
-        return self._create_cgsnapshot(context, group, name, description)
-
-    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,
@@ -744,13 +715,16 @@ class API(base.Base):
 
             snap_name = cgsnapshot.name
             snap_desc = cgsnapshot.description
-            self.volume_api.create_snapshots_in_db(
+            with group.obj_as_admin():
-                context, volumes, snap_name, snap_desc, True, cgsnapshot_id)
+                self.volume_api.create_snapshots_in_db(
+                    context, group.volumes, snap_name, snap_desc, True,
+                    cgsnapshot_id)
 
         except Exception:
             with excutils.save_and_reraise_exception():
                 try:
-                    if cgsnapshot:
+                    # If the cgsnapshot has been created
+                    if cgsnapshot.obj_attr_is_set('id'):
                         cgsnapshot.destroy()
                 finally:
                     LOG.error(_LE("Error occurred when creating cgsnapshot"
@@ -761,11 +735,15 @@ class API(base.Base):
         return cgsnapshot
 
     def delete_cgsnapshot(self, context, cgsnapshot, force=False):
-        if cgsnapshot.status not in ["available", "error"]:
+        values = {'status': 'deleting'}
-            msg = _("Cgsnapshot status must be available or error")
+        expected = {'status': ('available', 'error')}
+        filters = [~db.cg_creating_from_src(cgsnapshot_id=cgsnapshot.id)]
+        res = cgsnapshot.conditional_update(values, expected, filters)
+
+        if not res:
+            msg = _('CgSnapshot status must be available or error, and no CG '
+                    'can be currently using it as source for its creation.')
             raise exception.InvalidCgSnapshot(reason=msg)
-        cgsnapshot.update({'status': 'deleting'})
-        cgsnapshot.save()
         self.volume_rpcapi.delete_cgsnapshot(context.elevated(), cgsnapshot)
 
     def update_cgsnapshot(self, context, cgsnapshot, fields):

cinder/db/api.py

@@ -992,9 +992,9 @@ def consistencygroup_get_all(context, filters=None, marker=None, limit=None,
                                          sort_dirs=sort_dirs)
 
 
-def consistencygroup_create(context, values):
+def consistencygroup_create(context, values, cg_snap_id=None, cg_id=None):
     """Create a consistencygroup from the values dictionary."""
-    return IMPL.consistencygroup_create(context, values)
+    return IMPL.consistencygroup_create(context, values, cg_snap_id, cg_id)
 
 
 def consistencygroup_get_all_by_project(context, project_id, filters=None,
@@ -1022,6 +1022,18 @@ def consistencygroup_destroy(context, consistencygroup_id):
     return IMPL.consistencygroup_destroy(context, consistencygroup_id)
 
 
+def cg_has_cgsnapshot_filter():
+    return IMPL.cg_has_cgsnapshot_filter()
+
+
+def cg_has_volumes_filter(attached_or_with_snapshots=False):
+    return IMPL.cg_has_volumes_filter(attached_or_with_snapshots)
+
+
+def cg_creating_from_src(cg_id=None, cgsnapshot_id=None):
+    return IMPL.cg_creating_from_src(cg_id, cgsnapshot_id)
+
+
 ###################
 
 
@@ -1063,6 +1075,13 @@ def cgsnapshot_destroy(context, cgsnapshot_id):
     return IMPL.cgsnapshot_destroy(context, cgsnapshot_id)
 
 
+def cgsnapshot_creating_from_src():
+    return IMPL.cgsnapshot_creating_from_src()
+
+
+###################
+
+
 def purge_deleted_rows(context, age_in_days):
     """Purge deleted rows older than given age from cinder tables
 

cinder/db/sqlalchemy/api.py

@@ -47,6 +47,7 @@ from sqlalchemy.orm import joinedload, joinedload_all
 from sqlalchemy.orm import RelationshipProperty
 from sqlalchemy.schema import Table
 from sqlalchemy import sql
+from sqlalchemy.sql.expression import bindparam
 from sqlalchemy.sql.expression import desc
 from sqlalchemy.sql.expression import literal_column
 from sqlalchemy.sql.expression import true
@@ -4131,16 +4132,49 @@ def consistencygroup_get_all_by_project(context, project_id, filters=None,
 
 @handle_db_data_error
 @require_context
-def consistencygroup_create(context, values):
+def consistencygroup_create(context, values, cg_snap_id=None, cg_id=None):
-    consistencygroup = models.ConsistencyGroup()
+    cg_model = models.ConsistencyGroup
+
+    values = values.copy()
     if not values.get('id'):
         values['id'] = str(uuid.uuid4())
 
     session = get_session()
     with session.begin():
-        consistencygroup.update(values)
+        if cg_snap_id:
-        session.add(consistencygroup)
+            conditions = [cg_model.id == models.Cgsnapshot.consistencygroup_id,
+                          models.Cgsnapshot.id == cg_snap_id]
+        elif cg_id:
+            conditions = [cg_model.id == cg_id]
+        else:
+            conditions = None
 
+        if conditions:
+            # We don't want duplicated field values
+            values.pop('volume_type_id', None)
+            values.pop('availability_zone', None)
+            values.pop('host', None)
+
+            sel = session.query(cg_model.volume_type_id,
+                                cg_model.availability_zone,
+                                cg_model.host,
+                                *(bindparam(k, v) for k, v in values.items())
+                                ).filter(*conditions)
+            names = ['volume_type_id', 'availability_zone', 'host']
+            names.extend(values.keys())
+            insert_stmt = cg_model.__table__.insert().from_select(names, sel)
+            result = session.execute(insert_stmt)
+            # If we couldn't insert the row because of the conditions raise
+            # the right exception
+            if not result.rowcount:
+                if cg_id:
+                    raise exception.ConsistencyGroupNotFound(
+                        consistencygroup_id=cg_id)
+                raise exception.CgSnapshotNotFound(cgsnapshot_id=cg_snap_id)
+        else:
+            consistencygroup = cg_model()
+            consistencygroup.update(values)
+            session.add(consistencygroup)
         return _consistencygroup_get(context, values['id'], session=session)
 
 
@@ -4175,6 +4209,36 @@ def consistencygroup_destroy(context, consistencygroup_id):
                     'updated_at': literal_column('updated_at')})
 
 
+def cg_has_cgsnapshot_filter():
+    return sql.exists().where(and_(
+        models.Cgsnapshot.consistencygroup_id == models.ConsistencyGroup.id,
+        ~models.Cgsnapshot.deleted))
+
+
+def cg_has_volumes_filter(attached_or_with_snapshots=False):
+    query = sql.exists().where(
+        and_(models.Volume.consistencygroup_id == models.ConsistencyGroup.id,
+             ~models.Volume.deleted))
+
+    if attached_or_with_snapshots:
+        query = query.where(or_(
+            models.Volume.attach_status == 'attached',
+            sql.exists().where(
+                and_(models.Volume.id == models.Snapshot.volume_id,
+                     ~models.Snapshot.deleted))))
+    return query
+
+
+def cg_creating_from_src(cg_id=None, cgsnapshot_id=None):
+    model = aliased(models.ConsistencyGroup)
+    conditions = [~model.deleted, model.status == 'creating']
+    if cg_id:
+        conditions.append(model.source_cgid == cg_id)
+    if cgsnapshot_id:
+        conditions.append(model.cgsnapshot_id == cgsnapshot_id)
+    return sql.exists().where(and_(*conditions))
+
+
 ###############################
 
 
@@ -4247,15 +4311,43 @@ def cgsnapshot_get_all_by_project(context, project_id, filters=None):
 @handle_db_data_error
 @require_context
 def cgsnapshot_create(context, values):
-    cgsnapshot = models.Cgsnapshot()
     if not values.get('id'):
         values['id'] = str(uuid.uuid4())
 
+    cg_id = values.get('consistencygroup_id')
     session = get_session()
+    model = models.Cgsnapshot
     with session.begin():
-        cgsnapshot.update(values)
+        if cg_id:
-        session.add(cgsnapshot)
+            # There has to exist at least 1 volume in the CG and the CG cannot
+            # be updating the composing volumes or being created.
+            conditions = [
+                sql.exists().where(and_(
+                    ~models.Volume.deleted,
+                    models.Volume.consistencygroup_id == cg_id)),
+                ~models.ConsistencyGroup.deleted,
+                models.ConsistencyGroup.id == cg_id,
+                ~models.ConsistencyGroup.status.in_(('creating', 'updating'))]
 
+            # NOTE(geguileo): We build a "fake" from_select clause instead of
+            # using transaction isolation on the session because we would need
+            # SERIALIZABLE level and that would have a considerable performance
+            # penalty.
+            binds = (bindparam(k, v) for k, v in values.items())
+            sel = session.query(*binds).filter(*conditions)
+            insert_stmt = model.__table__.insert().from_select(values.keys(),
+                                                               sel)
+            result = session.execute(insert_stmt)
+            # If we couldn't insert the row because of the conditions raise
+            # the right exception
+            if not result.rowcount:
+                msg = _("Source CG cannot be empty or in 'creating' or "
+                        "'updating' state. No cgsnapshot will be created.")
+                raise exception.InvalidConsistencyGroup(reason=msg)
+        else:
+            cgsnapshot = model()
+            cgsnapshot.update(values)
+            session.add(cgsnapshot)
         return _cgsnapshot_get(context, values['id'], session=session)
 
 
@@ -4289,6 +4381,16 @@ def cgsnapshot_destroy(context, cgsnapshot_id):
                     'updated_at': literal_column('updated_at')})
 
 
+def cgsnapshot_creating_from_src():
+    return sql.exists().where(and_(
+        models.Cgsnapshot.consistencygroup_id == models.ConsistencyGroup.id,
+        ~models.Cgsnapshot.deleted,
+        models.Cgsnapshot.status == 'creating'))
+
+
+###############################
+
+
 @require_admin_context
 def purge_deleted_rows(context, age_in_days):
     """Purge deleted rows older than age from cinder tables."""

cinder/objects/consistencygroup.py

@@ -77,7 +77,13 @@ class ConsistencyGroup(base.CinderPersistentObject, base.CinderObject,
         return consistencygroup
 
     @base.remotable
-    def create(self):
+    def create(self, cg_snap_id=None, cg_id=None):
+        """Create a consistency group.
+
+        If cg_snap_id or cg_id are specified then volume_type_id,
+        availability_zone, and host will be taken from the source Consistency
+        Group.
+        """
         if self.obj_attr_is_set('id'):
             raise exception.ObjectActionError(action='create',
                                               reason=_('already_created'))
@@ -92,7 +98,9 @@ class ConsistencyGroup(base.CinderPersistentObject, base.CinderObject,
                                               reason=_('volumes assigned'))
 
         db_consistencygroups = db.consistencygroup_create(self._context,
-                                                          updates)
+                                                          updates,
+                                                          cg_snap_id,
+                                                          cg_id)
         self._from_db_object(self._context, self, db_consistencygroups)
 
     def obj_load_attr(self, attrname):

cinder/tests/unit/api/contrib/test_cgsnapshots.py

@@ -284,10 +284,7 @@ class CgsnapshotsAPITestCase(test.TestCase):
                          res_dict['itemNotFound']['message'])
         consistencygroup.destroy()
 
-    @mock.patch.object(objects.CGSnapshot, 'create')
+    def test_create_cgsnapshot_from_empty_consistencygroup(self):
-    def test_create_cgsnapshot_from_empty_consistencygroup(
-            self,
-            mock_cgsnapshot_create):
         consistencygroup = utils.create_consistencygroup(self.context)
 
         body = {"cgsnapshot": {"name": "cg1",
@@ -305,13 +302,15 @@ class CgsnapshotsAPITestCase(test.TestCase):
 
         self.assertEqual(400, res.status_int)
         self.assertEqual(400, res_dict['badRequest']['code'])
-        self.assertEqual('Invalid ConsistencyGroup: Consistency group is '
+        expected = ("Invalid ConsistencyGroup: Source CG cannot be empty or "
-                         'empty. No cgsnapshot will be created.',
+                    "in 'creating' or 'updating' state. No cgsnapshot will be "
-                         res_dict['badRequest']['message'])
+                    "created.")
+        self.assertEqual(expected, res_dict['badRequest']['message'])
 
         # If failed to create cgsnapshot, its DB object should not be created
-        self.assertFalse(mock_cgsnapshot_create.called)
+        self.assertListEqual(
-
+            [],
+            list(objects.CGSnapshotList.get_all(self.context)))
         consistencygroup.destroy()
 
     def test_delete_cgsnapshot_available(self):
@@ -339,6 +338,34 @@ class CgsnapshotsAPITestCase(test.TestCase):
                           volume_id)
         consistencygroup.destroy()
 
+    def test_delete_cgsnapshot_available_used_as_source(self):
+        consistencygroup = utils.create_consistencygroup(self.context)
+        volume_id = utils.create_volume(
+            self.context,
+            consistencygroup_id=consistencygroup.id)['id']
+        cgsnapshot = utils.create_cgsnapshot(
+            self.context,
+            consistencygroup_id=consistencygroup.id,
+            status='available')
+
+        cg2 = utils.create_consistencygroup(
+            self.context, status='creating', cgsnapshot_id=cgsnapshot.id)
+        req = webob.Request.blank('/v2/fake/cgsnapshots/%s' %
+                                  cgsnapshot.id)
+        req.method = 'DELETE'
+        req.headers['Content-Type'] = 'application/json'
+        res = req.get_response(fakes.wsgi_app())
+
+        cgsnapshot = objects.CGSnapshot.get_by_id(self.context, cgsnapshot.id)
+        self.assertEqual(400, res.status_int)
+        self.assertEqual('available', cgsnapshot.status)
+
+        cgsnapshot.destroy()
+        db.volume_destroy(context.get_admin_context(),
+                          volume_id)
+        consistencygroup.destroy()
+        cg2.destroy()
+
     def test_delete_cgsnapshot_with_cgsnapshot_NotFound(self):
         req = webob.Request.blank('/v2/%s/cgsnapshots/%s' %
                                   (fake.PROJECT_ID, fake.WILL_NOT_BE_FOUND_ID))
@@ -373,8 +400,10 @@ class CgsnapshotsAPITestCase(test.TestCase):
 
         self.assertEqual(400, res.status_int)
         self.assertEqual(400, res_dict['badRequest']['code'])
-        self.assertEqual('Invalid cgsnapshot',
+        expected = ('Invalid CgSnapshot: CgSnapshot status must be available '
-                         res_dict['badRequest']['message'])
+                    'or error, and no CG can be currently using it as source '
+                    'for its creation.')
+        self.assertEqual(expected, res_dict['badRequest']['message'])
 
         cgsnapshot.destroy()
         db.volume_destroy(context.get_admin_context(),

cinder/tests/unit/api/contrib/test_consistencygroups.py

@@ -58,7 +58,8 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
             volume_type_id=fake.VOLUME_TYPE_ID,
             availability_zone='az1',
             host='fakehost',
-            status=fields.ConsistencyGroupStatus.CREATING):
+            status=fields.ConsistencyGroupStatus.CREATING,
+            **kwargs):
         """Create a consistency group object."""
         ctxt = ctxt or self.ctxt
         consistencygroup = objects.ConsistencyGroup(ctxt)
@@ -70,6 +71,7 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
         consistencygroup.volume_type_id = volume_type_id
         consistencygroup.host = host
         consistencygroup.status = status
+        consistencygroup.update(kwargs)
         consistencygroup.create()
         return consistencygroup
 
@@ -385,7 +387,7 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
 
         # Create volume type
         vol_type = 'test'
-        db.volume_type_create(context.get_admin_context(),
+        db.volume_type_create(self.ctxt,
                               {'name': vol_type, 'extra_specs': {}})
 
         body = {"consistencygroup": {"name": "cg1",
@@ -405,7 +407,7 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
         self.assertTrue(mock_validate.called)
 
         group_id = res_dict['consistencygroup']['id']
-        cg = objects.ConsistencyGroup.get_by_id(context.get_admin_context(),
+        cg = objects.ConsistencyGroup.get_by_id(self.ctxt,
                                                 group_id)
         cg.destroy()
 
@@ -433,7 +435,44 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
                                   (fake.PROJECT_ID, consistencygroup.id))
         req.method = 'POST'
         req.headers['Content-Type'] = 'application/json'
-        body = {"consistencygroup": {"force": True}}
+        req.body = jsonutils.dump_as_bytes({})
+        res = req.get_response(fakes.wsgi_app())
+
+        consistencygroup = objects.ConsistencyGroup.get_by_id(
+            self.ctxt, consistencygroup.id)
+        self.assertEqual(202, res.status_int)
+        self.assertEqual('deleting', consistencygroup.status)
+
+        consistencygroup.destroy()
+
+    def test_delete_consistencygroup_available_used_as_source(self):
+        consistencygroup = self._create_consistencygroup(
+            status=fields.ConsistencyGroupStatus.AVAILABLE)
+        req = webob.Request.blank('/v2/%s/consistencygroups/%s/delete' %
+                                  (fake.PROJECT_ID, consistencygroup.id))
+        cg2 = self._create_consistencygroup(
+            status=fields.ConsistencyGroupStatus.CREATING,
+            source_cgid=consistencygroup.id)
+        req.method = 'POST'
+        req.headers['Content-Type'] = 'application/json'
+        req.body = jsonutils.dump_as_bytes({})
+        res = req.get_response(fakes.wsgi_app())
+
+        consistencygroup = objects.ConsistencyGroup.get_by_id(
+            self.ctxt, consistencygroup.id)
+        self.assertEqual(400, res.status_int)
+        self.assertEqual('available', consistencygroup.status)
+
+        consistencygroup.destroy()
+        cg2.destroy()
+
+    def test_delete_consistencygroup_available_no_force(self):
+        consistencygroup = self._create_consistencygroup(status='available')
+        req = webob.Request.blank('/v2/%s/consistencygroups/%s/delete' %
+                                  (fake.PROJECT_ID, consistencygroup.id))
+        req.method = 'POST'
+        req.headers['Content-Type'] = 'application/json'
+        body = {"consistencygroup": {"force": False}}
         req.body = jsonutils.dump_as_bytes(body)
         res = req.get_response(fakes.wsgi_app(
             fake_auth_context=self.user_ctxt))
@@ -463,25 +502,26 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
                          res_dict['itemNotFound']['message'])
 
     def test_delete_consistencygroup_with_Invalidconsistencygroup(self):
+        consistencygroup = self._create_consistencygroup(
+            status=fields.ConsistencyGroupStatus.IN_USE)
+        self._assert_deleting_result_400(consistencygroup.id)
+        consistencygroup.destroy()
+
+    def test_delete_consistencygroup_invalid_force(self):
         consistencygroup = self._create_consistencygroup(
             status=fields.ConsistencyGroupStatus.IN_USE)
         req = webob.Request.blank('/v2/%s/consistencygroups/%s/delete' %
                                   (fake.PROJECT_ID, consistencygroup.id))
         req.method = 'POST'
         req.headers['Content-Type'] = 'application/json'
-        body = {"consistencygroup": {"force": False}}
+        body = {"consistencygroup": {"force": True}}
         req.body = jsonutils.dump_as_bytes(body)
-        res = req.get_response(fakes.wsgi_app(
+        res = req.get_response(fakes.wsgi_app())
-            fake_auth_context=self.user_ctxt))
-        res_dict = jsonutils.loads(res.body)
 
-        self.assertEqual(400, res.status_int)
+        consistencygroup = objects.ConsistencyGroup.get_by_id(
-        self.assertEqual(400, res_dict['badRequest']['code'])
+            self.ctxt, consistencygroup.id)
-        msg = (_('Invalid ConsistencyGroup: Consistency group status must be '
+        self.assertEqual(202, res.status_int)
-                 'available or error, but current status is: in-use'))
+        self.assertEqual('deleting', consistencygroup.status)
-        self.assertEqual(msg, res_dict['badRequest']['message'])
-
-        consistencygroup.destroy()
 
     def test_delete_consistencygroup_no_host(self):
         consistencygroup = self._create_consistencygroup(
@@ -574,6 +614,116 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
 
         self.assertEqual(400, res.status_int)
 
+    def _assert_deleting_result_400(self, cg_id, force=False):
+        req = webob.Request.blank('/v2/%s/consistencygroups/%s/delete' %
+                                  (fake.PROJECT_ID, cg_id))
+        req.method = 'POST'
+        req.headers['Content-Type'] = 'application/json'
+        body = {"consistencygroup": {"force": force}}
+        req.body = jsonutils.dump_as_bytes(body)
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(400, res.status_int)
+
+        if force:
+            reason = _('Consistency group must not have attached volumes, '
+                       'volumes with snapshots, or dependent cgsnapshots')
+        else:
+            reason = _('Consistency group status must be available or '
+                       'error and must not have volumes or dependent '
+                       'cgsnapshots')
+        msg = (_('Invalid ConsistencyGroup: Cannot delete consistency group '
+                 '%(id)s. %(reason)s, and it cannot be the source for an '
+                 'ongoing CG or CG Snapshot creation.')
+               % {'id': cg_id, 'reason': reason})
+
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(400, res_dict['badRequest']['code'])
+        self.assertEqual(msg, res_dict['badRequest']['message'])
+
+    def test_delete_consistencygroup_with_volumes(self):
+        consistencygroup = self._create_consistencygroup(status='available')
+        utils.create_volume(self.ctxt, consistencygroup_id=consistencygroup.id,
+                            testcase_instance=self)
+        self._assert_deleting_result_400(consistencygroup.id)
+        consistencygroup.destroy()
+
+    def test_delete_consistencygroup_with_cgsnapshot(self):
+        consistencygroup = self._create_consistencygroup(status='available')
+        # If we don't add a volume to the CG the cgsnapshot creation will fail
+        utils.create_volume(self.ctxt,
+                            consistencygroup_id=consistencygroup.id,
+                            testcase_instance=self)
+        cg_snap = utils.create_cgsnapshot(self.ctxt, consistencygroup.id)
+        self._assert_deleting_result_400(consistencygroup.id)
+        cg_snap.destroy()
+        consistencygroup.destroy()
+
+    def test_delete_consistencygroup_with_cgsnapshot_force(self):
+        consistencygroup = self._create_consistencygroup(status='available')
+        # If we don't add a volume to the CG the cgsnapshot creation will fail
+        utils.create_volume(self.ctxt,
+                            consistencygroup_id=consistencygroup.id,
+                            testcase_instance=self)
+        cg_snap = utils.create_cgsnapshot(self.ctxt, consistencygroup.id)
+        self._assert_deleting_result_400(consistencygroup.id, force=True)
+        cg_snap.destroy()
+        consistencygroup.destroy()
+
+    def test_delete_consistencygroup_force_with_volumes(self):
+        consistencygroup = self._create_consistencygroup(status='available')
+        utils.create_volume(self.ctxt, consistencygroup_id=consistencygroup.id,
+                            testcase_instance=self)
+
+        req = webob.Request.blank('/v2/%s/consistencygroups/%s/delete' %
+                                  (fake.PROJECT_ID, consistencygroup.id))
+        req.method = 'POST'
+        req.headers['Content-Type'] = 'application/json'
+        body = {"consistencygroup": {"force": True}}
+        req.body = jsonutils.dump_as_bytes(body)
+        res = req.get_response(fakes.wsgi_app())
+
+        consistencygroup = objects.ConsistencyGroup.get_by_id(
+            self.ctxt, consistencygroup.id)
+        self.assertEqual(202, res.status_int)
+        self.assertEqual('deleting', consistencygroup.status)
+        consistencygroup.destroy()
+
+    def test_delete_consistencygroup_force_with_attached_volumes(self):
+        consistencygroup = self._create_consistencygroup(status='available')
+        utils.create_volume(self.ctxt, consistencygroup_id=consistencygroup.id,
+                            testcase_instance=self, attach_status='attached')
+        self._assert_deleting_result_400(consistencygroup.id, force=True)
+        consistencygroup.destroy()
+
+    def test_delete_consistencygroup_force_with_volumes_with_snapshots(self):
+        consistencygroup = self._create_consistencygroup(status='available')
+        vol = utils.create_volume(self.ctxt, testcase_instance=self,
+                                  consistencygroup_id=consistencygroup.id)
+        utils.create_snapshot(self.ctxt, vol.id)
+        self._assert_deleting_result_400(consistencygroup.id, force=True)
+        consistencygroup.destroy()
+
+    def test_delete_cg_force_with_volumes_with_deleted_snapshots(self):
+        consistencygroup = self._create_consistencygroup(status='available')
+        vol = utils.create_volume(self.ctxt, testcase_instance=self,
+                                  consistencygroup_id=consistencygroup.id)
+        utils.create_snapshot(self.ctxt, vol.id, status='deleted',
+                              deleted=True, testcase_instance=self)
+
+        req = webob.Request.blank('/v2/%s/consistencygroups/%s/delete' %
+                                  (fake.PROJECT_ID, consistencygroup.id))
+        req.method = 'POST'
+        req.headers['Content-Type'] = 'application/json'
+        body = {"consistencygroup": {"force": True}}
+        req.body = jsonutils.dump_as_bytes(body)
+        res = req.get_response(fakes.wsgi_app())
+
+        consistencygroup = objects.ConsistencyGroup.get_by_id(
+            self.ctxt, consistencygroup.id)
+        self.assertEqual(202, res.status_int)
+        self.assertEqual('deleting', consistencygroup.status)
+        consistencygroup.destroy()
+
     def test_create_consistencygroup_failed_no_volume_type(self):
         name = 'cg1'
         body = {"consistencygroup": {"name": name,
@@ -601,6 +751,13 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
             status=fields.ConsistencyGroupStatus.AVAILABLE,
             host='test_host')
 
+        # We create another CG from the one we are updating to confirm that
+        # it will not affect the update if it is not CREATING
+        cg2 = self._create_consistencygroup(
+            status=fields.ConsistencyGroupStatus.AVAILABLE,
+            host='test_host',
+            source_cgid=consistencygroup.id)
+
         remove_volume_id = utils.create_volume(
             self.ctxt,
             volume_type_id=volume_type_id,
@@ -657,6 +814,96 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
                          consistencygroup.status)
 
         consistencygroup.destroy()
+        cg2.destroy()
+
+    @mock.patch(
+        'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
+    def test_update_consistencygroup_sourcing_cg(self, mock_validate):
+        volume_type_id = fake.VOLUME_TYPE_ID
+        consistencygroup = self._create_consistencygroup(
+            status=fields.ConsistencyGroupStatus.AVAILABLE,
+            host='test_host')
+
+        cg2 = self._create_consistencygroup(
+            status=fields.ConsistencyGroupStatus.CREATING,
+            host='test_host',
+            source_cgid=consistencygroup.id)
+
+        remove_volume_id = utils.create_volume(
+            self.ctxt,
+            volume_type_id=volume_type_id,
+            consistencygroup_id=consistencygroup.id)['id']
+        remove_volume_id2 = utils.create_volume(
+            self.ctxt,
+            volume_type_id=volume_type_id,
+            consistencygroup_id=consistencygroup.id)['id']
+
+        req = webob.Request.blank('/v2/%s/consistencygroups/%s/update' %
+                                  (fake.PROJECT_ID, consistencygroup.id))
+        req.method = 'PUT'
+        req.headers['Content-Type'] = 'application/json'
+        name = 'newcg'
+        description = 'New Consistency Group Description'
+        remove_volumes = remove_volume_id + "," + remove_volume_id2
+        body = {"consistencygroup": {"name": name,
+                                     "description": description,
+                                     "remove_volumes": remove_volumes, }}
+        req.body = jsonutils.dump_as_bytes(body)
+        res = req.get_response(fakes.wsgi_app())
+
+        consistencygroup = objects.ConsistencyGroup.get_by_id(
+            self.ctxt, consistencygroup.id)
+        self.assertEqual(400, res.status_int)
+        self.assertEqual(fields.ConsistencyGroupStatus.AVAILABLE,
+                         consistencygroup.status)
+
+        consistencygroup.destroy()
+        cg2.destroy()
+
+    @mock.patch(
+        'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
+    def test_update_consistencygroup_creating_cgsnapshot(self, mock_validate):
+        volume_type_id = fake.VOLUME_TYPE_ID
+        consistencygroup = self._create_consistencygroup(
+            status=fields.ConsistencyGroupStatus.AVAILABLE,
+            host='test_host')
+
+        # If we don't add a volume to the CG the cgsnapshot creation will fail
+        utils.create_volume(self.ctxt,
+                            consistencygroup_id=consistencygroup.id,
+                            testcase_instance=self)
+
+        cgsnapshot = utils.create_cgsnapshot(
+            self.ctxt, consistencygroup_id=consistencygroup.id)
+
+        add_volume_id = utils.create_volume(
+            self.ctxt,
+            volume_type_id=volume_type_id)['id']
+        add_volume_id2 = utils.create_volume(
+            self.ctxt,
+            volume_type_id=volume_type_id)['id']
+
+        req = webob.Request.blank('/v2/%s/consistencygroups/%s/update' %
+                                  (fake.PROJECT_ID, consistencygroup.id))
+        req.method = 'PUT'
+        req.headers['Content-Type'] = 'application/json'
+        name = 'newcg'
+        description = 'New Consistency Group Description'
+        add_volumes = add_volume_id + "," + add_volume_id2
+        body = {"consistencygroup": {"name": name,
+                                     "description": description,
+                                     "add_volumes": add_volumes}}
+        req.body = jsonutils.dump_as_bytes(body)
+        res = req.get_response(fakes.wsgi_app())
+
+        consistencygroup = objects.ConsistencyGroup.get_by_id(
+            self.ctxt, consistencygroup.id)
+        self.assertEqual(400, res.status_int)
+        self.assertEqual(fields.ConsistencyGroupStatus.AVAILABLE,
+                         consistencygroup.status)
+
+        consistencygroup.destroy()
+        cgsnapshot.destroy()
 
     def test_update_consistencygroup_add_volume_not_found(self):
         consistencygroup = self._create_consistencygroup(
@@ -850,9 +1097,10 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
 
         self.assertEqual(400, res.status_int)
         self.assertEqual(400, res_dict['badRequest']['code'])
-        msg = _("Invalid ConsistencyGroup: Consistency group status must be "
+        msg = (_("Invalid ConsistencyGroup: Cannot update consistency group "
-                "available, but current status is: %s.") % (
+                 "%s, status must be available, and it cannot be the source "
-            fields.ConsistencyGroupStatus.IN_USE)
+                 "for an ongoing CG or CG Snapshot creation.")
+               % consistencygroup.id)
         self.assertEqual(msg, res_dict['badRequest']['message'])
 
         consistencygroup.destroy()
@@ -1111,10 +1359,14 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
             consistencygroup_id=consistencygroup.id)['id']
 
         test_cg_name = 'test cg'
-        body = {"consistencygroup-from-src": {"name": test_cg_name,
+        body = {
-                                              "description":
+            "consistencygroup-from-src":
-                                              "Consistency Group 1",
+            {
-                                              "cgsnapshot_id": "fake_cgsnap"}}
+                "name": test_cg_name,
+                "description": "Consistency Group 1",
+                "source_cgid": fake.CGSNAPSHOT_ID
+            }
+        }
         req = webob.Request.blank('/v2/%s/consistencygroups/create_from_src' %
                                   fake.PROJECT_ID)
         req.method = 'POST'
@@ -1133,10 +1385,14 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
 
     def test_create_consistencygroup_from_src_source_cg_notfound(self):
         test_cg_name = 'test cg'
-        body = {"consistencygroup-from-src": {"name": test_cg_name,
+        body = {
-                                              "description":
+            "consistencygroup-from-src":
-                                              "Consistency Group 1",
+            {
-                                              "source_cgid": "fake_source_cg"}}
+                "name": test_cg_name,
+                "description": "Consistency Group 1",
+                "source_cgid": fake.CONSISTENCY_GROUP_ID
+            }
+        }
         req = webob.Request.blank('/v2/%s/consistencygroups/create_from_src' %
                                   fake.PROJECT_ID)
         req.method = 'POST'

cinder/tests/unit/api/fakes.py

@@ -68,7 +68,8 @@ def wsgi_app(inner_app_v2=None, fake_auth=True, fake_auth_context=None,
         if fake_auth_context is not None:
             ctxt = fake_auth_context
         else:
-            ctxt = context.RequestContext('fake', 'fake', auth_token=True)
+            ctxt = context.RequestContext(fake.USER_ID, fake.PROJECT_ID,
+                                          auth_token=True)
         api_v2 = fault.FaultWrapper(auth.InjectContext(ctxt,
                                                        inner_app_v2))
     elif use_no_auth:

cinder/tests/unit/objects/test_objects.py

@@ -28,7 +28,7 @@ object_data = {
     'BackupList': '1.0-24591dabe26d920ce0756fe64cd5f3aa',
     'CGSnapshot': '1.0-de2586a31264d7647f40c762dece9d58',
     'CGSnapshotList': '1.0-e8c3f4078cd0ee23487b34d173eec776',
-    'ConsistencyGroup': '1.2-5365b1c670adc3973f0faa54a66af243',
+    'ConsistencyGroup': '1.2-dff0647c77d1b34682c7b3af7b50588e',
     'ConsistencyGroupList': '1.1-73916823b697dfa0c7f02508d87e0f28',
     'Service': '1.3-66c8e1683f58546c54551e9ff0a3b111',
     'ServiceList': '1.1-cb758b200f0a3a90efabfc5aa2ffb627',

cinder/tests/unit/test_db_api.py

@@ -31,6 +31,7 @@ from cinder.objects import fields
 from cinder import quota
 from cinder import test
 from cinder.tests.unit import fake_constants as fake
+from cinder.tests.unit import utils
 
 THREE = 3
 THREE_HUNDREDS = 300
@@ -93,7 +94,7 @@ class ModelsObjectComparatorMixin(object):
         sort_key = lambda d: [d[k] for k in sorted(d)]
         conv_and_sort = lambda obj: sorted(map(obj_to_dict, obj), key=sort_key)
 
-        self.assertEqual(conv_and_sort(objs1), conv_and_sort(objs2))
+        self.assertListEqual(conv_and_sort(objs1), conv_and_sort(objs2))
 
     def _assertEqualListsOfPrimitivesAsSets(self, primitives1, primitives2):
         self.assertEqual(len(primitives1), len(primitives2))
@@ -1455,16 +1456,23 @@ class DBAPISnapshotTestCase(BaseTest):
 class DBAPICgsnapshotTestCase(BaseTest):
     """Tests for cinder.db.api.cgsnapshot_*."""
 
+    def _cgsnapshot_create(self, values):
+        return utils.create_cgsnapshot(self.ctxt, return_vo=False, **values)
+
     def test_cgsnapshot_get_all_by_filter(self):
-        cgsnapshot1 = db.cgsnapshot_create(self.ctxt, {'id': 1,
+        cgsnapshot1 = self._cgsnapshot_create(
-                                           'consistencygroup_id': 'g1'})
+            {'id': fake.CGSNAPSHOT_ID,
-        cgsnapshot2 = db.cgsnapshot_create(self.ctxt, {'id': 2,
+             'consistencygroup_id': fake.CONSISTENCY_GROUP_ID})
-                                           'consistencygroup_id': 'g1'})
+        cgsnapshot2 = self._cgsnapshot_create(
-        cgsnapshot3 = db.cgsnapshot_create(self.ctxt, {'id': 3,
+            {'id': fake.CGSNAPSHOT2_ID,
-                                           'consistencygroup_id': 'g2'})
+             'consistencygroup_id': fake.CONSISTENCY_GROUP_ID})
+        cgsnapshot3 = self._cgsnapshot_create(
+            {'id': fake.CGSNAPSHOT3_ID,
+             'consistencygroup_id': fake.CONSISTENCY_GROUP2_ID})
         tests = [
-            ({'consistencygroup_id': 'g1'}, [cgsnapshot1, cgsnapshot2]),
+            ({'consistencygroup_id': fake.CONSISTENCY_GROUP_ID},
-            ({'id': 3}, [cgsnapshot3]),
+             [cgsnapshot1, cgsnapshot2]),
+            ({'id': fake.CGSNAPSHOT3_ID}, [cgsnapshot3]),
             ({'fake_key': 'fake'}, [])
         ]
 
@@ -1480,17 +1488,20 @@ class DBAPICgsnapshotTestCase(BaseTest):
                                                 filters))
 
     def test_cgsnapshot_get_all_by_group(self):
-        cgsnapshot1 = db.cgsnapshot_create(self.ctxt, {'id': 1,
+        cgsnapshot1 = self._cgsnapshot_create(
-                                           'consistencygroup_id': 'g1'})
+            {'id': fake.CGSNAPSHOT_ID,
-        cgsnapshot2 = db.cgsnapshot_create(self.ctxt, {'id': 2,
+             'consistencygroup_id': fake.CONSISTENCY_GROUP_ID})
-                                           'consistencygroup_id': 'g1'})
+        cgsnapshot2 = self._cgsnapshot_create(
-        db.cgsnapshot_create(self.ctxt, {'id': 3,
+            {'id': fake.CGSNAPSHOT2_ID,
-                             'consistencygroup_id': 'g2'})
+             'consistencygroup_id': fake.CONSISTENCY_GROUP_ID})
+        self._cgsnapshot_create(
+            {'id': fake.CGSNAPSHOT3_ID,
+             'consistencygroup_id': fake.CONSISTENCY_GROUP2_ID})
         tests = [
-            ({'consistencygroup_id': 'g1'}, [cgsnapshot1, cgsnapshot2]),
+            ({'consistencygroup_id': fake.CONSISTENCY_GROUP_ID},
-            ({'id': 3}, []),
+             [cgsnapshot1, cgsnapshot2]),
-            ({'fake_key': 'fake'}, []),
+            ({'id': fake.CGSNAPSHOT3_ID}, []),
-            ({'consistencygroup_id': 'g2'}, []),
+            ({'consistencygroup_id': fake.CONSISTENCY_GROUP2_ID}, []),
             (None, [cgsnapshot1, cgsnapshot2]),
         ]
 
@@ -1498,7 +1509,7 @@ class DBAPICgsnapshotTestCase(BaseTest):
             self._assertEqualListsOfObjects(expected,
                                             db.cgsnapshot_get_all_by_group(
                                                 self.ctxt,
-                                                'g1',
+                                                fake.CONSISTENCY_GROUP_ID,
                                                 filters))
 
         db.cgsnapshot_destroy(self.ctxt, '1')
@@ -1506,18 +1517,18 @@ class DBAPICgsnapshotTestCase(BaseTest):
         db.cgsnapshot_destroy(self.ctxt, '3')
 
     def test_cgsnapshot_get_all_by_project(self):
-        cgsnapshot1 = db.cgsnapshot_create(self.ctxt,
+        cgsnapshot1 = self._cgsnapshot_create(
-                                           {'id': 1,
+            {'id': fake.CGSNAPSHOT_ID,
-                                            'consistencygroup_id': 'g1',
+             'consistencygroup_id': fake.CONSISTENCY_GROUP_ID,
-                                            'project_id': 1})
+             'project_id': fake.PROJECT_ID})
-        cgsnapshot2 = db.cgsnapshot_create(self.ctxt,
+        cgsnapshot2 = self._cgsnapshot_create(
-                                           {'id': 2,
+            {'id': fake.CGSNAPSHOT2_ID,
-                                            'consistencygroup_id': 'g1',
+             'consistencygroup_id': fake.CONSISTENCY_GROUP_ID,
-                                            'project_id': 1})
+             'project_id': fake.PROJECT_ID})
-        project_id = 1
         tests = [
-            ({'id': 1}, [cgsnapshot1]),
+            ({'id': fake.CGSNAPSHOT_ID}, [cgsnapshot1]),
-            ({'consistencygroup_id': 'g1'}, [cgsnapshot1, cgsnapshot2]),
+            ({'consistencygroup_id': fake.CONSISTENCY_GROUP_ID},
+             [cgsnapshot1, cgsnapshot2]),
             ({'fake_key': 'fake'}, [])
         ]
 
@@ -1525,7 +1536,7 @@ class DBAPICgsnapshotTestCase(BaseTest):
             self._assertEqualListsOfObjects(expected,
                                             db.cgsnapshot_get_all_by_project(
                                                 self.ctxt,
-                                                project_id,
+                                                fake.PROJECT_ID,
                                                 filters))
 
 

cinder/tests/unit/utils.py

@@ -24,6 +24,7 @@ import oslo_versionedobjects
 
 from cinder import context
 from cinder import db
+from cinder import exception
 from cinder import objects
 from cinder.objects import fields
 from cinder.tests.unit import fake_constants as fake
@@ -107,6 +108,7 @@ def create_snapshot(ctxt,
                     display_description='this is a test snapshot',
                     cgsnapshot_id = None,
                     status=fields.SnapshotStatus.CREATING,
+                    testcase_instance=None,
                     **kwargs):
     vol = db.volume_get(ctxt, volume_id)
     snap = objects.Snapshot(ctxt)
@@ -119,6 +121,14 @@ def create_snapshot(ctxt,
     snap.display_description = display_description
     snap.cgsnapshot_id = cgsnapshot_id
     snap.create()
+    # We do the update after creating the snapshot in case we want to set
+    # deleted field
+    snap.update(kwargs)
+    snap.save()
+
+    # If we get a TestCase instance we add cleanup
+    if testcase_instance:
+        testcase_instance.addCleanup(snap.destroy)
     return snap
 
 
@@ -147,9 +157,12 @@ def create_consistencygroup(ctxt,
         cg.volume_type_id = volume_type_id
     cg.cgsnapshot_id = cgsnapshot_id
     cg.source_cgid = source_cgid
-    for key in kwargs:
+    new_id = kwargs.pop('id', None)
-        setattr(cg, key, kwargs[key])
+    cg.update(kwargs)
     cg.create()
+    if new_id and new_id != cg.id:
+        db.consistencygroup_update(ctxt, cg.id, {'id': new_id})
+        cg = objects.ConsistencyGroup.get_by_id(ctxt, new_id)
     return cg
 
 
@@ -158,19 +171,40 @@ def create_cgsnapshot(ctxt,
                       name='test_cgsnapshot',
                       description='this is a test cgsnapshot',
                       status='creating',
+                      recursive_create_if_needed=True,
+                      return_vo=True,
                       **kwargs):
     """Create a cgsnapshot object in the DB."""
-    cgsnap = objects.CGSnapshot(ctxt)
+    values = {
-    cgsnap.user_id = ctxt.user_id or fake.USER_ID
+        'user_id': ctxt.user_id or fake.USER_ID,
-    cgsnap.project_id = ctxt.project_id or fake.PROJECT_ID
+        'project_id': ctxt.project_id or fake.PROJECT_ID,
-    cgsnap.status = status
+        'status': status,
-    cgsnap.name = name
+        'name': name,
-    cgsnap.description = description
+        'description': description,
-    cgsnap.consistencygroup_id = consistencygroup_id
+        'consistencygroup_id': consistencygroup_id}
-    for key in kwargs:
+    values.update(kwargs)
-        setattr(cgsnap, key, kwargs[key])
+
-    cgsnap.create()
+    if recursive_create_if_needed and consistencygroup_id:
-    return cgsnap
+        create_cg = False
+        try:
+            objects.ConsistencyGroup.get_by_id(ctxt,
+                                               consistencygroup_id)
+            create_vol = not db.volume_get_all_by_group(
+                ctxt, consistencygroup_id)
+        except exception.ConsistencyGroupNotFound:
+            create_cg = True
+            create_vol = True
+        if create_cg:
+            create_consistencygroup(ctxt, id=consistencygroup_id)
+        if create_vol:
+            create_volume(ctxt, consistencygroup_id=consistencygroup_id)
+
+    cgsnap = db.cgsnapshot_create(ctxt, values)
+
+    if not return_vo:
+        return cgsnap
+
+    return objects.CGSnapshot.get_by_id(ctxt, cgsnap.id)
 
 
 def create_backup(ctxt,

tools/lintstack.py

@@ -52,7 +52,13 @@ ignore_messages = [
 
     # Note(aarefiev): this error message is for SQLAlchemy rename calls in
     # DB migration(033_add_encryption_unique_key).
-    "Instance of 'Table' has no 'rename' member"
+    "Instance of 'Table' has no 'rename' member",
+
+    # NOTE(geguileo): these error messages are for code [E1101], and they can
+    # be ignored because a SQLAlchemy ORM class will have __table__ member
+    # during runtime.
+    "Class 'ConsistencyGroup' has no '__table__' member",
+    "Class 'Cgsnapshot' has no '__table__' member",
 ]
 
 # Note(maoy):  We ignore cinder.tests for now due to high false