From 933a833b8d0138dae50a1b19095dd77a2a5cd55b Mon Sep 17 00:00:00 2001 From: katarimanojkumar Date: Tue, 4 Aug 2020 06:49:14 +0000 Subject: [PATCH] [Storwize]:Fix delete_group_snapshot cleanup issue delete_group_snapshot doesn't handle flash copy consistency group cleanup properly. In case of multiple snapshots in the group,existing code exits if any one snapshot deletion fails, but it should update error state and continue with deleting other snapshots. closes bug: #1890241 Change-Id: I87c9f35f34c94bff3ada7d428cd963fc8703a2ad --- .../volume/drivers/ibm/test_storwize_svc.py | 70 +++++++++++++++++++ .../ibm/storwize_svc/storwize_svc_common.py | 34 +++++---- ...e_group_snapshot_fix-2e491e74e1f73ba7.yaml | 8 +++ 3 files changed, 100 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/bug-1890241-strowize-delete_group_snapshot_fix-2e491e74e1f73ba7.yaml diff --git a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py index 31e9c3d4053..19429712478 100644 --- a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py +++ b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py @@ -6295,6 +6295,76 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): for volume in model_update[1]: self.assertEqual(fields.SnapshotStatus.DELETED, volume['status']) + @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', + new=testutils.ZeroIntervalLoopingCall) + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'delete_vdisk') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'delete_fc_consistgrp') + @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type') + def test_storwize_delete_consistgroup_snapshot(self, + is_grp_a_cg_snapshot_type, + delete_fc_consistgrp, + delete_vdisk): + is_grp_a_cg_snapshot_type.side_effect = [True, True, True, False, True] + type_ref = volume_types.create(self.ctxt, 'testtype', None) + group = testutils.create_group(self.ctxt, + group_type_id=fake.GROUP_TYPE_ID, + volume_type_ids=[type_ref['id']]) + + self._create_volume(volume_type_id=type_ref['id'], group_id=group.id) + self._create_volume(volume_type_id=type_ref['id'], group_id=group.id) + + group_snapshot, snapshots = self._create_group_snapshot(group.id) + cgsnapshot_id = group_snapshot.id + cg_name = 'cg_snap-' + cgsnapshot_id + + self.driver._helpers.delete_consistgrp_snapshots(cg_name, snapshots) + + delete_fc_consistgrp.assert_has_calls([mock.call(cg_name)]) + self.assertEqual(2, delete_fc_consistgrp.call_count) + + calls = [mock.call(snapshots[0]['name'], force_delete=True, + force_unmap=False), + mock.call(snapshots[1]['name'], force_delete=True, + force_unmap=False)] + delete_vdisk.assert_has_calls(calls, any_order=True) + + @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', + new=testutils.ZeroIntervalLoopingCall) + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'delete_vdisk') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'delete_fc_consistgrp') + @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type') + def test_storwize_delete_consistgroup_snapshot_1(self, + is_grp_a_cg_snapshot_type, + delete_fc_consistgrp, + delete_vdisk): + is_grp_a_cg_snapshot_type.side_effect = [True, True, True, False, True] + type_ref = volume_types.create(self.ctxt, 'testtype', None) + group = testutils.create_group(self.ctxt, + group_type_id=fake.GROUP_TYPE_ID, + volume_type_ids=[type_ref['id']]) + + self._create_volume(volume_type_id=type_ref['id'], group_id=group.id) + self._create_volume(volume_type_id=type_ref['id'], group_id=group.id) + + group_snapshot, snapshots = self._create_group_snapshot(group.id) + cgsnapshot_id = group_snapshot.id + cg_name = 'cg_snap-' + cgsnapshot_id + delete_vdisk.side_effect = exception.VolumeBackendAPIException(data='') + + (model_update, + snap_model_update) = self.driver._helpers.delete_consistgrp_snapshots( + cg_name, snapshots) + self.assertEqual(fields.GroupSnapshotStatus.ERROR_DELETING, + model_update['status']) + + for snapshot in snap_model_update: + self.assertEqual(fields.SnapshotStatus.ERROR_DELETING, + snapshot['status']) + @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', new=testutils.ZeroIntervalLoopingCall) def test_storwize_create_group_from_src_invalid(self): diff --git a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py index 26c3e9eaaa3..769c4fa1e47 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py @@ -53,6 +53,7 @@ from cinder.volume import volume_utils INTERVAL_1_SEC = 1 DEFAULT_TIMEOUT = 15 +CMMVC5753E = "CMMVC5753E" LOG = logging.getLogger(__name__) storwize_svc_opts = [ @@ -1851,21 +1852,30 @@ class StorwizeHelpers(object): snapshots_model_update = [] try: - for snapshot in snapshots: + self.delete_fc_consistgrp(fc_consistgrp) + except exception.VolumeBackendAPIException as err: + if CMMVC5753E in err.msg: + LOG.warning('Failed to delete as flash copy consistency ' + 'group %s does not exist,ignoring err: %s', + fc_consistgrp, err) + + for snapshot in snapshots: + try: self.delete_vdisk(snapshot['name'], force_unmap=False, force_delete=True) - except exception.VolumeBackendAPIException as err: - model_update['status'] = ( - fields.GroupSnapshotStatus.ERROR_DELETING) - LOG.error("Failed to delete the snapshot %(snap)s of " - "CGSnapshot. Exception: %(exception)s.", - {'snap': snapshot['name'], 'exception': err}) - - for snapshot in snapshots: - snapshots_model_update.append( - {'id': snapshot['id'], - 'status': model_update['status']}) + snapshots_model_update.append( + {'id': snapshot['id'], + 'status': fields.GroupSnapshotStatus.DELETED}) + except exception.VolumeBackendAPIException as err: + model_update['status'] = ( + fields.GroupSnapshotStatus.ERROR_DELETING) + snapshots_model_update.append( + {'id': snapshot['id'], + 'status': fields.GroupSnapshotStatus.ERROR_DELETING}) + LOG.error("Failed to delete the snapshot %(snap)s of " + "CGSnapshot. Exception: %(exception)s.", + {'snap': snapshot['name'], 'exception': err}) return model_update, snapshots_model_update diff --git a/releasenotes/notes/bug-1890241-strowize-delete_group_snapshot_fix-2e491e74e1f73ba7.yaml b/releasenotes/notes/bug-1890241-strowize-delete_group_snapshot_fix-2e491e74e1f73ba7.yaml new file mode 100644 index 00000000000..e4a8815164e --- /dev/null +++ b/releasenotes/notes/bug-1890241-strowize-delete_group_snapshot_fix-2e491e74e1f73ba7.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + `Bug #1890241 `_: + During delete_group_snapshot on IBM storwize, in case of multiple + snapshots in the group, delete flow exits if any one snapshot deletion + fails, but it should update error state and continue with deleting + other snapshots.