fix(vpc): close the route-table and ENI teardown leaks (#652)

* fix(vpc): close the route-table and ENI teardown leaks

DeleteVpc validated subnets and non-default security groups but never
non-main route tables, so it deleted the VPC out from under them and left
records naming a VPC that no longer resolves. It now returns
DependencyViolation, alongside the existing checks and before any cascade,
and still cascades the main route table on its own.

DeleteSubnet left route table associations naming the deleted subnet.
DeleteRouteTable blocks on any non-main association carrying a subnet ID,
and that subnet could never be deleted again to clear it, so the table was
pinned forever. Associations are now cleared before the subnet record goes,
leaving a mid-way failure retryable. routetable imports vpc, so the record
cannot be typed here; only the associations key is rewritten, and every
other field is passed through untouched.

DeleteLoadBalancer detached an ENI and then re-read it to delete, where a
lagging R3 replica could report it still in use. The failure was logged at
WARN and the LB record deleted anyway, orphaning the ENI and its security
group. DetachAndDeleteENI now reads once and carries the detach revision
into the delete via LastRevision, retrying the whole flow from a fresh read
on a conflict, bounded at 3 attempts. The IPAM release happens after the
delete, not before, so a retry cannot hand the same IP back twice. When
ENIs survive, the LB record is kept naming them and the call returns
DependencyViolation rather than dropping the only reference to the leak.

Instance termination routes through the same call, and both ENI paths now
distinguish a real delete from an already-absent one instead of logging a
deletion that did not happen.

TestRLC5_NoOrphanedRouteTableAfterSubnetThenVpcTeardown covers the ordering
that defeated the per-operation tests: subnet first, then VPC.

* fix(elbv2): route SetSubnets ENI teardown through the atomic detach+delete

SetSubnets detached every ENI in one loop, then deleted the removed subnets'
ENIs with force=false in another. That is the same shape as the
DeleteLoadBalancer leak: the delete re-reads the ENI, a lagging replica can
still show it attached, and the delete fails as in-use while the LB moves on
without it.

Removed subnets now go through DetachAndDeleteENI, which carries the detach
revision into the delete under one read. ENIs that survive the reshape are
still detached on their own, since they are re-attached to the relaunched VM.

spinifex/daemon/vm_adapters.go

@@ -725,20 +725,21 @@ func (a *instanceCleanerAdapter) DetachAndDeleteENI(instance *vm.VM) error {
 
 	var primaryErr error
 	if instance.ENIId != "" {
-		// Best-effort detach; a failure here must not block deletion — the force
-		// delete below bypasses the in-use guard for the owning instance, breaking
-		// the un-terminable-ENI deadlock (ADR-0003 §2).
-		if detachErr := a.d.vpcService.DetachENI(context.Background(), instance.AccountID, instance.ENIId); detachErr != nil {
-			slog.Warn("Failed to detach ENI on termination",
-				"eni", instance.ENIId, "instanceId", instance.ID, "err", detachErr)
-		}
-		if err := a.d.vpcService.ForceDeleteInstanceENI(context.Background(), instance.AccountID, instance.ENIId); err != nil {
+		// force=true bypasses the in-use guard for the owning instance,
+		// breaking the un-terminable-ENI deadlock (ADR-0003 §2). One call
+		// carries the detach's revision into the delete, so a lagging KV
+		// replica can never decide the outcome.
+		deleted, err := a.d.vpcService.DetachAndDeleteENI(context.Background(), instance.AccountID, instance.ENIId, true)
+		if err != nil {
 			slog.Error("Failed to delete ENI on termination",
 				"eni", instance.ENIId, "instanceId", instance.ID, "err", err)
 			primaryErr = err
-		} else {
+		} else if deleted {
 			slog.Info("Deleted ENI on termination",
 				"eni", instance.ENIId, "instanceId", instance.ID)
+		} else {
+			slog.Info("ENI already absent on termination",
+				"eni", instance.ENIId, "instanceId", instance.ID)
 		}
 	}
 
@@ -762,24 +763,35 @@ func (a *instanceCleanerAdapter) releaseAttachedENIs(instance *vm.VM) {
 
 	for _, record := range records {
 		eniId := record.NetworkInterfaceId
-		if detachErr := a.d.vpcService.DetachENI(context.Background(), instance.AccountID, eniId); detachErr != nil {
-			slog.Warn("Failed to detach attached ENI on termination",
-				"eni", eniId, "instanceId", instance.ID, "err", detachErr)
-		}
 
+		// DeleteOnTermination=false detaches only; there is no subsequent
+		// delete to race against, so a plain DetachENI is sufficient here.
 		if record.DeleteOnTermination != nil && !*record.DeleteOnTermination {
+			if detachErr := a.d.vpcService.DetachENI(context.Background(), instance.AccountID, eniId); detachErr != nil {
+				slog.Warn("Failed to detach attached ENI on termination",
+					"eni", eniId, "instanceId", instance.ID, "err", detachErr)
+			}
 			slog.Info("Detached attached ENI on termination, DeleteOnTermination=false",
 				"eni", eniId, "instanceId", instance.ID)
 			continue
 		}
 
-		if err := a.d.vpcService.ForceDeleteInstanceENI(context.Background(), instance.AccountID, eniId); err != nil {
+		// One call carries the detach's revision into the delete so a
+		// lagging KV replica can never decide the outcome (mirrors the
+		// primary ENI path above).
+		deleted, err := a.d.vpcService.DetachAndDeleteENI(context.Background(), instance.AccountID, eniId, true)
+		if err != nil {
 			slog.Error("Failed to delete attached ENI on termination",
 				"eni", eniId, "instanceId", instance.ID, "err", err)
 			continue
 		}
-		slog.Info("Deleted attached ENI on termination",
-			"eni", eniId, "instanceId", instance.ID)
+		if deleted {
+			slog.Info("Deleted attached ENI on termination",
+				"eni", eniId, "instanceId", instance.ID)
+		} else {
+			slog.Info("Attached ENI already absent on termination",
+				"eni", eniId, "instanceId", instance.ID)
+		}
 	}
 }
 

spinifex/daemon/vm_adapters_test.go

@@ -509,6 +509,25 @@ func TestInstanceCleanerAdapter_DetachAndDeleteENI_MultipleAttachedENIsReleased(
 	assert.Equal(t, "available", rec2.Status)
 }
 
+// TestInstanceCleanerAdapter_DetachAndDeleteENI_AbsentPrimaryDoesNotLogFalseSuccess
+// proves the false-success log is gone: a primary ENI that DetachAndDeleteENI
+// finds already absent must be logged as absent, never as "Deleted ENI on
+// termination" — that log used to fire unconditionally on a nil error, even
+// when the force path's stale-NotFound tolerance meant nothing was deleted.
+func TestInstanceCleanerAdapter_DetachAndDeleteENI_AbsentPrimaryDoesNotLogFalseSuccess(t *testing.T) {
+	f := newENIHotPlugFixture(t)
+	f.vmInst.AccountID = testAccountID
+	f.vmInst.ENIId = "eni-never-existed"
+
+	buf := captureSlogForTest(t)
+	cleaner := newInstanceCleanerAdapter(f.daemon)
+	require.NoError(t, cleaner.DetachAndDeleteENI(f.vmInst))
+
+	assert.NotContains(t, buf.String(), "Deleted ENI on termination",
+		"an already-absent ENI must never be logged as deleted")
+	assert.Contains(t, buf.String(), "ENI already absent on termination")
+}
+
 // TestInstanceCleanerAdapter_ReleaseAttachedENIs_ListInstanceENIsErrorTolerated
 // exercises the enumeration error branch: the connection backing vpcService's
 // KV is closed before terminate runs, so ListInstanceENIs fails with a real

spinifex/handlers/ec2/vpc/eni.go

@@ -238,29 +238,7 @@ func (s *VPCServiceImpl) deleteNetworkInterface(ctx context.Context, eniId, acco
 		return nil, errors.New(awserrors.ErrorInvalidNetworkInterfaceInUse)
 	}
 
-	// Release the private IP back to the IPAM pool
-	if err := s.ipam.ReleaseIP(ctx, record.SubnetId, record.PrivateIpAddress); err != nil {
-		slog.WarnContext(ctx, "Failed to release IP during ENI delete", "eni", eniId, "ip", record.PrivateIpAddress, "err", err)
-	}
-
-	// Release auto-assigned public IP (if any) and remove NAT rule.
-	// Skip if the public IP belongs to an EIP — those are managed independently.
-	if record.PublicIpAddress != "" && s.externalIPAM != nil {
-		owned, err := s.isEIPOwned(ctx, eniId, accountID)
-		if err != nil {
-			slog.ErrorContext(ctx, "DeleteNetworkInterface: failed to check EIP ownership, skipping public IP release to avoid data loss", "eniId", eniId, "err", err)
-		} else if owned {
-			slog.InfoContext(ctx, "DeleteNetworkInterface: public IP owned by EIP, skipping release", "eniId", eniId, "publicIp", record.PublicIpAddress)
-		} else {
-			portName := topology.Port(eniId)
-			s.publishNATEvent("vpc.delete-nat", record.VpcId, record.PublicIpAddress, record.PrivateIpAddress, portName, record.MacAddress)
-			if err := s.externalIPAM.ReleaseIP(ctx, record.PublicIpPool, record.PublicIpAddress, eniId); err != nil {
-				slog.WarnContext(ctx, "Failed to release public IP during ENI delete", "eni", eniId, "ip", record.PublicIpAddress, "pool", record.PublicIpPool, "err", err)
-			} else {
-				slog.InfoContext(ctx, "Released auto-assigned public IP during ENI delete", "eniId", eniId, "publicIp", record.PublicIpAddress, "pool", record.PublicIpPool)
-			}
-		}
-	}
+	s.releaseENISideEffects(ctx, eniId, accountID, &record)
 
 	if err := s.eniKV.Delete(ctx, key); err != nil {
 		return nil, errors.New(awserrors.ErrorServerInternal)
@@ -276,6 +254,119 @@ func (s *VPCServiceImpl) deleteNetworkInterface(ctx context.Context, eniId, acco
 	return &ec2.DeleteNetworkInterfaceOutput{}, nil
 }
 
+// releaseENISideEffects releases the IPAM allocation and any auto-assigned
+// public IP that belonged to record, ahead of the KV delete that finalizes
+// it. Best-effort: a release failure is logged, not returned — a stuck IP
+// release must never block the ENI delete it precedes.
+func (s *VPCServiceImpl) releaseENISideEffects(ctx context.Context, eniId, accountID string, record *ENIRecord) {
+	if err := s.ipam.ReleaseIP(ctx, record.SubnetId, record.PrivateIpAddress); err != nil {
+		slog.WarnContext(ctx, "Failed to release IP during ENI delete", "eni", eniId, "ip", record.PrivateIpAddress, "err", err)
+	}
+
+	// Release auto-assigned public IP (if any) and remove NAT rule.
+	// Skip if the public IP belongs to an EIP — those are managed independently.
+	if record.PublicIpAddress != "" && s.externalIPAM != nil {
+		owned, err := s.isEIPOwned(ctx, eniId, accountID)
+		if err != nil {
+			slog.ErrorContext(ctx, "releaseENISideEffects: failed to check EIP ownership, skipping public IP release to avoid data loss", "eniId", eniId, "err", err)
+		} else if owned {
+			slog.InfoContext(ctx, "releaseENISideEffects: public IP owned by EIP, skipping release", "eniId", eniId, "publicIp", record.PublicIpAddress)
+		} else {
+			portName := topology.Port(eniId)
+			s.publishNATEvent("vpc.delete-nat", record.VpcId, record.PublicIpAddress, record.PrivateIpAddress, portName, record.MacAddress)
+			if err := s.externalIPAM.ReleaseIP(ctx, record.PublicIpPool, record.PublicIpAddress, eniId); err != nil {
+				slog.WarnContext(ctx, "Failed to release public IP during ENI delete", "eni", eniId, "ip", record.PublicIpAddress, "pool", record.PublicIpPool, "err", err)
+			} else {
+				slog.InfoContext(ctx, "Released auto-assigned public IP during ENI delete", "eniId", eniId, "publicIp", record.PublicIpAddress, "pool", record.PublicIpPool)
+			}
+		}
+	}
+}
+
+// DetachAndDeleteENI detaches eniID and deletes it under a single KV read,
+// carrying the revision from the detach into the delete via
+// jetstream.LastRevision so a lagging replica's re-read can never decide the
+// outcome. eniKV direct-gets may be served by any replica at R3 (nats.go
+// hardcodes AllowDirect on KV buckets); reading twice — once for detach, once
+// for delete, as the previous detach-then-delete call sequence did — left a
+// window where the second read raced the first write. force skips the
+// in-use guard for owner-driven teardown (an LB or instance deleting its own
+// ENI). A revision conflict on either write means the local view was stale,
+// so the whole flow (read, detach, delete) retries from a fresh read, bounded
+// at 3 attempts.
+//
+// deleted reports whether this call actually removed the ENI, so callers can
+// distinguish a real delete from an already-absent one instead of logging a
+// deletion that did not happen.
+func (s *VPCServiceImpl) DetachAndDeleteENI(ctx context.Context, accountID, eniID string, force bool) (deleted bool, err error) {
+	const maxAttempts = 3
+	key := utils.AccountKey(accountID, eniID)
+	var lastErr error
+
+	for range maxAttempts {
+		entry, getErr := s.eniKV.Get(ctx, key)
+		if getErr != nil {
+			if errors.Is(getErr, jetstream.ErrKeyNotFound) {
+				// Force (owner-driven) teardown tolerates an already-gone ENI,
+				// matching ForceDeleteInstanceENI's convergence contract.
+				if force {
+					return false, nil
+				}
+				return false, errors.New(awserrors.ErrorInvalidNetworkInterfaceIDNotFound)
+			}
+			return false, errors.New(awserrors.ErrorServerInternal)
+		}
+
+		var record ENIRecord
+		if err := json.Unmarshal(entry.Value(), &record); err != nil {
+			return false, errors.New(awserrors.ErrorServerInternal)
+		}
+
+		if !force && eniIsLiveAttachment(&record) {
+			return false, errors.New(awserrors.ErrorInvalidNetworkInterfaceInUse)
+		}
+
+		record.Status = "available"
+		record.AttachmentId = ""
+		record.InstanceId = ""
+		record.DeviceIndex = 0
+
+		data, marshalErr := json.Marshal(record)
+		if marshalErr != nil {
+			return false, fmt.Errorf("marshal ENI record: %w", marshalErr)
+		}
+		revision, updateErr := s.eniKV.Update(ctx, key, data, entry.Revision())
+		if updateErr != nil {
+			// Stale read: the key moved under us between Get and Update.
+			// Retry the whole flow from a fresh read rather than trusting
+			// anything else about this attempt.
+			lastErr = updateErr
+			continue
+		}
+
+		if delErr := s.eniKV.Delete(ctx, key, jetstream.LastRevision(revision)); delErr != nil {
+			if errors.Is(delErr, jetstream.ErrKeyNotFound) {
+				return false, nil // a concurrent delete already won
+			}
+			lastErr = delErr
+			continue // revision moved again — retry the whole flow
+		}
+
+		// After the delete, never before it: a failed delete retries the whole
+		// flow, and releasing twice can hand back an IP already reallocated to
+		// another ENI.
+		s.releaseENISideEffects(ctx, eniID, accountID, &record)
+
+		slog.InfoContext(ctx, "DetachAndDeleteENI completed", "eniId", eniID, "accountID", accountID, "force", force)
+		s.publishPortEvent("vpc.delete-port", eniID, record.SubnetId, record.VpcId, record.PrivateIpAddress, record.MacAddress, record.SecurityGroupIds)
+		return true, nil
+	}
+
+	// Carry the last write error: exhaustion from a genuine NATS fault must not
+	// read as ordinary contention.
+	return false, fmt.Errorf("DetachAndDeleteENI: exhausted %d CAS attempts for %s: %w", maxAttempts, eniID, lastErr)
+}
+
 // ModifyNetworkInterfaceAttribute modifies ENI attributes (security groups,
 // description). SourceDestCheck=true is accepted as a no-op.
 func (s *VPCServiceImpl) ModifyNetworkInterfaceAttribute(ctx context.Context, input *ec2.ModifyNetworkInterfaceAttributeInput, accountID string) (*ec2.ModifyNetworkInterfaceAttributeOutput, error) {

spinifex/handlers/ec2/vpc/eni_test.go

@@ -276,6 +276,150 @@ func TestDetachENI(t *testing.T) {
 	assert.Nil(t, out.NetworkInterfaces[0].Attachment)
 }
 
+// failNTimesDeleteKV wraps a real KeyValue and fails the first n Delete calls
+// with a simulated CAS conflict, so DetachAndDeleteENI's bounded retry can be
+// exercised deterministically instead of racing a live writer.
+type failNTimesDeleteKV struct {
+	jetstream.KeyValue
+
+	failsLeft int
+}
+
+func (k *failNTimesDeleteKV) Delete(ctx context.Context, key string, opts ...jetstream.KVDeleteOpt) error {
+	if k.failsLeft > 0 {
+		k.failsLeft--
+		return fmt.Errorf("simulated CAS conflict on delete")
+	}
+	return k.KeyValue.Delete(ctx, key, opts...)
+}
+
+// TestDetachAndDeleteENI_RetriesOnDeleteCASConflict proves the bounded retry
+// path: a Delete rejected once by a stale revision must retry the whole flow
+// from a fresh read rather than giving up, so a lagging replica never causes
+// a leaked ENI.
+func TestDetachAndDeleteENI_RetriesOnDeleteCASConflict(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	eniId := createTestENI(t, svc, subnetId)
+	_, err := svc.AttachENI(testAccountID, eniId, "i-test123", 0)
+	require.NoError(t, err)
+
+	svc.eniKV = &failNTimesDeleteKV{KeyValue: svc.eniKV, failsLeft: 1}
+
+	deleted, err := svc.DetachAndDeleteENI(context.Background(), testAccountID, eniId, true)
+	require.NoError(t, err)
+	assert.True(t, deleted)
+
+	_, err = svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{
+		NetworkInterfaceIds: []*string{aws.String(eniId)},
+	}, testAccountID)
+	assert.ErrorContains(t, err, "InvalidNetworkInterfaceID.NotFound")
+}
+
+// TestDetachAndDeleteENI_ExhaustsRetriesReturnsError proves the retry is
+// bounded: a Delete that never stops conflicting must surface an error
+// rather than retry forever or silently drop the ENI.
+func TestDetachAndDeleteENI_ExhaustsRetriesReturnsError(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	eniId := createTestENI(t, svc, subnetId)
+
+	svc.eniKV = &failNTimesDeleteKV{KeyValue: svc.eniKV, failsLeft: 100}
+
+	_, err := svc.DetachAndDeleteENI(context.Background(), testAccountID, eniId, true)
+	require.Error(t, err)
+}
+
+// TestDetachAndDeleteENI_HoldsIPUntilDeleteSucceeds pins the ordering of the
+// IPAM release against the KV delete. Releasing before the delete means every
+// CAS retry releases again, and an IP handed back twice can be reallocated to
+// another ENI in between — so a delete that never succeeds must leave the
+// allocation exactly as it found it.
+func TestDetachAndDeleteENI_HoldsIPUntilDeleteSucceeds(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	eniId := createTestENI(t, svc, subnetId)
+
+	before, err := svc.ipam.AllocatedIPs(t.Context(), subnetId)
+	require.NoError(t, err)
+
+	svc.eniKV = &failNTimesDeleteKV{KeyValue: svc.eniKV, failsLeft: 100}
+	_, err = svc.DetachAndDeleteENI(context.Background(), testAccountID, eniId, true)
+	require.Error(t, err)
+
+	after, err := svc.ipam.AllocatedIPs(t.Context(), subnetId)
+	require.NoError(t, err)
+	assert.Equal(t, before, after, "a failed delete must not hand the ENI's IP back to the pool")
+}
+
+// TestDetachAndDeleteENI_ForceFalseOnLiveAttachment_StaysAWSFaithful asserts
+// the public-facing guard is unchanged by the new atomic path: a genuinely
+// attached ENI without force must still return InvalidNetworkInterface.InUse.
+func TestDetachAndDeleteENI_ForceFalseOnLiveAttachment_StaysAWSFaithful(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	eniId := createTestENI(t, svc, subnetId)
+	_, err := svc.AttachENI(testAccountID, eniId, "i-test123", 0)
+	require.NoError(t, err)
+
+	_, err = svc.DetachAndDeleteENI(context.Background(), testAccountID, eniId, false)
+	assert.ErrorContains(t, err, "InvalidNetworkInterface.InUse")
+
+	// Rejected call must not have mutated the record.
+	out, err := svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{
+		NetworkInterfaceIds: []*string{aws.String(eniId)},
+	}, testAccountID)
+	require.NoError(t, err)
+	assert.Equal(t, "in-use", *out.NetworkInterfaces[0].Status)
+}
+
+// TestDetachAndDeleteENI_ForceTrueDeletesLiveAttachment asserts the owner
+// teardown path (force=true) detaches and deletes an in-use ENI in one call,
+// with no separate re-read between the two — the mechanism that closes the
+// stale-replica window DetachENI+DeleteNetworkInterface used to open.
+func TestDetachAndDeleteENI_ForceTrueDeletesLiveAttachment(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+	eniId := createTestENI(t, svc, subnetId)
+	_, err := svc.AttachENI(testAccountID, eniId, "i-test123", 0)
+	require.NoError(t, err)
+
+	deleted, err := svc.DetachAndDeleteENI(context.Background(), testAccountID, eniId, true)
+	require.NoError(t, err)
+	assert.True(t, deleted)
+
+	_, err = svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{
+		NetworkInterfaceIds: []*string{aws.String(eniId)},
+	}, testAccountID)
+	assert.ErrorContains(t, err, "InvalidNetworkInterfaceID.NotFound")
+}
+
+// TestDetachAndDeleteENI_AbsentForceTrue_ReturnsNotDeletedNoError proves the
+// force path distinguishes "already absent" from "deleted" — this is what
+// stops the caller logging a false "Deleted ENI on termination" for an ENI
+// it never touched.
+func TestDetachAndDeleteENI_AbsentForceTrue_ReturnsNotDeletedNoError(t *testing.T) {
+	svc := setupTestVPCService(t)
+
+	deleted, err := svc.DetachAndDeleteENI(context.Background(), testAccountID, "eni-never-existed", true)
+	require.NoError(t, err)
+	assert.False(t, deleted, "an already-absent ENI must report deleted=false so callers don't log a false success")
+}
+
+// TestDetachAndDeleteENI_AbsentForceFalse_ReturnsNotFound asserts the public
+// (non-force) path stays AWS-faithful on an absent ENI.
+func TestDetachAndDeleteENI_AbsentForceFalse_ReturnsNotFound(t *testing.T) {
+	svc := setupTestVPCService(t)
+
+	_, err := svc.DetachAndDeleteENI(context.Background(), testAccountID, "eni-never-existed", false)
+	assert.ErrorContains(t, err, "InvalidNetworkInterfaceID.NotFound")
+}
+
 func TestGenerateENIMac(t *testing.T) {
 	mac := generateENIMac("eni-test123")
 	hw, err := net.ParseMAC(mac)

spinifex/handlers/ec2/vpc/lifecycle_invariants_test.go

@@ -11,6 +11,38 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// TestRLC5_NoOrphanedRouteTableAfterSubnetThenVpcTeardown enforces Common
+// Resource Lifecycle Contract rule #5 (no-orphan completeness) for the one
+// teardown ordering that used to defeat it: delete the subnet, then the VPC.
+// Deleting the route table before the subnet never exercises the leak, which
+// is why unit tests on each operation individually all passed while it
+// existed — this asserts the end state across the ordering that matters.
+func TestRLC5_NoOrphanedRouteTableAfterSubnetThenVpcTeardown(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcID := createTestVPC(t, svc, "10.70.0.0/16")
+	subnetID := createTestSubnet(t, svc, vpcID, "10.70.1.0/24")
+	rtbID, _ := putTestNonMainRouteTable(t, svc, testAccountID, vpcID, subnetID)
+
+	_, err := svc.DeleteSubnet(context.Background(), &ec2.DeleteSubnetInput{SubnetId: aws.String(subnetID)}, testAccountID)
+	require.NoError(t, err, "DeleteSubnet must succeed and clear the association naming it")
+
+	// The non-main route table still exists, so DeleteVpc must reject rather
+	// than deleting the VPC out from under it.
+	_, err = svc.DeleteVpc(context.Background(), &ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, testAccountID)
+	require.Errorf(t, err, "rule #5: DeleteVpc must not remove the VPC while a non-main route table survives")
+	assert.ErrorContains(t, err, awserrors.ErrorDependencyViolation)
+
+	// Deleting the now-unassociated route table, then the VPC, must leave
+	// nothing referencing a nonexistent VPC — the invariant this whole
+	// regression exists to protect.
+	require.NoError(t, svc.rtbKV.Delete(t.Context(), testAccountID+"."+rtbID))
+	_, err = svc.DeleteVpc(context.Background(), &ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, testAccountID)
+	require.NoError(t, err)
+
+	_, err = svc.rtbKV.Get(t.Context(), testAccountID+"."+rtbID)
+	assert.Error(t, err, "rule #5: no route table may survive referencing a VPC that no longer exists")
+}
+
 // TestRLC1_VPCDeleteNotFoundOnAbsent enforces the Common Resource Lifecycle
 // Contract rule #1 (AWS-faithful delete, per-service): the EC2/VPC delete API
 // returns the service's canonical InvalidX.NotFound for an absent resource —

spinifex/handlers/ec2/vpc/service_impl.go

@@ -403,6 +403,46 @@ func (s *VPCServiceImpl) DeleteVpc(ctx context.Context, input *ec2.DeleteVpcInpu
 		defaultSGId = sg.GroupId
 	}
 
+	// Reject if any non-main route table remains in this VPC; DeleteVpc only
+	// auto-reaps the main route table (matches AWS DeleteVpc semantics).
+	rtbKeys, err := s.rtbKV.Keys(ctx)
+	if err != nil && !errors.Is(err, jetstream.ErrNoKeysFound) {
+		return nil, errors.New(awserrors.ErrorServerInternal)
+	}
+	for _, k := range rtbKeys {
+		if k == utils.VersionKey || !strings.HasPrefix(k, prefix) {
+			continue
+		}
+		entry, err := s.rtbKV.Get(ctx, k)
+		if err != nil {
+			// ErrKeyNotFound means the route table was deleted between Keys()
+			// and Get() — fine to skip. Any other error is fail-closed: a
+			// transient read error must not let DeleteVpc orphan a
+			// non-main route table it can't see.
+			if errors.Is(err, jetstream.ErrKeyNotFound) {
+				continue
+			}
+			slog.WarnContext(ctx, "DeleteVpc: route table read failed", "key", k, "err", err)
+			return nil, errors.New(awserrors.ErrorServerInternal)
+		}
+		var rtb struct {
+			RouteTableId string `json:"route_table_id"`
+			VpcId        string `json:"vpc_id"`
+			IsMain       bool   `json:"is_main"`
+		}
+		if err := json.Unmarshal(entry.Value(), &rtb); err != nil {
+			slog.WarnContext(ctx, "DeleteVpc: route table unmarshal failed", "key", k, "err", err)
+			return nil, errors.New(awserrors.ErrorServerInternal)
+		}
+		if rtb.VpcId != vpcID {
+			continue
+		}
+		if !rtb.IsMain {
+			return nil, awserrors.Errorf(awserrors.ErrorDependencyViolation,
+				"the VPC has a dependent route table %s that must be deleted first", rtb.RouteTableId)
+		}
+	}
+
 	// Cascade-delete the default SG before removing the VPC record so a vpcd
 	// failure surfaces to the caller and leaves both records intact for retry.
 	if defaultSGId != "" {
@@ -675,6 +715,15 @@ func (s *VPCServiceImpl) DeleteSubnet(ctx context.Context, input *ec2.DeleteSubn
 		return nil, err
 	}
 
+	// Clear route table associations naming this subnet before the subnet
+	// record is gone, so DeleteRouteTable's association check can never be
+	// pinned on a subnet nothing can ever delete again. Runs before the
+	// subnet delete so a mid-way failure leaves the subnet present and the
+	// whole operation retryable.
+	if err := s.clearRouteTableAssociationsForSubnet(ctx, accountID, subnetID); err != nil {
+		return nil, err
+	}
+
 	if err := s.subnetKV.Delete(ctx, key); err != nil {
 		return nil, errors.New(awserrors.ErrorServerInternal)
 	}
@@ -722,6 +771,93 @@ func (s *VPCServiceImpl) checkSubnetResidents(ctx context.Context, accountID, su
 	return nil
 }
 
+// clearRouteTableAssociationsForSubnet removes every route table association
+// naming subnetID. Without this, DeleteRouteTable's association check blocks
+// on a subnet ID that can never be deleted again once the subnet itself is
+// gone, permanently pinning the route table. Fail-closed on a KV read error,
+// matching checkSubnetResidents.
+func (s *VPCServiceImpl) clearRouteTableAssociationsForSubnet(ctx context.Context, accountID, subnetID string) error {
+	// routetable imports vpc, so routetable.RouteTableRecord cannot be named
+	// here. Rather than mirror the whole record and silently drop any field the
+	// mirror lacks on write-back, decode to raw fields and rewrite only
+	// "associations".
+	type associationRecord struct {
+		AssociationId string `json:"association_id"`
+		SubnetId      string `json:"subnet_id,omitempty"`
+		Main          bool   `json:"main"`
+	}
+
+	keys, err := s.rtbKV.Keys(ctx)
+	if err != nil {
+		if errors.Is(err, jetstream.ErrNoKeysFound) {
+			return nil
+		}
+		slog.ErrorContext(ctx, "DeleteSubnet: route table scan failed, blocking delete to avoid stranding an association", "subnetId", subnetID, "err", err)
+		return errors.New(awserrors.ErrorServerInternal)
+	}
+	prefix := accountID + "."
+	for _, key := range keys {
+		if key == utils.VersionKey || !strings.HasPrefix(key, prefix) {
+			continue
+		}
+		entry, err := s.rtbKV.Get(ctx, key)
+		if err != nil {
+			if errors.Is(err, jetstream.ErrKeyNotFound) {
+				continue
+			}
+			slog.ErrorContext(ctx, "DeleteSubnet: route table read failed, blocking delete to avoid stranding an association", "key", key, "err", err)
+			return errors.New(awserrors.ErrorServerInternal)
+		}
+
+		var record map[string]json.RawMessage
+		if err := json.Unmarshal(entry.Value(), &record); err != nil {
+			slog.ErrorContext(ctx, "DeleteSubnet: route table unmarshal failed", "key", key, "err", err)
+			return errors.New(awserrors.ErrorServerInternal)
+		}
+		var associations []associationRecord
+		if raw, ok := record["associations"]; ok {
+			if err := json.Unmarshal(raw, &associations); err != nil {
+				slog.ErrorContext(ctx, "DeleteSubnet: route table associations unmarshal failed", "key", key, "err", err)
+				return errors.New(awserrors.ErrorServerInternal)
+			}
+		}
+
+		kept := associations[:0]
+		changed := false
+		for _, assoc := range associations {
+			if assoc.SubnetId == subnetID {
+				changed = true
+				continue
+			}
+			kept = append(kept, assoc)
+		}
+		if !changed {
+			continue
+		}
+
+		// Route table ID is for logging only, so a missing one must not fail the clear.
+		var routeTableID string
+		_ = json.Unmarshal(record["route_table_id"], &routeTableID)
+
+		updated, err := json.Marshal(kept)
+		if err != nil {
+			return fmt.Errorf("marshal associations for route table %s: %w", routeTableID, err)
+		}
+		record["associations"] = updated
+
+		data, err := json.Marshal(record)
+		if err != nil {
+			return fmt.Errorf("marshal route table %s: %w", routeTableID, err)
+		}
+		if _, err := s.rtbKV.Update(ctx, key, data, entry.Revision()); err != nil {
+			slog.ErrorContext(ctx, "DeleteSubnet: route table association clear failed", "routeTableId", routeTableID, "err", err)
+			return errors.New(awserrors.ErrorServerInternal)
+		}
+		slog.InfoContext(ctx, "DeleteSubnet: cleared route table association", "routeTableId", routeTableID, "subnetId", subnetID)
+	}
+	return nil
+}
+
 // DescribeSubnets describes subnets.
 func (s *VPCServiceImpl) DescribeSubnets(ctx context.Context, input *ec2.DescribeSubnetsInput, accountID string) (*ec2.DescribeSubnetsOutput, error) {
 	var subnets []*ec2.Subnet

spinifex/handlers/ec2/vpc/service_impl_test.go

@@ -653,6 +653,202 @@ func countMainRouteTablesForVPC(t *testing.T, svc *VPCServiceImpl, vpcID string)
 	return n
 }
 
+// nonMainRTBRecord mirrors routetable.RouteTableRecord (see
+// clearRouteTableAssociationsForSubnet) so tests can seed a non-main route
+// table with an association directly in rtbKV, bypassing the routetable
+// package to avoid a circular import.
+type nonMainRTBRecord struct {
+	RouteTableId string `json:"route_table_id"`
+	VpcId        string `json:"vpc_id"`
+	AccountID    string `json:"account_id"`
+	IsMain       bool   `json:"is_main"`
+	Associations []struct {
+		AssociationId string `json:"association_id"`
+		SubnetId      string `json:"subnet_id,omitempty"`
+		Main          bool   `json:"main"`
+	} `json:"associations"`
+	Tags      map[string]string `json:"tags"`
+	CreatedAt time.Time         `json:"created_at"`
+}
+
+// putTestNonMainRouteTable writes a non-main route table associated with
+// subnetID directly to rtbKV and returns its ID and association ID.
+func putTestNonMainRouteTable(t *testing.T, svc *VPCServiceImpl, accountID, vpcID, subnetID string) (rtbID, assocID string) {
+	t.Helper()
+	rtbID = "rtb-" + t.Name() + "-" + subnetID
+	assocID = "rtbassoc-" + t.Name() + "-" + subnetID
+	rec := nonMainRTBRecord{
+		RouteTableId: rtbID,
+		VpcId:        vpcID,
+		AccountID:    accountID,
+		IsMain:       false,
+		Associations: []struct {
+			AssociationId string `json:"association_id"`
+			SubnetId      string `json:"subnet_id,omitempty"`
+			Main          bool   `json:"main"`
+		}{
+			{AssociationId: assocID, SubnetId: subnetID, Main: false},
+		},
+		Tags:      make(map[string]string),
+		CreatedAt: time.Now(),
+	}
+	data, err := json.Marshal(rec)
+	require.NoError(t, err)
+	_, err = svc.rtbKV.Put(t.Context(), accountID+"."+rtbID, data)
+	require.NoError(t, err)
+	return rtbID, assocID
+}
+
+// getTestRouteTableAssociations reads back the associations of rtbID for
+// assertions.
+func getTestRouteTableAssociations(t *testing.T, svc *VPCServiceImpl, accountID, rtbID string) []string {
+	t.Helper()
+	entry, err := svc.rtbKV.Get(t.Context(), accountID+"."+rtbID)
+	require.NoError(t, err)
+	var rec nonMainRTBRecord
+	require.NoError(t, json.Unmarshal(entry.Value(), &rec))
+	subnetIDs := make([]string, 0, len(rec.Associations))
+	for _, a := range rec.Associations {
+		subnetIDs = append(subnetIDs, a.SubnetId)
+	}
+	return subnetIDs
+}
+
+// TestDeleteVpc_RejectsNonMainRouteTable asserts DeleteVpc rejects with
+// DependencyViolation while a non-main route table exists, matching AWS:
+// non-main route tables must be deleted first. Without this check the VPC
+// is removed out from under the route table, orphaning it permanently since
+// DeleteRouteTable can never resolve a VpcId that no longer exists.
+func TestDeleteVpc_RejectsNonMainRouteTable(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcID := createTestVPC(t, svc, "10.60.0.0/16")
+	subnetID := createTestSubnet(t, svc, vpcID, "10.60.1.0/24")
+	rtbID, _ := putTestNonMainRouteTable(t, svc, testAccountID, vpcID, subnetID)
+
+	_, err := svc.DeleteVpc(context.Background(), &ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, testAccountID)
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "DependencyViolation")
+
+	// Rejected call must mutate nothing: VPC and route table both persist.
+	desc, err := svc.DescribeVpcs(context.Background(), &ec2.DescribeVpcsInput{VpcIds: []*string{aws.String(vpcID)}}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, desc.Vpcs, 1, "VPC must persist so the caller can delete the route table and retry")
+
+	_, err = svc.rtbKV.Get(t.Context(), testAccountID+"."+rtbID)
+	require.NoError(t, err, "non-main route table must not be reaped by a rejected DeleteVpc")
+}
+
+// TestDeleteVpc_RouteTableCheckFailsClosedOnCorruptRTB asserts a corrupt
+// route table record blocks DeleteVpc rather than being silently skipped —
+// a transient/corrupt read must never let DeleteVpc orphan a route table it
+// could not evaluate.
+func TestDeleteVpc_RouteTableCheckFailsClosedOnCorruptRTB(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcID := createTestVPC(t, svc, "10.61.0.0/16")
+
+	_, err := svc.rtbKV.Put(t.Context(), testAccountID+".rtb-corrupt", []byte("{not json"))
+	require.NoError(t, err)
+
+	_, err = svc.DeleteVpc(context.Background(), &ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, testAccountID)
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "ServerInternal")
+}
+
+// TestDeleteSubnet_ClearsRouteTableAssociations asserts DeleteSubnet removes
+// every association naming the deleted subnet, so the route table it leaves
+// behind never carries a reference to a subnet that no longer exists.
+func TestDeleteSubnet_ClearsRouteTableAssociations(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcID := createTestVPC(t, svc, "10.62.0.0/16")
+	subnetID := createTestSubnet(t, svc, vpcID, "10.62.1.0/24")
+	rtbID, _ := putTestNonMainRouteTable(t, svc, testAccountID, vpcID, subnetID)
+
+	_, err := svc.DeleteSubnet(context.Background(), &ec2.DeleteSubnetInput{SubnetId: aws.String(subnetID)}, testAccountID)
+	require.NoError(t, err)
+
+	assert.NotContains(t, getTestRouteTableAssociations(t, svc, testAccountID, rtbID), subnetID,
+		"DeleteSubnet must clear associations naming the deleted subnet")
+}
+
+// TestDeleteSubnet_PreservesUnknownRouteTableFields pins the write-back to
+// editing only "associations". routetable owns this record and imports vpc, so
+// it cannot be typed here; a mirror struct would silently erase every field
+// added to RouteTableRecord that the mirror had not caught up with.
+func TestDeleteSubnet_PreservesUnknownRouteTableFields(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcID := createTestVPC(t, svc, "10.63.0.0/16")
+	subnetID := createTestSubnet(t, svc, vpcID, "10.63.1.0/24")
+	rtbID, _ := putTestNonMainRouteTable(t, svc, testAccountID, vpcID, subnetID)
+
+	// Re-write the stored record with a field no struct in this package knows.
+	key := testAccountID + "." + rtbID
+	entry, err := svc.rtbKV.Get(t.Context(), key)
+	require.NoError(t, err)
+	var stored map[string]json.RawMessage
+	require.NoError(t, json.Unmarshal(entry.Value(), &stored))
+	stored["propagating_vgws"] = json.RawMessage(`["vgw-deadbeef"]`)
+	data, err := json.Marshal(stored)
+	require.NoError(t, err)
+	_, err = svc.rtbKV.Put(t.Context(), key, data)
+	require.NoError(t, err)
+
+	_, err = svc.DeleteSubnet(context.Background(), &ec2.DeleteSubnetInput{SubnetId: aws.String(subnetID)}, testAccountID)
+	require.NoError(t, err)
+
+	after, err := svc.rtbKV.Get(t.Context(), key)
+	require.NoError(t, err)
+	var reread map[string]json.RawMessage
+	require.NoError(t, json.Unmarshal(after.Value(), &reread))
+
+	assert.NotContains(t, getTestRouteTableAssociations(t, svc, testAccountID, rtbID), subnetID,
+		"the association naming the subnet must still be cleared")
+	assert.JSONEq(t, `["vgw-deadbeef"]`, string(reread["propagating_vgws"]),
+		"a field this package does not model must survive the association clear")
+}
+
+// TestDeleteSubnet_LeavesOtherVPCsAssociationsAlone asserts clearing
+// associations for one subnet does not touch a route table association
+// belonging to a different subnet/VPC.
+func TestDeleteSubnet_LeavesOtherVPCsAssociationsAlone(t *testing.T) {
+	svc := setupTestVPCService(t)
+
+	vpcA := createTestVPC(t, svc, "10.63.0.0/16")
+	subnetA := createTestSubnet(t, svc, vpcA, "10.63.1.0/24")
+	rtbA, _ := putTestNonMainRouteTable(t, svc, testAccountID, vpcA, subnetA)
+
+	vpcB := createTestVPC(t, svc, "10.64.0.0/16")
+	subnetB := createTestSubnet(t, svc, vpcB, "10.64.1.0/24")
+	rtbB, _ := putTestNonMainRouteTable(t, svc, testAccountID, vpcB, subnetB)
+
+	_, err := svc.DeleteSubnet(context.Background(), &ec2.DeleteSubnetInput{SubnetId: aws.String(subnetA)}, testAccountID)
+	require.NoError(t, err)
+
+	assert.NotContains(t, getTestRouteTableAssociations(t, svc, testAccountID, rtbA), subnetA)
+	assert.Contains(t, getTestRouteTableAssociations(t, svc, testAccountID, rtbB), subnetB,
+		"deleting a subnet in one VPC must not touch another VPC's route table associations")
+}
+
+// TestDeleteSubnet_RouteTableCheckFailsClosedOnCorruptRTB asserts a corrupt
+// route table record blocks DeleteSubnet, leaving the subnet present and the
+// operation retryable, rather than deleting the subnet with a stale
+// association strand behind it unseen.
+func TestDeleteSubnet_RouteTableCheckFailsClosedOnCorruptRTB(t *testing.T) {
+	svc := setupTestVPCService(t)
+	vpcID := createTestVPC(t, svc, "10.65.0.0/16")
+	subnetID := createTestSubnet(t, svc, vpcID, "10.65.1.0/24")
+
+	_, err := svc.rtbKV.Put(t.Context(), testAccountID+".rtb-corrupt", []byte("{not json"))
+	require.NoError(t, err)
+
+	_, err = svc.DeleteSubnet(context.Background(), &ec2.DeleteSubnetInput{SubnetId: aws.String(subnetID)}, testAccountID)
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "ServerInternal")
+
+	desc, err := svc.DescribeSubnets(context.Background(), &ec2.DescribeSubnetsInput{SubnetIds: []*string{aws.String(subnetID)}}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, desc.Subnets, 1, "subnet must persist so a KV read failure never strands an association unseen")
+}
+
 // TestEnsureDefaultVPC_NoVpcdResponder simulates the daemon-startup race where
 // EnsureDefaultVPC runs before vpcd has subscribed. The SG step is best-effort,
 // so subnet and RTB must still land in KV when vpcd is absent.

spinifex/handlers/elbv2/service_impl.go

@@ -1797,17 +1797,28 @@ func (s *ELBv2ServiceImpl) DeleteLoadBalancer(ctx context.Context, input *elbv2.
 		}()
 	}
 
-	// Delete system-managed ENIs. Detach first to clear in-use status.
+	// Delete system-managed ENIs under one detach+delete flow — the LB is the
+	// ENI's owner tearing it down, so force=true is correct (same reason it's
+	// correct for instance terminate).
 	if s.VPCService != nil {
+		var survivingENIs []string
 		for _, eniID := range lb.ENIs {
-			if detachErr := s.VPCService.DetachENI(ctx, accountID, eniID); detachErr != nil {
-				slog.WarnContext(ctx, "Failed to detach ALB ENI during cleanup", "eniId", eniID, "err", detachErr)
+			if _, eniErr := s.VPCService.DetachAndDeleteENI(ctx, accountID, eniID, true); eniErr != nil {
+				slog.ErrorContext(ctx, "Failed to detach+delete ALB ENI during cleanup", "eniId", eniID, "err", eniErr)
+				survivingENIs = append(survivingENIs, eniID)
 			}
-			if _, eniErr := s.VPCService.DeleteNetworkInterface(ctx, &ec2.DeleteNetworkInterfaceInput{
-				NetworkInterfaceId: aws.String(eniID),
-			}, accountID); eniErr != nil {
-				slog.WarnContext(ctx, "Failed to delete ALB ENI during cleanup", "eniId", eniID, "err", eniErr)
+		}
+		if len(survivingENIs) > 0 {
+			// Do not drop the LB record while its ENIs survive: it is the only
+			// thing that still names them, and losing it makes the leak
+			// unrecoverable. Keep it with the surviving set so a retried
+			// delete (or the orphan reaper) can still find and finish them.
+			lb.ENIs = survivingENIs
+			if putErr := s.store.PutLoadBalancer(ctx, lb); putErr != nil {
+				slog.ErrorContext(ctx, "DeleteLoadBalancer: failed to persist surviving ENIs", "lbId", lb.LoadBalancerID, "err", putErr)
 			}
+			return nil, awserrors.Errorf(awserrors.ErrorDependencyViolation,
+				"load balancer ENIs could not be deleted: %v", survivingENIs)
 		}
 		// The managed NLB SG can only be removed once its ENIs are gone.
 		s.deleteNLBManagedSG(ctx, lb.NLBManagedSGID, accountID)

spinifex/handlers/elbv2/service_impl_lb_network.go

@@ -204,19 +204,28 @@ func (s *ELBv2ServiceImpl) SetSubnets(ctx context.Context, input *elbv2.SetSubne
 		}
 	}
 
-	// Detach all ENIs explicitly: TerminateSystemInstance doesn't clear in-use status.
+	removed := make(map[string]bool, len(toRemove))
+	for _, sn := range toRemove {
+		removed[current[sn]] = true
+	}
+
+	// Detach the ENIs that survive the reshape: TerminateSystemInstance doesn't
+	// clear in-use status, and these are re-attached to the relaunched VM.
 	for _, eniID := range current {
+		if removed[eniID] {
+			continue
+		}
 		if detachErr := s.VPCService.DetachENI(ctx, accountID, eniID); detachErr != nil {
 			slog.WarnContext(ctx, "SetSubnets: failed to detach ENI before relaunch", "eni", eniID, "err", detachErr)
 		}
 	}
 
-	// Delete ENIs for removed subnets now that they are detached.
+	// ENIs for removed subnets go through the single detach+delete flow. Two
+	// separate calls let a lagging replica's re-read decide the outcome and
+	// leak the ENI; force is correct because the LB owns them.
 	for _, sn := range toRemove {
 		eniID := current[sn]
-		if _, delErr := s.VPCService.DeleteNetworkInterface(ctx, &ec2.DeleteNetworkInterfaceInput{
-			NetworkInterfaceId: aws.String(eniID),
-		}, accountID); delErr != nil && !awserrors.IsNotFound(delErr) {
+		if _, delErr := s.VPCService.DetachAndDeleteENI(ctx, accountID, eniID, true); delErr != nil && !awserrors.IsNotFound(delErr) {
 			slog.ErrorContext(ctx, "SetSubnets: failed to delete removed ENI", "subnet", sn, "eni", eniID, "err", delErr)
 		}
 	}

spinifex/handlers/elbv2/service_impl_lb_network_test.go

@@ -326,6 +326,49 @@ func TestSetSubnets_RemoveSubnet(t *testing.T) {
 	assert.Equal(t, []string{sub1}, subnetIDsOfLB(lb))
 }
 
+// TestSetSubnets_RemovedENIStillAttachedIsDeleted pins the removed-subnet
+// teardown to the single detach+delete flow. The old shape detached in one loop
+// and then deleted with force=false in another, so a delete whose read had not
+// yet caught up with the detach saw the ENI in use, logged, and left it behind.
+// An ENI still marked attached at delete time is that state, made deterministic.
+func TestSetSubnets_RemovedENIStillAttachedIsDeleted(t *testing.T) {
+	svc, vpcSvc, _ := setupSubnetTestService(t)
+	vid := vpcID(t, vpcSvc)
+	sub1 := getTestSubnetID(t, vpcSvc, vid, "10.0.30.0/24", "us-east-1a")
+	sub2 := getTestSubnetID(t, vpcSvc, vid, "10.0.31.0/24", "us-east-1b")
+
+	out, err := svc.CreateLoadBalancer(context.Background(), &elbv2.CreateLoadBalancerInput{
+		Name:    aws.String("stale-rm-lb"),
+		Subnets: []*string{aws.String(sub1), aws.String(sub2)},
+	}, testAccountID)
+	require.NoError(t, err)
+	svc.WaitLaunches()
+	arn := *out.LoadBalancers[0].LoadBalancerArn
+	require.Equal(t, 2, countManagedENIs(t, vpcSvc))
+
+	enis, err := vpcSvc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{}, testAccountID)
+	require.NoError(t, err)
+	var removedENI string
+	for _, eni := range enis.NetworkInterfaces {
+		if eni.SubnetId != nil && *eni.SubnetId == sub2 {
+			removedENI = *eni.NetworkInterfaceId
+		}
+	}
+	require.NotEmpty(t, removedENI, "sub2 must have a managed ENI to remove")
+
+	_, err = vpcSvc.AttachENI(testAccountID, removedENI, "i-stale", 1)
+	require.NoError(t, err)
+
+	_, err = svc.SetSubnets(context.Background(), &elbv2.SetSubnetsInput{
+		LoadBalancerArn: aws.String(arn),
+		Subnets:         []*string{aws.String(sub1)},
+	}, testAccountID)
+	require.NoError(t, err)
+
+	assert.Equal(t, 1, countManagedENIs(t, vpcSvc),
+		"the removed subnet's ENI must be deleted, not left behind as a leak")
+}
+
 func TestSetSubnets_Replace(t *testing.T) {
 	svc, vpcSvc, _ := setupSubnetTestService(t)
 	vid := vpcID(t, vpcSvc)

spinifex/handlers/elbv2/service_impl_vpc_test.go

@@ -259,6 +259,66 @@ func TestDeleteLoadBalancer_CleansUpENIs(t *testing.T) {
 	assert.Empty(t, eniDesc.NetworkInterfaces)
 }
 
+// TestDeleteLoadBalancer_PersistsSurvivingENIsOnPersistentDeleteFailure
+// asserts DeleteLoadBalancer does not delete the LB record while an ENI
+// delete fails persistently — the ownership edge (lb.ENIs) is the only thing
+// that would ever let a retry (or the orphan reaper) find and finish it.
+func TestDeleteLoadBalancer_PersistsSurvivingENIsOnPersistentDeleteFailure(t *testing.T) {
+	_, nc, js := testutil.StartTestJetStream(t)
+	testutil.StubVpcdSGResponder(t, nc)
+
+	vpcSvc, err := handlers_ec2_vpc.NewVPCServiceImplWithNATS(t.Context(), nil, nc)
+	require.NoError(t, err)
+
+	cfg := &config.Config{Daemon: config.DaemonConfig{DevNetworking: true}}
+	masterKey, err := handlers_iam.GenerateMasterKey()
+	require.NoError(t, err)
+	svc, err := NewELBv2ServiceImplWithNATS(cfg, nc, masterKey)
+	require.NoError(t, err)
+	svc.VPCService = vpcSvc
+
+	vpcOut, err := vpcSvc.CreateVpc(context.Background(), &ec2.CreateVpcInput{
+		CidrBlock: aws.String("10.0.0.0/16"),
+	}, testAccountID)
+	require.NoError(t, err)
+	subOut, err := vpcSvc.CreateSubnet(context.Background(), &ec2.CreateSubnetInput{
+		VpcId:            vpcOut.Vpc.VpcId,
+		CidrBlock:        aws.String("10.0.1.0/24"),
+		AvailabilityZone: aws.String("us-east-1a"),
+	}, testAccountID)
+	require.NoError(t, err)
+
+	lbOut, err := svc.CreateLoadBalancer(context.Background(), &elbv2.CreateLoadBalancerInput{
+		Name:    aws.String("persist-alb"),
+		Subnets: []*string{subOut.Subnet.SubnetId},
+	}, testAccountID)
+	require.NoError(t, err)
+	lbArn := *lbOut.LoadBalancers[0].LoadBalancerArn
+
+	lb, err := svc.store.GetLoadBalancerByArn(context.Background(), lbArn)
+	require.NoError(t, err)
+	require.NotEmpty(t, lb.ENIs, "CreateLoadBalancer must have allocated a system-managed ENI")
+	eniID := lb.ENIs[0]
+
+	// Corrupt the ENI record so every DetachAndDeleteENI read fails closed —
+	// a persistent failure, not a one-shot race.
+	eniKV, err := js.KeyValue(context.Background(), handlers_ec2_vpc.KVBucketENIs)
+	require.NoError(t, err)
+	_, err = eniKV.Put(context.Background(), testAccountID+"."+eniID, []byte("{not json"))
+	require.NoError(t, err)
+
+	_, err = svc.DeleteLoadBalancer(context.Background(), &elbv2.DeleteLoadBalancerInput{
+		LoadBalancerArn: aws.String(lbArn),
+	}, testAccountID)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "DependencyViolation")
+
+	persisted, err := svc.store.GetLoadBalancerByArn(context.Background(), lbArn)
+	require.NoError(t, err)
+	require.NotNil(t, persisted, "the LB record must survive while its ENI could not be deleted")
+	assert.Contains(t, persisted.ENIs, eniID, "the surviving ENI id must stay recorded for retry")
+}
+
 func TestCreateLoadBalancer_InvalidSubnet(t *testing.T) {
 	svc, _ := setupTestServiceWithVPC(t)