Skip to content

Add HPE Nimble driver patch to fix clone snapshot issue #1137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Yaguang Tang (yaguangtang) wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add HPE Nimble driver patch to fix clone snapshot issue #1137

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
270 changes: 270 additions & 0 deletions patches/openstack/cinder/0004-Fix-Nimble-clone-snapshot-cleanup-on-delete.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,270 @@
From 07077c3f2b4b45caa0428a0c500aa966fa945b42 Mon Sep 17 00:00:00 2001
From: Yaguang Tang <heut2008@gmail.com>
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
+ <https://bugs.launchpad.net/cinder/+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)

Loading