From 0b4e7a0dee7f840549d23412043e16d2d9e03502 Mon Sep 17 00:00:00 2001 From: tonic Date: Thu, 30 Jul 2026 22:58:07 +0800 Subject: [PATCH 1/3] feat: add the keep-snapshot-on-delete pod flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A hibernatePolicy=release suspend deletes the pod to free its scheduling seat while the VM state stays claimable from the :hibernate tag. vk-cocoon cannot tell that deletion apart from a real teardown, so it GCs the node-local snapshot — and with it the only thing that would let a wake landing back on the same node skip the registry pull. This is the signal that tells them apart: the operator flags the pod just before a seat-release delete, and vk-cocoon keeps the local snapshot as a warm-wake cache. Read/Mark mirror the restore-from-hibernate pair, and PatchKeepSnapshotOnDelete follows PatchHibernateState's short-circuit. Keeping the snapshot cannot restore stale state: resolveWakeSource still verifies any local copy against the SnapshotID in the tag's config blob and discards a mismatch, so the tag stays the sole authority. --- k8s/clientpatch.go | 9 +++++++++ k8s/clientpatch_test.go | 20 ++++++++++++++++++++ meta/hibernate.go | 14 ++++++++++++++ meta/hibernate_test.go | 23 +++++++++++++++++++++++ meta/keys.go | 5 +++++ 5 files changed, 71 insertions(+) diff --git a/k8s/clientpatch.go b/k8s/clientpatch.go index b4ff770..cd1d212 100644 --- a/k8s/clientpatch.go +++ b/k8s/clientpatch.go @@ -37,6 +37,15 @@ func PatchHibernateState(ctx context.Context, cli client.Client, pod *corev1.Pod }) } +// PatchKeepSnapshotOnDelete flags the pod's coming deletion as a seat release so +// vk-cocoon keeps its local snapshot. Short-circuits if already flagged. +func PatchKeepSnapshotOnDelete(ctx context.Context, cli client.Client, pod *corev1.Pod) error { + if meta.ReadKeepSnapshotOnDelete(pod) { + return nil + } + return Patch(ctx, cli, pod, meta.MarkKeepSnapshotOnDelete) +} + // PatchCocoonSetGeneration stamps the owning CocoonSet's metadata.generation // onto the pod so vk-cocoon can read it back as lifecycle-observed-generation. // Short-circuits when the annotation is already correct. diff --git a/k8s/clientpatch_test.go b/k8s/clientpatch_test.go index 0d24ac0..1c958bb 100644 --- a/k8s/clientpatch_test.go +++ b/k8s/clientpatch_test.go @@ -121,3 +121,23 @@ func newFakeClient(t *testing.T, objs ...client.Object) client.Client { } return ctrlfake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() } + +func TestPatchKeepSnapshotOnDeletePersistsAndShortCircuits(t *testing.T) { + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "ns"}} + cli := newFakeClient(t, pod.DeepCopy()) + + if err := PatchKeepSnapshotOnDelete(t.Context(), cli, pod); err != nil { + t.Fatalf("PatchKeepSnapshotOnDelete: %v", err) + } + var got corev1.Pod + if err := cli.Get(t.Context(), client.ObjectKey{Namespace: "ns", Name: "demo"}, &got); err != nil { + t.Fatalf("get: %v", err) + } + if !meta.ReadKeepSnapshotOnDelete(&got) { + t.Errorf("flag must reach the API server before the delete lands: %v", got.Annotations) + } + // The fake client errors on an empty-body Patch, so success proves the no-op guard. + if err := PatchKeepSnapshotOnDelete(t.Context(), cli, &got); err != nil { + t.Fatalf("re-flagging an already-flagged pod must be a no-op: %v", err) + } +} diff --git a/meta/hibernate.go b/meta/hibernate.go index 79b8d19..da96761 100644 --- a/meta/hibernate.go +++ b/meta/hibernate.go @@ -34,3 +34,17 @@ func MarkRestoreFromHibernate(pod *corev1.Pod) { a := ensurePodAnnotations(pod) a[AnnotationRestoreFromHibernate] = annotationTrue } + +//nolint:dupl // mirrors the restore-from-hibernate pair by design; merging them would hide which key is read. + +// ReadKeepSnapshotOnDelete reports whether this pod's deletion releases a +// scheduling seat, in which case the node-local snapshot must outlive the pod. +func ReadKeepSnapshotOnDelete(pod *corev1.Pod) bool { + return pod.Annotations[AnnotationKeepSnapshotOnDelete] == annotationTrue +} + +// MarkKeepSnapshotOnDelete flags a pod's deletion as a seat release. +func MarkKeepSnapshotOnDelete(pod *corev1.Pod) { + a := ensurePodAnnotations(pod) + a[AnnotationKeepSnapshotOnDelete] = annotationTrue +} diff --git a/meta/hibernate_test.go b/meta/hibernate_test.go index b814fbd..eb05a8c 100644 --- a/meta/hibernate_test.go +++ b/meta/hibernate_test.go @@ -80,3 +80,26 @@ func TestDefaultSnapshotTagConstant(t *testing.T) { t.Errorf("DefaultSnapshotTag must differ from HibernateSnapshotTag") } } + +func TestKeepSnapshotOnDeleteRoundTrip(t *testing.T) { + pod := &corev1.Pod{} + if ReadKeepSnapshotOnDelete(pod) { + t.Error("an unflagged pod must not keep its snapshot: a plain teardown has to GC it") + } + MarkKeepSnapshotOnDelete(pod) + if pod.Annotations[AnnotationKeepSnapshotOnDelete] != annotationTrue { + t.Errorf("MarkKeepSnapshotOnDelete should set %s=%s, got %q", AnnotationKeepSnapshotOnDelete, annotationTrue, pod.Annotations[AnnotationKeepSnapshotOnDelete]) + } + if !ReadKeepSnapshotOnDelete(pod) { + t.Error("Read must see what Mark wrote") + } +} + +func TestReadKeepSnapshotOnDeleteRejectsNonTrue(t *testing.T) { + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ + AnnotationKeepSnapshotOnDelete: "1", + }}} + if ReadKeepSnapshotOnDelete(pod) { + t.Error("only the literal \"true\" may keep a snapshot alive past its pod") + } +} diff --git a/meta/keys.go b/meta/keys.go index 51db498..3973765 100644 --- a/meta/keys.go +++ b/meta/keys.go @@ -55,6 +55,11 @@ const ( // restore its VM from the :hibernate snapshot (cross-node migration) instead // of cloning from the base image. Written by the operator on the rebuilt pod. AnnotationRestoreFromHibernate = "vm.cocoonstack.io/restore-from-hibernate" + // AnnotationKeepSnapshotOnDelete marks a pod deletion as a scheduling-seat + // release rather than a teardown: the VM state stays claimable from the + // :hibernate tag, so the node-local snapshot must survive as the same-node + // warm-wake cache. Written by the operator just before a release-policy delete. + AnnotationKeepSnapshotOnDelete = "vm.cocoonstack.io/keep-snapshot-on-delete" // AnnotationForkFrom names a VM to fork the new VM from. AnnotationForkFrom = "vm.cocoonstack.io/fork-from" // AnnotationCloneFromDir names a host directory to clone the VM image from (vk-cocoon-specific). From ed112fc8ac576689322d672cde4313e2cf065242 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 31 Jul 2026 11:50:49 +0800 Subject: [PATCH 2/3] review: trim comments, dead nolint, and redundant test coverage --- k8s/clientpatch.go | 3 +-- k8s/clientpatch_test.go | 18 +++++++++--------- meta/hibernate.go | 5 +---- meta/hibernate_test.go | 18 +++--------------- meta/keys.go | 7 +++---- 5 files changed, 17 insertions(+), 34 deletions(-) diff --git a/k8s/clientpatch.go b/k8s/clientpatch.go index cd1d212..26312ee 100644 --- a/k8s/clientpatch.go +++ b/k8s/clientpatch.go @@ -37,8 +37,7 @@ func PatchHibernateState(ctx context.Context, cli client.Client, pod *corev1.Pod }) } -// PatchKeepSnapshotOnDelete flags the pod's coming deletion as a seat release so -// vk-cocoon keeps its local snapshot. Short-circuits if already flagged. +// PatchKeepSnapshotOnDelete flags the pod's deletion as a seat release, short-circuiting if already flagged. func PatchKeepSnapshotOnDelete(ctx context.Context, cli client.Client, pod *corev1.Pod) error { if meta.ReadKeepSnapshotOnDelete(pod) { return nil diff --git a/k8s/clientpatch_test.go b/k8s/clientpatch_test.go index 1c958bb..1a33d67 100644 --- a/k8s/clientpatch_test.go +++ b/k8s/clientpatch_test.go @@ -113,15 +113,6 @@ func TestPatchCocoonSetGenerationShortCircuitsNoOp(t *testing.T) { } } -func newFakeClient(t *testing.T, objs ...client.Object) client.Client { - t.Helper() - scheme := runtime.NewScheme() - if err := clientgoscheme.AddToScheme(scheme); err != nil { - t.Fatalf("add client-go scheme: %v", err) - } - return ctrlfake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() -} - func TestPatchKeepSnapshotOnDeletePersistsAndShortCircuits(t *testing.T) { pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "ns"}} cli := newFakeClient(t, pod.DeepCopy()) @@ -141,3 +132,12 @@ func TestPatchKeepSnapshotOnDeletePersistsAndShortCircuits(t *testing.T) { t.Fatalf("re-flagging an already-flagged pod must be a no-op: %v", err) } } + +func newFakeClient(t *testing.T, objs ...client.Object) client.Client { + t.Helper() + scheme := runtime.NewScheme() + if err := clientgoscheme.AddToScheme(scheme); err != nil { + t.Fatalf("add client-go scheme: %v", err) + } + return ctrlfake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() +} diff --git a/meta/hibernate.go b/meta/hibernate.go index da96761..aed15e9 100644 --- a/meta/hibernate.go +++ b/meta/hibernate.go @@ -35,10 +35,7 @@ func MarkRestoreFromHibernate(pod *corev1.Pod) { a[AnnotationRestoreFromHibernate] = annotationTrue } -//nolint:dupl // mirrors the restore-from-hibernate pair by design; merging them would hide which key is read. - -// ReadKeepSnapshotOnDelete reports whether this pod's deletion releases a -// scheduling seat, in which case the node-local snapshot must outlive the pod. +// ReadKeepSnapshotOnDelete reports whether the pod's deletion is flagged as a seat release. func ReadKeepSnapshotOnDelete(pod *corev1.Pod) bool { return pod.Annotations[AnnotationKeepSnapshotOnDelete] == annotationTrue } diff --git a/meta/hibernate_test.go b/meta/hibernate_test.go index eb05a8c..fbf8e35 100644 --- a/meta/hibernate_test.go +++ b/meta/hibernate_test.go @@ -81,25 +81,13 @@ func TestDefaultSnapshotTagConstant(t *testing.T) { } } -func TestKeepSnapshotOnDeleteRoundTrip(t *testing.T) { +func TestMarkKeepSnapshotOnDelete(t *testing.T) { pod := &corev1.Pod{} if ReadKeepSnapshotOnDelete(pod) { - t.Error("an unflagged pod must not keep its snapshot: a plain teardown has to GC it") + t.Fatal("fresh pod should not be flagged as a seat release") } MarkKeepSnapshotOnDelete(pod) - if pod.Annotations[AnnotationKeepSnapshotOnDelete] != annotationTrue { - t.Errorf("MarkKeepSnapshotOnDelete should set %s=%s, got %q", AnnotationKeepSnapshotOnDelete, annotationTrue, pod.Annotations[AnnotationKeepSnapshotOnDelete]) - } if !ReadKeepSnapshotOnDelete(pod) { - t.Error("Read must see what Mark wrote") - } -} - -func TestReadKeepSnapshotOnDeleteRejectsNonTrue(t *testing.T) { - pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ - AnnotationKeepSnapshotOnDelete: "1", - }}} - if ReadKeepSnapshotOnDelete(pod) { - t.Error("only the literal \"true\" may keep a snapshot alive past its pod") + t.Error("MarkKeepSnapshotOnDelete should round-trip through ReadKeepSnapshotOnDelete") } } diff --git a/meta/keys.go b/meta/keys.go index 3973765..f8046d9 100644 --- a/meta/keys.go +++ b/meta/keys.go @@ -55,10 +55,9 @@ const ( // restore its VM from the :hibernate snapshot (cross-node migration) instead // of cloning from the base image. Written by the operator on the rebuilt pod. AnnotationRestoreFromHibernate = "vm.cocoonstack.io/restore-from-hibernate" - // AnnotationKeepSnapshotOnDelete marks a pod deletion as a scheduling-seat - // release rather than a teardown: the VM state stays claimable from the - // :hibernate tag, so the node-local snapshot must survive as the same-node - // warm-wake cache. Written by the operator just before a release-policy delete. + // AnnotationKeepSnapshotOnDelete marks a pod deletion as a seat release, so + // vk-cocoon keeps the node-local snapshot for a same-node warm wake. Written + // by the operator just before a release-policy delete. AnnotationKeepSnapshotOnDelete = "vm.cocoonstack.io/keep-snapshot-on-delete" // AnnotationForkFrom names a VM to fork the new VM from. AnnotationForkFrom = "vm.cocoonstack.io/fork-from" From 698dd8199860f32fd93f654b54a407440d28abfb Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 31 Jul 2026 12:15:45 +0800 Subject: [PATCH 3/3] review: compress annotation comment --- meta/keys.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/meta/keys.go b/meta/keys.go index f8046d9..aa18939 100644 --- a/meta/keys.go +++ b/meta/keys.go @@ -55,9 +55,7 @@ const ( // restore its VM from the :hibernate snapshot (cross-node migration) instead // of cloning from the base image. Written by the operator on the rebuilt pod. AnnotationRestoreFromHibernate = "vm.cocoonstack.io/restore-from-hibernate" - // AnnotationKeepSnapshotOnDelete marks a pod deletion as a seat release, so - // vk-cocoon keeps the node-local snapshot for a same-node warm wake. Written - // by the operator just before a release-policy delete. + // AnnotationKeepSnapshotOnDelete requests node-local snapshot retention for a seat-release pod deletion. AnnotationKeepSnapshotOnDelete = "vm.cocoonstack.io/keep-snapshot-on-delete" // AnnotationForkFrom names a VM to fork the new VM from. AnnotationForkFrom = "vm.cocoonstack.io/fork-from"