feat: keep local snapshots on a seat-release delete - #61
Merged
Conversation
DeletePod GC'd the local snapshot on every delete, including the one the operator issues to free a scheduling seat under hibernatePolicy=release. That threw away the warm-wake cache: a wake landing back on the same node (the operator soft-prefers it at weight 100) had to cold-pull state that was still sitting on local disk. Honor the operator's keep-snapshot-on-delete flag and skip the GC in both branches — the forgotten-VM path a hibernate leaves behind, and the live-VM path an interrupted hibernate can leave. The seat-release delete now reports reason=seat_release so the cache-keep rate is countable at the source. Safety is unchanged: resolveWakeSource verifies any local copy against the SnapshotID in the :hibernate tag's config blob, so a stale copy is discarded rather than restored. A missing flag only costs a pull.
The seat-release delete now keeps fork-<vm>, so it can outlive a real lineage break: a set deleted while released never gets another DeletePod (the pod is already gone), and a same-name recreate on the same node would hand the dead incarnation's fork to new sub-agents via ensureForkSnapshot's create-once reuse. The operator already GCs the :hibernate tag at teardown so recreates start clean; this restores the node-local half of that guarantee. Hoist the run-mode fork invalidation to cover every fresh (non-restore) bring-up arm, before boot. A hibernate restore continues the lineage and keeps the fork; the operator creates sub-agents only after the main is Ready, so pre-boot invalidation has no race window.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on cocoonstack/cocoon-common#14 — merged; pinned here as
v0.2.9-0.20260731042413-9ca4f1c8fc0f(cocoon-common main HEAD).Why
DeletePodGC'd the local snapshot on every delete — including the one the operator issues to free a scheduling seat underhibernatePolicy: release. That threw away the warm-wake cache: a wake landing back on the same node (the operator soft-prefers it at weight 100 viahibernated-on-node) had to cold-pull from the registry state that was still sitting on local disk.Note the wake path was already local-first —
resolveWakeSourceprefers a verified local snapshot and only pulls on a miss, andcleanupWakeImportexplicitly keeps the local copy "live for the next wake".retainis fast today precisely because its pod is never deleted, so its snapshot is never GC'd. This closes the gap forrelease, which frees the seat.What
vm.cocoonstack.io/keep-snapshot-on-deleteand skipremoveLocalSnapshotsin both branches: the forgotten-VM path a completed hibernate leaves behind, and the live-VM path an interrupted hibernate can leave (the VM is still removed there — only the snapshot survives).reason=seat_releaseinstead ofno_vm, so the cache-keep rate is countable at the source.fork-<vm>on every fresh (non-restore) bring-up, generalizing the run-mode-only invalidation. The kept fork can outlive a real lineage break — a set deleted while released never gets anotherDeletePod, and a same-name recreate would hand the dead incarnation's fork to new sub-agents viaensureForkSnapshot's create-once reuse. A hibernate restore continues the lineage and keeps the fork; the operator creates sub-agents only after the main is Ready, so pre-boot invalidation has no race window. Pair it withsnapshot_verify_total{result=ok}andsnapshot_pull_totalto see the pulls actually avoided.Safety
The
delete.gocomment this replaces justified the GC as "so a later restore cannot prefer stale local state over the registry tag" — butverifyLocalSnapshotalready defends that more strongly: it compares the local snapshot's ID against theSnapshotIDin the:hibernatetag's config blob, judges a missing tag stale, and fails closed when the registry is unreachable. The tag is the sole authority; the local copy is strictly a cache keyed by it. A missing flag only costs a pull, never correctness.Follow-up (not in this PR)
Nothing reclaims a kept snapshot — the orphan policy covers VMs, not snapshots. This needs node-level
cocoon gc --snapshot --snapshot-size/-ageto bound the cache, and vk's storage allocatable (read once at startup) to stop hiding it from the scheduler, or it trades silent disk oversubscription for memory oversubscription.Test
Two new cases in
delete_test.go(forgotten-VM and live-VM seat release).go build ./...,go test ./...,make lint(linux + darwin) all green.