From 04ea9a9092fad49a05e8b2de4504b54b5dd7c6dd Mon Sep 17 00:00:00 2001 From: Yaguang Tang Date: Thu, 25 Jun 2026 22:41:19 +0800 Subject: [PATCH] Add HPE Nimble driver patch to fix clone snapshot issue Signed-off-by: Yaguang Tang --- ...ble-clone-snapshot-cleanup-on-delete.patch | 270 ++++++++++++++++++ 1 file changed, 270 insertions(+) create mode 100644 patches/openstack/cinder/0004-Fix-Nimble-clone-snapshot-cleanup-on-delete.patch diff --git a/patches/openstack/cinder/0004-Fix-Nimble-clone-snapshot-cleanup-on-delete.patch b/patches/openstack/cinder/0004-Fix-Nimble-clone-snapshot-cleanup-on-delete.patch new file mode 100644 index 00000000..13f1fcbc --- /dev/null +++ b/patches/openstack/cinder/0004-Fix-Nimble-clone-snapshot-cleanup-on-delete.patch @@ -0,0 +1,270 @@ +From 07077c3f2b4b45caa0428a0c500aa966fa945b42 Mon Sep 17 00:00:00 2001 +From: Yaguang Tang +Date: Thu, 25 Jun 2026 22:24:50 +0800 +Subject: [PATCH] Fix Nimble clone snapshot cleanup on delete + +The Nimble driver already cleans up temporary snapshots used by +backup clone workflows, but it leaves the base snapshot behind for +source-volume clones. Detect the temporary openstack-clone-volume +snapshot and remove it after the cloned volume is deleted. +Add unit test coverage and a release note for the fix. + +Closes-Bug: #1807626 + +Change-Id: Id75066bf0dd9d33feb22709042b456b11b26c502 +--- + .../unit/volume/drivers/hpe/test_nimble.py | 85 +++++++++++++++++++ + cinder/volume/drivers/hpe/nimble.py | 59 +++++++++++-- + ...one-snapshot-cleanup-7f3be6d4d6b64df1.yaml | 9 ++ + 3 files changed, 145 insertions(+), 8 deletions(-) + create mode 100644 releasenotes/notes/bug-1807626-nimble-clone-snapshot-cleanup-7f3be6d4d6b64df1.yaml + +diff --git a/cinder/tests/unit/volume/drivers/hpe/test_nimble.py b/cinder/tests/unit/volume/drivers/hpe/test_nimble.py +index 0fba131ae..025ef99c7 100644 +--- a/cinder/tests/unit/volume/drivers/hpe/test_nimble.py ++++ b/cinder/tests/unit/volume/drivers/hpe/test_nimble.py +@@ -125,6 +125,15 @@ FAKE_GET_VOL_INFO_BACKUP_RESPONSE = {'name': 'testvolume', + fake.VOLUME2_ID, + 'base_snap_id': 'test-backup-snap'} + ++FAKE_GET_VOL_INFO_CLONE_RESPONSE = {'name': 'testvolume', ++ 'clone': True, ++ 'target_name': 'iqn.test', ++ 'online': False, ++ 'agent_type': 'openstack', ++ 'parent_vol_id': 'volume-' + ++ fake.VOLUME2_ID, ++ 'base_snap_id': 'test-clone-snap'} ++ + FAKE_GET_SNAP_INFO_BACKUP_RESPONSE = { + 'description': "backup-vol-" + fake.VOLUME2_ID, + 'name': 'test-backup-snap', +@@ -132,6 +141,13 @@ FAKE_GET_SNAP_INFO_BACKUP_RESPONSE = { + 'vol_id': fake.VOLUME_ID, + 'volume_name': 'volume-' + fake.VOLUME_ID} + ++FAKE_GET_SNAP_INFO_CLONE_RESPONSE = { ++ 'description': '', ++ 'name': 'openstack-clone-volume-' + fake.VOLUME_ID + '-tmpclone', ++ 'id': fake.SNAPSHOT_ID, ++ 'vol_id': fake.VOLUME2_ID, ++ 'volume_name': 'volume-' + fake.VOLUME2_ID} ++ + FAKE_POSITIVE_GROUP_CONFIG_RESPONSE = { + 'name': 'group-test', + 'version_current': '0.0.0.0', +@@ -823,6 +839,8 @@ class NimbleDriverVolumeTestCase(NimbleDriverBaseTestCase): + mock.Mock(return_value=[])) + @NimbleDriverBaseTestCase.client_mock_decorator(create_configuration( + 'nimble', 'nimble_pass', '10.18.108.55', 'default', '*')) ++ @mock.patch(NIMBLE_ISCSI_DRIVER + ".is_volume_clone", mock.Mock( ++ return_value=['', ''])) + @mock.patch(NIMBLE_ISCSI_DRIVER + ".is_volume_backup_clone", mock.Mock( + return_value=['', ''])) + def test_delete_volume(self): +@@ -843,6 +861,8 @@ class NimbleDriverVolumeTestCase(NimbleDriverBaseTestCase): + mock.Mock(return_value=[])) + @NimbleDriverBaseTestCase.client_mock_decorator(create_configuration( + 'nimble', 'nimble_pass', '10.18.108.55', 'default', '*')) ++ @mock.patch(NIMBLE_ISCSI_DRIVER + ".is_volume_clone", mock.Mock( ++ return_value=['', ''])) + @mock.patch(NIMBLE_ISCSI_DRIVER + ".is_volume_backup_clone", mock.Mock( + return_value=['', ''])) + def test_delete_volume_with_clone(self): +@@ -868,6 +888,8 @@ class NimbleDriverVolumeTestCase(NimbleDriverBaseTestCase): + @mock.patch(NIMBLE_CLIENT) + @NimbleDriverBaseTestCase.client_mock_decorator(create_configuration( + 'nimble', 'nimble_pass', '10.18.108.55', 'default', '*')) ++ @mock.patch(NIMBLE_ISCSI_DRIVER + ".is_volume_clone", mock.Mock( ++ return_value=['', ''])) + @mock.patch(NIMBLE_ISCSI_DRIVER + ".is_volume_backup_clone", mock.Mock( + return_value=['test-backup-snap', 'volume-' + fake.VOLUME_ID])) + @mock.patch.object(obj_volume.VolumeList, 'get_all_by_host') +@@ -894,6 +916,38 @@ class NimbleDriverVolumeTestCase(NimbleDriverBaseTestCase): + + self.mock_client_service.assert_has_calls(expected_calls) + ++ @mock.patch(NIMBLE_URLLIB2) ++ @mock.patch(NIMBLE_CLIENT) ++ @NimbleDriverBaseTestCase.client_mock_decorator(create_configuration( ++ 'nimble', 'nimble_pass', '10.18.108.55', 'default', '*')) ++ @mock.patch(NIMBLE_ISCSI_DRIVER + ".is_volume_clone", mock.Mock( ++ return_value=['test-clone-snap', 'volume-' + fake.VOLUME2_ID])) ++ @mock.patch(NIMBLE_ISCSI_DRIVER + ".is_volume_backup_clone", mock.Mock( ++ return_value=['', ''])) ++ @mock.patch.object(obj_volume.VolumeList, 'get_all_by_host') ++ def test_delete_volume_with_source_clone(self, mock_volume_list): ++ mock_volume_list.return_value = [] ++ self.mock_client_service.online_vol.return_value = ( ++ FAKE_GENERIC_POSITIVE_RESPONSE) ++ self.mock_client_service.delete_vol.return_value = ( ++ FAKE_GENERIC_POSITIVE_RESPONSE) ++ self.mock_client_service.online_snap.return_value = ( ++ FAKE_GENERIC_POSITIVE_RESPONSE) ++ self.mock_client_service.delete_snap.return_value = ( ++ FAKE_GENERIC_POSITIVE_RESPONSE) ++ ++ self.driver.delete_volume({'name': 'testvolume'}) ++ expected_calls = [mock.call.online_vol( ++ 'testvolume', False), ++ mock.call.delete_vol('testvolume'), ++ mock.call.online_snap('volume-' + fake.VOLUME2_ID, ++ False, ++ 'test-clone-snap'), ++ mock.call.delete_snap('volume-' + fake.VOLUME2_ID, ++ 'test-clone-snap')] ++ ++ self.mock_client_service.assert_has_calls(expected_calls) ++ + @mock.patch(NIMBLE_URLLIB2) + @mock.patch(NIMBLE_CLIENT) + @mock.patch.object(obj_volume.VolumeList, 'get_all_by_host', +@@ -901,6 +955,8 @@ class NimbleDriverVolumeTestCase(NimbleDriverBaseTestCase): + @NimbleDriverBaseTestCase.client_mock_decorator(create_configuration( + NIMBLE_SAN_LOGIN, NIMBLE_SAN_PASS, NIMBLE_MANAGEMENT_IP, + 'default', '*', devices=REPL_DEVICES)) ++ @mock.patch(NIMBLE_ISCSI_DRIVER + ".is_volume_clone", mock.Mock( ++ return_value=['', ''])) + @mock.patch(NIMBLE_ISCSI_DRIVER + ".is_volume_backup_clone", mock.Mock( + return_value=['', ''])) + def test_delete_volume_replicated(self): +@@ -1277,6 +1333,35 @@ class NimbleDriverVolumeTestCase(NimbleDriverBaseTestCase): + ] + self.mock_client_service.assert_has_calls(expected_calls) + ++ @mock.patch(NIMBLE_URLLIB2) ++ @mock.patch(NIMBLE_CLIENT) ++ @mock.patch.object(obj_volume.VolumeList, 'get_all_by_host', ++ mock.Mock(return_value=[])) ++ @NimbleDriverBaseTestCase.client_mock_decorator(create_configuration( ++ 'nimble', 'nimble_pass', '10.18.108.55', 'default', '*')) ++ def test_is_volume_clone(self): ++ self.mock_client_service.get_vol_info.return_value = ( ++ FAKE_GET_VOL_INFO_CLONE_RESPONSE) ++ self.mock_client_service.get_snap_info_by_id.return_value = ( ++ FAKE_GET_SNAP_INFO_CLONE_RESPONSE) ++ self.mock_client_service.get_snap_info_detail.return_value = ( ++ FAKE_GET_SNAP_INFO_CLONE_RESPONSE) ++ self.mock_client_service.get_volume_name.return_value = ( ++ 'volume-' + fake.VOLUME2_ID) ++ ++ volume = obj_volume.Volume(context.get_admin_context(), ++ id=fake.VOLUME_ID, ++ _name_id=None) ++ self.assertEqual(("openstack-clone-volume-" + fake.VOLUME_ID + ++ "-tmpclone", "volume-" + fake.VOLUME2_ID), ++ self.driver.is_volume_clone(volume)) ++ expected_calls = [ ++ mock.call.get_vol_info('volume-' + fake.VOLUME_ID), ++ mock.call.get_snap_info_by_id('test-clone-snap', ++ 'volume-' + fake.VOLUME2_ID) ++ ] ++ self.mock_client_service.assert_has_calls(expected_calls) ++ + @mock.patch(NIMBLE_URLLIB2) + @mock.patch(NIMBLE_CLIENT) + @NimbleDriverBaseTestCase.client_mock_decorator(create_configuration( +diff --git a/cinder/volume/drivers/hpe/nimble.py b/cinder/volume/drivers/hpe/nimble.py +index 55715a0f7..00dceb1fd 100644 +--- a/cinder/volume/drivers/hpe/nimble.py ++++ b/cinder/volume/drivers/hpe/nimble.py +@@ -63,6 +63,7 @@ DEFAULT_IOPS_LIMIT_SETTING = None + DEFAULT_FOLDER_SETTING = None + DEFAULT_SNAP_QUOTA = sys.maxsize + BACKUP_VOL_PREFIX = 'backup-vol-' ++CLONE_VOL_PREFIX = 'openstack-clone-volume' + AGENT_TYPE_OPENSTACK = 'openstack' + AGENT_TYPE_OPENSTACK_GST = 'openstackv2' + AGENT_TYPE_NONE = 'none' +@@ -243,8 +244,42 @@ class NimbleBaseVolumeDriver(san.SanDriver): + return snap_info['name'], snap_vol_name + return "", "" + ++ def is_volume_clone(self, volume): ++ """Check if the volume was created through the source-volume workflow. ++ ++ :param volume ++ """ ++ vol_info = self.APIExecutor.get_vol_info(volume['name']) ++ LOG.debug("is_clone: %(is_clone)s base_snap_id: %(snap)s, " ++ "parent_vol_id: %(vol)s", ++ {'is_clone': vol_info['clone'], ++ 'snap': vol_info['base_snap_id'], ++ 'vol': vol_info['parent_vol_id']}) ++ ++ if vol_info['base_snap_id'] and ( ++ vol_info['parent_vol_id'] is not None): ++ LOG.debug("Nimble base-snap exists for volume %(vol)s", ++ {'vol': volume['name']}) ++ volume_name_prefix = volume['name'].replace(volume['id'], "") ++ LOG.debug("volume_name_prefix : %(prefix)s", ++ {'prefix': volume_name_prefix}) ++ snap_id = self.APIExecutor.get_snap_info_by_id( ++ vol_info['base_snap_id'], ++ vol_info['parent_vol_id']) ++ snap_info = self.APIExecutor.get_snap_info_detail(snap_id['id']) ++ LOG.debug("snap_info name %(snap_info)s", ++ {'snap_info': snap_info['name']}) ++ if snap_info['name'] and CLONE_VOL_PREFIX in snap_info['name']: ++ snap_vol_name = self.APIExecutor.get_volume_name( ++ snap_info['vol_id']) ++ LOG.debug("snap_vol_name %(snap)s", ++ {'snap': snap_vol_name}) ++ return snap_info['name'], snap_vol_name ++ return "", "" ++ + def delete_volume(self, volume): + """Delete the specified volume.""" ++ clone_snap_name, clone_vol_name = self.is_volume_clone(volume) + backup_snap_name, backup_vol_name = self.is_volume_backup_clone(volume) + time.sleep(DEFAULT_SLEEP) + +@@ -281,14 +316,22 @@ class NimbleBaseVolumeDriver(san.SanDriver): + self.APIExecutor.online_vol(volume['name'], True) + raise exception.VolumeIsBusy(volume_name=volume['name']) + raise +- # Nimble backend does not delete the snapshot from the parent volume +- # if there is a dependent clone. So the deletes need to be in reverse +- # order i.e. +- # 1. First delete the clone volume used for backup +- # 2. Delete the base snapshot used for clone from the parent volume. +- # This is only done for the force backup clone operation as it is +- # a temporary operation in which we are certain that the snapshot does +- # not need to be preserved after the backup is completed. ++ # Nimble backend does not delete the temporary base snapshot from the ++ # parent volume when a dependent clone is deleted. For source-volume ++ # clones and force-backup clones, remove the snapshot after the clone ++ # volume has been deleted. ++ ++ if (clone_snap_name != "" and clone_vol_name != "") and ( ++ clone_snap_name is not None): ++ LOG.debug("Delete volume clone vol: %(vol)s snap: %(snap)s", ++ {'vol': clone_vol_name, ++ 'snap': clone_snap_name}) ++ self.APIExecutor.online_snap(clone_vol_name, ++ False, ++ clone_snap_name) ++ ++ self.APIExecutor.delete_snap(clone_vol_name, ++ clone_snap_name) + + if (backup_snap_name != "" and backup_vol_name != "") and ( + backup_snap_name is not None): +diff --git a/releasenotes/notes/bug-1807626-nimble-clone-snapshot-cleanup-7f3be6d4d6b64df1.yaml b/releasenotes/notes/bug-1807626-nimble-clone-snapshot-cleanup-7f3be6d4d6b64df1.yaml +new file mode 100644 +index 000000000..ef3205f1e +--- /dev/null ++++ b/releasenotes/notes/bug-1807626-nimble-clone-snapshot-cleanup-7f3be6d4d6b64df1.yaml +@@ -0,0 +1,9 @@ ++--- ++fixes: ++ - | ++ Nimble driver `bug #1807626 ++ `_: Fixed an issue ++ where deleting a volume created from a source volume or volume-backed ++ image could leave the temporary ``openstack-clone-volume-*`` snapshot ++ on the parent Nimble volume. The driver now removes that temporary ++ base snapshot after the cloned volume is deleted. +-- +2.50.1 (Apple Git-155) +