Switch to using generic groups with Pure driver

This adds implementations of consistent generic volume
groups to the Pure volume drivers. They pretty much
just re-use the CG implementations, and if the group
being created isn't a CG then we bail out and allow
the generic group handling to take care of it.

Change-Id: I02080a1c2eddf93513793ae13adf88d869770a2e
Implements: blueprint pure-generic-volume-groups

cinder/tests/unit/fake_group.py

@@ -18,6 +18,32 @@ from cinder import objects
 from cinder.tests.unit import fake_constants as fake
 
 
+def fake_db_group(**updates):
+    db_group = {
+        'id': fake.GROUP_ID,
+        'name': 'group-1',
+        'status': 'available',
+        'user_id': fake.USER_ID,
+        'project_id': fake.PROJECT_ID,
+        'group_type_id': fake.GROUP_TYPE_ID,
+    }
+
+    for name, field in objects.Group.fields.items():
+        if name in db_group:
+            continue
+        if field.nullable:
+            db_group[name] = None
+        elif field.default != fields.UnspecifiedDefault:
+            db_group[name] = field.default
+        else:
+            raise Exception('fake_db_group needs help with %s.' % name)
+
+    if updates:
+        db_group.update(updates)
+
+    return db_group
+
+
 def fake_db_group_type(**updates):
     db_group_type = {
         'id': fake.GROUP_TYPE_ID,
@@ -44,6 +70,11 @@ def fake_db_group_type(**updates):
     return db_group_type
 
 
+def fake_group_obj(context, **updates):
+    return objects.Group._from_db_object(
+        context, objects.Group(), fake_db_group(**updates))
+
+
 def fake_group_type_obj(context, **updates):
     return objects.GroupType._from_db_object(
         context, objects.GroupType(), fake_db_group_type(**updates))

cinder/tests/unit/test_volume_utils.py

@@ -35,6 +35,7 @@ from cinder.objects import fields
 from cinder import test
 from cinder.tests.unit.backup import fake_backup
 from cinder.tests.unit import fake_constants as fake
+from cinder.tests.unit import fake_group
 from cinder.tests.unit import fake_snapshot
 from cinder.tests.unit import fake_volume
 from cinder import utils
@@ -1010,3 +1011,38 @@ class VolumeUtilsTestCase(test.TestCase):
     def test_is_replicated_spec_false(self, enabled):
         res = volume_utils.is_replicated_spec({'replication_enabled': enabled})
         self.assertFalse(res)
+
+    @mock.patch('cinder.db.group_get')
+    def test_group_get_by_id(self, mock_db_group_get):
+        expected = mock.Mock()
+        mock_db_group_get.return_value = expected
+        group_id = fake.GROUP_ID
+        actual = volume_utils.group_get_by_id(group_id)
+        self.assertEqual(expected, actual)
+
+    @mock.patch('cinder.db.group_get')
+    def test_group_get_by_id_group_not_found(self, mock_db_group_get):
+        group_id = fake.GROUP_ID
+        mock_db_group_get.side_effect = exception.GroupNotFound(
+            group_id=group_id)
+        self.assertRaises(
+            exception.GroupNotFound,
+            volume_utils.group_get_by_id,
+            group_id
+        )
+
+    @ddt.data('<is> False', None, 'notASpecValueWeCareAbout')
+    def test_is_group_a_cg_snapshot_type_is_false(self, spec_value):
+        with mock.patch('cinder.volume.group_types'
+                        '.get_group_type_specs') as mock_get_specs:
+            mock_get_specs.return_value = spec_value
+            group = fake_group.fake_group_obj(
+                None, group_type_id=fake.GROUP_TYPE_ID)
+            self.assertFalse(volume_utils.is_group_a_cg_snapshot_type(group))
+
+    @mock.patch('cinder.volume.group_types.get_group_type_specs')
+    def test_is_group_a_cg_snapshot_type_is_true(self, mock_get_specs):
+        mock_get_specs.return_value = '<is> True'
+        group = fake_group.fake_group_obj(
+            None, group_type_id=fake.GROUP_TYPE_ID)
+        self.assertTrue(volume_utils.is_group_a_cg_snapshot_type(group))

cinder/tests/unit/volume/drivers/test_pure.py

@@ -23,6 +23,7 @@ from oslo_utils import units
 from cinder import exception
 from cinder import test
 from cinder.tests.unit import fake_constants as fake
+from cinder.tests.unit import fake_group
 from cinder.tests.unit import fake_snapshot
 from cinder.tests.unit import fake_volume
 
@@ -91,10 +92,12 @@ VOLUME = {
     "volume_type_id": VOLUME_TYPE_ID,
     "replication_status": None,
     "consistencygroup_id": None,
-    "provider_location": GET_ARRAY_PRIMARY["id"]
+    "provider_location": GET_ARRAY_PRIMARY["id"],
+    "group_id": None,
 }
 VOLUME_PURITY_NAME = VOLUME['name'] + '-cinder'
 VOLUME_WITH_CGROUP = VOLUME.copy()
+VOLUME_WITH_CGROUP['group_id'] = "4a2f7e3a-312a-40c5-96a8-536b8a0fe074"
 VOLUME_WITH_CGROUP['consistencygroup_id'] = \
     "4a2f7e3a-312a-40c5-96a8-536b8a0fe074"
 SRC_VOL_ID = "dc7a294d-5964-4379-a15f-ce5554734efc"
@@ -107,6 +110,7 @@ SRC_VOL = {
     "volume_type": None,
     "volume_type_id": None,
     "consistencygroup_id": None,
+    "group_id": None,
 }
 SNAPSHOT_ID = "04fe2f9a-d0c4-4564-a30d-693cc3657b47"
 SNAPSHOT = {
@@ -117,11 +121,15 @@ SNAPSHOT = {
     "volume_size": 2,
     "display_name": "fake_snapshot",
     "cgsnapshot_id": None,
+    "cgsnapshot": None,
+    "group_snapshot_id": None,
+    "group_snapshot": None,
 }
 SNAPSHOT_PURITY_NAME = SRC_VOL["name"] + '-cinder.' + SNAPSHOT["name"]
 SNAPSHOT_WITH_CGROUP = SNAPSHOT.copy()
-SNAPSHOT_WITH_CGROUP['cgsnapshot_id'] = \
+SNAPSHOT_WITH_CGROUP['group_snapshot'] = {
-    "4a2f7e3a-312a-40c5-96a8-536b8a0fe075"
+    "group_id": "4a2f7e3a-312a-40c5-96a8-536b8a0fe044",
+}
 INITIATOR_IQN = "iqn.1993-08.org.debian:01:222"
 INITIATOR_WWN = "5001500150015081abc"
 ISCSI_CONNECTOR = {"initiator": INITIATOR_IQN, "host": HOSTNAME}
@@ -624,32 +632,23 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
         self.assertEqual(49, len(result))
         self.assertTrue(pure.GENERATED_NAME.match(result))
 
+    @mock.patch(BASE_DRIVER_OBJ + "._add_to_group_if_needed")
     @mock.patch(BASE_DRIVER_OBJ + "._is_volume_replicated_type", autospec=True)
-    def test_create_volume(self, mock_is_replicated_type):
+    def test_create_volume(self, mock_is_replicated_type, mock_add_to_group):
         mock_is_replicated_type.return_value = False
         self.driver.create_volume(VOLUME)
+        vol_name = VOLUME["name"] + "-cinder"
         self.array.create_volume.assert_called_with(
-            VOLUME["name"] + "-cinder", 2 * units.Gi)
+            vol_name, 2 * units.Gi)
+        mock_add_to_group.assert_called_once_with(VOLUME,
+                                                  vol_name)
         self.assert_error_propagates([self.array.create_volume],
                                      self.driver.create_volume, VOLUME)
 
-    @mock.patch(BASE_DRIVER_OBJ + "._add_volume_to_consistency_group",
+    @mock.patch(BASE_DRIVER_OBJ + "._add_to_group_if_needed")
-                autospec=True)
     @mock.patch(BASE_DRIVER_OBJ + "._is_volume_replicated_type", autospec=True)
-    def test_create_volume_with_cgroup(self, mock_is_replicated_type,
+    def test_create_volume_from_snapshot(self, mock_is_replicated_type,
-                                       mock_add_to_cgroup):
+                                         mock_add_to_group):
-        vol_name = VOLUME_WITH_CGROUP["name"] + "-cinder"
-        mock_is_replicated_type.return_value = False
-
-        self.driver.create_volume(VOLUME_WITH_CGROUP)
-
-        mock_add_to_cgroup\
-            .assert_called_with(self.driver,
-                                VOLUME_WITH_CGROUP['consistencygroup_id'],
-                                vol_name)
-
-    @mock.patch(BASE_DRIVER_OBJ + "._is_volume_replicated_type", autospec=True)
-    def test_create_volume_from_snapshot(self, mock_is_replicated_type):
         vol_name = VOLUME["name"] + "-cinder"
         snap_name = SNAPSHOT["volume_name"] + "-cinder." + SNAPSHOT["name"]
         mock_is_replicated_type.return_value = False
@@ -658,21 +657,32 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
         self.driver.create_volume_from_snapshot(VOLUME, SNAPSHOT)
         self.array.copy_volume.assert_called_with(snap_name, vol_name)
         self.assertFalse(self.array.extend_volume.called)
+        mock_add_to_group.assert_called_once_with(VOLUME,
+                                                  vol_name)
         self.assert_error_propagates(
             [self.array.copy_volume],
             self.driver.create_volume_from_snapshot, VOLUME, SNAPSHOT)
         self.assertFalse(self.array.extend_volume.called)
 
+    @mock.patch(BASE_DRIVER_OBJ + "._add_to_group_if_needed")
+    @mock.patch(BASE_DRIVER_OBJ + "._is_volume_replicated_type",
+                autospec=True)
+    def test_create_volume_from_snapshot_with_extend(self,
+                                                     mock_is_replicated_type,
+                                                     mock_add_to_group):
+        vol_name = VOLUME["name"] + "-cinder"
+        snap_name = SNAPSHOT["volume_name"] + "-cinder." + SNAPSHOT["name"]
+        mock_is_replicated_type.return_value = False
+
         # Branch where extend needed
-        SNAPSHOT["volume_size"] = 1  # resize so smaller than VOLUME
+        src = deepcopy(SNAPSHOT)
-        self.driver.create_volume_from_snapshot(VOLUME, SNAPSHOT)
+        src["volume_size"] = 1  # resize so smaller than VOLUME
+        self.driver.create_volume_from_snapshot(VOLUME, src)
         expected = [mock.call.copy_volume(snap_name, vol_name),
                     mock.call.extend_volume(vol_name, 2 * units.Gi)]
         self.array.assert_has_calls(expected)
-        self.assert_error_propagates(
+        mock_add_to_group.assert_called_once_with(VOLUME,
-            [self.array.copy_volume, self.array.extend_volume],
+                                                  vol_name)
-            self.driver.create_volume_from_snapshot, VOLUME, SNAPSHOT)
-        SNAPSHOT["volume_size"] = 2  # reset size
 
     @mock.patch(BASE_DRIVER_OBJ + "._get_snap_name")
     def test_create_volume_from_snapshot_cant_get_name(self, mock_get_name):
@@ -688,15 +698,14 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
                           self.driver.create_volume_from_snapshot,
                           VOLUME, SNAPSHOT_WITH_CGROUP)
 
-    @mock.patch(BASE_DRIVER_OBJ + "._add_volume_to_consistency_group",
+    @mock.patch(BASE_DRIVER_OBJ + "._add_to_group_if_needed")
-                autospec=True)
     @mock.patch(BASE_DRIVER_OBJ + "._extend_if_needed", autospec=True)
     @mock.patch(BASE_DRIVER_OBJ + "._get_pgroup_snap_name_from_snapshot")
     @mock.patch(BASE_DRIVER_OBJ + "._is_volume_replicated_type", autospec=True)
     def test_create_volume_from_cgsnapshot(self, mock_is_replicated_type,
                                            mock_get_snap_name,
                                            mock_extend_if_needed,
-                                           mock_add_to_cgroup):
+                                           mock_add_to_group):
         vol_name = VOLUME_WITH_CGROUP["name"] + "-cinder"
         snap_name = "consisgroup-4a2f7e3a-312a-40c5-96a8-536b8a0f" \
                     "e074-cinder.4a2f7e3a-312a-40c5-96a8-536b8a0fe075."\
@@ -713,14 +722,15 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
 
         self.driver.create_volume_from_snapshot(VOLUME_WITH_CGROUP,
                                                 SNAPSHOT_WITH_CGROUP)
-        mock_add_to_cgroup\
+        mock_add_to_group\
-            .assert_called_with(self.driver,
+            .assert_called_with(VOLUME_WITH_CGROUP,
-                                VOLUME_WITH_CGROUP['consistencygroup_id'],
                                 vol_name)
 
     # Tests cloning a volume that is not replicated type
+    @mock.patch(BASE_DRIVER_OBJ + "._add_to_group_if_needed")
     @mock.patch(BASE_DRIVER_OBJ + "._is_volume_replicated_type", autospec=True)
-    def test_create_cloned_volume(self, mock_is_replicated_type):
+    def test_create_cloned_volume(self, mock_is_replicated_type,
+                                  mock_add_to_group):
         vol_name = VOLUME["name"] + "-cinder"
         src_name = SRC_VOL["name"] + "-cinder"
         mock_is_replicated_type.return_value = False
@@ -728,35 +738,40 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
         self.driver.create_cloned_volume(VOLUME, SRC_VOL)
         self.array.copy_volume.assert_called_with(src_name, vol_name)
         self.assertFalse(self.array.extend_volume.called)
+        mock_add_to_group.assert_called_once_with(VOLUME,
+                                                  vol_name)
         self.assert_error_propagates(
             [self.array.copy_volume],
             self.driver.create_cloned_volume, VOLUME, SRC_VOL)
         self.assertFalse(self.array.extend_volume.called)
-        # Branch where extend needed
+
-        SRC_VOL["size"] = 1  # resize so smaller than VOLUME
+    @mock.patch(BASE_DRIVER_OBJ + "._add_to_group_if_needed")
-        self.driver.create_cloned_volume(VOLUME, SRC_VOL)
+    @mock.patch(BASE_DRIVER_OBJ + "._is_volume_replicated_type",
+                autospec=True)
+    def test_create_cloned_volume_and_extend(self, mock_is_replicated_type,
+                                             mock_add_to_group):
+        vol_name = VOLUME["name"] + "-cinder"
+        src_name = SRC_VOL["name"] + "-cinder"
+        src = deepcopy(SRC_VOL)
+        src["size"] = 1  # resize so smaller than VOLUME
+        self.driver.create_cloned_volume(VOLUME, src)
         expected = [mock.call.copy_volume(src_name, vol_name),
                     mock.call.extend_volume(vol_name, 2 * units.Gi)]
         self.array.assert_has_calls(expected)
-        self.assert_error_propagates(
+        mock_add_to_group.assert_called_once_with(VOLUME,
-            [self.array.copy_volume, self.array.extend_volume],
+                                                  vol_name)
-            self.driver.create_cloned_volume, VOLUME, SRC_VOL)
-        SRC_VOL["size"] = 2  # reset size
 
     # Tests cloning a volume that is part of a consistency group
-    @mock.patch(BASE_DRIVER_OBJ + "._add_volume_to_consistency_group",
+    @mock.patch(BASE_DRIVER_OBJ + "._add_to_group_if_needed")
-                autospec=True)
     @mock.patch(BASE_DRIVER_OBJ + "._is_volume_replicated_type", autospec=True)
     def test_create_cloned_volume_with_cgroup(self, mock_is_replicated_type,
-                                              mock_add_to_cgroup):
+                                              mock_add_to_group):
         vol_name = VOLUME_WITH_CGROUP["name"] + "-cinder"
         mock_is_replicated_type.return_value = False
 
         self.driver.create_cloned_volume(VOLUME_WITH_CGROUP, SRC_VOL)
 
-        mock_add_to_cgroup\
+        mock_add_to_group.assert_called_with(VOLUME_WITH_CGROUP,
-            .assert_called_with(self.driver,
-                                VOLUME_WITH_CGROUP['consistencygroup_id'],
                                              vol_name)
 
     def test_delete_volume_already_deleted(self):
@@ -964,9 +979,10 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
         self.assertEqual(expected_name, actual_name)
 
     def test_get_pgroup_snap_suffix(self):
-        cgsnap = mock.Mock()
+        cgsnap = {
-        cgsnap.id = "4a2f7e3a-312a-40c5-96a8-536b8a0fe074"
+            'id': "4a2f7e3a-312a-40c5-96a8-536b8a0fe074"
-        expected_suffix = "cgsnapshot-%s-cinder" % cgsnap.id
+        }
+        expected_suffix = "cgsnapshot-%s-cinder" % cgsnap['id']
         actual_suffix = self.driver._get_pgroup_snap_suffix(cgsnap)
         self.assertEqual(expected_suffix, actual_suffix)
 
@@ -974,33 +990,35 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
         cg_id = "4a2f7e3a-312a-40c5-96a8-536b8a0fe074"
         cgsnap_id = "4a2f7e3a-312a-40c5-96a8-536b8a0fe075"
 
-        mock_cgsnap = mock.Mock()
+        cgsnap = {
-        mock_cgsnap.consistencygroup_id = cg_id
+            'id': cgsnap_id,
-        mock_cgsnap.id = cgsnap_id
+            'group_id': cg_id
+        }
         expected_name = "consisgroup-%(cg)s-cinder.cgsnapshot-%(snap)s-cinder"\
                         % {"cg": cg_id, "snap": cgsnap_id}
 
-        actual_name = self.driver._get_pgroup_snap_name(mock_cgsnap)
+        actual_name = self.driver._get_pgroup_snap_name(cgsnap)
 
         self.assertEqual(expected_name, actual_name)
 
     def test_get_pgroup_snap_name_from_snapshot(self):
 
-        cgsnapshot_id = 'b919b266-23b4-4b83-9a92-e66031b9a921'
+        groupsnapshot_id = 'b919b266-23b4-4b83-9a92-e66031b9a921'
         volume_name = 'volume-a3b8b294-8494-4a72-bec7-9aadec561332'
         cg_id = '0cfc0e4e-5029-4839-af20-184fbc42a9ed'
         pgsnap_name_base = (
             'consisgroup-%s-cinder.cgsnapshot-%s-cinder.%s-cinder')
-        pgsnap_name = pgsnap_name_base % (cg_id, cgsnapshot_id, volume_name)
+        pgsnap_name = pgsnap_name_base % (cg_id, groupsnapshot_id, volume_name)
 
         self.driver.db = mock.MagicMock()
-        mock_cgsnap = mock.MagicMock()
+        cgsnap = {
-        mock_cgsnap.id = cgsnapshot_id
+            'id': groupsnapshot_id,
-        mock_cgsnap.consistencygroup_id = cg_id
+            'group_id': cg_id
-        self.driver.db.cgsnapshot_get.return_value = mock_cgsnap
+        }
+        self.driver.db.group_snapshot_get.return_value = cgsnap
 
-        mock_snap = mock.Mock()
+        mock_snap = mock.MagicMock()
-        mock_snap.cgsnapshot_id = cgsnapshot_id
+        mock_snap.group_snapshot = cgsnap
         mock_snap.volume_name = volume_name
 
         actual_name = self.driver._get_pgroup_snap_name_from_snapshot(
@@ -1252,17 +1270,17 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
         )
 
     def test_create_cgsnapshot(self):
-        mock_cgsnap = mock.Mock()
+        mock_cgsnap = {
-        mock_cgsnap.id = "4a2f7e3a-312a-40c5-96a8-536b8a0fe074"
+            'id': "4a2f7e3a-312a-40c5-96a8-536b8a0fe074",
-        mock_cgsnap.consistencygroup_id = \
+            'group_id': "4a2f7e3a-312a-40c5-96a8-536b8a0fe075",
-            "4a2f7e3a-312a-40c5-96a8-536b8a0fe075"
+        }
         mock_context = mock.Mock()
         mock_snap = mock.MagicMock()
 
         model_update, snapshots = self.driver.create_cgsnapshot(mock_context,
                                                                 mock_cgsnap,
                                                                 [mock_snap])
-        cg_id = mock_cgsnap.consistencygroup_id
+        cg_id = mock_cgsnap["group_id"]
         expected_pgroup_name = self.driver._get_pgroup_name_from_id(cg_id)
         expected_snap_suffix = self.driver._get_pgroup_snap_suffix(mock_cgsnap)
         self.array.create_pgroup_snapshot\
@@ -2718,3 +2736,226 @@ class PureVolumeUpdateStatsTestCase(PureBaseSharedDriverTestCase):
         self.assertFalse(self.array.list_volumes.called)
         self.assertFalse(self.array.list_hosts.called)
         self.assertFalse(self.array.list_pgroups.called)
+
+
+class PureVolumeGroupsTestCase(PureBaseSharedDriverTestCase):
+    def setUp(self):
+        super(PureVolumeGroupsTestCase, self).setUp()
+        self.array.get.side_effect = self.fake_get_array
+        self.mock_context = mock.Mock()
+        self.driver.db = mock.Mock()
+        self.driver.db.group_get = mock.Mock()
+
+    @mock.patch('cinder.db.group_get')
+    @mock.patch(BASE_DRIVER_OBJ + '._add_volume_to_consistency_group')
+    @mock.patch('cinder.volume.utils.is_group_a_cg_snapshot_type')
+    def test_add_to_group_if_needed(self, mock_is_cg, mock_add_to_cg,
+                                    mock_db_group_get):
+        mock_is_cg.return_value = False
+        vol_name = 'foo'
+        group_id = fake.GROUP_ID
+        volume = fake_volume.fake_volume_obj(None, group_id=group_id)
+        group = mock.MagicMock()
+        mock_db_group_get.return_value = group
+
+        self.driver._add_to_group_if_needed(volume, vol_name)
+
+        mock_is_cg.assert_called_once_with(group)
+        mock_add_to_cg.assert_not_called()
+
+    @mock.patch('cinder.db.group_get')
+    @mock.patch(BASE_DRIVER_OBJ + '._add_volume_to_consistency_group')
+    @mock.patch('cinder.volume.utils.is_group_a_cg_snapshot_type')
+    def test_add_to_group_if_needed_with_cg(self, mock_is_cg, mock_add_to_cg,
+                                            mock_db_group_get):
+        mock_is_cg.return_value = True
+        vol_name = 'foo'
+        group_id = fake.GROUP_ID
+        volume = fake_volume.fake_volume_obj(None, group_id=group_id)
+        group = mock.MagicMock()
+        mock_db_group_get.return_value = group
+
+        self.driver._add_to_group_if_needed(volume, vol_name)
+
+        mock_is_cg.assert_called_once_with(group)
+        mock_add_to_cg.assert_called_once_with(
+            group_id,
+            vol_name
+        )
+
+    @mock.patch('cinder.volume.utils.is_group_a_cg_snapshot_type')
+    def test_create_group(self, mock_is_cg):
+        mock_is_cg.return_value = False
+        group = fake_group.fake_group_type_obj(None)
+        self.assertRaises(
+            NotImplementedError,
+            self.driver.create_group,
+            self.mock_context, group
+        )
+        mock_is_cg.assert_called_once_with(group)
+
+    @mock.patch('cinder.volume.utils.is_group_a_cg_snapshot_type')
+    def test_delete_group(self, mock_is_cg):
+        mock_is_cg.return_value = False
+        group = mock.MagicMock()
+        volumes = [fake_volume.fake_volume_obj(None)]
+        self.assertRaises(
+            NotImplementedError,
+            self.driver.delete_group,
+            self.mock_context, group, volumes
+        )
+        mock_is_cg.assert_called_once_with(group)
+
+    @mock.patch('cinder.volume.utils.is_group_a_cg_snapshot_type')
+    def test_update_group(self, mock_is_cg):
+        mock_is_cg.return_value = False
+        group = mock.MagicMock()
+        self.assertRaises(
+            NotImplementedError,
+            self.driver.update_group,
+            self.mock_context, group
+        )
+        mock_is_cg.assert_called_once_with(group)
+
+    @mock.patch('cinder.volume.utils.is_group_a_cg_snapshot_type')
+    def test_create_group_from_src(self, mock_is_cg):
+        mock_is_cg.return_value = False
+        group = mock.MagicMock()
+        volumes = [fake_volume.fake_volume_obj(None)]
+        self.assertRaises(
+            NotImplementedError,
+            self.driver.create_group_from_src,
+            self.mock_context, group, volumes
+        )
+        mock_is_cg.assert_called_once_with(group)
+
+    @mock.patch('cinder.volume.utils.is_group_a_cg_snapshot_type')
+    def test_create_group_snapshot(self, mock_is_cg):
+        mock_is_cg.return_value = False
+        group_snapshot = mock.MagicMock()
+        snapshots = [fake_snapshot.fake_snapshot_obj(None)]
+        self.assertRaises(
+            NotImplementedError,
+            self.driver.create_group_snapshot,
+            self.mock_context, group_snapshot, snapshots
+        )
+        mock_is_cg.assert_called_once_with(group_snapshot)
+
+    @mock.patch('cinder.volume.utils.is_group_a_cg_snapshot_type')
+    def test_delete_group_snapshot(self, mock_is_cg):
+        mock_is_cg.return_value = False
+        group_snapshot = mock.MagicMock()
+        snapshots = [fake_snapshot.fake_snapshot_obj(None)]
+        self.assertRaises(
+            NotImplementedError,
+            self.driver.create_group_snapshot,
+            self.mock_context, group_snapshot, snapshots
+        )
+        mock_is_cg.assert_called_once_with(group_snapshot)
+
+    @mock.patch(BASE_DRIVER_OBJ + '.create_consistencygroup')
+    @mock.patch('cinder.volume.group_types.get_group_type_specs')
+    def test_create_group_with_cg(self, mock_get_specs, mock_create_cg):
+        mock_get_specs.return_value = '<is> True'
+        group = mock.MagicMock()
+        self.driver.create_group(self.mock_context, group)
+        mock_create_cg.assert_called_once_with(self.mock_context, group)
+
+    @mock.patch(BASE_DRIVER_OBJ + '.delete_consistencygroup')
+    @mock.patch('cinder.volume.group_types.get_group_type_specs')
+    def test_delete_group_with_cg(self, mock_get_specs, mock_delete_cg):
+        mock_get_specs.return_value = '<is> True'
+        group = mock.MagicMock()
+        volumes = [fake_volume.fake_volume_obj(None)]
+        self.driver.delete_group(self.mock_context, group, volumes)
+        mock_delete_cg.assert_called_once_with(self.mock_context,
+                                               group,
+                                               volumes)
+
+    @mock.patch(BASE_DRIVER_OBJ + '.update_consistencygroup')
+    @mock.patch('cinder.volume.group_types.get_group_type_specs')
+    def test_update_group_with_cg(self, mock_get_specs, mock_update_cg):
+        mock_get_specs.return_value = '<is> True'
+        group = mock.MagicMock()
+        addvollist = [mock.Mock()]
+        remvollist = [mock.Mock()]
+        self.driver.update_group(
+            self.mock_context,
+            group,
+            addvollist,
+            remvollist
+        )
+        mock_update_cg.assert_called_once_with(
+            self.mock_context,
+            group,
+            addvollist,
+            remvollist
+        )
+
+    @mock.patch(BASE_DRIVER_OBJ + '.create_consistencygroup_from_src')
+    @mock.patch('cinder.volume.group_types.get_group_type_specs')
+    def test_create_group_from_src_with_cg(self, mock_get_specs, mock_create):
+        mock_get_specs.return_value = '<is> True'
+        group = mock.MagicMock()
+        volumes = [mock.Mock()]
+        group_snapshot = mock.Mock()
+        snapshots = [mock.Mock()]
+        source_group = mock.MagicMock()
+        source_vols = [mock.Mock()]
+
+        self.driver.create_group_from_src(
+            self.mock_context,
+            group,
+            volumes,
+            group_snapshot,
+            snapshots,
+            source_group,
+            source_vols
+        )
+        mock_create.assert_called_once_with(
+            self.mock_context,
+            group,
+            volumes,
+            group_snapshot,
+            snapshots,
+            source_group,
+            source_vols
+        )
+
+    @mock.patch(BASE_DRIVER_OBJ + '.create_cgsnapshot')
+    @mock.patch('cinder.volume.group_types.get_group_type_specs')
+    def test_create_group_snapshot_with_cg(self, mock_get_specs,
+                                           mock_create_cgsnap):
+        mock_get_specs.return_value = '<is> True'
+        group_snapshot = mock.MagicMock()
+        snapshots = [mock.Mock()]
+
+        self.driver.create_group_snapshot(
+            self.mock_context,
+            group_snapshot,
+            snapshots
+        )
+        mock_create_cgsnap.assert_called_once_with(
+            self.mock_context,
+            group_snapshot,
+            snapshots
+        )
+
+    @mock.patch(BASE_DRIVER_OBJ + '.delete_cgsnapshot')
+    @mock.patch('cinder.volume.group_types.get_group_type_specs')
+    def test_delete_group_snapshot_with_cg(self, mock_get_specs,
+                                           mock_delete_cg):
+        mock_get_specs.return_value = '<is> True'
+        group_snapshot = mock.MagicMock()
+        snapshots = [mock.Mock()]
+
+        self.driver.delete_group_snapshot(
+            self.mock_context,
+            group_snapshot,
+            snapshots
+        )
+        mock_delete_cg.assert_called_once_with(
+            self.mock_context,
+            group_snapshot,
+            snapshots
+        )

cinder/volume/driver.py

@@ -1755,7 +1755,7 @@ class BaseVD(object):
         """Creates a group.
 
         :param context: the context of the caller.
-        :param group: the dictionary of the group to be created.
+        :param group: the Group object of the group to be created.
         :returns: model_update
 
         model_update will be in this format: {'status': xxx, ......}.
@@ -1776,8 +1776,8 @@ class BaseVD(object):
         """Deletes a group.
 
         :param context: the context of the caller.
-        :param group: the dictionary of the group to be deleted.
+        :param group: the Group object of the group to be deleted.
-        :param volumes: a list of volume dictionaries in the group.
+        :param volumes: a list of Volume objects in the group.
         :returns: model_update, volumes_model_update
 
         param volumes is retrieved directly from the db. It is a list of
@@ -1821,9 +1821,9 @@ class BaseVD(object):
         """Updates a group.
 
         :param context: the context of the caller.
-        :param group: the dictionary of the group to be updated.
+        :param group: the Group object of the group to be updated.
-        :param add_volumes: a list of volume dictionaries to be added.
+        :param add_volumes: a list of Volume objects to be added.
-        :param remove_volumes: a list of volume dictionaries to be removed.
+        :param remove_volumes: a list of Volume objects to be removed.
         :returns: model_update, add_volumes_update, remove_volumes_update
 
         model_update is a dictionary that the driver wants the manager

cinder/volume/drivers/pure.py

@@ -282,19 +282,14 @@ class PureBaseVolumeDriver(san.SanDriver):
         current_array = self._get_current_array()
         current_array.create_volume(vol_name, vol_size)
 
-        if volume['consistencygroup_id']:
+        self._add_to_group_if_needed(volume, vol_name)
-            self._add_volume_to_consistency_group(
-                volume['consistencygroup_id'],
-                vol_name
-            )
-
         self._enable_replication_if_needed(current_array, volume)
 
     @pure_driver_debug_trace
     def create_volume_from_snapshot(self, volume, snapshot):
         """Creates a volume from a snapshot."""
         vol_name = self._get_vol_name(volume)
-        if snapshot['cgsnapshot_id']:
+        if snapshot['group_snapshot'] or snapshot['cgsnapshot']:
             snap_name = self._get_pgroup_snap_name_from_snapshot(snapshot)
         else:
             snap_name = self._get_snap_name(snapshot)
@@ -312,11 +307,7 @@ class PureBaseVolumeDriver(san.SanDriver):
                                snapshot["volume_size"],
                                volume["size"])
 
-        if volume['consistencygroup_id']:
+        self._add_to_group_if_needed(volume, vol_name)
-            self._add_volume_to_consistency_group(
-                volume['consistencygroup_id'],
-                vol_name)
-
         self._enable_replication_if_needed(current_array, volume)
 
     def _enable_replication_if_needed(self, array, volume):
@@ -352,11 +343,7 @@ class PureBaseVolumeDriver(san.SanDriver):
                                src_vref["size"],
                                volume["size"])
 
-        if volume['consistencygroup_id']:
+        self._add_to_group_if_needed(volume, vol_name)
-            self._add_volume_to_consistency_group(
-                volume['consistencygroup_id'],
-                vol_name)
-
         self._enable_replication_if_needed(current_array, volume)
 
     def _extend_if_needed(self, array, vol_name, src_size, vol_size):
@@ -632,8 +619,8 @@ class PureBaseVolumeDriver(san.SanDriver):
         new_size = new_size * units.Gi
         current_array.extend_volume(vol_name, new_size)
 
-    def _add_volume_to_consistency_group(self, consistencygroup_id, vol_name):
+    def _add_volume_to_consistency_group(self, group_id, vol_name):
-        pgroup_name = self._get_pgroup_name_from_id(consistencygroup_id)
+        pgroup_name = self._get_pgroup_name_from_id(group_id)
         current_array = self._get_current_array()
         current_array.set_pgroup(pgroup_name, addvollist=[vol_name])
 
@@ -753,7 +740,7 @@ class PureBaseVolumeDriver(san.SanDriver):
     def create_cgsnapshot(self, context, cgsnapshot, snapshots):
         """Creates a cgsnapshot."""
 
-        cg_id = cgsnapshot.consistencygroup_id
+        cg_id = self._get_group_id_from_snap(cgsnapshot)
         pgroup_name = self._get_pgroup_name_from_id(cg_id)
         pgsnap_suffix = self._get_pgroup_snap_suffix(cgsnapshot)
         current_array = self._get_current_array()
@@ -832,6 +819,129 @@ class PureBaseVolumeDriver(san.SanDriver):
             existing_ref=existing_ref,
             reason=_("Unable to find Purity ref with name=%s") % ref_vol_name)
 
+    def _add_to_group_if_needed(self, volume, vol_name):
+        if volume['group_id']:
+            # If the query blows up just let it raise up the stack, the volume
+            # should be put into an error state
+            group = volume_utils.group_get_by_id(volume['group_id'])
+            if volume_utils.is_group_a_cg_snapshot_type(group):
+                self._add_volume_to_consistency_group(
+                    volume['group_id'],
+                    vol_name
+                )
+        elif volume['consistencygroup_id']:
+            self._add_volume_to_consistency_group(
+                volume['consistencygroup_id'],
+                vol_name
+            )
+
+    def create_group(self, ctxt, group):
+        """Creates a group.
+
+        :param ctxt: the context of the caller.
+        :param group: the Group object of the group to be created.
+        :returns: model_update
+        """
+        if volume_utils.is_group_a_cg_snapshot_type(group):
+            return self.create_consistencygroup(ctxt, group)
+
+        # If it wasn't a consistency group request ignore it and we'll rely on
+        # the generic group implementation.
+        raise NotImplementedError()
+
+    def delete_group(self, ctxt, group, volumes):
+        """Deletes a group.
+
+        :param ctxt: the context of the caller.
+        :param group: the Group object of the group to be deleted.
+        :param volumes: a list of Volume objects in the group.
+        :returns: model_update, volumes_model_update
+        """
+        if volume_utils.is_group_a_cg_snapshot_type(group):
+            return self.delete_consistencygroup(ctxt, group, volumes)
+
+        # If it wasn't a consistency group request ignore it and we'll rely on
+        # the generic group implementation.
+        raise NotImplementedError()
+
+    def update_group(self, ctxt, group,
+                     add_volumes=None, remove_volumes=None):
+        """Updates a group.
+
+        :param ctxt: the context of the caller.
+        :param group: the Group object of the group to be updated.
+        :param add_volumes: a list of Volume objects to be added.
+        :param remove_volumes: a list of Volume objects to be removed.
+        :returns: model_update, add_volumes_update, remove_volumes_update
+        """
+
+        if volume_utils.is_group_a_cg_snapshot_type(group):
+            return self.update_consistencygroup(ctxt,
+                                                group,
+                                                add_volumes,
+                                                remove_volumes)
+
+        # If it wasn't a consistency group request ignore it and we'll rely on
+        # the generic group implementation.
+        raise NotImplementedError()
+
+    def create_group_from_src(self, ctxt, group, volumes,
+                              group_snapshot=None, snapshots=None,
+                              source_group=None, source_vols=None):
+        """Creates a group from source.
+
+        :param ctxt: the context of the caller.
+        :param group: the Group object to be created.
+        :param volumes: a list of Volume objects in the group.
+        :param group_snapshot: the GroupSnapshot object as source.
+        :param snapshots: a list of snapshot objects in group_snapshot.
+        :param source_group: the Group object as source.
+        :param source_vols: a list of volume objects in the source_group.
+        :returns: model_update, volumes_model_update
+        """
+        if volume_utils.is_group_a_cg_snapshot_type(group):
+            return self.create_consistencygroup_from_src(ctxt,
+                                                         group,
+                                                         volumes,
+                                                         group_snapshot,
+                                                         snapshots,
+                                                         source_group,
+                                                         source_vols)
+
+        # If it wasn't a consistency group request ignore it and we'll rely on
+        # the generic group implementation.
+        raise NotImplementedError()
+
+    def create_group_snapshot(self, ctxt, group_snapshot, snapshots):
+        """Creates a group_snapshot.
+
+        :param ctxt: the context of the caller.
+        :param group_snapshot: the GroupSnapshot object to be created.
+        :param snapshots: a list of Snapshot objects in the group_snapshot.
+        :returns: model_update, snapshots_model_update
+        """
+        if volume_utils.is_group_a_cg_snapshot_type(group_snapshot):
+            return self.create_cgsnapshot(ctxt, group_snapshot, snapshots)
+
+        # If it wasn't a consistency group request ignore it and we'll rely on
+        # the generic group implementation.
+        raise NotImplementedError()
+
+    def delete_group_snapshot(self, ctxt, group_snapshot, snapshots):
+        """Deletes a group_snapshot.
+
+        :param ctxt: the context of the caller.
+        :param group_snapshot: the GroupSnapshot object to be deleted.
+        :param snapshots: a list of snapshot objects in the group_snapshot.
+        :returns: model_update, snapshots_model_update
+        """
+        if volume_utils.is_group_a_cg_snapshot_type(group_snapshot):
+            return self.delete_cgsnapshot(ctxt, group_snapshot, snapshots)
+
+        # If it wasn't a consistency group request ignore it and we'll rely on
+        # the generic group implementation.
+        raise NotImplementedError()
+
     @pure_driver_debug_trace
     def manage_existing(self, volume, existing_ref):
         """Brings an existing backend storage object under Cinder management.
@@ -1097,15 +1207,31 @@ class PureBaseVolumeDriver(san.SanDriver):
         return "consisgroup-%s-cinder" % id
 
     @staticmethod
-    def _get_pgroup_snap_suffix(cgsnapshot):
+    def _get_pgroup_snap_suffix(group_snapshot):
-        return "cgsnapshot-%s-cinder" % cgsnapshot.id
+        return "cgsnapshot-%s-cinder" % group_snapshot['id']
+
+    @staticmethod
+    def _get_group_id_from_snap(group_snap):
+        # We don't really care what kind of group it is, if we are calling
+        # this look for a group_id and fall back to using a consistencygroup_id
+        id = None
+        try:
+            id = group_snap['group_id']
+        except AttributeError:
+            pass
+        if id is None:
+            try:
+                id = group_snap['consistencygroup_id']
+            except AttributeError:
+                pass
+        return id
 
     @classmethod
-    def _get_pgroup_snap_name(cls, cgsnapshot):
+    def _get_pgroup_snap_name(cls, group_snapshot):
         """Return the name of the pgroup snapshot that Purity will use"""
-        cg_id = cgsnapshot.consistencygroup_id
+        group_id = cls._get_group_id_from_snap(group_snapshot)
-        return "%s.%s" % (cls._get_pgroup_name_from_id(cg_id),
+        return "%s.%s" % (cls._get_pgroup_name_from_id(group_id),
-                          cls._get_pgroup_snap_suffix(cgsnapshot))
+                          cls._get_pgroup_snap_suffix(group_snapshot))
 
     @staticmethod
     def _get_pgroup_vol_snap_name(pg_name, pgsnap_suffix, volume_name):
@@ -1118,13 +1244,14 @@ class PureBaseVolumeDriver(san.SanDriver):
     def _get_pgroup_snap_name_from_snapshot(self, snapshot):
         """Return the name of the snapshot that Purity will use."""
 
-        # TODO(patrickeast): Remove DB calls once the cgsnapshot objects are
+        group_snap = None
-        # available to use and can be associated with the snapshot objects.
+        if snapshot.group_snapshot:
-        ctxt = context.get_admin_context()
+            group_snap = snapshot.group_snapshot
-        cgsnapshot = self.db.cgsnapshot_get(ctxt, snapshot.cgsnapshot_id)
+        elif snapshot.cgsnapshot:
+            group_snap = snapshot.cgsnapshot
 
         pg_vol_snap_name = "%(group_snap)s.%(volume_name)s-cinder" % {
-            'group_snap': self._get_pgroup_snap_name(cgsnapshot),
+            'group_snap': self._get_pgroup_snap_name(group_snap),
             'volume_name': snapshot.volume_name
         }
         return pg_vol_snap_name

cinder/volume/utils.py

@@ -43,6 +43,7 @@ from cinder.i18n import _, _LI, _LW, _LE
 from cinder import objects
 from cinder import rpc
 from cinder import utils
+from cinder.volume import group_types
 from cinder.volume import throttling
 from cinder.volume import volume_types
 
@@ -888,3 +889,21 @@ def is_replicated_str(str):
 def is_replicated_spec(extra_specs):
     return (extra_specs and
             is_replicated_str(extra_specs.get('replication_enabled')))
+
+
+def group_get_by_id(group_id):
+    ctxt = context.get_admin_context()
+    group = db.group_get(ctxt, group_id)
+    return group
+
+
+def is_group_a_cg_snapshot_type(group_or_snap):
+    LOG.debug("Checking if %s is a consistent snapshot group",
+              group_or_snap)
+    if group_or_snap["group_type_id"] is not None:
+        spec = group_types.get_group_type_specs(
+            group_or_snap["group_type_id"],
+            key="consistent_group_snapshot_enabled"
+        )
+        return spec == "<is> True"
+    return False

releasenotes/notes/pure-generic-volume-groups-2b0941103f7c01cb.yaml

@@ -0,0 +1,3 @@
+---
+features:
+  - Add consistent group capability to generic volume groups in Pure drivers.