From bac35336109c2c71cf5faef729a227168d57e216 Mon Sep 17 00:00:00 2001 From: xing-yang Date: Fri, 17 Mar 2017 06:33:12 -0400 Subject: [PATCH] Update db from drivers in default groups methods The following default generic volume group methods in volume/manager.py are not using returned model_update from drivers. As a result database is not properly updated for drivers that are using fields such as provider_id to save info on volumes/snapshots. _create_group_snapshot_generic() _create_group_from_src_generic() This patch fixed the problem. Change-Id: I118175fc5be0f61146d8762a81113578c338037c Closes-Bug: #1674454 --- cinder/tests/unit/fake_constants.py | 2 + cinder/tests/unit/fake_group.py | 2 + cinder/tests/unit/fake_group_snapshot.py | 51 ++++++++ .../tests/unit/group/test_groups_manager.py | 111 ++++++++++++++++++ cinder/volume/manager.py | 65 ++++++---- 5 files changed, 211 insertions(+), 20 deletions(-) create mode 100644 cinder/tests/unit/fake_group_snapshot.py diff --git a/cinder/tests/unit/fake_constants.py b/cinder/tests/unit/fake_constants.py index 5f195880448..c1436711628 100644 --- a/cinder/tests/unit/fake_constants.py +++ b/cinder/tests/unit/fake_constants.py @@ -65,6 +65,7 @@ VOLUME4_ID = '904d4602-4301-4e9b-8df1-8133b51904e6' VOLUME4_NAME = 'volume-904d4602-4301-4e9b-8df1-8133b51904e6' VOLUME5_ID = '17b0e01d-3d2d-4c31-a1aa-c962420bc3dc' VOLUME5_NAME = 'volume-17b0e01d-3d2d-4c31-a1aa-c962420bc3dc' +VOLUME6_ID = '84375761-46e0-4df2-a567-02f0113428d7' VOLUME_NAME_ID = 'ee73d33c-52ed-4cb7-a8a9-2687c1205c22' VOLUME2_NAME_ID = '63fbdd21-03bc-4309-b867-2893848f86af' VOLUME_TYPE_ID = '4e9e6d23-eed0-426d-b90a-28f87a94b6fe' @@ -78,6 +79,7 @@ GROUP_TYPE2_ID = 'f8645498-1323-47a2-9442-5c57724d2e3c' GROUP_TYPE3_ID = '1b7915f4-b899-4510-9eff-bd67508c3334' GROUP_ID = '9a965cc6-ee3a-468d-a721-cebb193f696f' GROUP2_ID = '40a85639-abc3-4461-9230-b131abd8ee07' +GROUP3_ID = '1078414b-380c-474c-bf76-57e2c235841c' GROUP_SNAPSHOT_ID = '1e2ab152-44f0-11e6-819f-000c29d19d84' GROUP_SNAPSHOT2_ID = '33e2ff04-44f0-11e6-819f-000c29d19d84' diff --git a/cinder/tests/unit/fake_group.py b/cinder/tests/unit/fake_group.py index 1afec2b8e27..be44fceb520 100644 --- a/cinder/tests/unit/fake_group.py +++ b/cinder/tests/unit/fake_group.py @@ -26,6 +26,8 @@ def fake_db_group(**updates): 'user_id': fake.USER_ID, 'project_id': fake.PROJECT_ID, 'group_type_id': fake.GROUP_TYPE_ID, + 'group_snapshot_id': None, + 'source_group_id': None, } for name, field in objects.Group.fields.items(): diff --git a/cinder/tests/unit/fake_group_snapshot.py b/cinder/tests/unit/fake_group_snapshot.py new file mode 100644 index 00000000000..0a880ef41f3 --- /dev/null +++ b/cinder/tests/unit/fake_group_snapshot.py @@ -0,0 +1,51 @@ +# Copyright 2016 EMC Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_versionedobjects import fields + +from cinder import objects +from cinder.tests.unit import fake_constants as fake + + +def fake_db_group_snapshot(**updates): + db_group_snapshot = { + 'id': fake.GROUP_SNAPSHOT_ID, + 'name': 'group-1', + 'status': 'available', + 'user_id': fake.USER_ID, + 'project_id': fake.PROJECT_ID, + 'group_type_id': fake.GROUP_TYPE_ID, + 'group_id': fake.GROUP_ID, + } + + for name, field in objects.GroupSnapshot.fields.items(): + if name in db_group_snapshot: + continue + if field.nullable: + db_group_snapshot[name] = None + elif field.default != fields.UnspecifiedDefault: + db_group_snapshot[name] = field.default + else: + raise Exception('fake_db_group_snapshot needs help with %s.' + % name) + + if updates: + db_group_snapshot.update(updates) + + return db_group_snapshot + + +def fake_group_snapshot_obj(context, **updates): + return objects.GroupSnapshot._from_db_object( + context, objects.GroupSnapshot(), fake_db_group_snapshot(**updates)) diff --git a/cinder/tests/unit/group/test_groups_manager.py b/cinder/tests/unit/group/test_groups_manager.py index 39ac194cde4..ff324eb021b 100644 --- a/cinder/tests/unit/group/test_groups_manager.py +++ b/cinder/tests/unit/group/test_groups_manager.py @@ -27,6 +27,8 @@ from cinder import quota from cinder import test from cinder.tests.unit import conf_fixture from cinder.tests.unit import fake_constants as fake +from cinder.tests.unit import fake_group +from cinder.tests.unit import fake_group_snapshot from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume from cinder.tests.unit import utils as tests_utils @@ -780,3 +782,112 @@ class GroupManagerTestCase(test.TestCase): context.get_admin_context(), group_snapshot.id).id) self.assertTrue(mock_create_grpsnap.called) + + @mock.patch( + 'cinder.tests.fake_driver.FakeLoggingVolumeDriver.create_snapshot') + def test_create_group_snapshot_generic(self, mock_create_snap): + grp_snp = {'id': fake.GROUP_SNAPSHOT_ID, 'group_id': fake.GROUP_ID, + 'name': 'group snap 1'} + snp1 = {'id': fake.SNAPSHOT_ID, 'name': 'snap 1', + 'group_snapshot_id': fake.GROUP_SNAPSHOT_ID, + 'volume_id': fake.VOLUME_ID} + snp2 = {'id': fake.SNAPSHOT2_ID, 'name': 'snap 2', + 'group_snapshot_id': fake.GROUP_SNAPSHOT_ID, + 'volume_id': fake.VOLUME2_ID} + snp1_obj = fake_snapshot.fake_snapshot_obj(self.context, **snp1) + snp2_obj = fake_snapshot.fake_snapshot_obj(self.context, **snp2) + snapshots = [] + snapshots.append(snp1_obj) + snapshots.append(snp2_obj) + + driver_update = {'test_snap_key': 'test_val'} + mock_create_snap.return_value = driver_update + model_update, snapshot_model_updates = ( + self.volume._create_group_snapshot_generic( + self.context, grp_snp, snapshots)) + for update in snapshot_model_updates: + self.assertEqual(driver_update['test_snap_key'], + update['test_snap_key']) + + @mock.patch( + 'cinder.tests.fake_driver.FakeLoggingVolumeDriver.' + 'create_volume_from_snapshot') + @mock.patch( + 'cinder.tests.fake_driver.FakeLoggingVolumeDriver.' + 'create_cloned_volume') + def test_create_group_from_src_generic(self, mock_create_clone, + mock_create_vol_from_snap): + grp = {'id': fake.GROUP_ID, 'name': 'group 1'} + grp_snp = {'id': fake.GROUP_SNAPSHOT_ID, 'group_id': fake.GROUP_ID, + 'name': 'group snap 1'} + grp2 = {'id': fake.GROUP2_ID, 'name': 'group 2', + 'group_snapshot_id': fake.GROUP_SNAPSHOT_ID} + vol1 = {'id': fake.VOLUME_ID, 'name': 'volume 1', + 'group_id': fake.GROUP_ID} + vol2 = {'id': fake.VOLUME2_ID, 'name': 'volume 2', + 'group_id': fake.GROUP_ID} + snp1 = {'id': fake.SNAPSHOT_ID, 'name': 'snap 1', + 'group_snapshot_id': fake.GROUP_SNAPSHOT_ID, + 'volume_id': fake.VOLUME_ID} + snp2 = {'id': fake.SNAPSHOT2_ID, 'name': 'snap 2', + 'group_snapshot_id': fake.GROUP_SNAPSHOT_ID, + 'volume_id': fake.VOLUME2_ID} + snp1_obj = fake_snapshot.fake_snapshot_obj(self.context, **snp1) + snp2_obj = fake_snapshot.fake_snapshot_obj(self.context, **snp2) + snapshots = [] + snapshots.append(snp1_obj) + snapshots.append(snp2_obj) + vol3 = {'id': fake.VOLUME3_ID, 'name': 'volume 3', + 'snapshot_id': fake.SNAPSHOT_ID, + 'group_id': fake.GROUP2_ID} + vol4 = {'id': fake.VOLUME4_ID, 'name': 'volume 4', + 'snapshot_id': fake.SNAPSHOT2_ID, + 'group_id': fake.GROUP2_ID} + vol3_obj = fake_volume.fake_volume_obj(self.context, **vol3) + vol4_obj = fake_volume.fake_volume_obj(self.context, **vol4) + vols2 = [] + vols2.append(vol3_obj) + vols2.append(vol4_obj) + grp2_obj = fake_group.fake_group_obj(self.context, **grp2) + grp_snp_obj = fake_group_snapshot.fake_group_snapshot_obj( + self.context, **grp_snp) + + driver_update = {'test_key': 'test_val'} + mock_create_vol_from_snap.return_value = driver_update + model_update, vol_model_updates = ( + self.volume._create_group_from_src_generic( + self.context, grp2_obj, vols2, grp_snp_obj, snapshots)) + for update in vol_model_updates: + self.assertEqual(driver_update['test_key'], + update['test_key']) + + vol1_obj = fake_volume.fake_volume_obj(self.context, **vol1) + vol2_obj = fake_volume.fake_volume_obj(self.context, **vol2) + vols = [] + vols.append(vol1_obj) + vols.append(vol2_obj) + grp_obj = fake_group.fake_group_obj(self.context, **grp) + + grp3 = {'id': fake.GROUP3_ID, 'name': 'group 3', + 'source_group_id': fake.GROUP_ID} + grp3_obj = fake_group.fake_group_obj(self.context, **grp3) + vol5 = {'id': fake.VOLUME5_ID, 'name': 'volume 5', + 'source_volid': fake.VOLUME_ID, + 'group_id': fake.GROUP3_ID} + vol6 = {'id': fake.VOLUME6_ID, 'name': 'volume 6', + 'source_volid': fake.VOLUME2_ID, + 'group_id': fake.GROUP3_ID} + vol5_obj = fake_volume.fake_volume_obj(self.context, **vol5) + vol6_obj = fake_volume.fake_volume_obj(self.context, **vol6) + vols3 = [] + vols3.append(vol5_obj) + vols3.append(vol6_obj) + + driver_update = {'test_key2': 'test_val2'} + mock_create_clone.return_value = driver_update + model_update, vol_model_updates = ( + self.volume._create_group_from_src_generic( + self.context, grp3_obj, vols3, None, None, grp_obj, vols)) + for update in vol_model_updates: + self.assertEqual(driver_update['test_key2'], + update['test_key2']) diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index a78bf8ad05b..a7fdedbc4e5 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -2908,25 +2908,46 @@ class VolumeManager(manager.CleanableManager, :param source_vols: a list of volume objects in the source_group. :returns: model_update, volumes_model_update """ + model_update = {'status': 'available'} + volumes_model_update = [] for vol in volumes: - try: - if snapshots: - for snapshot in snapshots: - if vol.snapshot_id == snapshot.id: - self.driver.create_volume_from_snapshot( - vol, snapshot) - break - except Exception: - raise - try: - if source_vols: - for source_vol in source_vols: - if vol.source_volid == source_vol.id: - self.driver.create_cloned_volume(vol, source_vol) - break - except Exception: - raise - return None, None + if snapshots: + for snapshot in snapshots: + if vol.snapshot_id == snapshot.id: + vol_model_update = {'id': vol.id} + try: + driver_update = ( + self.driver.create_volume_from_snapshot( + vol, snapshot)) + if driver_update: + driver_update.pop('id', None) + vol_model_update.update(driver_update) + if 'status' not in vol_model_update: + vol_model_update['status'] = 'available' + except Exception: + vol_model_update['status'] = 'error' + model_update['status'] = 'error' + volumes_model_update.append(vol_model_update) + break + elif source_vols: + for source_vol in source_vols: + if vol.source_volid == source_vol.id: + vol_model_update = {'id': vol.id} + try: + driver_update = self.driver.create_cloned_volume( + vol, source_vol) + if driver_update: + driver_update.pop('id', None) + vol_model_update.update(driver_update) + if 'status' not in vol_model_update: + vol_model_update['status'] = 'available' + except Exception: + vol_model_update['status'] = 'error' + model_update['status'] = 'error' + volumes_model_update.append(vol_model_update) + break + + return model_update, volumes_model_update def _sort_snapshots(self, volumes, snapshots): # Sort source snapshots so that they are in the same order as their @@ -3899,8 +3920,12 @@ class VolumeManager(manager.CleanableManager, for snapshot in snapshots: snapshot_model_update = {'id': snapshot.id} try: - self.driver.create_snapshot(snapshot) - snapshot_model_update['status'] = 'available' + driver_update = self.driver.create_snapshot(snapshot) + if driver_update: + driver_update.pop('id', None) + snapshot_model_update.update(driver_update) + if 'status' not in snapshot_model_update: + snapshot_model_update['status'] = 'available' except Exception: snapshot_model_update['status'] = 'error' model_update['status'] = 'error'