Actor crash leaves external volumes attached to the node; delete then skips detach #1715

Summary

When an actor crashes, its external volumes are never detached from the worker node. The crash clears Status.WorkerAssignment, and detachActorVolumes treats a nil assignment as "nothing to do" — so the later delete, which is the only exit from CRASHED, skips the detach too. The disk stays ControllerPublished to a node that will never use it again.

Steps to reproduce

1. Create an actor from a template with an externalVolumeTemplate volume, and resume it so the disk is attached.
2. Force a crash that leaves the worker and node healthy — e.g. make the snapshot upload fail, which raises ReasonFaileSaveSnapshot (cmd/atelet/main.go:702). Any of atelet's DataLoss crash directives works.
3. Observe the actor reaches ACTOR_STATE_CRASHED.
4. DeleteActor.

Expected: the volume is detached from the node before it is deleted.

Actual: the delete logs Actor has no assigned worker pod during delete, skipping detach volumes, then calls DeleteVolume on a still-attached disk. On CSI hostpath this silently succeeds; on a real driver such as GCE PD, DeleteVolume fails because the disk is in use, the delete returns an error, and the actor is stuck in DELETING until someone detaches the disk out of band.

Root cause

crashActor clears the assignment as part of moving to the terminal state:

// cmd/ateapi/internal/controlapi/crash.go:100
toUpdate.Status.WorkerAssignment = nil

detachActorVolumes derives the node exclusively from that assignment, and bails out when it is absent:

// cmd/ateapi/internal/controlapi/volumes.go:198
assignment := actor.GetStatus().GetWorkerAssignment()
if assignment == nil {
    slog.WarnContext(ctx, ... "skipping detach volumes" ...)
    return nil
}

The crash path itself never detaches, so by the time ensureVolumesDetachedForDelete (workflow_delete.go:70) runs, the only record of which node held the disk is already gone. The skip returns nil, so volumesDetachedErr is nil and the delete proceeds to ensureVolumesDeleted (:87) as if the detach had succeeded.

The same applies to the second crash path in workflow_worker_delete.go.

Impact

Today, with per-actor provisioned volumes, the damage is contained to the crashed actor: one orphaned attachment and a delete that may fail on drivers that refuse to delete an attached disk.

It gets substantially worse with shared volumes (ExternalVolume), where the disk outlives the actor. A crash releases the reference and makes the volume claimable again, but the stale publish to the old node remains. The next actor to claim it cannot ControllerPublish an RWO disk to a second node, so its resume fails — and nothing in the system ever clears the old attachment. One crashed actor permanently poisons a shared volume for every other actor.

Suggested fix

Record the node on ActorVolumeStatus when the volume is attached, and have detachActorVolumes read the node from the volume record rather than walking WorkerAssignment → GetWorker → NodeName. The message already carries storage_volume_id, volume_type, and volume_context; the node is the one piece of attach state that isn't durable.

That makes detach correct for every actor whose assignment is gone or stale — crashed actors, and the existing GetWorker → ErrNotFound skip a few lines below — without adding CSI work to the crash path, which is the worst moment to depend on a best-effort remote call. It is also the shape shared volumes need anyway, since per-(volume, node) references make the node part of the reference identity.

A narrower alternative is a best-effort detach inside crashActor before the assignment is cleared. It's a smaller diff, but it only covers the crash path and leaves the worker-not-found skip as-is.

Workaround

Detach the disk manually (gcloud compute instances detach-disk, or the driver's equivalent) before deleting a crashed actor that had external volumes.