diff --git a/cocoonset/slotrelease.go b/cocoonset/slotrelease.go index 3a792dd..19b8770 100644 --- a/cocoonset/slotrelease.go +++ b/cocoonset/slotrelease.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" cocoonv1 "github.com/cocoonstack/cocoon-common/apis/v1" + commonk8s "github.com/cocoonstack/cocoon-common/k8s" "github.com/cocoonstack/cocoon-common/meta" "github.com/cocoonstack/cocoon-operator/metrics" "github.com/cocoonstack/cocoon-operator/snapshot" @@ -63,6 +64,10 @@ func (r *Reconciler) reconcileSuspendRelease(ctx context.Context, cs *cocoonv1.C return nil } logger.Infof(ctx, "slot release: deleting hibernated pod %s/%s (node=%s)", pod.Namespace, pod.Name, pod.Spec.NodeName) + // Best-effort: a lost flag costs the wake a registry pull; failing here would forfeit the seat. + if err := commonk8s.PatchKeepSnapshotOnDelete(ctx, r.Client, pod); err != nil { + logger.Errorf(ctx, err, "slot release: flag keep-snapshot on %s/%s; wake will cold-pull", pod.Namespace, pod.Name) + } if err := r.Delete(ctx, pod); err != nil && !apierrors.IsNotFound(err) { return fmt.Errorf("slot release: delete pod %s/%s: %w", pod.Namespace, pod.Name, err) } diff --git a/cocoonset/slotrelease_test.go b/cocoonset/slotrelease_test.go index e9b1549..c9d48db 100644 --- a/cocoonset/slotrelease_test.go +++ b/cocoonset/slotrelease_test.go @@ -1,6 +1,7 @@ package cocoonset import ( + "context" "errors" "slices" "strings" @@ -11,6 +12,7 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" cocoonv1 "github.com/cocoonstack/cocoon-common/apis/v1" "github.com/cocoonstack/cocoon-common/meta" @@ -108,6 +110,55 @@ func TestSuspendReleaseKeepsTerminalPod(t *testing.T) { } } +func TestSuspendReleaseFlagsKeepSnapshotBeforeDelete(t *testing.T) { + cs := relCocoonSet() + pod := relHibernatedPod(t, cs, "node-a") + reg := &fakeRegistry{present: map[string]bool{relHibernateTagKey: true}} + flaggedAtDelete := false + cli := relInterceptedClient(t, interceptor.Funcs{ + Delete: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.DeleteOption) error { + // Re-read from the API: the flag must be durable, not merely set in memory. + var live corev1.Pod + if err := c.Get(ctx, client.ObjectKeyFromObject(obj), &live); err == nil { + flaggedAtDelete = meta.ReadKeepSnapshotOnDelete(&live) + } + return c.Delete(ctx, obj, opts...) + }, + }, cs, pod) + r := &Reconciler{Client: cli, Scheme: testScheme(t), Registry: reg} + + if _, err := r.reconcileSuspendRelease(t.Context(), cs, singlePod(pod)); err != nil { + t.Fatalf("reconcileSuspendRelease: %v", err) + } + if !flaggedAtDelete { + t.Error("pod must carry keep-snapshot-on-delete in the API before it is deleted") + } +} + +func TestSuspendReleaseFreesSeatWhenFlagPatchFails(t *testing.T) { + cs := relCocoonSet() + pod := relHibernatedPod(t, cs, "node-a") + reg := &fakeRegistry{present: map[string]bool{relHibernateTagKey: true}} + cli := relInterceptedClient(t, interceptor.Funcs{ + Patch: func(ctx context.Context, c client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + // Only the keep-snapshot patch fails; the suspend patch must still land. + if p, ok := obj.(*corev1.Pod); ok && meta.ReadKeepSnapshotOnDelete(p) { + return errors.New("webhook rejected the pod patch") + } + return c.Patch(ctx, obj, patch, opts...) + }, + }, cs, pod) + r := &Reconciler{Client: cli, Scheme: testScheme(t), Registry: reg} + + if _, err := r.reconcileSuspendRelease(t.Context(), cs, singlePod(pod)); err != nil { + t.Fatalf("a failed flag patch must not fail the reconcile: %v", err) + } + var got corev1.Pod + if err := cli.Get(t.Context(), types.NamespacedName{Namespace: "ns", Name: "demo-0"}, &got); !apierrors.IsNotFound(err) { + t.Errorf("seat must be released even when the flag patch fails, err=%v", err) + } +} + func TestWakeRecreatesWithRestoreAndPreferredAffinity(t *testing.T) { cs := relCocoonSet(func(cs *cocoonv1.CocoonSet) { cs.Spec.Suspend = false @@ -488,9 +539,15 @@ func singlePod(pod *corev1.Pod) classifiedPods { } func relClient(t *testing.T, objs ...client.Object) client.WithWatch { + t.Helper() + return relInterceptedClient(t, interceptor.Funcs{}, objs...) +} + +func relInterceptedClient(t *testing.T, funcs interceptor.Funcs, objs ...client.Object) client.WithWatch { t.Helper() return ctrlfake.NewClientBuilder().WithScheme(testScheme(t)). - WithObjects(objs...).WithStatusSubresource(&cocoonv1.CocoonSet{}).Build() + WithObjects(objs...).WithStatusSubresource(&cocoonv1.CocoonSet{}). + WithInterceptorFuncs(funcs).Build() } func mustGetCS(t *testing.T, cli client.Client) cocoonv1.CocoonSet { diff --git a/docs/cocoonset.md b/docs/cocoonset.md index e18521e..2f331e0 100644 --- a/docs/cocoonset.md +++ b/docs/cocoonset.md @@ -6,7 +6,7 @@ 4. List owned pods by `cocoonset.cocoonstack.io/name=`, drop any with stale labels that aren't actually controller-owned, and classify the rest by role label. 5. **Lifecycle-bridge stamp**: patch `cs.Generation` onto each owned pod's `cocoonset.cocoonstack.io/generation` annotation so vk-cocoon can echo it back as `lifecycle-observed-generation`, giving clients a counter-based completion signal immune to wallclock skew. 6. **Failed-state short-circuit**: if the main pod is terminal (`Pod.Phase=Failed`, or it carries `vm.cocoonstack.io/lifecycle-state=Failed` from vk-cocoon while still Running), patch `Phase=Failed` and emit `MainAgentFailed` / `PodLifecycleFailed`. The Failed phase is recoverable: when the main pod becomes `Ready` again the operator emits `RecoveredFromFailure` and resumes normal reconciliation. -7. **Suspend short-circuit**: if `spec.suspend == true`, write `meta.HibernateState(true)` onto every owned pod and poll for completion on every managed VM: vk-cocoon must report `lifecycle-state=hibernated` with `lifecycle-observed-generation >= cs.Generation` AND the `:hibernate` manifest must be in the registry. vk writes state and observed-generation atomically, so the gate rejects a stale tag from a prior suspend cycle (unsuspend never deletes tags) and a lagging informer snapshot of a prior round alike. Terminal pods are skipped: they have no live VM to snapshot, and the normal flow triages them after unsuspend. Stay in `Phase=Suspending` (requeueing every 5 s) until then, and transition to `Phase=Suspended`. +7. **Suspend short-circuit**: if `spec.suspend == true`, write `meta.HibernateState(true)` onto every owned pod and poll for completion on every managed VM: vk-cocoon must report `lifecycle-state=hibernated` with `lifecycle-observed-generation >= cs.Generation` AND the `:hibernate` manifest must be in the registry. vk writes state and observed-generation atomically, so the gate rejects a stale tag from a prior suspend cycle (unsuspend never deletes tags) and a lagging informer snapshot of a prior round alike. Terminal pods are skipped: they have no live VM to snapshot, and the normal flow triages them after unsuspend. Stay in `Phase=Suspending` (requeueing every 5 s) until then, and transition to `Phase=Suspended`. With `spec.hibernatePolicy: release` the same gate is followed by a **slot release**: stash the VM names and the main's node onto `cocoonset.cocoonstack.io/hibernated-on-node`, flag each pod with `vm.cocoonstack.io/keep-snapshot-on-delete` so vk-cocoon keeps the node-local snapshot as the warm-wake cache, then delete the pods so their scheduling seats free. The flag is best-effort — a failed patch costs the wake a registry pull, whereas blocking the delete would forfeit the seat the policy exists to free. Wake recreates the main with a weight-100 preferred affinity on the stashed node, so it lands back on its snapshot whenever that node still has room and cold-pulls elsewhere when it does not. 8. **Cross-node migration**: when `spec.nodeName` pins the main agent (slot 0) to a node it is not currently on, `reconcileMigration` moves it — quiesce the old pod, snapshot it, delete it, recreate on the target node with restore-from-hibernate, and drop the snapshot only once the new VM runs. `Phase=Migrating` persists throughout. It runs before un-suspend so the migration's own hibernate annotation is not cleared, and short-circuits when the target pod is already owned by a `desire=Hibernate` CocoonHibernation CR to avoid racing that reconciler. Live state is never lost: the old pod dies only after the snapshot exists and this controller quiesced it; the snapshot drops only after the new VM runs. 9. **Un-suspend**: if `spec.suspend == false` and any owned pod still carries the hibernate annotation from a prior suspend, clear it via `PatchHibernateState(false)` so vk-cocoon wakes the VMs. Pods that are the active target of a `desire=Hibernate` CocoonHibernation CR are skipped to avoid racing the hibernation reconciler. `PatchHibernateState(false)` is a no-op on pods whose annotation is already absent, so this is cheap in the common "never suspended" case. 10. Ensure the **main agent** (slot 0). If the existing pod has drifted from spec, delete it for recreate. If it is not yet `Ready`, requeue in 5 s and report `Phase=Pending`. diff --git a/go.mod b/go.mod index d9092ac..0ba4447 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/cocoonstack/cocoon-operator go 1.26.5 require ( - github.com/cocoonstack/cocoon-common v0.2.8 + github.com/cocoonstack/cocoon-common v0.2.9-0.20260731042413-9ca4f1c8fc0f github.com/go-logr/logr v1.4.3 github.com/google/go-containerregistry v0.21.7 github.com/projecteru2/core v0.0.0-20241016125006-ff909eefe04c diff --git a/go.sum b/go.sum index db78532..7d5893e 100644 --- a/go.sum +++ b/go.sum @@ -29,8 +29,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.8 h1:xpfNVTBmP/VGD/XB4oDzCI7bO1Ygb8Yd9Gnq8U/VJh8= -github.com/cocoonstack/cocoon-common v0.2.8/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=