feat: keep local snapshots on a seat-release delete

docs/lifecycle.md

@@ -35,7 +35,11 @@ and the hibernate transition.
      it, same as the UpdatePod wake path.
    - **Fork-from** (`spec.ForkFrom`): snapshot the named source VM once
      (deduped via `ensureForkSnapshot`) and clone every fork off that
-     shared snapshot.
+     shared snapshot. The fork snapshot is a per-lineage baseline: every
+     fresh (non-restore) bring-up drops `fork-<vm>` before booting, so a
+     recreated same-name VM can never hand a dead incarnation's baseline
+     to new sub-agents; a hibernate restore keeps it, since a wake
+     continues the same lineage.
    - **`Managed=false`** (static / externally-managed VMs, e.g. Windows
      toolboxes on an external QEMU host): skip the runtime entirely and
      adopt the pre-assigned `VMID` / `IP` / `VNCPort` the operator
@@ -107,7 +111,15 @@ and the hibernate transition.
      i.e. slot 0 of its CocoonSet).
    - `never`: skip snapshots entirely.
 3. `Runtime.Remove(vmID)` to destroy the VM.
-4. Forget the pod from the in-memory tables.
+4. Drop the local snapshot and its fork snapshot, **unless** the pod carries
+   `vm.cocoonstack.io/keep-snapshot-on-delete`. The operator sets that flag
+   when the delete is a `hibernatePolicy: release` seat release: the VM state
+   stays claimable from the `:hibernate` tag, so the node-local snapshot is
+   kept as the warm-wake cache that lets a wake landing back on this node
+   skip the registry pull. `resolveWakeSource` still verifies any local copy
+   against the tag's `SnapshotID`, so keeping it cannot restore stale state.
+   A missing flag only costs a pull, never correctness.
+5. Forget the pod from the in-memory tables.
 
 ## UpdatePod
 

go.mod

@@ -3,7 +3,7 @@ module github.com/cocoonstack/vk-cocoon
 go 1.26.5
 
 require (
-	github.com/cocoonstack/cocoon-common v0.2.9-0.20260728170736-b0a8b954c944
+	github.com/cocoonstack/cocoon-common v0.2.9-0.20260731042413-9ca4f1c8fc0f
 	github.com/google/go-containerregistry v0.21.7
 	github.com/projecteru2/core v0.0.0-20241016125006-ff909eefe04c
 	github.com/prometheus/client_golang v1.23.2

go.sum

@@ -43,8 +43,8 @@ github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZe
 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
 github.com/cockroachdb/redact v1.1.3 h1:AKZds10rFSIj7qADf0g46UixK8NNLwWTNdCIGS5wfSQ=
 github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
-github.com/cocoonstack/cocoon-common v0.2.9-0.20260728170736-b0a8b954c944 h1:Jru/8LxzwHZKVjFklwClA/UU6oRV9wVVTIJxLSeMBlg=
-github.com/cocoonstack/cocoon-common v0.2.9-0.20260728170736-b0a8b954c944/go.mod h1:VSfgYiWxoHRnWybzQNaxmK4kVoLT9ffiMKll5/i92CM=
+github.com/cocoonstack/cocoon-common v0.2.9-0.20260731042413-9ca4f1c8fc0f h1:mcHAOv4VUN1cc3CSAeoWyKh/iwPQ4WithgB2sKAVi0Y=
+github.com/cocoonstack/cocoon-common v0.2.9-0.20260731042413-9ca4f1c8fc0f/go.mod h1:VSfgYiWxoHRnWybzQNaxmK4kVoLT9ffiMKll5/i92CM=
 github.com/codegangsta/inject v0.0.0-20150114235600-33e0aa1cb7c0/go.mod h1:4Zcjuz89kmFXt9morQgcfYZAYZ5n8WHjt81YYWIwtTM=
 github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
 github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk=

metrics/metrics.go

@@ -21,7 +21,7 @@ var (
 			Name:      "pod_lifecycle_total",
 			Help:      "Number of pod lifecycle operations by op, result, and reason.",
 		},
-		// result=ok|failed|skipped; reason=adopted|noop|missing_vmname|no_vm
+		// result=ok|failed|skipped; reason=adopted|noop|missing_vmname|no_vm|seat_release
 		[]string{"op", labelResult, "reason"},
 	)
 

provider/cocoon/create.go

@@ -194,8 +194,7 @@ func (p *Provider) hibernateEvidence(ctx context.Context, vmName string) (bool,
 	return true, m.Annotations[manifest.AnnotationSnapshotBaseImage], nil
 }
 
-// bringUpVM dispatches on mode: unmanaged, clone, run, or fork. The
-// returned sourceImage feeds post-clone classification.
+// bringUpVM boots the VM for its mode; the returned sourceImage feeds post-clone classification.
 func (p *Provider) bringUpVM(ctx context.Context, pod *corev1.Pod, spec meta.VMSpec) (*vm.VM, string, error) {
 	if !spec.Managed {
 		runtime := meta.ParseVMRuntime(pod)
@@ -212,8 +211,7 @@ func (p *Provider) bringUpVM(ctx context.Context, pod *corev1.Pod, spec meta.VMS
 	if err != nil {
 		return nil, "", err
 	}
-	switch {
-	case meta.ReadRestoreFromHibernate(pod):
+	if meta.ReadRestoreFromHibernate(pod) {
 		sourceName, snapshot, err := p.resolveWakeSource(ctx, spec.VMName)
 		if err != nil {
 			return nil, "", err
@@ -223,7 +221,15 @@ func (p *Provider) bringUpVM(ctx context.Context, pod *corev1.Pod, spec meta.VMS
 			return nil, "", err
 		}
 		return v, "", nil
+	}
+
+	// Invalidate the fork snapshot from a previous incarnation so sub-agents clone from current state.
+	forkName := forkSnapshotName(spec.VMName)
+	if err := p.Runtime.SnapshotRemoveIfExists(ctx, forkName); err != nil {
+		log.WithFunc("Provider.bringUpVM").Errorf(ctx, err, "invalidate fork snapshot %s", forkName)
+	}
 
+	switch {
 	case fromDir != "":
 		if mode == string(cocoonv1.AgentModeRun) {
 			return nil, "", fmt.Errorf("annotation %s is incompatible with mode=run", meta.AnnotationCloneFromDir)
@@ -284,12 +290,6 @@ func (p *Provider) bringUpVM(ctx context.Context, pod *corev1.Pod, spec meta.VMS
 		if err != nil {
 			return nil, "", fmt.Errorf("run vm %s: %w", spec.VMName, err)
 		}
-		// Invalidate fork snapshot from a previous incarnation so later
-		// sub-agents clone from current state.
-		forkName := forkSnapshotName(spec.VMName)
-		if err := p.Runtime.SnapshotRemoveIfExists(ctx, forkName); err != nil {
-			log.WithFunc("Provider.bringUpVM").Errorf(ctx, err, "invalidate fork snapshot %s", forkName)
-		}
 		return v, "", nil
 
 	default: // clone is the default
@@ -334,10 +334,9 @@ func (p *Provider) imagePresent(ctx context.Context, digest string) bool {
 	return digest != "" && p.Runtime.Image(ctx, digest) == nil
 }
 
-// ensureSnapshotBaseImage materializes a snapshot's OCI-ref base image before a
-// clone or restore. cocoon's `vm clone --pull` only fetches http(s) bases, so an
-// OCI-ref base must be imported here. Dedup by digest — the same bytes may be
-// local under another name (epoch→AR ref migration).
+// cocoon's `vm clone --pull` only fetches http(s) bases, so an OCI-ref base
+// must be imported here. Dedup by digest — the same bytes may be local under
+// another name (epoch→AR ref migration).
 func (p *Provider) ensureSnapshotBaseImage(ctx context.Context, snapshot *vm.Snapshot) error {
 	if snapshot == nil || snapshot.Image == "" || isHTTPURL(snapshot.Image) || p.imagePresent(ctx, snapshot.ImageDigest) {
 		return nil
@@ -543,8 +542,6 @@ func awaitFlight[T any](ctx context.Context, ch <-chan singleflight.Result, zero
 	}
 }
 
-// parseCloneFromDirAnnotation returns the validated absolute, canonical
-// path from the clone-from-dir annotation, or "" when absent.
 func parseCloneFromDirAnnotation(pod *corev1.Pod) (string, error) {
 	raw := strings.TrimSpace(pod.Annotations[meta.AnnotationCloneFromDir])
 	if raw == "" {
@@ -573,8 +570,6 @@ func isClonedBoot(pod *corev1.Pod, spec meta.VMSpec) bool {
 	return hasExplicitCloneSource(pod, spec) || strings.ToLower(spec.Mode) != string(cocoonv1.AgentModeRun)
 }
 
-// hasExplicitCloneSource reports whether the pod names its own clone source
-// (clone-from-dir or fork-from).
 func hasExplicitCloneSource(pod *corev1.Pod, spec meta.VMSpec) bool {
 	return strings.TrimSpace(pod.Annotations[meta.AnnotationCloneFromDir]) != "" || spec.ForkFrom != ""
 }
@@ -593,7 +588,6 @@ func assertSnapshotBackend(snapshot *vm.Snapshot, targetBackend string) error {
 		snapshot.Name, snapshot.Hypervisor, targetBackend)
 }
 
-// isHTTPURL reports whether ref looks like an HTTP(S) cloud-image URL.
 func isHTTPURL(ref string) bool {
 	return strings.HasPrefix(ref, "http://") || strings.HasPrefix(ref, "https://")
 }

provider/cocoon/create_test.go

@@ -215,24 +215,37 @@ func TestEnsureForkSnapshotDedupsConcurrentSaves(t *testing.T) {
 	}
 }
 
-func TestCreatePodRunModeInvalidatesForkSnapshot(t *testing.T) {
-	// A fresh main VM must drop the old fork snapshot so sub-agents cloned
-	// after a main recreate pick up current state, not the stale checkpoint.
-	rt := &fakeRuntime{runVM: &vm.VM{ID: "vmid-main", Name: "vk-ns-demo-0"}}
-	p := newTestProvider(t)
-	p.Runtime = rt
-
-	pod := newPodWithSpec(meta.VMSpec{
-		VMName: "vk-ns-demo-0",
-		Image:  "registry.example/cocoon/ubuntu:24.04",
-		Mode:   "run",
-		OS:     "linux",
-	})
-	if err := p.CreatePod(t.Context(), pod); err != nil {
-		t.Fatalf("create: %v", err)
+// TestCreatePodInvalidatesForkSnapshot locks the fresh-boot invalidation: a recreated main must drop the old fork so sub-agents pick up current state, not the stale checkpoint.
+func TestCreatePodInvalidatesForkSnapshot(t *testing.T) {
+	tests := []struct {
+		name string
+		rt   *fakeRuntime
+		spec meta.VMSpec
+	}{
+		{
+			name: "run mode",
+			rt:   &fakeRuntime{runVM: &vm.VM{ID: "vmid-main", Name: "vk-ns-demo-0"}},
+			spec: meta.VMSpec{VMName: "vk-ns-demo-0", Image: "registry.example/cocoon/ubuntu:24.04", Mode: "run", OS: "linux"},
+		},
+		{
+			name: "clone mode",
+			rt:   &fakeRuntime{snapshots: map[string]*vm.Snapshot{"snapshot-repo": {Name: "snapshot-repo"}}},
+			spec: meta.VMSpec{VMName: "vk-ns-demo-0", Image: "snapshot-repo:latest", Mode: "clone"},
+		},
 	}
-	if len(rt.snapshotRemoveCalls) != 1 || rt.snapshotRemoveCalls[0] != "fork-vk-ns-demo-0" {
-		t.Fatalf("SnapshotRemoveIfExists calls = %v, want [fork-vk-ns-demo-0]", rt.snapshotRemoveCalls)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := newTestProvider(t)
+			p.Runtime = tt.rt
+
+			pod := newPodWithSpec(tt.spec)
+			if err := p.CreatePod(t.Context(), pod); err != nil {
+				t.Fatalf("create: %v", err)
+			}
+			if len(tt.rt.snapshotRemoveCalls) != 1 || tt.rt.snapshotRemoveCalls[0] != "fork-vk-ns-demo-0" {
+				t.Fatalf("SnapshotRemoveIfExists calls = %v, want [fork-vk-ns-demo-0]", tt.rt.snapshotRemoveCalls)
+			}
+		})
 	}
 }
 
@@ -1265,7 +1278,7 @@ func TestStartupReconcileOrphanDestroyRemovesUnmatchedVM(t *testing.T) {
 	p := newTestProvider(t)
 	p.NodeName = "cocoon-pool"
 	p.Runtime = rt
-	p.Clientset = fake.NewSimpleClientset() // no pods
+	p.Clientset = fake.NewSimpleClientset()
 	p.OrphanPolicy = provider.OrphanDestroy
 
 	if err := p.StartupReconcile(t.Context()); err != nil {
@@ -2026,7 +2039,6 @@ func newTestProvider(t *testing.T) *Provider {
 	return p
 }
 
-// newLeaseParser writes a one-entry leases.json and returns a parser for it.
 func newLeaseParser(t *testing.T, mac, ip string) *network.LeaseParser {
 	t.Helper()
 	path := filepath.Join(t.TempDir(), "leases.json")

provider/cocoon/delete.go

@@ -21,12 +21,19 @@ func (p *Provider) DeletePod(ctx context.Context, pod *corev1.Pod) error {
 		return err
 	}
 	spec := meta.ParseVMSpec(pod)
+	// A seat release keeps the local snapshot as the same-node warm-wake cache; resolveWakeSource still gates it on the :hibernate tag.
+	keepSnapshots := meta.ReadKeepSnapshotOnDelete(pod)
 
 	v := p.vmForPod(pod.Namespace, pod.Name)
 	if v == nil {
-		p.removeLocalSnapshots(ctx, spec.VMName)
+		reason := "no_vm"
+		if keepSnapshots {
+			reason = "seat_release"
+		} else {
+			p.removeLocalSnapshots(ctx, spec.VMName)
+		}
 		p.forgetPod(pod.Namespace, pod.Name)
-		metrics.PodLifecycleTotal.WithLabelValues("delete", "skipped", "no_vm").Inc()
+		metrics.PodLifecycleTotal.WithLabelValues("delete", "skipped", reason).Inc()
 		return nil
 	}
 
@@ -39,7 +46,9 @@ func (p *Provider) DeletePod(ctx context.Context, pod *corev1.Pod) error {
 		return fmt.Errorf("remove vm %s: %w", v.ID, err)
 	}
 
-	p.removeLocalSnapshots(ctx, v.Name)
+	if !keepSnapshots {
+		p.removeLocalSnapshots(ctx, v.Name)
+	}
 
 	p.forgetPod(pod.Namespace, pod.Name)
 	pod.Status.Phase = corev1.PodSucceeded
@@ -48,7 +57,6 @@ func (p *Provider) DeletePod(ctx context.Context, pod *corev1.Pod) error {
 	return nil
 }
 
-// removeLocalSnapshots drops the clone source and its fork snapshot so a later restore cannot prefer stale local state over the registry tag.
 func (p *Provider) removeLocalSnapshots(ctx context.Context, vmName string) {
 	if vmName == "" {
 		return

provider/cocoon/delete_test.go

@@ -1,6 +1,7 @@
 package cocoon
 
 import (
+	"slices"
 	"strings"
 	"testing"
 
@@ -9,31 +10,60 @@ import (
 	"github.com/cocoonstack/vk-cocoon/vm"
 )
 
-// TestDeletePodForgottenVMRemovesLocalSnapshots locks the GC fix: deleting a pod whose VM was already forgotten must still remove its local snapshots.
-func TestDeletePodForgottenVMRemovesLocalSnapshots(t *testing.T) {
-	rt := &fakeRuntime{}
-	p := newTestProvider(t)
-	p.Runtime = rt
+// TestDeletePodSnapshotRetention locks the delete GC table: a plain delete removes the local snapshots, a seat release keeps them as the warm-wake cache.
+func TestDeletePodSnapshotRetention(t *testing.T) {
+	tests := []struct {
+		name          string
+		track         *vm.VM
+		keep          bool
+		wantRemovedID string
+		wantSnapshots []string
+	}{
+		{
+			name:          "forgotten vm removes snapshots",
+			wantSnapshots: []string{"vk-ns-demo-0", forkSnapshotName("vk-ns-demo-0")},
+		},
+		{
+			name: "seat release keeps snapshots",
+			keep: true,
+		},
+		{
+			name:          "seat release with live vm removes only the vm",
+			track:         &vm.VM{ID: "live-vmid", Name: "vk-ns-demo-0", State: vm.StateRunning},
+			keep:          true,
+			wantRemovedID: "live-vmid",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rt := &fakeRuntime{}
+			p := newTestProvider(t)
+			p.Runtime = rt
 
-	// Pod is never tracked → vmForPod returns nil (VM already forgotten).
-	pod := newPodWithSpec(meta.VMSpec{VMName: "vk-ns-demo-0"})
+			pod := newPodWithSpec(meta.VMSpec{VMName: "vk-ns-demo-0", Mode: "clone"})
+			if tt.keep {
+				meta.MarkKeepSnapshotOnDelete(pod)
+			}
+			if tt.track != nil {
+				p.trackPod(pod, tt.track)
+			}
 
-	if err := p.DeletePod(t.Context(), pod); err != nil {
-		t.Fatalf("DeletePod: %v", err)
-	}
+			if err := p.DeletePod(t.Context(), pod); err != nil {
+				t.Fatalf("DeletePod: %v", err)
+			}
 
-	removed := map[string]bool{}
-	for _, name := range rt.snapshotRemoveCalls {
-		removed[name] = true
-	}
-	if !removed["vk-ns-demo-0"] || !removed[forkSnapshotName("vk-ns-demo-0")] || len(removed) != 2 {
-		t.Errorf("snapshotRemoveCalls = %v, want exactly [vk-ns-demo-0 %s]", rt.snapshotRemoveCalls, forkSnapshotName("vk-ns-demo-0"))
-	}
-	if rt.removedID != "" {
-		t.Errorf("forgotten-VM delete must not call Runtime.Remove, got %q", rt.removedID)
-	}
-	if rt.snapshotSaveCount != 0 {
-		t.Errorf("forgotten-VM delete must not save a snapshot, got %d", rt.snapshotSaveCount)
+			if rt.removedID != tt.wantRemovedID {
+				t.Errorf("removedID = %q, want %q", rt.removedID, tt.wantRemovedID)
+			}
+			got := slices.Sorted(slices.Values(rt.snapshotRemoveCalls))
+			want := slices.Sorted(slices.Values(tt.wantSnapshots))
+			if !slices.Equal(got, want) {
+				t.Errorf("snapshotRemoveCalls = %v, want %v", rt.snapshotRemoveCalls, tt.wantSnapshots)
+			}
+			if rt.snapshotSaveCount != 0 {
+				t.Errorf("delete must not save a snapshot, got %d", rt.snapshotSaveCount)
+			}
+		})
 	}
 }
 

provider/cocoon/restore_test.go

@@ -56,6 +56,9 @@ func TestBringUpVMRestoreFromHibernate(t *testing.T) {
 			if tc.wantNICs && (rt.cloned.NICs == nil || *rt.cloned.NICs != 1) {
 				t.Errorf("CH+Windows restore must clone with --nics 1; got %v", rt.cloned.NICs)
 			}
+			if len(rt.snapshotRemoveCalls) != 0 {
+				t.Errorf("restore must keep the fork snapshot, removed %v", rt.snapshotRemoveCalls)
+			}
 		})
 	}
 }