From 59fb14f58566bc92be1bc398d2cd1402aa2168e0 Mon Sep 17 00:00:00 2001 From: bmeagherix <118192357+bmeagherix@users.noreply.github.com> Date: Wed, 18 Dec 2024 14:17:30 +0000 Subject: [PATCH] NAS-133001 / 25.04 / Add flag to delete extents when removing a target (#15226) * Add optional delete_extents parameter to iscsi.target.delete * Add iSCSI unit test test__target_delete_extents --- .../middlewared/api/v25_04_0/iscsi_target.py | 1 + .../middlewared/plugins/iscsi_/targets.py | 4 +- tests/api2/assets/websocket/iscsi.py | 10 ++-- tests/api2/test_261_iscsi_cmd.py | 46 ++++++++++++++++++- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/middlewared/middlewared/api/v25_04_0/iscsi_target.py b/src/middlewared/middlewared/api/v25_04_0/iscsi_target.py index 5a9bab136c064..c5ede91e1020e 100644 --- a/src/middlewared/middlewared/api/v25_04_0/iscsi_target.py +++ b/src/middlewared/middlewared/api/v25_04_0/iscsi_target.py @@ -86,6 +86,7 @@ class IscsiTargetUpdateResult(BaseModel): class IscsiTargetDeleteArgs(BaseModel): id: int force: bool = False + delete_extents: bool = False class IscsiTargetDeleteResult(BaseModel): diff --git a/src/middlewared/middlewared/plugins/iscsi_/targets.py b/src/middlewared/middlewared/plugins/iscsi_/targets.py index 8741b53e196e7..62ed2c9a6da9a 100644 --- a/src/middlewared/middlewared/plugins/iscsi_/targets.py +++ b/src/middlewared/middlewared/plugins/iscsi_/targets.py @@ -334,7 +334,7 @@ async def do_update(self, audit_callback, id_, data): audit='Delete iSCSI target', audit_callback=True ) - async def do_delete(self, audit_callback, id_, force): + async def do_delete(self, audit_callback, id_, force, delete_extents): """ Delete iSCSI Target of `id`. @@ -350,6 +350,8 @@ async def do_delete(self, audit_callback, id_, force): raise CallError(f'Target {target["name"]} is in use.') for target_to_extent in await self.middleware.call('iscsi.targetextent.query', [['target', '=', id_]]): await self.middleware.call('iscsi.targetextent.delete', target_to_extent['id'], force) + if delete_extents: + await self.middleware.call('iscsi.extent.delete', target_to_extent['extent'], False, force) await self.middleware.call( 'datastore.delete', 'services.iscsitargetgroups', [['iscsi_target', '=', id_]] diff --git a/tests/api2/assets/websocket/iscsi.py b/tests/api2/assets/websocket/iscsi.py index 85a3c3b5dfbaa..6d31e3f78fe63 100644 --- a/tests/api2/assets/websocket/iscsi.py +++ b/tests/api2/assets/websocket/iscsi.py @@ -72,7 +72,8 @@ def target(target_name, groups, alias=None): try: yield target_config finally: - call('iscsi.target.delete', target_config['id'], True) + if call('iscsi.target.query', [['id', '=', target_config['id']]]): + call('iscsi.target.delete', target_config['id'], True) @contextlib.contextmanager @@ -86,7 +87,8 @@ def zvol_extent(zvol, extent_name): try: yield config finally: - call('iscsi.extent.delete', config['id'], True, True) + if call('iscsi.extent.query', [['id', '=', config['id']]]): + call('iscsi.extent.delete', config['id'], True, True) @contextlib.contextmanager @@ -105,7 +107,9 @@ def target_extent_associate(target_id, extent_id, lun_id=0): try: yield associate_config finally: - call('iscsi.targetextent.delete', associate_config['id'], True) + # We may have deleted the association + if call('iscsi.targetextent.query', [['id', '=', associate_config['id']]]): + call('iscsi.targetextent.delete', associate_config['id'], True) if alua_enabled: sleep(2) diff --git a/tests/api2/test_261_iscsi_cmd.py b/tests/api2/test_261_iscsi_cmd.py index 3f6ec516252ca..212e6eafe0d36 100644 --- a/tests/api2/test_261_iscsi_cmd.py +++ b/tests/api2/test_261_iscsi_cmd.py @@ -23,7 +23,7 @@ from pyscsi.pyscsi.scsi_sense import sense_ascq_dict, sense_key_dict from pytest_dependency import depends -from middlewared.service_exception import InstanceNotFound, ValidationError, ValidationErrors +from middlewared.service_exception import CallError, InstanceNotFound, ValidationError, ValidationErrors from middlewared.test.integration.assets.iscsi import target_login_test from middlewared.test.integration.assets.pool import dataset, snapshot from middlewared.test.integration.utils import call, ssh @@ -2950,3 +2950,47 @@ def check_readonly_state(zvolid, extentid, readonly): read_lbas(s, True) write_lbas(s) read_lbas(s) + + +def test__target_delete_extents(iscsi_running): + """Validate that we can delete a target and its extents.""" + name1 = f"{target_name}x1" + name2 = f"{target_name}x2" + name3 = f"{target_name}x3" + iqn1 = f'{basename}:{name1}' + iqn2 = f'{basename}:{name2}' + + with initiator_portal() as config: + with configured_target(config, name1, 'VOLUME') as target1_config: + with iscsi_scsi_connection(truenas_server.ip, iqn1): + # Without force we cannot delete a target that is logged into + with pytest.raises(CallError) as ve: + call('iscsi.target.delete', target1_config['target']['id']) + assert f'Target {name1} is in use' in ve.value.errmsg + + # Force the target delete, but do NOT remove the associated + # extents + call('iscsi.target.delete', target1_config['target']['id'], True) + + # Ensure the extent still exists + extents = call('iscsi.extent.query', [['id', '=', target1_config['extent']['id']]]) + assert len(extents) == 1, extents + + with configured_target(config, name2, 'VOLUME') as target2_config: + with iscsi_scsi_connection(truenas_server.ip, iqn2): + # Force the target delete, and DO remove the associated + # extents + call('iscsi.target.delete', target2_config['target']['id'], True, True) + + # Ensure the extent does not exist + extents = call('iscsi.extent.query', [['id', '=', target2_config['extent']['id']]]) + assert len(extents) == 0, extents + + with configured_target(config, name3, 'VOLUME') as target3_config: + # Force the target delete, and DO remove the associated + # extents. but no force necessary + call('iscsi.target.delete', target3_config['target']['id'], False, True) + + # Ensure the extent does not exist + extents = call('iscsi.extent.query', [['id', '=', target3_config['extent']['id']]]) + assert len(extents) == 0, extents