From eab964c5a2ab40309739cb52d5c82d8158370540 Mon Sep 17 00:00:00 2001
From: Kevin Adams <kevin.adams@ksatechnologies.com>
Date: Wed, 27 May 2026 08:47:49 -0400
Subject: [PATCH] feat: rollback orphaned TrueNAS resources on alloc_image
 failure (#250)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Steps 2-4 of alloc_image (extent create, target find/create, targetextent
create) are now wrapped in an eval. On any failure the rollback runs in
reverse order, best-effort: target removed if it has no remaining extents
(i.e. it was just created for this disk), extent deleted with force=true,
zvol deleted recursively. This prevents the orphaned-resources-on-failure
state that previously required manual cleanup.

Also restore alias ("Proxmox VM <vmid>") on target creation. The alias was
removed during SCALE 25.04 debugging as a red herring — the actual bug was
int() type coercion (#264). Alias is unique per-VM and _resolve_vm_target
always checks for an existing target by name before attempting a POST, so
there is no alias-collision risk in normal operation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 perl5/PVE/Storage/Custom/TrueNAS.pm | 62 ++++++++++++++++++++---------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/perl5/PVE/Storage/Custom/TrueNAS.pm b/perl5/PVE/Storage/Custom/TrueNAS.pm
index 1119bae..d741f6a 100644
--- a/perl5/PVE/Storage/Custom/TrueNAS.pm
+++ b/perl5/PVE/Storage/Custom/TrueNAS.pm
@@ -320,6 +320,7 @@ sub _resolve_vm_target {
         my $groups = _portal_groups_for_new_target($scfg);
         my $new    = _api($scfg, 'POST', '/iscsi/target', {
             name   => $target_name,
+            alias  => "Proxmox VM $vmid",
             mode   => 'ISCSI',
             groups => $groups,
         });
@@ -444,26 +445,51 @@ sub alloc_image {
         sparse  => JSON::true,
     });
 
-    # 2. Create the iSCSI extent
-    my $extent = _api($scfg, 'POST', '/iscsi/extent', {
-        name => $name,
-        type => 'DISK',
-        disk => "zvol/$zvol",
-        ro   => JSON::false,
-    });
+    # Steps 2-4 wrapped in eval so the zvol is cleaned up on any failure.
+    my ($extent, $target, $lun_id);
+    eval {
+        # 2. Create the iSCSI extent
+        $extent = _api($scfg, 'POST', '/iscsi/extent', {
+            name => $name,
+            type => 'DISK',
+            disk => "zvol/$zvol",
+            ro   => JSON::false,
+        });
 
-    # 3. Associate extent with the per-VM target
-    my $target = _resolve_vm_target($scfg, $vmid);
-    my $lun_id = _next_lun_id($scfg, $target->{id});
+        # 3. Associate extent with the per-VM target
+        $target = _resolve_vm_target($scfg, $vmid);
+        $lun_id = _next_lun_id($scfg, $target->{id});
 
-    # int() forces fresh IV scalars — decode_json IDs can become dual-vars
-    # (string+int) after log interpolation or hash-key lookup, which causes
-    # encode_json to emit "7" instead of 7, rejected by Pydantic v2.
-    _api($scfg, 'POST', '/iscsi/targetextent', {
-        target => int($target->{id}),
-        extent => int($extent->{id}),
-        lunid  => int($lun_id),
-    });
+        # int() forces fresh IV scalars — decode_json IDs can become dual-vars
+        # (string+int) after log interpolation or hash-key lookup, which causes
+        # encode_json to emit "7" instead of 7, rejected by Pydantic v2.
+        _api($scfg, 'POST', '/iscsi/targetextent', {
+            target => int($target->{id}),
+            extent => int($extent->{id}),
+            lunid  => int($lun_id),
+        });
+    };
+
+    if (my $err = $@) {
+        _log('warning', "alloc_image: partial failure — rolling back: $err");
+
+        # Reverse order, best-effort. Target: only removed if it has no
+        # remaining extents (i.e. it was just created for this disk).
+        if ($target) {
+            eval { _maybe_cleanup_vm_target($scfg, $vmid, $target->{id}) };
+        }
+        if ($extent) {
+            eval { _api($scfg, 'DELETE', "/iscsi/extent/id/$extent->{id}",
+                        { force => JSON::true }) };
+            _log('warning', "alloc_image rollback: extent delete failed: $@") if $@;
+        }
+        my $zvol_id = uri_escape($zvol, "^A-Za-z0-9\\-_.~");
+        eval { _api($scfg, 'DELETE', "/pool/dataset/id/$zvol_id",
+                    { recursive => JSON::true }) };
+        _log('warning', "alloc_image rollback: zvol delete failed: $@") if $@;
+
+        die $err;
+    }
 
     _reload_iscsi($scfg);