From abf53e0815014b1ffc9d8d03bec1570faef18327 Mon Sep 17 00:00:00 2001 From: Patrick East Date: Wed, 4 Jan 2017 14:12:03 -0800 Subject: [PATCH] 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 | 31 ++ cinder/tests/unit/test_volume_utils.py | 36 ++ cinder/tests/unit/volume/drivers/test_pure.py | 377 ++++++++++++++---- cinder/volume/driver.py | 12 +- cinder/volume/drivers/pure.py | 189 +++++++-- cinder/volume/utils.py | 19 + ...eneric-volume-groups-2b0941103f7c01cb.yaml | 3 + 7 files changed, 562 insertions(+), 105 deletions(-) create mode 100644 releasenotes/notes/pure-generic-volume-groups-2b0941103f7c01cb.yaml diff --git a/cinder/tests/unit/fake_group.py b/cinder/tests/unit/fake_group.py index 2bbc680f830..1afec2b8e27 100644 --- a/cinder/tests/unit/fake_group.py +++ b/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)) diff --git a/cinder/tests/unit/test_volume_utils.py b/cinder/tests/unit/test_volume_utils.py index 180322c205b..52f8c147045 100644 --- a/cinder/tests/unit/test_volume_utils.py +++ b/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(' 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 = ' 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)) diff --git a/cinder/tests/unit/volume/drivers/test_pure.py b/cinder/tests/unit/volume/drivers/test_pure.py index 93d7eab938a..d8a0b9d78e4 100644 --- a/cinder/tests/unit/volume/drivers/test_pure.py +++ b/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'] = \ - "4a2f7e3a-312a-40c5-96a8-536b8a0fe075" +SNAPSHOT_WITH_CGROUP['group_snapshot'] = { + "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", - autospec=True) + @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_with_cgroup(self, mock_is_replicated_type, - mock_add_to_cgroup): - 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): + def test_create_volume_from_snapshot(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 @@ -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 - self.driver.create_volume_from_snapshot(VOLUME, SNAPSHOT) + src = deepcopy(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( - [self.array.copy_volume, self.array.extend_volume], - self.driver.create_volume_from_snapshot, VOLUME, SNAPSHOT) - SNAPSHOT["volume_size"] = 2 # reset size + mock_add_to_group.assert_called_once_with(VOLUME, + vol_name) @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", - autospec=True) + @mock.patch(BASE_DRIVER_OBJ + "._add_to_group_if_needed") @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\ - .assert_called_with(self.driver, - VOLUME_WITH_CGROUP['consistencygroup_id'], + mock_add_to_group\ + .assert_called_with(VOLUME_WITH_CGROUP, 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,36 +738,41 @@ 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 - self.driver.create_cloned_volume(VOLUME, SRC_VOL) + + @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_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( - [self.array.copy_volume, self.array.extend_volume], - self.driver.create_cloned_volume, VOLUME, SRC_VOL) - SRC_VOL["size"] = 2 # reset size + mock_add_to_group.assert_called_once_with(VOLUME, + vol_name) # Tests cloning a volume that is part of a consistency group - @mock.patch(BASE_DRIVER_OBJ + "._add_volume_to_consistency_group", - autospec=True) + @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_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\ - .assert_called_with(self.driver, - VOLUME_WITH_CGROUP['consistencygroup_id'], - vol_name) + mock_add_to_group.assert_called_with(VOLUME_WITH_CGROUP, + vol_name) def test_delete_volume_already_deleted(self): self.array.list_volume_private_connections.side_effect = \ @@ -964,9 +979,10 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): self.assertEqual(expected_name, actual_name) def test_get_pgroup_snap_suffix(self): - cgsnap = mock.Mock() - cgsnap.id = "4a2f7e3a-312a-40c5-96a8-536b8a0fe074" - expected_suffix = "cgsnapshot-%s-cinder" % cgsnap.id + cgsnap = { + 'id': "4a2f7e3a-312a-40c5-96a8-536b8a0fe074" + } + 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() - mock_cgsnap.consistencygroup_id = cg_id - mock_cgsnap.id = cgsnap_id + 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() - mock_cgsnap.id = cgsnapshot_id - mock_cgsnap.consistencygroup_id = cg_id - self.driver.db.cgsnapshot_get.return_value = mock_cgsnap + cgsnap = { + 'id': groupsnapshot_id, + 'group_id': cg_id + } + self.driver.db.group_snapshot_get.return_value = cgsnap - mock_snap = mock.Mock() - mock_snap.cgsnapshot_id = cgsnapshot_id + mock_snap = mock.MagicMock() + 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.id = "4a2f7e3a-312a-40c5-96a8-536b8a0fe074" - mock_cgsnap.consistencygroup_id = \ - "4a2f7e3a-312a-40c5-96a8-536b8a0fe075" + mock_cgsnap = { + 'id': "4a2f7e3a-312a-40c5-96a8-536b8a0fe074", + 'group_id': "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 = ' 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 = ' 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 = ' 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 = ' 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 = ' 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 = ' 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 + ) diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index c177dd080c1..db8e05339f4 100644 --- a/cinder/volume/driver.py +++ b/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 volumes: a list of volume dictionaries in the group. + :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 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 add_volumes: a list of volume dictionaries to be added. - :param remove_volumes: a list of volume dictionaries to be removed. + :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 model_update is a dictionary that the driver wants the manager diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py index 634fc655c6a..fed489fc2f0 100644 --- a/cinder/volume/drivers/pure.py +++ b/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_volume_to_consistency_group( - volume['consistencygroup_id'], - vol_name - ) - + self._add_to_group_if_needed(volume, 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_volume_to_consistency_group( - volume['consistencygroup_id'], - vol_name) - + self._add_to_group_if_needed(volume, 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_volume_to_consistency_group( - volume['consistencygroup_id'], - vol_name) - + self._add_to_group_if_needed(volume, 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): - pgroup_name = self._get_pgroup_name_from_id(consistencygroup_id) + def _add_volume_to_consistency_group(self, group_id, vol_name): + 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): - return "cgsnapshot-%s-cinder" % cgsnapshot.id + def _get_pgroup_snap_suffix(group_snapshot): + 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 - return "%s.%s" % (cls._get_pgroup_name_from_id(cg_id), - cls._get_pgroup_snap_suffix(cgsnapshot)) + group_id = cls._get_group_id_from_snap(group_snapshot) + return "%s.%s" % (cls._get_pgroup_name_from_id(group_id), + 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 - # available to use and can be associated with the snapshot objects. - ctxt = context.get_admin_context() - cgsnapshot = self.db.cgsnapshot_get(ctxt, snapshot.cgsnapshot_id) + group_snap = None + if snapshot.group_snapshot: + group_snap = snapshot.group_snapshot + 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 diff --git a/cinder/volume/utils.py b/cinder/volume/utils.py index ce77dba1dbe..f86b6bcca12 100644 --- a/cinder/volume/utils.py +++ b/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 == " True" + return False diff --git a/releasenotes/notes/pure-generic-volume-groups-2b0941103f7c01cb.yaml b/releasenotes/notes/pure-generic-volume-groups-2b0941103f7c01cb.yaml new file mode 100644 index 00000000000..07492be36cf --- /dev/null +++ b/releasenotes/notes/pure-generic-volume-groups-2b0941103f7c01cb.yaml @@ -0,0 +1,3 @@ +--- +features: + - Add consistent group capability to generic volume groups in Pure drivers.