fix: make volume deletion asynchronous so a large volume cannot fail its own delete (#1356)

DeleteVolume returned only once every object behind the volume was gone, which
made the call's latency a function of the volume's size. Object removal costs
one round trip per object, so no timeout is both short enough for a control
plane request and long enough for a large volume: on a 100 GiB volume the
provider request overran its 30 s budget at 52 s, the tenant was told the
delete failed, and the teardown that had already started was left with no
record accountable for it.

AWS does not work this way. DeleteVolume's output in the EC2 model is
smithy.api#Unit, VolumeState includes deleting, and the documentation says a
volume can remain in that state for several minutes.

DeleteVolume now validates, decides every AWS-returnable error, records
State=deleting and returns. VolumeReclaimReaper converges the teardown and
removes the record only once it is complete, so an unreachable or saturated
blob node delays reclamation instead of failing a delete the tenant has already
been told succeeded. The reaper is cluster-wide and leader-gated, which is also
what serialises the teardown: three concurrent destructive handlers ran one
volume's objects on prod because a retry had nothing to collapse against.

A volume already recorded as deleting answers a second delete from the record,
which collapses both the tenant's retry and DeleteOnTermination's own delete.

Two ordering fixes behind it. The provider handler now tears down local state
before deleting objects, because the local half is fixed-cost and sat behind the
half that scales, so a timeout left this node's WAL and dirty marker behind for
good. And volume.delete and snapshot.delete are classified as data movement,
since both reference real data and neither answers from state already held.

scripts/README.md

@@ -6,7 +6,7 @@ Node installation, dev-environment lifecycle, guest image builds, and verificati
 
 | Script | What it does |
 |---|---|
-| `setup.sh` | The production binary installer, served from `install.mulgadc.com`. Installs deps, users, sudoers, the host firewall policy, files, systemd units, logrotate and udev rules, and loads `openvswitch`, `geneve`, `vhost_net` and `tun`, listing them in `/etc/modules-load.d/spinifex.conf` for later boots. It masks firewalld wherever it is installed, since the Spinifex nftables policy is the only supported node firewall. An unset `--firewall` keeps the mode recorded in `/etc/spinifex/firewall/mode`, and otherwise arms the policy when firewalld was active. Where systemd-resolved is installed but not running, resolvconf is absent and NetworkManager runs, it hands host DNS to resolved: it saves `/etc/resolv.conf` once to `/var/lib/spinifex-reset/resolv.conf.orig`, links `/etc/resolv.conf` to the resolved stub, and stops unless resolved has upstream servers and the package repository host still resolves. It links `/usr/bin/spx` to `/usr/local/bin/spx` where sudo's PATH leaves out `/usr/local/bin`, and takes the nbdkit plugin directory from `nbdkit --dump-config`, stopping when nbdkit cannot say. `SETUP_STAGES` runs a subset; `ISO_BUILD=1` runs it inside the ISO builder's chroot. |
+| `setup.sh` | The production binary installer, served from `install.mulgadc.com`. Installs deps, users, sudoers, the host firewall policy, files, systemd units, logrotate and udev rules, and loads `openvswitch`, `geneve`, `vhost_net` and `tun`, listing them in `/etc/modules-load.d/spinifex.conf` for later boots. It masks firewalld wherever it is installed, since the Spinifex nftables policy is the only supported node firewall. It writes a needrestart drop-in so an apt upgrade never restarts the Spinifex, OVS or OVN units; restarting them after an upgrade is left to the operator. An unset `--firewall` keeps the mode recorded in `/etc/spinifex/firewall/mode`, and otherwise arms the policy when firewalld was active. Where systemd-resolved is installed but not running, resolvconf is absent and NetworkManager runs, it hands host DNS to resolved: it saves `/etc/resolv.conf` once to `/var/lib/spinifex-reset/resolv.conf.orig`, links `/etc/resolv.conf` to the resolved stub, and stops unless resolved has upstream servers and the package repository host still resolves. It links `/usr/bin/spx` to `/usr/local/bin/spx` where sudo's PATH leaves out `/usr/local/bin`, and takes the nbdkit plugin directory from `nbdkit --dump-config`, stopping when nbdkit cannot say. `SETUP_STAGES` runs a subset; `ISO_BUILD=1` runs it inside the ISO builder's chroot. |
 | `setup-ovn.sh` | Bootstraps a compute node for OVN VPC networking: installs OVN/OVS, creates `br-int`, configures the WAN bridge (bridged, DHCP, or routed-NAT uplink), sets the chassis identity, and applies overlay sysctls. It runs OVS and OVN as root, setting `OVS_USER_ID` and `OVN_USER_ID` where `/etc/sysconfig` holds them, and hooks the socket-permission helper onto `ovs-vswitchd`, `ovn-controller`, `openvswitch-ipsec` and, on EL, `ovn-northd`. `--management` also starts the OVN central services, writes their `ovn-ctl` flags to `/etc/default/ovn-central` (Debian) or `/etc/sysconfig/ovn-northd` (EL), and records the role in `/etc/spinifex/ovn-role`. With clustered databases it returns only once this node's NB and SB members are back in their RAFT clusters under a known leader, and fails after `OVN_RAFT_REJOIN_WAIT` seconds (default 60) otherwise, so reruns one node at a time never take two members of a cluster down. A later run without `--management` refuses while that file, or the `OVN_CTL_OPTS` line older nodes carry in `/etc/default/ovn-central`, records the role. It adds `maxsize 100M` to the OVS and OVN logrotate files, plus `rotate 5` where the file sets no count, and rotates as root, matching the daemons. On a host whose IPsec stack is libreswan it enables, stops and masks `openvswitch-ipsec.service` and `ipsec.service` for the daemon to unmask when IPsec is on, unless `openvswitch-ipsec.service` is already running; elsewhere it enables and starts `openvswitch-ipsec.service`, unless the daemon has masked it to keep IPsec off. The links it creates persist through systemd-networkd where that is enabled, and otherwise through `/etc/spinifex/hostnet.json`, which `spinifex-hostnet.service` (`spx admin hostnet apply`) replays at boot and on every OVS start, starting `ovn-controller` again when an OVS stop took it down; the first run records the choice in `/etc/spinifex/hostnet-backend`. Where NetworkManager runs, it leaves the links Spinifex owns unmanaged. |
 | `install-node.sh` | Forms a multi-node cluster across hosts that are already running Spinifex: rebuilds the OVN database in clustered form, runs `spx admin init` on the first host and `spx admin join` on the rest, then verifies. Before touching any host it refuses hosts of different OS families (`distro_os_family`) or `spx` versions, and `--ipsec on` when any host uses libreswan. The scripted form of [install-multi-node](../docs/install/install-multi-node/README.md). Destroys the joining nodes' logical network state and crypto — see its `--help`. |
 | `dev-install.sh` | Full local dev setup through the production installer: builds from source, packs a tarball, runs `setup.sh`, initialises the cluster, and starts services under systemd. |

scripts/setup.sh

@@ -16,8 +16,8 @@
 #   VERBOSE                    Set to 1 to echo "[setup] <stage>" before each top-level step.
 #   SETUP_STAGES               Comma-separated subset of stages to run:
 #                                deps, aws, dns, users, sudoers, firewall, timesync, files,
-#                                directories, env, systemd, sysctl, modules, logrotate,
-#                                udev, fixown, migrations
+#                                directories, env, systemd, sysctl, needrestart, modules,
+#                                logrotate, udev, fixown, migrations
 #                              Unset = run every stage appropriate for the current mode.
 
 set -e
@@ -1936,6 +1936,28 @@ SYSCTL
     info "Kernel tunables installed (net.core.rmem_max/wmem_max = 16MB)"
 }
 
+# --- Keep needrestart off the Spinifex, OVS and OVN units ---
+# Guest QEMU runs in spinifex-daemon's cgroup, so needrestart charges QEMU's
+# replaced libraries to the daemon and restarts it after every dpkg run, which
+# never clears them and ends at the unit's start limit. Restarting the stack
+# after an upgrade is the operator's call; needrestart still lists the units.
+install_needrestart_conf() {
+    stage "installing needrestart override"
+    local dir="${DISTRO_ROOT:-}/etc/needrestart/conf.d"
+    $SUDO install -d "$dir"
+    $SUDO tee "$dir/90-spinifex.conf" > /dev/null << 'NEEDRESTART'
+# Managed by spinifex setup.sh — edits are overwritten.
+# Never restart these automatically; restart them by hand when it suits.
+$nrconf{override_rc}{qr(^spinifex-)} = 0;
+$nrconf{override_rc}{qr(^ovs-)} = 0;
+$nrconf{override_rc}{qr(^ovsdb-)} = 0;
+$nrconf{override_rc}{qr(^ovn-)} = 0;
+$nrconf{override_rc}{qr(^openvswitch-)} = 0;
+NEEDRESTART
+    $SUDO chmod 0644 "$dir/90-spinifex.conf"
+    info "needrestart will not restart the spinifex, OVS or OVN units"
+}
+
 # --- Install logrotate ---
 install_logrotate() {
     stage "installing logrotate config"
@@ -2130,6 +2152,7 @@ main() {
     stage_enabled fixown     && fix_file_ownership
     stage_enabled systemd    && install_systemd
     stage_enabled sysctl     && install_sysctl
+    stage_enabled needrestart && install_needrestart_conf
     stage_enabled modules    && install_kernel_modules
     stage_enabled logrotate  && install_logrotate
     stage_enabled udev       && install_udev

scripts/setup_host_test.sh

@@ -1,6 +1,6 @@
 #!/bin/sh
 # setup_host_test.sh — tests for setup.sh's DNS handover to systemd-resolved,
-# its chrony drop-in and its nbdkit plugin directory.
+# its chrony and needrestart drop-ins and its nbdkit plugin directory.
 #
 # setup.sh is sourced with INSTALL_SPINIFEX_LIB_ONLY=1 so main never runs. PATH
 # holds stub commands and links to a few real tools only, so no host resolver,
@@ -17,10 +17,10 @@ fails=0
 pass() { echo "  ok   - $1"; }
 fail() { echo "  FAIL - $1"; fails=$((fails + 1)); }
 
-echo "-- setup.sh host DNS, chrony and nbdkit"
+echo "-- setup.sh host DNS, chrony, needrestart and nbdkit"
 
 mkdir -p "$TMP/sys"
-for tool in bash awk sed grep readlink basename dirname cp ln mkdir cat head tail wc tee rm chmod; do
+for tool in bash awk sed grep readlink basename dirname cp ln mkdir cat head tail wc tee rm chmod install; do
     ln -s "$(command -v "$tool")" "$TMP/sys/$tool"
 done
 
@@ -249,6 +249,50 @@ case "$RC:$OUT" in
     *) fail "no chrony: rc=$RC: $OUT" ;;
 esac
 
+# --------------------------------------------------------------------------
+# needrestart override.
+# --------------------------------------------------------------------------
+# restarts <conf> <unit> — how needrestart would treat <unit> under <conf>:
+# "no" when an override_rc pattern deselects it, "yes" otherwise.
+restarts() {
+    perl -e 'our %nrconf; do $ARGV[0]; die $@ if $@;
+        for my $re (keys %{$nrconf{override_rc}}) {
+            if ($ARGV[1] =~ $re) { print $nrconf{override_rc}{$re} ? "yes" : "no"; exit }
+        }
+        print "yes"' "$1" "$2"
+}
+
+host
+run install_needrestart_conf
+conf="$TMP/root/etc/needrestart/conf.d/90-spinifex.conf"
+if [ "$RC" != 0 ] || [ ! -f "$conf" ]; then
+    fail "needrestart: rc=$RC: $OUT"
+elif ! command -v perl > /dev/null; then
+    fail "needrestart: perl is not installed, so the override cannot be checked"
+else
+    wrong=""
+    for unit in spinifex-daemon.service spinifex-viperblock.service ovs-vswitchd.service \
+        ovsdb-server.service ovn-controller.service openvswitch-switch.service; do
+        [ "$(restarts "$conf" "$unit")" = no ] || wrong="$wrong $unit"
+    done
+    for unit in ssh.service rsyslog.service polkit.service; do
+        [ "$(restarts "$conf" "$unit")" = yes ] || wrong="$wrong $unit"
+    done
+    if [ -n "$wrong" ]; then
+        fail "needrestart override treats these wrongly:$wrong"
+    else
+        pass "needrestart keeps off the stack's units and still restarts everything else"
+    fi
+fi
+
+before=$(cat "$conf" 2>/dev/null)
+run install_needrestart_conf
+if [ "$RC" = 0 ] && [ "$(cat "$conf" 2>/dev/null)" = "$before" ]; then
+    pass "needrestart override is unchanged by a second install"
+else
+    fail "needrestart override changed on re-run: rc=$RC: $OUT"
+fi
+
 # --------------------------------------------------------------------------
 # nbdkit plugin directory.
 # --------------------------------------------------------------------------

scripts/uninstall-spx.sh

@@ -29,8 +29,9 @@
 #
 # /usr/local/bin/spx, the nbdkit plugin, /usr/local/{share,lib}/spinifex,
 # /usr/share/spinifex, every spinifex systemd unit and slice, the eight
-# spinifex-* service users, the sudoers grant, the sysctl/logrotate/chrony/udev
-# drop-ins, the host firewall policy and its loaded nft table, the OVS bridges
+# spinifex-* service users, the sudoers grant, the sysctl, logrotate, chrony,
+# needrestart and udev drop-ins, the host firewall policy and its loaded nft
+# table, the OVS bridges
 # and OVN databases, the Spinifex resolved drop-in, and the Spinifex CA from the
 # host trust store. Where setup.sh turned on systemd-resolved, it restores
 # /etc/resolv.conf and turns resolved off.
@@ -457,6 +458,7 @@ rm_path /etc/sysctl.d/*spinifex* \
         /etc/sudoers.d/spinifex-network \
         /etc/chrony/conf.d/spinifex.conf \
         /etc/chrony.d/spinifex.conf \
+        /etc/needrestart/conf.d/90-spinifex.conf \
         /etc/logrotate.d/spinifex \
         /etc/tmpfiles.d/spinifex.conf \
         /etc/udev/rules.d/*spinifex* \
@@ -615,6 +617,7 @@ if ! $DRY_RUN; then
              /usr/share/spinifex "$ETC_DIR" "$LOG_DIR" \
              /etc/sudoers.d/spinifex-network /etc/logrotate.d/spinifex /etc/tmpfiles.d/spinifex.conf \
              /etc/chrony/conf.d/spinifex.conf /etc/chrony.d/spinifex.conf "$RESOLV_CONF_SAVED" \
+             /etc/needrestart/conf.d/90-spinifex.conf \
              "$RESOLVED_DROPIN"; do
         exists "$p" && LEFT+=("$p")
     done

spinifex/domains/ec2/image/account_termination_current_behaviour_test.go

@@ -111,6 +111,10 @@ func TestCurrentBehaviour_AccountTermination_CreateImageSnapshotDoesNotPinSource
 	imageID := s.createImage(t, "vol-imaged", "ct-image")
 	require.NoError(t, s.deleteVolume("vol-imaged"), "a CreateImage snapshot does not block deleting its source volume")
 
+	// DeleteVolume records the obligation and returns, so the document is gone
+	// once the reclaimer has converged it rather than when the call returns.
+	_, err = s.volumes.NewVolumeReclaimReaper().Sweep(ctx)
+	require.NoError(t, err)
 	_, err = s.metadata.GetVolume(ctx, ctAccountID, "vol-imaged")
 	require.Error(t, err, "the source volume's document is gone")
 

spinifex/domains/ec2/volume/delete_volume_unreadable_test.go

@@ -103,8 +103,10 @@ func (s *racingDeleteStore) DeleteObject(ctx context.Context, input *s3.DeleteOb
 	return s.MemoryObjectStore.DeleteObject(ctx, input)
 }
 
-// A document another delete removed after this one read it is deleted, so the
-// call must succeed rather than answer InternalError for a finished delete.
+// A document another sweep removed after this one read it is deleted, so the
+// reclaimer must count it done rather than fail the sweep for a finished
+// delete. The record is the obligation, so retiring it is the reclaimer's step
+// and this is where a concurrent remover is now observed.
 func TestDeleteVolume_ConcurrentlyDeletedDocumentIsSuccess(t *testing.T) {
 	memory := objectstore.NewMemoryObjectStore()
 	svc := newTestVolumeServiceWithStore("ap-southeast-2a", memory)
@@ -115,15 +117,19 @@ func TestDeleteVolume_ConcurrentlyDeletedDocumentIsSuccess(t *testing.T) {
 		VolumeID: volumeID, CapacityGiB: 10, State: "available",
 	})
 
-	racing := &racingDeleteStore{MemoryObjectStore: memory}
-	svc.store = racing
-	svc.metadata = ebsmetadata.NewStore(racing, "test-bucket")
-
 	_, err := svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{
 		VolumeId: aws.String(volumeID),
 	}, testVolAccountID)
 	require.NoError(t, err)
 
+	racing := &racingDeleteStore{MemoryObjectStore: memory}
+	svc.store = racing
+	svc.metadata = ebsmetadata.NewStore(racing, "test-bucket")
+
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err, "a document a concurrent sweep removed is a finished delete, not a sweep failure")
+	assert.Equal(t, 1, reclaimed)
+
 	_, getErr := ebsmetadata.NewStore(memory, "test-bucket").GetVolume(context.Background(), testVolAccountID, volumeID)
 	assert.True(t, objectstore.IsNoSuchKeyError(getErr), "the document must be gone, got %v", getErr)
 }

spinifex/domains/ec2/volume/lifecycle_invariants_test.go

@@ -97,6 +97,52 @@ func TestRLC3_VolumeDeleteRequiresDetach(t *testing.T) {
 		"ADR-0003:S2 behaviour; historic PROP-LIFECYCLE-005 §2 mechanism: deleting an attached volume must return VolumeInUse (detach-before-delete, rule #3)")
 }
 
+// TestS2_VolumeDeleteRecordsTheObligationBeforeRemovingTheRecord enforces
+// ADR-0003:S2's delete contract: "Delete first creates a durable obligation and
+// prevents new dependants ... The active record is removed only when all
+// required cleanup is durably complete."
+//
+// A delete that removes the record as part of the request cannot satisfy this,
+// because the cleanup it depends on costs one round trip per object and so has
+// no bound a request can carry. On prod a 100 GiB volume's delete overran its
+// 30 s budget, the tenant was told it failed, and the teardown that had been
+// started was left with no record accountable for it.
+func TestS2_VolumeDeleteRecordsTheObligationBeforeRemovingTheRecord(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)
+	svc.snapshotKV = setupTestVolumeKV(t)
+	seedVolume(t, svc, "vol-obligation000", "available", "")
+
+	_, err := svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{VolumeId: aws.String("vol-obligation000")}, testVolAccountID)
+	require.NoError(t, err)
+
+	owed, err := svc.GetVolumeMetadata(testVolAccountID, "vol-obligation000")
+	require.NoErrorf(t, err, "ADR-0003:S2: the active record must survive the request so the obligation stays readable (service_impl.go DeleteVolume: record the state, do not remove the document)")
+	assert.Equalf(t, volumeStateDeleting, owed.State,
+		"ADR-0003:S2: delete must first create a durable obligation (service_impl.go DeleteVolume: write State=%q before returning)", volumeStateDeleting)
+
+	// The obligation is what callers see, so describe must report it rather than
+	// the available state the volume no longer has. The attach refusal reads the
+	// same field (runtime/daemon/daemon_handlers_volume.go: State != "available").
+	described, err := svc.DescribeVolumes(context.Background(), &ec2.DescribeVolumesInput{
+		VolumeIds: []*string{aws.String("vol-obligation000")},
+	}, testVolAccountID)
+	require.NoError(t, err)
+	require.Len(t, described.Volumes, 1)
+	assert.Equalf(t, volumeStateDeleting, aws.StringValue(described.Volumes[0].State),
+		"ADR-0003:S2: a delete must prevent new dependants, and %q is the state every dependant path gates on", volumeStateDeleting)
+
+	// A second delete adds nothing rather than racing the first teardown.
+	_, err = svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{VolumeId: aws.String("vol-obligation000")}, testVolAccountID)
+	require.NoErrorf(t, err, "ADR-0003:S2: the obligation is durable, so a repeated delete is answered from it (service_impl.go DeleteVolume: return early on State==%q)", volumeStateDeleting)
+
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, 1, reclaimed)
+	_, err = svc.GetVolumeMetadata(testVolAccountID, "vol-obligation000")
+	require.Errorf(t, err, "ADR-0003:S2: the active record is removed once cleanup is durably complete (volume_reclaim_reaper.go Sweep: delete the document last)")
+}
+
 // TestVolumeTagMirror_WritesOnlyControlPlaneDocument locks the single-writer
 // invariant: tag mutations must never rewrite config.json or route an encrypted
 // volume through the ebs.config keyholder queue.

spinifex/domains/ec2/volume/service_impl.go

@@ -33,6 +33,11 @@ import (
 	"github.com/nats-io/nats.go/jetstream"
 )
 
+// volumeStateDeleting is the state a delete records as its durable obligation.
+// The record carries it until the teardown is complete, which is what makes a
+// delete survive a restart and what prevents a second one repeating the work.
+const volumeStateDeleting = string(ebsprovider.VolumeStateDeleting)
+
 // Ensure VolumeServiceImpl implements VolumeService.
 var _ VolumeService = (*VolumeServiceImpl)(nil)
 
@@ -1078,6 +1083,14 @@ func (s *VolumeServiceImpl) ModifyVolume(ctx context.Context, input *ec2.ModifyV
 // is always a metadata-only clear, never a QMP call. Errors from either step
 // are returned, not swallowed.
 func (s *VolumeServiceImpl) DeleteVolumeOnTerminate(ctx context.Context, volumeID, accountID string) error {
+	// Clearing the attachment would overwrite a recorded deletion, so a volume
+	// already owed is left alone: the reclaimer owns it from here, and resetting
+	// it to available would let a second teardown run against the same data.
+	meta, err := s.metadata.GetVolume(ctx, accountID, volumeID)
+	if err == nil && meta.State == volumeStateDeleting {
+		return nil
+	}
+
 	if err := s.UpdateVolumeState(accountID, volumeID, "available", "", ""); err != nil {
 		// No metadata doc means the volume is already gone, per GetVolume's own
 		// contract, so absence here is success, not a teardown failure.
@@ -1086,7 +1099,7 @@ func (s *VolumeServiceImpl) DeleteVolumeOnTerminate(ctx context.Context, volumeI
 		}
 		return fmt.Errorf("clear attachment before terminate delete: %w", err)
 	}
-	_, err := s.DeleteVolume(ctx, &ec2.DeleteVolumeInput{VolumeId: &volumeID}, accountID)
+	_, err = s.DeleteVolume(ctx, &ec2.DeleteVolumeInput{VolumeId: &volumeID}, accountID)
 	return err
 }
 
@@ -1154,8 +1167,17 @@ func (s *VolumeServiceImpl) ForceDetachVolume(ctx context.Context, input *ec2.De
 	}, nil
 }
 
-// DeleteVolume deletes an EBS volume: validates state, asks the EBS provider to
-// destroy the backing data, then removes the control-plane metadata document.
+// DeleteVolume records a durable obligation to destroy an EBS volume and
+// returns, matching AWS: DeleteVolume's output is an empty shape and the
+// documented state machine has the volume "remain in the deleting state for
+// several minutes". It validates, decides every AWS-returnable error, moves the
+// volume to "deleting", and leaves the object teardown to VolumeReclaimReaper.
+//
+// The caller never waits on data. Object deletion costs one round trip per
+// object, so the old synchronous form made this call's latency a function of
+// the volume's size: no timeout can be both short enough for a control-plane
+// request and long enough for a large volume, and the overrun contributed no
+// error a caller could act on.
 func (s *VolumeServiceImpl) DeleteVolume(ctx context.Context, input *ec2.DeleteVolumeInput, accountID string) (*ec2.DeleteVolumeOutput, error) {
 	if input == nil || input.VolumeId == nil || *input.VolumeId == "" {
 		return nil, errors.New(awserrors.ErrorInvalidParameterValue)
@@ -1182,6 +1204,14 @@ func (s *VolumeServiceImpl) DeleteVolume(ctx context.Context, input *ec2.DeleteV
 		return nil, errors.New(awserrors.ErrorInvalidVolumeNotFound)
 	}
 
+	// The obligation is already recorded, so this request has nothing to add.
+	// That makes a tenant's retry and DeleteOnTermination's own delete for the
+	// same volume collapse into one, rather than racing each other's teardown.
+	if meta.State == volumeStateDeleting {
+		slog.InfoContext(ctx, "DeleteVolume: already deleting", "volumeId", volumeID)
+		return &ec2.DeleteVolumeOutput{}, nil
+	}
+
 	// An unattached volume is deletable. State must be "available" OR empty: a
 	// detach/terminate that failed to write back "available" leaves the State
 	// drifted to empty with no attachment, and gating on State=="available"
@@ -1196,19 +1226,16 @@ func (s *VolumeServiceImpl) DeleteVolume(ctx context.Context, input *ec2.DeleteV
 	if err := s.checkVolumeHasNoSnapshots(ctx, volumeID); err != nil {
 		return nil, err
 	}
-	if err := s.provider.DeleteVolume(ctx, ebsprovider.DeleteVolumeRequest{
-		Versioned: ebsprovider.NewVersioned(), VolumeID: volumeID, Handle: meta.ProviderHandle,
-	}); err != nil {
-		slog.ErrorContext(ctx, "DeleteVolume: provider deletion failed", "volumeId", volumeID, "err", err)
-		// The terminate reaper retries this every two minutes and logs whatever
-		// it gets back, so a bare code makes a persistent fault unreadable.
-		return nil, awserrors.Errorf(awserrors.ErrorServerInternal, "delete volume %s: %v", volumeID, err)
-	}
-	if err := s.metadata.DeleteVolume(ctx, accountID, volumeID); err != nil && !objectstore.IsNoSuchKeyError(err) {
-		return nil, awserrors.Errorf(awserrors.ErrorServerInternal, "delete volume metadata %s: %v", volumeID, err)
+
+	// Last write of the request, and the only one that can fail from here: the
+	// record is the obligation, so until this lands nothing is owed and the
+	// caller must not be told otherwise.
+	if err := s.UpdateVolumeState(accountID, volumeID, volumeStateDeleting, "", ""); err != nil {
+		slog.ErrorContext(ctx, "DeleteVolume: failed to record the deletion", "volumeId", volumeID, "err", err)
+		return nil, awserrors.Errorf(awserrors.ErrorServerInternal, "mark volume %s deleting: %v", volumeID, err)
 	}
 
-	slog.InfoContext(ctx, "DeleteVolume completed", "volumeId", volumeID)
+	slog.InfoContext(ctx, "DeleteVolume accepted", "volumeId", volumeID)
 
 	return &ec2.DeleteVolumeOutput{}, nil
 }

spinifex/domains/ec2/volume/service_impl_test.go

@@ -326,6 +326,12 @@ func TestCreateVolume_UsesInjectedProvider(t *testing.T) {
 	require.NoError(t, svc.UpdateVolumeState(testVolAccountID, aws.StringValue(vol.VolumeId), "available", "", ""))
 	_, err = svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{VolumeId: vol.VolumeId}, testVolAccountID)
 	require.NoError(t, err)
+	deleting, err := svc.metadata.GetVolume(context.Background(), testVolAccountID, aws.StringValue(vol.VolumeId))
+	require.NoError(t, err, "the record carries the obligation until the teardown is complete")
+	assert.Equal(t, "deleting", deleting.State)
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, 1, reclaimed)
 	_, err = svc.metadata.GetVolume(context.Background(), testVolAccountID, aws.StringValue(vol.VolumeId))
 	assert.Error(t, err)
 }
@@ -1764,8 +1770,16 @@ func TestDeleteVolumeOnTerminate_ClearsAttachmentThenDeletes(t *testing.T) {
 	err := svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID)
 	require.NoError(t, err, "terminate implies detach: a stale attachment must not block the terminate delete")
 
+	deleting, err := svc.GetVolumeMetadata(testVolAccountID, volumeID)
+	require.NoError(t, err)
+	assert.Equal(t, "deleting", deleting.State, "the volume must be recorded as deleting, not merely detached")
+	assert.Empty(t, deleting.AttachedInstance)
+
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, 1, reclaimed)
 	_, err = svc.GetVolumeMetadata(testVolAccountID, volumeID)
-	require.Error(t, err, "the volume must actually be deleted, not merely detached")
+	require.Error(t, err, "the volume must actually be deleted once the teardown completes")
 	assert.Contains(t, err.Error(), awserrors.ErrorInvalidVolumeNotFound)
 }
 
@@ -1809,11 +1823,12 @@ func (p *failingDeleteProvider) DeleteVolume(context.Context, ebsprovider.Delete
 	return p.err
 }
 
-// TestDeleteVolume_ProviderFailureCarriesItsCause pins that a provider delete
-// failure keeps its cause on the way out. The terminate reaper retries this
-// every two minutes and logs whatever it is handed, so a bare ServerInternal
-// makes a persistent fault indistinguishable from a transient one.
-func TestDeleteVolume_ProviderFailureCarriesItsCause(t *testing.T) {
+// TestDeleteVolume_ProviderFailureDoesNotFailTheRequest pins the behaviour a
+// 100 GiB volume on prod needed: an unreachable blob node must not turn an
+// accepted delete into an error the tenant retries. The obligation is already
+// recorded, so the request succeeds and the teardown is the reclaimer's to
+// retry until it lands.
+func TestDeleteVolume_ProviderFailureDoesNotFailTheRequest(t *testing.T) {
 	kv := setupTestVolumeKV(t)
 	store := objectstore.NewMemoryObjectStore()
 	svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)
@@ -1829,12 +1844,74 @@ func TestDeleteVolume_ProviderFailureCarriesItsCause(t *testing.T) {
 	})
 
 	_, err := svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{VolumeId: aws.String(volumeID)}, testVolAccountID)
-	require.Error(t, err)
+	require.NoError(t, err, "a provider that cannot be reached must not fail a delete the control plane has accepted")
 
-	assert.Equal(t, awserrors.ErrorServerInternal, awserrors.ValidErrorCodeFromError(err),
-		"the code stays ServerInternal so callers classify it unchanged")
-	assert.Contains(t, err.Error(), "dial node 6",
-		"an operator reading the reaper log must learn why the delete keeps failing")
+	meta, err := svc.metadata.GetVolume(context.Background(), testVolAccountID, volumeID)
+	require.NoError(t, err)
+	assert.Equal(t, "deleting", meta.State)
+
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err, "a failed teardown is retried, not reported as a sweep failure")
+	assert.Equal(t, 0, reclaimed)
+	meta, err = svc.metadata.GetVolume(context.Background(), testVolAccountID, volumeID)
+	require.NoError(t, err, "the record survives so the obligation is still readable")
+	assert.Equal(t, "deleting", meta.State)
+
+	// The provider comes back, and the same record is what drives the retry.
+	svc.SetEBSProvider(&failingDeleteProvider{EBSProvider: svc.provider, err: nil})
+	reclaimed, err = svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, 1, reclaimed)
+	_, err = svc.metadata.GetVolume(context.Background(), testVolAccountID, volumeID)
+	assert.Error(t, err)
+}
+
+// TestDeleteVolume_SecondDeleteIsAcceptedWithoutASecondTeardown pins the dedupe
+// the deleting state buys. A tenant retry and DeleteOnTermination's own delete
+// both arrive for the same volume, and on prod three destructive handlers ran
+// one volume's teardown concurrently because nothing collapsed them.
+func TestDeleteVolume_SecondDeleteIsAcceptedWithoutASecondTeardown(t *testing.T) {
+	kv := setupTestVolumeKV(t)
+	store := objectstore.NewMemoryObjectStore()
+	svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)
+	svc.snapshotKV = kv
+
+	volumeID := "vol-deleted-twice"
+	createVolumeInStoreWithMeta(t, svc, store, volumeID, ebsmetadata.Volume{
+		VolumeID: volumeID, CapacityGiB: 10, State: "available",
+	})
+	counting := &countingDeleteProvider{EBSProvider: svc.provider}
+	svc.SetEBSProvider(counting)
+
+	for range 3 {
+		_, err := svc.DeleteVolume(context.Background(), &ec2.DeleteVolumeInput{VolumeId: aws.String(volumeID)}, testVolAccountID)
+		require.NoError(t, err, "AWS answers a repeated delete with the same empty success")
+	}
+	assert.Equal(t, 0, counting.calls, "no teardown runs in the request path at all")
+
+	// DeleteOnTermination arriving for a volume already owed must not reset it
+	// to available, which would let a second teardown through.
+	require.NoError(t, svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID))
+	meta, err := svc.metadata.GetVolume(context.Background(), testVolAccountID, volumeID)
+	require.NoError(t, err)
+	assert.Equal(t, "deleting", meta.State)
+
+	reclaimed, err := svc.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, 1, reclaimed)
+	assert.Equal(t, 1, counting.calls, "one recorded obligation is one teardown")
+}
+
+// countingDeleteProvider counts the deletes that reach the provider boundary.
+type countingDeleteProvider struct {
+	ebsprovider.EBSProvider
+
+	calls int
+}
+
+func (p *countingDeleteProvider) DeleteVolume(context.Context, ebsprovider.DeleteVolumeRequest) error {
+	p.calls++
+	return nil
 }
 
 // TestDeleteVolumeOnTerminate_SurfacesDeleteFailure verifies a DeleteVolume

spinifex/domains/ec2/volume/volume_reclaim_reaper.go

@@ -0,0 +1,93 @@
+package volume
+
+import (
+	"context"
+	"log/slog"
+
+	ebsprovider "github.com/mulgadc/spinifex/spinifex/providers/ebs"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
+)
+
+// VolumeReclaimReaper converges the volumes a tenant has already asked to
+// delete. DeleteVolume records the obligation as State="deleting" and returns,
+// so the object teardown runs here: its cost is one round trip per object and
+// therefore a function of the volume, which is not work a caller can be held
+// on. The active record is removed only once the teardown reports complete.
+//
+// Its licence to delete volume data is narrow and is not VolumeLeakReaper's.
+// That reaper may only mark and alarm because it would be deciding, on its own
+// judgement, that a volume is garbage. This one decides nothing: the tenant
+// decided, the decision is recorded on the volume itself, and the sweep only
+// finishes what the API accepted.
+type VolumeReclaimReaper struct {
+	svc *VolumeServiceImpl
+}
+
+var _ vm.Reaper = (*VolumeReclaimReaper)(nil)
+
+// NewVolumeReclaimReaper builds the reclaimer for volumes recorded as deleting.
+func (s *VolumeServiceImpl) NewVolumeReclaimReaper() *VolumeReclaimReaper {
+	return &VolumeReclaimReaper{svc: s}
+}
+
+func (r *VolumeReclaimReaper) Class() string { return "volume-reclaim" }
+
+// Scope is cluster-wide, which is also what serialises the teardown. A delete
+// that ran on every node would put several destructive handlers on one volume's
+// objects at once, which is what the synchronous path did when a caller retried.
+func (r *VolumeReclaimReaper) Scope() vm.ReaperScope { return vm.ScopeClusterWide }
+
+// Sweep finishes every recorded deletion it can and reports how many records it
+// retired. A volume whose teardown fails keeps its record and is retried on the
+// next sweep: predastore being slow, saturated or down delays reclamation and
+// must not undo a delete the tenant has already been told succeeded.
+func (r *VolumeReclaimReaper) Sweep(ctx context.Context) (int, error) {
+	if r.svc.provider == nil {
+		return 0, nil
+	}
+
+	// The sweep has no caller account, so each document supplies the account its
+	// own key is under.
+	volumes, err := r.svc.metadata.ListAllVolumes(ctx)
+	if err != nil {
+		return 0, err
+	}
+
+	reclaimed := 0
+	for _, meta := range volumes {
+		select {
+		case <-ctx.Done():
+			return reclaimed, ctx.Err()
+		default:
+		}
+
+		if meta.State != volumeStateDeleting {
+			continue
+		}
+
+		if err := r.svc.provider.DeleteVolume(ctx, ebsprovider.DeleteVolumeRequest{
+			Versioned: ebsprovider.NewVersioned(),
+			VolumeID:  meta.VolumeID,
+			Handle:    meta.ProviderHandle,
+		}); err != nil {
+			slog.WarnContext(ctx, "volume-reclaim: teardown incomplete, retrying next sweep",
+				"volumeId", meta.VolumeID, "accountId", meta.TenantID, "err", err)
+			continue
+		}
+
+		// Removing the record is the last step by design: while it exists the
+		// obligation is readable, so a sweep interrupted anywhere above resumes
+		// rather than leaving objects nobody is accountable for.
+		if err := r.svc.metadata.DeleteVolume(ctx, meta.TenantID, meta.VolumeID); err != nil && !objectstore.IsNoSuchKeyError(err) {
+			slog.ErrorContext(ctx, "volume-reclaim: failed to remove the volume record",
+				"volumeId", meta.VolumeID, "accountId", meta.TenantID, "err", err)
+			continue
+		}
+
+		slog.InfoContext(ctx, "volume-reclaim: deletion complete",
+			"volumeId", meta.VolumeID, "accountId", meta.TenantID)
+		reclaimed++
+	}
+	return reclaimed, nil
+}

spinifex/providers/ebs/nats.go

@@ -43,9 +43,11 @@ const dataMovementTimeout = 180 * time.Second
 // Everything absent answers from state already held and keeps the caller's
 // timeout, so a subject added without a deliberate decision stays fast.
 var dataMovementSubjects = map[string]struct{}{
-	CreateVolumeSubject: {},
-	ExpandVolumeSubject: {},
-	CopySnapshotSubject: {},
+	CreateVolumeSubject:   {},
+	ExpandVolumeSubject:   {},
+	CopySnapshotSubject:   {},
+	DeleteVolumeSubject:   {},
+	DeleteSnapshotSubject: {},
 }
 
 // timeoutFor sizes a request to the work its subject names. The per-volume

spinifex/providers/ebs/nats_test.go

@@ -713,8 +713,13 @@ func TestTimeoutForSizesEachSubjectToItsWork(t *testing.T) {
 	ownerDescribe, err := GetVolumeOwnerSubject("vol-1")
 	require.NoError(t, err)
 
+	// A delete costs one round trip per object, so its duration is a function
+	// of the volume. On prod a 100 GiB volume's delete took 52 s against a 30 s
+	// budget, and the overrun was reported to the tenant as a 500 for a delete
+	// that was working.
 	slow := []string{
 		CreateVolumeSubject, ExpandVolumeSubject, CopySnapshotSubject,
+		DeleteVolumeSubject, DeleteSnapshotSubject,
 		snapshotCreate, ownerCopy, ownerExpand, ownerCreate,
 	}
 	for _, subject := range slow {
@@ -724,8 +729,7 @@ func TestTimeoutForSizesEachSubjectToItsWork(t *testing.T) {
 
 	fast := []string{
 		CapabilitiesSubject, GetVolumeSubject, ListVolumesSubject,
-		DeleteVolumeSubject, DeleteSnapshotSubject, ListSnapshotsSubject,
-		ownerDescribe,
+		ListSnapshotsSubject, ownerDescribe,
 	}
 	for _, subject := range fast {
 		assert.Equal(t, callerTimeout, p.timeoutFor(subject),

spinifex/providers/ebs/provider.go

@@ -182,6 +182,11 @@ const (
 	VolumeStateCreating  VolumeState = "creating"
 	VolumeStateAvailable VolumeState = "available"
 	VolumeStateInUse     VolumeState = "in-use"
+	// VolumeStateDeleting is a volume whose teardown is owed but not finished.
+	// AWS documents a volume remaining here for several minutes, because the
+	// object removal behind it scales with the volume and no request can carry
+	// that. Every dependant path refuses it.
+	VolumeStateDeleting VolumeState = "deleting"
 )
 
 // Volume contains only facts the control plane can rely on across providers.

spinifex/providers/ebs/viperblock/provider_handlers.go

@@ -892,6 +892,19 @@ func handleDeleteVolume(ctx context.Context, cfg *Config, nc *nats.Conn, msg *na
 		return
 	}
 
+	// Local teardown runs first because its cost is fixed while the object
+	// deletion below scales with the volume. Behind it, a timeout on a large
+	// volume leaves this node's WAL and dirty marker behind for good.
+	if localPath, err := localVolumeDir(cfg.BaseDir, req.VolumeID); err == nil {
+		if err := os.RemoveAll(localPath); err != nil {
+			slog.Error("ebs.provider.volume.delete: failed to remove local volume directory", "volume", req.VolumeID, "path", localPath, "err", err)
+		}
+	}
+
+	// The volume is going, so a marker pinning it to this node pins nothing.
+	// Left behind it would outlive every copy of the data it describes.
+	cfg.purgeVolumeDirty(ctx, req.VolumeID)
+
 	store := providerObjectStoreFactory(cfg)
 	if err := deleteObjectPrefix(ctx, store, cfg.Bucket, req.VolumeID+"-efi/"); err != nil {
 		slog.Error("ebs.provider.volume.delete: failed to delete aux objects", "volume", req.VolumeID, "err", err)
@@ -904,18 +917,6 @@ func handleDeleteVolume(ctx context.Context, cfg *Config, nc *nats.Conn, msg *na
 		return
 	}
 
-	// Delete is permanent: remove any on-disk WAL/checkpoint cache left on
-	// this node regardless of mount-tracking state, mirroring ebs.delete.
-	if localPath, err := localVolumeDir(cfg.BaseDir, req.VolumeID); err == nil {
-		if err := os.RemoveAll(localPath); err != nil {
-			slog.Error("ebs.provider.volume.delete: failed to remove local volume directory", "volume", req.VolumeID, "path", localPath, "err", err)
-		}
-	}
-
-	// The volume is gone, so a marker pinning it to this node pins nothing.
-	// Left behind it would outlive every copy of the data it describes.
-	cfg.purgeVolumeDirty(ctx, req.VolumeID)
-
 	slog.Info("ebs.provider.volume.delete: deleted", "volume", req.VolumeID)
 	respondProvider(ctx, msg, ebsprovider.DeleteVolumeResponse{Versioned: ebsprovider.NewVersioned()})
 }

spinifex/providers/ebs/viperblock/provider_handlers_test.go

@@ -802,6 +802,54 @@ func TestProviderHandlers_DeleteVolume_MountedVolumeRefused(t *testing.T) {
 	assert.NoError(t, statErr, "a refused delete must leave the NBD socket alone")
 }
 
+// listFailingObjectStore fails every listing, which is the shape of a blob
+// node that stops answering partway through a delete.
+type listFailingObjectStore struct {
+	objectstore.ObjectStore
+
+	err error
+}
+
+func (s *listFailingObjectStore) ListObjectsV2(context.Context, *s3.ListObjectsV2Input) (*s3.ListObjectsV2Output, error) {
+	return nil, s.err
+}
+
+// TestProviderHandlers_DeleteVolume_LocalStateGoesBeforeTheObjects pins the
+// ordering a 100 GiB volume on prod needed. Object deletion costs one round
+// trip per object, so it is the half that runs out of time; behind it the local
+// WAL and the dirty marker are never reached and stay on the node for good.
+func TestProviderHandlers_DeleteVolume_LocalStateGoesBeforeTheObjects(t *testing.T) {
+	_, natsURL := setupEmbeddedNATS(t)
+	cfg := setupTestConfig(t, natsURL)
+	nc := startProviderSubjects(t, cfg, natsURL)
+
+	prev := providerObjectStoreFactory
+	providerObjectStoreFactory = func(*Config) objectstore.ObjectStore {
+		return &listFailingObjectStore{
+			ObjectStore: objectstore.NewMemoryObjectStore(),
+			err:         errors.New("dial node 6: quic dial 10.10.8.5:6660: timeout"),
+		}
+	}
+	t.Cleanup(func() { providerObjectStoreFactory = prev })
+
+	const volumeName = "vol-deleteordering1"
+	localPath, err := localVolumeDir(cfg.BaseDir, volumeName)
+	require.NoError(t, err)
+	require.NoError(t, os.MkdirAll(localPath, 0o750))
+	require.NoError(t, os.WriteFile(filepath.Join(localPath, "wal.0000000001.bin"), []byte("unflushed"), 0o600))
+
+	body := marshalRequest(t, map[string]any{"schema_version": ebsprovider.SchemaVersion, "volume_id": volumeName})
+	msg := requestProvider(t, nc, ebsprovider.DeleteVolumeSubject, body)
+
+	var resp ebsprovider.DeleteVolumeResponse
+	require.NoError(t, json.Unmarshal(msg.Data, &resp))
+	require.NotNil(t, resp.Error, "an unreachable object store must be reported")
+
+	_, statErr := os.Stat(localPath)
+	assert.True(t, os.IsNotExist(statErr),
+		"local teardown is fixed-cost and must not sit behind the half that scales with the volume")
+}
+
 // TestProviderHandlers_PublishVolume_IdempotentRepublish covers the double-
 // writer hazard this provider boundary exists to prevent: republishing a
 // volume this node already has mounted must return the existing attachment

spinifex/runtime/daemon/daemon.go

@@ -2448,6 +2448,7 @@ func (d *Daemon) startCluster() error {
 		}
 		if d.volumeService != nil {
 			reapers = append(reapers, d.volumeService.NewVolumeLeakReaper(d.leakedVolumeInstances))
+			reapers = append(reapers, d.volumeService.NewVolumeReclaimReaper())
 		}
 		if d.eksService != nil {
 			reapers = append(reapers, d.eksService.NewBillableReaper(d.nodeRunningVMs))

spinifex/runtime/daemon/daemon_handlers_volume_test.go

@@ -258,6 +258,60 @@ func TestAttachVolume_InUseDifferentInstance(t *testing.T) {
 	assert.Contains(t, string(resp.Data), "VolumeInUse")
 }
 
+// TestAttachVolume_DeletingVolumeRefused pins the half of the delete contract
+// that is not about the delete itself: a volume whose teardown is already owed
+// must take no new dependant, because the objects behind it are being removed.
+// DeleteVolume records the obligation and returns, so between the call and the
+// reclaimer finishing it the volume is detached and would otherwise look
+// attachable.
+func TestAttachVolume_DeletingVolumeRefused(t *testing.T) {
+	daemon, store := createFullTestDaemonWithStore(t, sharedNATSURL)
+
+	instanceID := "i-attach-vol-deleting"
+	volumeID := "vol-deleting-attach"
+
+	instance := &vm.VM{
+		ID:           instanceID,
+		Status:       vm.StateRunning,
+		AccountID:    testAccountID,
+		InstanceType: getTestInstanceType(t),
+		Instance:     &ec2.Instance{},
+		QMPClient:    &qmp.QMPClient{},
+	}
+	daemon.vmMgr.Insert(instance)
+
+	seedVolumeConfig(t, daemon, store, ebsmetadata.Volume{
+		VolumeID:    volumeID,
+		CapacityGiB: 10,
+		State:       "deleting",
+		TenantID:    testAccountID,
+	})
+
+	sub, err := daemon.natsConn.Subscribe(
+		fmt.Sprintf("ec2.cmd.%s", instanceID),
+		daemon.handleEC2Events,
+	)
+	require.NoError(t, err)
+	defer sub.Unsubscribe()
+
+	command := ec2v1.EC2InstanceCommand{
+		ID:               instanceID,
+		Attributes:       ec2v1.EC2CommandAttributes{AttachVolume: true},
+		AttachVolumeData: &ec2v1.AttachVolumeData{VolumeID: volumeID},
+	}
+	cmdData, err := json.Marshal(command)
+	require.NoError(t, err)
+
+	resp, err := natsRequest(daemon.natsConn,
+		fmt.Sprintf("ec2.cmd.%s", instanceID),
+		cmdData,
+		5*time.Second,
+	)
+	require.NoError(t, err)
+	assert.Contains(t, string(resp.Data), "VolumeInUse",
+		"a volume already recorded for deletion must not take a new attachment")
+}
+
 // TestAttachVolume_IdempotentSameInstance_DeviceMismatch verifies that a
 // same-instance re-attach requesting a DIFFERENT device than the one
 // already attached is treated as a real CSI conflict — AWS returns

spinifex/runtime/daemon/daemon_test.go

@@ -1597,6 +1597,10 @@ func TestInstanceCleanerAdapter_DeleteVolumes_BootVolumeDeletedAfterAttachmentCl
 	err = cleaner.DeleteVolumes(instance)
 	require.NoError(t, err, "the still-attached boot volume must be deleted, not rejected as VolumeInUse")
 
+	// The delete records the obligation and returns, so the reclaimer is what
+	// retires the document.
+	_, err = daemon.volumeService.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
 	_, err = daemon.volumeService.GetVolumeMetadata(testAccountID, volumeID)
 	require.Error(t, err, "the volume must actually be deleted")
 	assert.Contains(t, err.Error(), awserrors.ErrorInvalidVolumeNotFound)
@@ -1745,6 +1749,9 @@ func TestTerminatedTeardownReaper_SelfHealsFailedVolumeTeardown(t *testing.T) {
 	_, err = reaper.Sweep(context.Background())
 	require.NoError(t, err)
 
+	// The retried delete records the obligation; the reclaimer retires it.
+	_, err = daemon.volumeService.NewVolumeReclaimReaper().Sweep(context.Background())
+	require.NoError(t, err)
 	_, err = daemon.volumeService.GetVolumeMetadata(testAccountID, volumeID)
 	require.Error(t, err, "the retried delete must actually remove the volume")
 	assert.Contains(t, err.Error(), awserrors.ErrorInvalidVolumeNotFound)