From a0d075219a1776f255e44982ffae652d5ab80506 Mon Sep 17 00:00:00 2001 From: Atsushi Kawai Date: Tue, 20 Jun 2023 19:24:01 +0900 Subject: [PATCH] Hitachi: Fix exception when deleted volume is busy This patch is to fix an exception name when deleted volume is busy in delete_volume(). Although exception ``VolumeIsBusy`` should be issued in that case, ``VolumeDriverException`` is issued. It should be fixed. Closes-Bug: #2024418 Change-Id: I5405790c7cd4ca513ceea70380be723a54c3fe3c --- .../hitachi/test_hitachi_hbsd_mirror_fc.py | 64 +++++++++++++++---- .../hitachi/test_hitachi_hbsd_rest_fc.py | 46 ++++++++++++- cinder/volume/drivers/hitachi/hbsd_common.py | 6 +- ...ix-except-in-del-vol-ca8b4c5d40d69531.yaml | 6 ++ 4 files changed, 104 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/hitachi-vsp-fix-except-in-del-vol-ca8b4c5d40d69531.yaml diff --git a/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_mirror_fc.py b/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_mirror_fc.py index e65c2fde7c8..051d1f77005 100644 --- a/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_mirror_fc.py +++ b/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_mirror_fc.py @@ -141,19 +141,20 @@ def _volume_get(context, volume_id): TEST_SNAPSHOT = [] -snapshot = {} -snapshot['id'] = '10000000-0000-0000-0000-{0:012d}'.format(0) -snapshot['name'] = 'TEST_SNAPSHOT{0:d}'.format(0) -snapshot['provider_location'] = '{0:d}'.format(1) -snapshot['status'] = 'available' -snapshot['volume_id'] = '00000000-0000-0000-0000-{0:012d}'.format(0) -snapshot['volume'] = _volume_get(None, snapshot['volume_id']) -snapshot['volume_name'] = 'test-volume{0:d}'.format(0) -snapshot['volume_size'] = 128 -snapshot = obj_snap.Snapshot._from_db_object( - CTXT, obj_snap.Snapshot(), - fake_snapshot.fake_db_snapshot(**snapshot)) -TEST_SNAPSHOT.append(snapshot) +for i in range(2): + snapshot = {} + snapshot['id'] = '10000000-0000-0000-0000-{0:012d}'.format(i) + snapshot['name'] = 'TEST_SNAPSHOT{0:d}'.format(i) + snapshot['provider_location'] = '{0:d}'.format(i + 1) + snapshot['status'] = 'available' + snapshot['volume_id'] = '00000000-0000-0000-0000-{0:012d}'.format(i) + snapshot['volume'] = _volume_get(None, snapshot['volume_id']) + snapshot['volume_name'] = 'test-volume{0:d}'.format(i) + snapshot['volume_size'] = 128 + snapshot = obj_snap.Snapshot._from_db_object( + CTXT, obj_snap.Snapshot(), + fake_snapshot.fake_db_snapshot(**snapshot)) + TEST_SNAPSHOT.append(snapshot) TEST_GROUP = [] for i in range(2): @@ -373,6 +374,18 @@ GET_SNAPSHOTS_RESULT_BUSY = { ], } +GET_SNAPSHOTS_RESULT_TEST = { + "data": [ + { + "primaryOrSecondary": "S-VOL", + "status": "PSUS", + "pvolLdevId": 1, + "muNumber": 1, + "svolLdevId": 1, + }, + ], +} + GET_LUNS_RESULT = { "data": [ { @@ -999,6 +1012,23 @@ class HBSDMIRRORFCDriverTest(test.TestCase): self.driver.delete_snapshot(TEST_SNAPSHOT[0]) self.assertEqual(14, request.call_count) + @mock.patch.object(requests.Session, "request") + def test_delete_snapshot_pldev_in_loc(self, request): + self.assertRaises(exception.VolumeDriverException, + self.driver.delete_snapshot, + TEST_SNAPSHOT[1]) + self.assertEqual(1, request.call_count) + + @mock.patch.object(requests.Session, "request") + def test_delete_snapshot_snapshot_is_busy(self, request): + request.side_effect = [FakeResponse(200, GET_LDEV_RESULT_PAIR), + FakeResponse(200, NOTFOUND_RESULT), + FakeResponse(200, GET_SNAPSHOTS_RESULT_TEST)] + self.assertRaises(exception.SnapshotIsBusy, + self.driver.delete_snapshot, + TEST_SNAPSHOT[0]) + self.assertEqual(3, request.call_count) + @mock.patch.object(requests.Session, "request") @mock.patch.object(volume_types, 'get_volume_type') @mock.patch.object(volume_types, 'get_volume_type_extra_specs') @@ -1232,6 +1262,14 @@ class HBSDMIRRORFCDriverTest(test.TestCase): self.driver.unmanage(TEST_VOLUME[0]) self.assertEqual(3, request.call_count) + @mock.patch.object(requests.Session, "request") + def test_unmanage_has_rep_pair_true(self, request): + request.return_value = FakeResponse(200, GET_LDEV_RESULT_REP) + self.assertRaises(exception.VolumeDriverException, + self.driver.unmanage, + TEST_VOLUME[4]) + self.assertEqual(1, request.call_count) + @mock.patch.object(requests.Session, "request") def test_copy_image_to_volume(self, request): image_service = 'fake_image_service' diff --git a/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_fc.py b/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_fc.py index 63e48b5bdd0..f4e7183e7a9 100644 --- a/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_fc.py +++ b/cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_fc.py @@ -243,6 +243,14 @@ GET_LDEV_RESULT_PAIR = { "status": "NML", } +GET_LDEV_RESULT_PAIR_TEST = { + "emulationType": "OPEN-V-CVS", + "blockCapacity": 2097152, + "attributes": ["CVS", "HDP", "HTI", "111"], + "status": "NML", + "snapshotPoolId": 0 +} + GET_LDEV_RESULT_PAIR_STATUS_TEST = { "emulationType": "OPEN-V-CVS", "blockCapacity": 2097152, @@ -834,7 +842,7 @@ class HBSDRESTFCDriverTest(test.TestCase): self.assertEqual(4, request.call_count) @mock.patch.object(requests.Session, "request") - def test_delete_volume_temporary_busy(self, request): + def test_delete_volume_wait_copy_pair_deleting(self, request): request.side_effect = [FakeResponse(200, GET_LDEV_RESULT_PAIR), FakeResponse(200, GET_SNAPSHOTS_RESULT_BUSY), FakeResponse(200, GET_LDEV_RESULT), @@ -849,7 +857,7 @@ class HBSDRESTFCDriverTest(test.TestCase): @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', new=test_utils.ZeroIntervalLoopingCall) @mock.patch.object(requests.Session, "request") - def test_delete_volume_busy_timeout(self, request): + def test_delete_volume_request_failed(self, request): request.side_effect = [FakeResponse(200, GET_LDEV_RESULT_PAIR), FakeResponse(200, GET_SNAPSHOTS_RESULT_BUSY), FakeResponse(200, GET_LDEV_RESULT_PAIR), @@ -860,6 +868,15 @@ class HBSDRESTFCDriverTest(test.TestCase): TEST_VOLUME[0]) self.assertGreater(request.call_count, 2) + @mock.patch.object(requests.Session, "request") + def test_delete_volume_volume_is_busy(self, request): + request.side_effect = [FakeResponse(200, GET_LDEV_RESULT_PAIR), + FakeResponse(200, GET_SNAPSHOTS_RESULT_PAIR)] + self.assertRaises(exception.VolumeIsBusy, + self.driver.delete_volume, + TEST_VOLUME[0]) + self.assertEqual(2, request.call_count) + @mock.patch.object(requests.Session, "request") def test_extend_volume(self, request): request.side_effect = [FakeResponse(200, GET_LDEV_RESULT), @@ -1243,6 +1260,31 @@ class HBSDRESTFCDriverTest(test.TestCase): self.driver.unmanage(TEST_VOLUME[0]) self.assertEqual(2, request.call_count) + @mock.patch.object(requests.Session, "request") + def test_unmanage_volume_is_busy(self, request): + request.side_effect = [ + FakeResponse(200, GET_LDEV_RESULT_PAIR), + FakeResponse(200, GET_LDEV_RESULT_PAIR), + FakeResponse(200, NOTFOUND_RESULT), + FakeResponse(200, GET_SNAPSHOTS_RESULT_PAIR), + ] + self.assertRaises(exception.VolumeIsBusy, + self.driver.unmanage, + TEST_VOLUME[1]) + self.assertEqual(4, request.call_count) + + @mock.patch.object(requests.Session, "request") + def test_unmanage_volume_is_busy_raise_ex(self, request): + request.side_effect = [ + FakeResponse(200, GET_LDEV_RESULT_PAIR), + FakeResponse(200, GET_LDEV_RESULT_PAIR), + FakeResponse(400, GET_SNAPSHOTS_RESULT_BUSY) + ] + self.assertRaises(exception.VolumeDriverException, + self.driver.unmanage, + TEST_VOLUME[0]) + self.assertEqual(3, request.call_count) + @mock.patch.object(requests.Session, "request") def test_copy_image_to_volume(self, request): image_service = 'fake_image_service' diff --git a/cinder/volume/drivers/hitachi/hbsd_common.py b/cinder/volume/drivers/hitachi/hbsd_common.py index ec8170dc9a5..da015ba6fe7 100644 --- a/cinder/volume/drivers/hitachi/hbsd_common.py +++ b/cinder/volume/drivers/hitachi/hbsd_common.py @@ -400,7 +400,7 @@ class HBSDCommon(): try: self.delete_ldev(ldev) except exception.VolumeDriverException as ex: - if ex.msg == utils.BUSY_MESSAGE: + if utils.BUSY_MESSAGE in ex.msg: raise exception.VolumeIsBusy(volume_name=volume['name']) else: raise ex @@ -437,7 +437,7 @@ class HBSDCommon(): try: self.delete_ldev(ldev) except exception.VolumeDriverException as ex: - if ex.msg == utils.BUSY_MESSAGE: + if utils.BUSY_MESSAGE in ex.msg: raise exception.SnapshotIsBusy(snapshot_name=snapshot['name']) else: raise ex @@ -592,7 +592,7 @@ class HBSDCommon(): try: self.delete_pair(ldev) except exception.VolumeDriverException as ex: - if ex.msg == utils.BUSY_MESSAGE: + if utils.BUSY_MESSAGE in ex.msg: raise exception.VolumeIsBusy(volume_name=volume['name']) else: raise ex diff --git a/releasenotes/notes/hitachi-vsp-fix-except-in-del-vol-ca8b4c5d40d69531.yaml b/releasenotes/notes/hitachi-vsp-fix-except-in-del-vol-ca8b4c5d40d69531.yaml new file mode 100644 index 00000000000..d8a0dcbc4ba --- /dev/null +++ b/releasenotes/notes/hitachi-vsp-fix-except-in-del-vol-ca8b4c5d40d69531.yaml @@ -0,0 +1,6 @@ +fixes: + - | + Hitachi driver `bug #2024418 + `_: Fixed to raise + correct exception when volume is busy while performing delete volume + operation. \ No newline at end of file