Skip to content

Commit

Permalink
NAS-133001 / 25.04 / Add flag to delete extents when removing a target (
Browse files Browse the repository at this point in the history
#15226)

* Add optional delete_extents parameter to iscsi.target.delete

* Add iSCSI unit test test__target_delete_extents
  • Loading branch information
bmeagherix authored Dec 18, 2024
1 parent 45a6e98 commit 59fb14f
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 5 deletions.
1 change: 1 addition & 0 deletions src/middlewared/middlewared/api/v25_04_0/iscsi_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class IscsiTargetUpdateResult(BaseModel):
class IscsiTargetDeleteArgs(BaseModel):
id: int
force: bool = False
delete_extents: bool = False


class IscsiTargetDeleteResult(BaseModel):
Expand Down
4 changes: 3 additions & 1 deletion src/middlewared/middlewared/plugins/iscsi_/targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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_]]
Expand Down
10 changes: 7 additions & 3 deletions tests/api2/assets/websocket/iscsi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)

Expand Down
46 changes: 45 additions & 1 deletion tests/api2/test_261_iscsi_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

0 comments on commit 59fb14f

Please sign in to comment.