From 91c672e340f8b7e525b63e8da7aed465be864305 Mon Sep 17 00:00:00 2001 From: TommyLike Date: Wed, 31 Jan 2018 08:58:30 +0800 Subject: [PATCH] Revert consumed quota when failed to create group from source group When failed to created corresponding volumes in group, Cinder will destory the volume object, but consumed quota is left. Change-Id: Ief0637768cf1fe04bb4162e02008b4884a184051 --- cinder/group/api.py | 8 +- cinder/tests/unit/group/test_groups_api.py | 135 +++++++++++++++++++++ 2 files changed, 141 insertions(+), 2 deletions(-) diff --git a/cinder/group/api.py b/cinder/group/api.py index 9f721bb5db5..a5cffbe6073 100644 --- a/cinder/group/api.py +++ b/cinder/group/api.py @@ -318,7 +318,9 @@ class API(base.Base): volumes = objects.VolumeList.get_all_by_generic_group( context, group.id) for vol in volumes: - vol.destroy() + # NOTE(tommylikehu): `delete` is used here in order to + # revert consumed quota. + self.volume_api.delete(context, vol) group.destroy() finally: LOG.error("Error occurred when creating group " @@ -399,7 +401,9 @@ class API(base.Base): volumes = objects.VolumeList.get_all_by_generic_group( context, group.id) for vol in volumes: - vol.destroy() + # NOTE(tommylikehu): `delete` is used here in order to + # revert consumed quota. + self.volume_api.delete(context, vol) group.destroy() finally: LOG.error("Error occurred when creating " diff --git a/cinder/tests/unit/group/test_groups_api.py b/cinder/tests/unit/group/test_groups_api.py index d5d3e39a1b1..bf5834016ff 100644 --- a/cinder/tests/unit/group/test_groups_api.py +++ b/cinder/tests/unit/group/test_groups_api.py @@ -374,6 +374,84 @@ class GroupAPITestCase(test.TestCase): self.group_api.delete_group_snapshot(self.ctxt, ret_group_snap) mock_delete_api.assert_called_once_with(mock.ANY, ret_group_snap) + @mock.patch('cinder.volume.api.API.delete') + @mock.patch('cinder.objects.VolumeType.get_by_name_or_id') + @mock.patch('cinder.db.group_volume_type_mapping_create') + @mock.patch('cinder.volume.api.API.create') + @mock.patch('cinder.objects.GroupSnapshot.get_by_id') + @mock.patch('cinder.objects.SnapshotList.get_all_for_group_snapshot') + @mock.patch('cinder.volume.rpcapi.VolumeAPI.create_group_from_src') + @mock.patch('cinder.objects.VolumeList.get_all_by_generic_group') + def test_create_group_from_snap_volume_failed( + self, mock_volume_get_all, + mock_rpc_create_group_from_src, + mock_snap_get_all, mock_group_snap_get, + mock_volume_api_create, + mock_mapping_create, + mock_get_volume_type, mock_volume_delete): + mock_volume_api_create.side_effect = [exception.CinderException] + vol_type = fake_volume.fake_volume_type_obj( + self.ctxt, + id=fake.VOLUME_TYPE_ID, + name='fake_volume_type') + mock_get_volume_type.return_value = vol_type + + grp_snap = utils.create_group_snapshot( + self.ctxt, fake.GROUP_ID, + group_type_id=fake.GROUP_TYPE_ID, + status=fields.GroupStatus.CREATING) + mock_group_snap_get.return_value = grp_snap + vol1 = utils.create_volume( + self.ctxt, + availability_zone='nova', + volume_type_id=vol_type['id'], + group_id=fake.GROUP_ID) + + snap = utils.create_snapshot(self.ctxt, vol1.id, + volume_type_id=vol_type['id'], + status=fields.GroupStatus.CREATING) + mock_snap_get_all.return_value = [snap] + + name = "test_group" + description = "this is a test group" + grp = utils.create_group(self.ctxt, group_type_id=fake.GROUP_TYPE_ID, + volume_type_ids=[vol_type['id']], + availability_zone='nova', + name=name, description=description, + group_snapshot_id=grp_snap.id, + status=fields.GroupStatus.CREATING) + + vol2 = utils.create_volume( + self.ctxt, + availability_zone=grp.availability_zone, + volume_type_id=vol_type['id'], + group_id=grp.id, + snapshot_id=snap.id) + mock_volume_get_all.return_value = [vol2] + + self.assertRaises( + exception.CinderException, + self.group_api._create_group_from_group_snapshot, + self.ctxt, grp, grp_snap.id) + + mock_volume_api_create.assert_called_once_with( + self.ctxt, 1, None, None, + availability_zone=grp.availability_zone, + group_snapshot=grp_snap, + group=grp, + snapshot=snap, + volume_type=vol_type) + + mock_rpc_create_group_from_src.assert_not_called() + + mock_volume_delete.assert_called_once_with(self.ctxt, vol2) + + vol2.destroy() + grp.destroy() + snap.destroy() + vol1.destroy() + grp_snap.destroy() + @mock.patch('cinder.objects.VolumeType.get_by_name_or_id') @mock.patch('cinder.db.group_volume_type_mapping_create') @mock.patch('cinder.volume.api.API.create') @@ -510,6 +588,63 @@ class GroupAPITestCase(test.TestCase): vol.destroy() grp.destroy() + @mock.patch('cinder.volume.api.API.delete') + @mock.patch('cinder.objects.VolumeType.get_by_name_or_id') + @mock.patch('cinder.db.group_volume_type_mapping_create') + @mock.patch('cinder.volume.api.API.create') + @mock.patch('cinder.objects.Group.get_by_id') + @mock.patch('cinder.volume.rpcapi.VolumeAPI.create_group_from_src') + @mock.patch('cinder.objects.VolumeList.get_all_by_generic_group') + def test_create_group_from_group_create_volume_failed( + self, mock_volume_get_all, mock_rpc_create_group_from_src, + mock_group_get, mock_volume_api_create, mock_mapping_create, + mock_get_volume_type, mock_volume_delete): + vol_type = fake_volume.fake_volume_type_obj( + self.ctxt, + id=fake.VOLUME_TYPE_ID, + name='fake_volume_type') + mock_get_volume_type.return_value = vol_type + + grp = utils.create_group(self.ctxt, group_type_id=fake.GROUP_TYPE_ID, + volume_type_ids=[vol_type['id']], + availability_zone='nova', + status=fields.GroupStatus.CREATING) + mock_group_get.return_value = grp + + vol1 = utils.create_volume( + self.ctxt, + availability_zone=grp.availability_zone, + volume_type_id=fake.VOLUME_TYPE_ID, + group_id=grp.id) + vol2 = utils.create_volume( + self.ctxt, + availability_zone=grp.availability_zone, + volume_type_id=fake.VOLUME_TYPE_ID, + group_id=grp.id) + mock_volume_get_all.side_effect = [[vol1, vol2], [vol1]] + + grp2 = utils.create_group(self.ctxt, + group_type_id=fake.GROUP_TYPE_ID, + volume_type_ids=[vol_type['id']], + availability_zone='nova', + source_group_id=grp.id, + status=fields.GroupStatus.CREATING) + + mock_volume_api_create.side_effect = [None, exception.CinderException] + + self.assertRaises( + exception.CinderException, + self.group_api._create_group_from_source_group, + self.ctxt, grp2, grp.id) + + mock_rpc_create_group_from_src.assert_not_called() + mock_volume_delete.assert_called_once_with(self.ctxt, vol1) + + grp2.destroy() + vol2.destroy() + vol1.destroy() + grp.destroy() + @mock.patch('cinder.group.api.API._create_group_from_group_snapshot') @mock.patch('cinder.group.api.API._create_group_from_source_group') @mock.patch('cinder.group.api.API.update_quota')