From 91a5722947ab94b75495cb1610db98247bfb465e Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 3 Sep 2026 01:49:36 +0800 Subject: [PATCH 1/6] fix: budget drift rebuilds for every owned pod; dead-letter parks at a generation The drift-delete path had no rebuild budget on main, sub-agent or toolbox pods. Any defaulter the spec does not pin (a namespace LimitRange with a cpu or memory default, a resource-rewriting webhook) makes podSpecMatchesAgent permanently false, so the pod was deleted and recreated every requeue, one microVM built and destroyed per cycle, with no metric moving. Every terminal or drifted pod now goes through one budget keyed by pod name: four attempts with 0/1/5/30 s backoff, then dead-letter. A dead-lettered pod records the CocoonSet generation and stays parked until a spec edit, which resets its budget; drift alone no longer lifts it, because a permanently rewritten pod would otherwise loop at the slower cadence. The history annotation keeps only pods the spec still names. --- cocoonset/agents.go | 118 +--------------------------- cocoonset/pods.go | 6 +- cocoonset/rebuild.go | 145 ++++++++++++++++++++++++++++++++--- cocoonset/rebuild_test.go | 44 ++++++----- cocoonset/reconciler.go | 36 +++++---- cocoonset/reconciler_test.go | 25 +++--- cocoonset/restore_test.go | 2 +- cocoonset/toolboxes.go | 49 +++++------- docs/observability.md | 2 +- metrics/metrics.go | 4 +- 10 files changed, 227 insertions(+), 204 deletions(-) diff --git a/cocoonset/agents.go b/cocoonset/agents.go index ecde94e..742990d 100644 --- a/cocoonset/agents.go +++ b/cocoonset/agents.go @@ -12,13 +12,8 @@ import ( "golang.org/x/sync/errgroup" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" - "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" ) // subAgentCreateConcurrency caps parallel creates so a scale-up does not burst the apiserver. @@ -37,7 +32,7 @@ func (r *Reconciler) ensureSubAgents(ctx context.Context, cs *cocoonv1.CocoonSet missing = append(missing, slot) continue } - deleted, wait, err := r.triageSubAgent(ctx, logger, pod, cs, slot) + deleted, wait, err := r.triagePod(ctx, logger, cs, pod, podSpecMatchesAgent(pod, cs, slot)) if err != nil { return changed, requeueAfter, err } @@ -103,114 +98,3 @@ func (r *Reconciler) createSubAgents(ctx context.Context, logger *log.Fields, cs waitErr := g.Wait() return created.Load(), waitErr } - -// triageSubAgent returns a non-zero requeueAfter while the slot waits out rebuild backoff. -func (r *Reconciler) triageSubAgent(ctx context.Context, logger *log.Fields, pod *corev1.Pod, cs *cocoonv1.CocoonSet, slot int32) (bool, time.Duration, error) { - if pod.Annotations[annotationDeadLetter] == "true" { - return r.rebuildDeadLetteredOnDrift(ctx, logger, pod, cs, slot) - } - switch { - case podIsTerminal(pod): - return r.rebuildSubAgent(ctx, logger, pod, cs, slot) - case !podSpecMatchesAgent(pod, cs, slot): - logger.Infof(ctx, "sub-agent %s/%s slot %d spec drifted, deleting for recreate", pod.Namespace, pod.Name, slot) - if err := r.Delete(ctx, pod); err != nil && !apierrors.IsNotFound(err) { - return false, 0, fmt.Errorf("delete drifted sub-agent slot %d: %w", slot, err) - } - return true, 0, nil - default: - return false, 0, nil - } -} - -// rebuildDeadLetteredOnDrift leaves a dead-lettered pod alone until a spec edit, which earns a fresh rebuild budget. -func (r *Reconciler) rebuildDeadLetteredOnDrift(ctx context.Context, logger *log.Fields, pod *corev1.Pod, cs *cocoonv1.CocoonSet, slot int32) (bool, time.Duration, error) { - if podSpecMatchesAgent(pod, cs, slot) { - return false, 0, nil - } - history := readRebuildHistory(cs) - if _, ok := history[slot]; ok { - delete(history, slot) - if err := r.patchRebuildHistory(ctx, cs, history); err != nil { - return false, 0, fmt.Errorf("reset rebuild history for slot %d: %w", slot, err) - } - } - logger.Infof(ctx, "dead-lettered sub-agent %s/%s slot %d spec drifted, rebuilding with a fresh budget", pod.Namespace, pod.Name, slot) - if err := r.Delete(ctx, pod); err != nil && !apierrors.IsNotFound(err) { - return false, 0, fmt.Errorf("delete dead-lettered sub-agent slot %d: %w", slot, err) - } - return true, 0, nil -} - -// rebuildSubAgent persists history before the delete so a failed delete cannot bypass the gate. -func (r *Reconciler) rebuildSubAgent(ctx context.Context, logger *log.Fields, pod *corev1.Pod, cs *cocoonv1.CocoonSet, slot int32) (bool, time.Duration, error) { - history := readRebuildHistory(cs) - entry := history[slot] - if entry.Count >= maxRebuildAttempts { - if err := r.patchAnnotation(ctx, pod, annotationDeadLetter, "true"); err != nil { - return false, 0, err - } - metrics.SubAgentDeadLetterTotal.WithLabelValues(cs.Namespace, cs.Name).Inc() - commonk8s.Eventf(r.Recorder, cs, corev1.EventTypeWarning, "SubAgentDeadLetter", - "slot %d exhausted %d rebuilds; pod %s left in dead-letter", slot, maxRebuildAttempts, pod.Name) - return false, 0, nil - } - if wait := backoffDelay(entry.Count); wait > 0 { - remaining := wait - time.Since(entry.LastDeleted) - if remaining > 0 { - return false, remaining, nil - } - } - entry.Count++ - entry.LastDeleted = time.Now() - history[slot] = entry - if err := r.patchRebuildHistory(ctx, cs, history); err != nil { - return false, 0, fmt.Errorf("persist rebuild history: %w", err) - } - logger.Infof(ctx, "sub-agent %s/%s slot %d terminal (phase=%s lifecycle=%s), rebuild attempt %d/%d", - pod.Namespace, pod.Name, slot, pod.Status.Phase, meta.ReadLifecycleState(pod), entry.Count, maxRebuildAttempts) - if err := r.Delete(ctx, pod); err != nil && !apierrors.IsNotFound(err) { - return false, 0, fmt.Errorf("delete terminal sub-agent slot %d: %w", slot, err) - } - metrics.SubAgentRebuildTotal.WithLabelValues(cs.Namespace, cs.Name).Inc() - commonk8s.Eventf(r.Recorder, cs, corev1.EventTypeNormal, "SubAgentRebuilding", - "slot %d attempt %d/%d", slot, entry.Count, maxRebuildAttempts) - return true, 0, nil -} - -// patchAnnotation merge-patches one annotation on obj; an empty value deletes the key. -func (r *Reconciler) patchAnnotation(ctx context.Context, obj client.Object, key, value string) error { - var v any = value - if value == "" { - v = nil - } - patch, err := commonk8s.AnnotationsMergePatch(map[string]any{key: v}) - if err != nil { - return fmt.Errorf("build patch for %T %s/%s annotation %s: %w", obj, obj.GetNamespace(), obj.GetName(), key, err) - } - if err := r.Patch(ctx, obj, client.RawPatch(types.MergePatchType, patch)); err != nil { - return fmt.Errorf("patch %T %s/%s annotation %s: %w", obj, obj.GetNamespace(), obj.GetName(), key, err) - } - return nil -} - -// patchRebuildHistory mirrors the annotation onto cs so later slots in this reconcile see fresh history. -func (r *Reconciler) patchRebuildHistory(ctx context.Context, cs *cocoonv1.CocoonSet, history map[int32]rebuildEntry) error { - enc, err := encodeRebuildHistory(cs.Spec.Agent.Replicas, history) - if err != nil { - return fmt.Errorf("encode rebuild history: %w", err) - } - csCopy := cs.DeepCopy() - if csCopy.Annotations == nil { - csCopy.Annotations = map[string]string{} - } - csCopy.Annotations[annotationRebuildHistory] = enc - if err := r.Patch(ctx, csCopy, client.MergeFrom(cs)); err != nil { - return fmt.Errorf("patch rebuild history: %w", err) - } - if cs.Annotations == nil { - cs.Annotations = map[string]string{} - } - cs.Annotations[annotationRebuildHistory] = enc - return nil -} diff --git a/cocoonset/pods.go b/cocoonset/pods.go index 182463f..6a212c6 100644 --- a/cocoonset/pods.go +++ b/cocoonset/pods.go @@ -81,7 +81,7 @@ func buildAgentPod(cs *cocoonv1.CocoonSet, slot int32, mainVMName, bindNodeName } vmName := meta.VMNameForDeployment(cs.Namespace, cs.Name, int(slot)) - podName := fmt.Sprintf("%s-%d", cs.Name, slot) + podName := agentPodName(cs.Name, slot) pod, err := newManagedPod(cs, podName, role, strconv.FormatInt(int64(slot), 10), scheme) if err != nil { @@ -157,6 +157,10 @@ func buildToolboxPod(cs *cocoonv1.CocoonSet, tb cocoonv1.ToolboxSpec, scheme *ru } // toolboxPodName is shared by the builder and the collision check so the two cannot diverge. +func agentPodName(csName string, slot int32) string { + return fmt.Sprintf("%s-%d", csName, slot) +} + func toolboxPodName(csName, tbName string) string { return fmt.Sprintf("%s-%s", csName, tbName) } diff --git a/cocoonset/rebuild.go b/cocoonset/rebuild.go index 18f5d93..55acac6 100644 --- a/cocoonset/rebuild.go +++ b/cocoonset/rebuild.go @@ -1,11 +1,23 @@ package cocoonset import ( + "context" "encoding/json" + "fmt" "maps" + "strconv" "time" + "github.com/projecteru2/core/log" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "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" ) const ( @@ -15,34 +27,149 @@ const ( maxRebuildAttempts = 4 ) -// rebuildEntry persists in a CocoonSet annotation so the count survives the pod delete. +// rebuildEntry persists in a CocoonSet annotation, keyed by pod name, so the count survives the pod delete. type rebuildEntry struct { Count int `json:"count"` LastDeleted time.Time `json:"lastDeleted"` } -func readRebuildHistory(cs *cocoonv1.CocoonSet) map[int32]rebuildEntry { - m := map[int32]rebuildEntry{} +// triagePod deletes a terminal or drifted pod for recreate within its rebuild budget; a dead-lettered pod waits for a spec edit. +func (r *Reconciler) triagePod(ctx context.Context, logger *log.Fields, cs *cocoonv1.CocoonSet, pod *corev1.Pod, matches bool) (bool, time.Duration, error) { + var reason string + switch { + case podIsTerminal(pod): + reason = fmt.Sprintf("terminal (phase=%s lifecycle=%s)", pod.Status.Phase, meta.ReadLifecycleState(pod)) + case !matches: + reason = "spec drifted" + default: + return false, 0, nil + } + parkedAt, parked := pod.Annotations[annotationDeadLetter] + if !parked { + return r.rebuildPod(ctx, logger, cs, pod, reason) + } + if parkedAt == strconv.FormatInt(cs.Generation, 10) { + return false, 0, nil + } + history := readRebuildHistory(cs) + if _, ok := history[pod.Name]; ok { + delete(history, pod.Name) + if err := r.patchRebuildHistory(ctx, cs, history); err != nil { + return false, 0, fmt.Errorf("reset rebuild history for %s: %w", pod.Name, err) + } + } + logger.Infof(ctx, "dead-lettered pod %s/%s %s after a spec edit, rebuilding with a fresh budget", pod.Namespace, pod.Name, reason) + if err := r.Delete(ctx, pod); err != nil && !apierrors.IsNotFound(err) { + return false, 0, fmt.Errorf("delete dead-lettered %s: %w", pod.Name, err) + } + return true, 0, nil +} + +// rebuildPod persists history before the delete so a failed delete cannot bypass the gate; past the budget it dead-letters the pod at the current generation. +func (r *Reconciler) rebuildPod(ctx context.Context, logger *log.Fields, cs *cocoonv1.CocoonSet, pod *corev1.Pod, reason string) (bool, time.Duration, error) { + history := readRebuildHistory(cs) + entry := history[pod.Name] + if entry.Count >= maxRebuildAttempts { + if err := r.patchAnnotation(ctx, pod, annotationDeadLetter, strconv.FormatInt(cs.Generation, 10)); err != nil { + return false, 0, err + } + metrics.SubAgentDeadLetterTotal.WithLabelValues(cs.Namespace, cs.Name).Inc() + commonk8s.Eventf(r.Recorder, cs, corev1.EventTypeWarning, "SubAgentDeadLetter", + "pod %s exhausted %d rebuilds (%s); left in dead-letter", pod.Name, maxRebuildAttempts, reason) + return false, 0, nil + } + if wait := backoffDelay(entry.Count); wait > 0 { + if remaining := wait - time.Since(entry.LastDeleted); remaining > 0 { + return false, remaining, nil + } + } + entry.Count++ + entry.LastDeleted = time.Now() + history[pod.Name] = entry + if err := r.patchRebuildHistory(ctx, cs, history); err != nil { + return false, 0, fmt.Errorf("persist rebuild history: %w", err) + } + logger.Infof(ctx, "pod %s/%s %s, rebuild attempt %d/%d", pod.Namespace, pod.Name, reason, entry.Count, maxRebuildAttempts) + if err := r.Delete(ctx, pod); err != nil && !apierrors.IsNotFound(err) { + return false, 0, fmt.Errorf("delete %s for rebuild: %w", pod.Name, err) + } + metrics.SubAgentRebuildTotal.WithLabelValues(cs.Namespace, cs.Name).Inc() + commonk8s.Eventf(r.Recorder, cs, corev1.EventTypeNormal, "SubAgentRebuilding", + "pod %s attempt %d/%d (%s)", pod.Name, entry.Count, maxRebuildAttempts, reason) + return true, 0, nil +} + +// patchRebuildHistory mirrors the annotation onto cs so later pods in this reconcile see fresh history. +func (r *Reconciler) patchRebuildHistory(ctx context.Context, cs *cocoonv1.CocoonSet, history map[string]rebuildEntry) error { + enc, err := encodeRebuildHistory(cs, history) + if err != nil { + return fmt.Errorf("encode rebuild history: %w", err) + } + csCopy := cs.DeepCopy() + if csCopy.Annotations == nil { + csCopy.Annotations = map[string]string{} + } + csCopy.Annotations[annotationRebuildHistory] = enc + if err := r.Patch(ctx, csCopy, client.MergeFrom(cs)); err != nil { + return fmt.Errorf("patch rebuild history: %w", err) + } + if cs.Annotations == nil { + cs.Annotations = map[string]string{} + } + cs.Annotations[annotationRebuildHistory] = enc + return nil +} + +// patchAnnotation merge-patches one annotation on obj; an empty value deletes the key. +func (r *Reconciler) patchAnnotation(ctx context.Context, obj client.Object, key, value string) error { + var v any = value + if value == "" { + v = nil + } + patch, err := commonk8s.AnnotationsMergePatch(map[string]any{key: v}) + if err != nil { + return fmt.Errorf("build patch for %T %s/%s annotation %s: %w", obj, obj.GetNamespace(), obj.GetName(), key, err) + } + if err := r.Patch(ctx, obj, client.RawPatch(types.MergePatchType, patch)); err != nil { + return fmt.Errorf("patch %T %s/%s annotation %s: %w", obj, obj.GetNamespace(), obj.GetName(), key, err) + } + return nil +} + +func readRebuildHistory(cs *cocoonv1.CocoonSet) map[string]rebuildEntry { + m := map[string]rebuildEntry{} if raw := cs.Annotations[annotationRebuildHistory]; raw != "" { // json "null" leaves m nil; callers write to it if err := json.Unmarshal([]byte(raw), &m); err != nil || m == nil { - return map[int32]rebuildEntry{} + return map[string]rebuildEntry{} } } return m } -func encodeRebuildHistory(replicas int32, m map[int32]rebuildEntry) (string, error) { - maps.DeleteFunc(m, func(slot int32, _ rebuildEntry) bool { - return slot > replicas - }) - raw, err := json.Marshal(m) +// encodeRebuildHistory keeps only the pods the spec still names. +func encodeRebuildHistory(cs *cocoonv1.CocoonSet, m map[string]rebuildEntry) (string, error) { + keep := desiredPodNames(cs) + kept := maps.Clone(m) + maps.DeleteFunc(kept, func(name string, _ rebuildEntry) bool { return !keep[name] }) + raw, err := json.Marshal(kept) if err != nil { return "", err } return string(raw), nil } +func desiredPodNames(cs *cocoonv1.CocoonSet) map[string]bool { + names := map[string]bool{} + for slot := range cs.Spec.Agent.Replicas + 1 { + names[agentPodName(cs.Name, slot)] = true + } + for _, tb := range cs.Spec.Toolboxes { + names[toolboxPodName(cs.Name, tb.Name)] = true + } + return names +} + // backoffDelay returns the wait before the next rebuild attempt: 0, 1s, 5s, 30s. func backoffDelay(priorCount int) time.Duration { switch priorCount { diff --git a/cocoonset/rebuild_test.go b/cocoonset/rebuild_test.go index c36bcc8..7d758fb 100644 --- a/cocoonset/rebuild_test.go +++ b/cocoonset/rebuild_test.go @@ -10,40 +10,48 @@ import ( func TestRebuildHistoryRoundTrip(t *testing.T) { cs := &cocoonv1.CocoonSet{} + cs.Name = "demo" cs.Spec.Agent.Replicas = 3 - in := map[int32]rebuildEntry{ - 1: {Count: 2, LastDeleted: time.Date(2026, 5, 14, 1, 0, 0, 0, time.UTC)}, - 2: {Count: 1, LastDeleted: time.Date(2026, 5, 14, 1, 0, 30, 0, time.UTC)}, + in := map[string]rebuildEntry{ + "demo-1": {Count: 2, LastDeleted: time.Date(2026, 5, 14, 1, 0, 0, 0, time.UTC)}, + "demo-2": {Count: 1, LastDeleted: time.Date(2026, 5, 14, 1, 0, 30, 0, time.UTC)}, } - enc, err := encodeRebuildHistory(cs.Spec.Agent.Replicas, in) + enc, err := encodeRebuildHistory(cs, in) if err != nil { t.Fatalf("encodeRebuildHistory: %v", err) } cs.Annotations = map[string]string{annotationRebuildHistory: enc} got := readRebuildHistory(cs) - if got[1].Count != 2 || got[2].Count != 1 { + if got["demo-1"].Count != 2 || got["demo-2"].Count != 1 { t.Fatalf("round-trip lost counts: %+v", got) } } -func TestRebuildHistoryGarbageCollectsStaleSlots(t *testing.T) { - in := map[int32]rebuildEntry{ - 1: {Count: 1}, - 2: {Count: 2}, - 7: {Count: 3}, +func TestRebuildHistoryGarbageCollectsStalePods(t *testing.T) { + cs := &cocoonv1.CocoonSet{} + cs.Name = "demo" + cs.Spec.Agent.Replicas = 2 + cs.Spec.Toolboxes = []cocoonv1.ToolboxSpec{{Name: "tb"}} + in := map[string]rebuildEntry{ + "demo-0": {Count: 1}, + "demo-2": {Count: 2}, + "demo-7": {Count: 3}, + "demo-tb": {Count: 1}, + "demo-gone": {Count: 1}, } - enc, err := encodeRebuildHistory(2, in) + enc, err := encodeRebuildHistory(cs, in) if err != nil { t.Fatalf("encodeRebuildHistory: %v", err) } - cs := &cocoonv1.CocoonSet{} cs.Annotations = map[string]string{annotationRebuildHistory: enc} got := readRebuildHistory(cs) - if _, ok := got[7]; ok { - t.Fatalf("expected slot 7 pruned, got %+v", got) + for _, name := range []string{"demo-7", "demo-gone"} { + if _, ok := got[name]; ok { + t.Fatalf("expected %s pruned, got %+v", name, got) + } } - if len(got) != 2 { - t.Fatalf("expected 2 surviving slots, got %d: %+v", len(got), got) + if len(got) != 3 || len(in) != 5 { + t.Fatalf("expected 3 surviving pods and an untouched input, got %d / %d: %+v", len(got), len(in), got) } } @@ -82,8 +90,8 @@ func TestReadRebuildHistoryHandlesNullPayload(t *testing.T) { if got == nil { t.Fatal("null payload must yield non-nil map so downstream writes don't panic") } - got[1] = rebuildEntry{Count: 1} - if _, err := encodeRebuildHistory(2, got); err != nil { + got["demo-1"] = rebuildEntry{Count: 1} + if _, err := encodeRebuildHistory(cs, got); err != nil { t.Fatalf("encodeRebuildHistory on normalized null payload: %v", err) } } diff --git a/cocoonset/reconciler.go b/cocoonset/reconciler.go index 2b9d775..ce64526 100644 --- a/cocoonset/reconciler.go +++ b/cocoonset/reconciler.go @@ -56,7 +56,6 @@ func (r *Reconciler) SetupWithManager(_ context.Context, mgr ctrl.Manager) error Complete(r) } -// Reconcile drives one CocoonSet toward its declared agent and toolbox pods. func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := log.WithFunc("cocoonset.Reconciler.Reconcile") @@ -122,12 +121,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, err } - if classified.main != nil && !podSpecMatchesAgent(classified.main, &cs, 0) { - logger.Infof(ctx, "main agent %s/%s spec drifted, deleting for recreate", classified.main.Namespace, classified.main.Name) - if err := r.Delete(ctx, classified.main); err != nil && !apierrors.IsNotFound(err) { - return ctrl.Result{}, fmt.Errorf("delete drifted main agent: %w", err) - } - return ctrl.Result{RequeueAfter: requeueAfterWrite}, nil + if handled, res, err := r.rebuildDriftedMain(ctx, logger, &cs, classified); handled { + return res, err } intent := r.newRestoreIntent(ctx, cs.Namespace) if classified.main == nil { @@ -147,7 +142,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if err != nil { return ctrl.Result{}, err } - tbChanged, err := r.ensureToolboxes(ctx, &cs, classified, intent) + tbChanged, tbRequeue, err := r.ensureToolboxes(ctx, &cs, classified, intent) if err != nil { return ctrl.Result{}, err } @@ -158,16 +153,31 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if err := r.patchStatus(ctx, &cs, buildStatus(&cs, classified, "")); err != nil { return ctrl.Result{}, err } + if subRequeue == 0 || (tbRequeue > 0 && tbRequeue < subRequeue) { + subRequeue = tbRequeue + } return ctrl.Result{RequeueAfter: subRequeue}, nil } +// rebuildDriftedMain deletes a main that no longer matches the spec, within its rebuild budget; handled reports a delete or a pending backoff. +func (r *Reconciler) rebuildDriftedMain(ctx context.Context, logger *log.Fields, cs *cocoonv1.CocoonSet, classified classifiedPods) (bool, ctrl.Result, error) { + if classified.main == nil || podSpecMatchesAgent(classified.main, cs, 0) { + return false, ctrl.Result{}, nil + } + deleted, wait, err := r.triagePod(ctx, logger, cs, classified.main, false) + if err != nil { + return true, ctrl.Result{}, err + } + if deleted || wait > 0 { + return true, ctrl.Result{RequeueAfter: cmp.Or(wait, requeueAfterWrite)}, nil + } + return false, ctrl.Result{}, nil +} + // handleFailedMainAgent recreates a drifted terminal main; parked in Failed it would wait for a Ready it can never reach. func (r *Reconciler) handleFailedMainAgent(ctx context.Context, cs *cocoonv1.CocoonSet, classified classifiedPods, reason string) (ctrl.Result, error) { - if !podSpecMatchesAgent(classified.main, cs, 0) { - if err := r.Delete(ctx, classified.main); err != nil && !apierrors.IsNotFound(err) { - return ctrl.Result{}, fmt.Errorf("delete terminal drifted main agent: %w", err) - } - return ctrl.Result{RequeueAfter: requeueAfterWrite}, nil + if handled, res, err := r.rebuildDriftedMain(ctx, log.WithFunc("cocoonset.Reconciler.handleFailedMainAgent"), cs, classified); handled { + return res, err } r.observeMainPodFailed(cs, classified.main, reason) return ctrl.Result{}, r.patchStatus(ctx, cs, buildStatus(cs, classified, cocoonv1.CocoonSetPhaseFailed)) diff --git a/cocoonset/reconciler_test.go b/cocoonset/reconciler_test.go index 2256757..75cda7f 100644 --- a/cocoonset/reconciler_test.go +++ b/cocoonset/reconciler_test.go @@ -121,7 +121,7 @@ func TestEnsureToolboxesCollisionReturnsError(t *testing.T) { allByName: map[string]*corev1.Pod{agentPod.Name: agentPod}, } - _, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) + _, _, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) if err == nil { t.Fatal("ensureToolboxes should return error on name collision with agent pod") } @@ -143,7 +143,7 @@ func TestEnsureToolboxesRejectsIntegerName(t *testing.T) { allByName: map[string]*corev1.Pod{}, } - _, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) + _, _, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) if err == nil { t.Fatal("ensureToolboxes must reject a toolbox name that collides with agent slot pod naming") } @@ -166,7 +166,7 @@ func TestEnsureToolboxesRejectsDuplicateNames(t *testing.T) { allByName: map[string]*corev1.Pod{}, } - _, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) + _, _, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) if err == nil { t.Fatal("ensureToolboxes must reject a spec with duplicate toolbox names") } @@ -192,7 +192,7 @@ func TestEnsureToolboxesIdempotentOnExistingToolbox(t *testing.T) { allByName: map[string]*corev1.Pod{}, } - changed, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) + changed, _, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) if err != nil { t.Fatalf("ensureToolboxes: %v", err) } @@ -384,15 +384,15 @@ func TestEnsureSubAgentsReplacesTerminalPod(t *testing.T) { } } -func TestEnsureSubAgentsDeadLetterYieldsToSpecDrift(t *testing.T) { +func TestEnsureSubAgentsDeadLetterStaysUntilSpecEdit(t *testing.T) { scheme := testScheme(t) cs := newCocoonSet("demo", func(cs *cocoonv1.CocoonSet) { cs.Spec.Agent.Replicas = 1 }) subPod := mustBuildAgentPod(t, cs, 1, "vk-ns-demo-0", "", scheme) - subPod.Annotations[annotationDeadLetter] = "true" + subPod.Annotations[annotationDeadLetter] = "0" - enc, err := encodeRebuildHistory(1, map[int32]rebuildEntry{1: {Count: maxRebuildAttempts}}) + enc, err := encodeRebuildHistory(cs, map[string]rebuildEntry{subPod.Name: {Count: maxRebuildAttempts}}) if err != nil { t.Fatalf("encodeRebuildHistory: %v", err) } @@ -415,12 +415,13 @@ func TestEnsureSubAgentsDeadLetterYieldsToSpecDrift(t *testing.T) { } cs.Spec.Agent.Image = "ghcr.io/cocoonstack/cocoon/ubuntu:26.04" + cs.Generation = 1 changed, _, err = r.ensureSubAgents(t.Context(), cs, classified, "vk-ns-demo-0", "", r.newRestoreIntent(t.Context(), cs.Namespace)) if err != nil { t.Fatalf("ensureSubAgents after spec fix: %v", err) } if !changed { - t.Fatal("spec drift must rebuild a dead-lettered pod") + t.Fatal("a spec edit must rebuild a dead-lettered pod") } if err := cli.Get(t.Context(), types.NamespacedName{Namespace: subPod.Namespace, Name: subPod.Name}, &corev1.Pod{}); err == nil { t.Error("dead-lettered drifted pod should have been deleted") @@ -429,7 +430,7 @@ func TestEnsureSubAgentsDeadLetterYieldsToSpecDrift(t *testing.T) { if err := cli.Get(t.Context(), types.NamespacedName{Namespace: cs.Namespace, Name: cs.Name}, &out); err != nil { t.Fatalf("get CocoonSet: %v", err) } - if _, ok := readRebuildHistory(&out)[1]; ok { + if _, ok := readRebuildHistory(&out)[subPod.Name]; ok { t.Error("rebuild history for the slot must be reset so the new spec gets a fresh budget") } } @@ -576,7 +577,7 @@ func TestEnsureToolboxesReplacesTerminalPod(t *testing.T) { cli := ctrlfake.NewClientBuilder(). WithScheme(scheme). - WithObjects(tbPod). + WithObjects(cs, tbPod). Build() r := &Reconciler{Client: cli, Scheme: scheme} classified := classifiedPods{ @@ -585,7 +586,7 @@ func TestEnsureToolboxesReplacesTerminalPod(t *testing.T) { allByName: map[string]*corev1.Pod{tbPod.Name: tbPod}, } - changed, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) + changed, _, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) if err != nil { t.Fatalf("ensureToolboxes: %v", err) } @@ -950,7 +951,7 @@ func TestEnsureToolboxesStashesRemovedToolboxVMName(t *testing.T) { pod := mustBuildToolboxPod(t, cs, tb, scheme) cli := ctrlfake.NewClientBuilder().WithScheme(scheme).WithObjects(cs, pod).Build() r := &Reconciler{Client: cli, Scheme: scheme} - if _, err := r.ensureToolboxes(t.Context(), cs, classifyPods([]corev1.Pod{*pod}), r.newRestoreIntent(t.Context(), cs.Namespace)); err != nil { + if _, _, err := r.ensureToolboxes(t.Context(), cs, classifyPods([]corev1.Pod{*pod}), r.newRestoreIntent(t.Context(), cs.Namespace)); err != nil { t.Fatalf("ensureToolboxes: %v", err) } if names := stashedVMNames(t, cli); !slices.Contains(names, "vk-ns-demo-tb") { diff --git a/cocoonset/restore_test.go b/cocoonset/restore_test.go index 0553434..5f75f95 100644 --- a/cocoonset/restore_test.go +++ b/cocoonset/restore_test.go @@ -118,7 +118,7 @@ func TestEnsureToolboxesRestoresHibernated(t *testing.T) { Registry: &fakeRegistry{present: map[string]bool{tbVMName + ":hibernate": true}}, } - changed, err := r.ensureToolboxes(t.Context(), cs, classifyPods(nil), r.newRestoreIntent(t.Context(), cs.Namespace)) + changed, _, err := r.ensureToolboxes(t.Context(), cs, classifyPods(nil), r.newRestoreIntent(t.Context(), cs.Namespace)) if err != nil { t.Fatalf("ensureToolboxes: %v", err) } diff --git a/cocoonset/toolboxes.go b/cocoonset/toolboxes.go index b856ed5..383a34f 100644 --- a/cocoonset/toolboxes.go +++ b/cocoonset/toolboxes.go @@ -6,6 +6,7 @@ import ( "maps" "slices" "strconv" + "time" "github.com/projecteru2/core/log" corev1 "k8s.io/api/core/v1" @@ -17,46 +18,51 @@ import ( "github.com/cocoonstack/cocoon-common/meta" ) -func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet, classified classifiedPods, intent restoreIntent) (bool, error) { +// ensureToolboxes reports changed on any mutation and the shortest rebuild backoff still pending. +func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet, classified classifiedPods, intent restoreIntent) (bool, time.Duration, error) { logger := log.WithFunc("cocoonset.Reconciler.ensureToolboxes") // Webhook already rejects duplicates; validating before any Create/Delete keeps a bypass from leaving partial state. desired := make(map[string]bool, len(cs.Spec.Toolboxes)) for _, tb := range cs.Spec.Toolboxes { if desired[tb.Name] { - return false, fmt.Errorf("duplicate toolbox name %q in spec", tb.Name) + return false, 0, fmt.Errorf("duplicate toolbox name %q in spec", tb.Name) } if _, convErr := strconv.Atoi(tb.Name); convErr == nil { - return false, fmt.Errorf("toolbox name %q must not be an integer: collides with agent slot pod naming", tb.Name) + return false, 0, fmt.Errorf("toolbox name %q must not be an integer: collides with agent slot pod naming", tb.Name) } desired[tb.Name] = true } changed := false + var requeueAfter time.Duration for _, tb := range cs.Spec.Toolboxes { podName := toolboxPodName(cs.Name, tb.Name) if classified.allByName[podName] != nil && classified.toolbox[tb.Name] == nil { - return changed, fmt.Errorf("create toolbox %s: name collision with existing pod %s", tb.Name, podName) + return changed, requeueAfter, fmt.Errorf("create toolbox %s: name collision with existing pod %s", tb.Name, podName) } if pod, exists := classified.toolbox[tb.Name]; exists { - deleted, err := r.triageToolbox(ctx, logger, pod, cs, tb) + deleted, wait, err := r.triagePod(ctx, logger, cs, pod, podSpecMatchesToolbox(pod, cs, tb)) if err != nil { - return changed, err + return changed, requeueAfter, err } changed = changed || deleted + if wait > 0 && (requeueAfter == 0 || wait < requeueAfter) { + requeueAfter = wait + } continue } tbPod, err := buildToolboxPod(cs, tb, r.Scheme) if err != nil { - return changed, fmt.Errorf("build toolbox %s: %w", tb.Name, err) + return changed, requeueAfter, fmt.Errorf("build toolbox %s: %w", tb.Name, err) } if err := r.markRestoreFromIntent(ctx, tbPod, intent); err != nil { - return changed, fmt.Errorf("mark restore toolbox %s: %w", tb.Name, err) + return changed, requeueAfter, fmt.Errorf("mark restore toolbox %s: %w", tb.Name, err) } if err := r.Create(ctx, tbPod); err != nil { if !apierrors.IsAlreadyExists(err) { - return changed, fmt.Errorf("create toolbox %s: %w", tb.Name, err) + return changed, requeueAfter, fmt.Errorf("create toolbox %s: %w", tb.Name, err) } if collisionErr := r.checkToolboxCollision(ctx, cs, tbPod, tb.Name); collisionErr != nil { - return changed, collisionErr + return changed, requeueAfter, collisionErr } continue } @@ -69,35 +75,18 @@ func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet } pod := classified.toolbox[name] if err := r.stashDeleteVMNames(ctx, cs, []corev1.Pod{*pod}); err != nil { - return changed, fmt.Errorf("stash vm name of toolbox %s: %w", name, err) + return changed, requeueAfter, fmt.Errorf("stash vm name of toolbox %s: %w", name, err) } if err := r.Delete(ctx, pod); err != nil { if apierrors.IsNotFound(err) { continue } - return changed, fmt.Errorf("delete extra toolbox %s: %w", name, err) + return changed, requeueAfter, fmt.Errorf("delete extra toolbox %s: %w", name, err) } logger.Infof(ctx, "deleted extra toolbox %s/%s", pod.Namespace, pod.Name) changed = true } - return changed, nil -} - -func (r *Reconciler) triageToolbox(ctx context.Context, logger *log.Fields, pod *corev1.Pod, cs *cocoonv1.CocoonSet, tb cocoonv1.ToolboxSpec) (bool, error) { - var reason string - switch { - case podIsTerminal(pod): - reason = fmt.Sprintf("terminal (phase=%s lifecycle=%s)", pod.Status.Phase, meta.ReadLifecycleState(pod)) - case !podSpecMatchesToolbox(pod, cs, tb): - reason = "spec drifted" - default: - return false, nil - } - logger.Infof(ctx, "toolbox %s/%s %q %s, deleting for recreate", pod.Namespace, pod.Name, tb.Name, reason) - if err := r.Delete(ctx, pod); err != nil && !apierrors.IsNotFound(err) { - return false, fmt.Errorf("delete toolbox %s for recreate: %w", tb.Name, err) - } - return true, nil + return changed, requeueAfter, nil } func (r *Reconciler) checkToolboxCollision(ctx context.Context, cs *cocoonv1.CocoonSet, tbPod *corev1.Pod, tbName string) error { diff --git a/docs/observability.md b/docs/observability.md index d72289e..38ccb91 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -38,4 +38,4 @@ elsewhere and cold-pulled from the registry. A rising `slot_release_wake_unschedulable_total` means wakes are queuing behind cluster capacity, which is the cost side of the policy. -`CocoonSet` consumes the `vm.cocoonstack.io/lifecycle-state=Failed` annotation that vk-cocoon writes on terminal failures (hibernate, wake, post-clone, SAC); the operator treats it as terminal on every owned pod role (main, sub-agent, toolbox) so reconciliation reacts immediately instead of waiting for `Pod.Status.Phase` to follow. `triageSubAgent` rebuilds a terminal sub pod up to four times with `0/1/5/30 s` exponential backoff between attempts, then marks the pod `cocoonset.cocoonstack.io/dead-letter=true` and leaves it in place so a permanently broken slot stops consuming the apiserver budget. A spec edit lifts the dead-letter: when the pod no longer matches the current spec it is rebuilt and the slot's rebuild budget resets. Rebuild count persists in the `cocoonset.cocoonstack.io/rebuild-history` annotation on the CocoonSet so the count survives the pod delete; entries for slots beyond the current `spec.agent.replicas` are garbage-collected on every write. +`CocoonSet` consumes the `vm.cocoonstack.io/lifecycle-state=Failed` annotation that vk-cocoon writes on terminal failures (hibernate, wake, post-clone, SAC); the operator treats it as terminal on every owned pod role (main, sub-agent, toolbox) so reconciliation reacts immediately instead of waiting for `Pod.Status.Phase` to follow. A terminal or drifted pod (main, sub-agent or toolbox) is deleted for recreate up to four times with `0/1/5/30 s` backoff between attempts, then marked `cocoonset.cocoonstack.io/dead-letter=` and left in place, so a slot that cannot converge (a terminal image, or a pod an external defaulter such as a LimitRange keeps rewriting) stops churning VMs. A spec edit lifts the dead-letter: at a newer generation the pod is rebuilt with a fresh budget. Rebuild counts persist in the `cocoonset.cocoonstack.io/rebuild-history` annotation on the CocoonSet, keyed by pod name, so they survive the pod delete; entries for pods the spec no longer names are garbage-collected on every write. diff --git a/metrics/metrics.go b/metrics/metrics.go index 4498de4..357c5d0 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -17,7 +17,7 @@ var ( Namespace: metricNamespace, Subsystem: metricSubsystem, Name: "subagent_rebuild_total", - Help: "Number of sub-agent rebuilds triggered by triageSubAgent.", + Help: "Number of owned pods deleted for recreate within their rebuild budget.", }, []string{labelNamespace, labelCocoonSet}, ) @@ -27,7 +27,7 @@ var ( Namespace: metricNamespace, Subsystem: metricSubsystem, Name: "subagent_dead_letter_total", - Help: "Number of sub-agents marked dead-letter after exhausting rebuild attempts.", + Help: "Number of owned pods parked in dead-letter after exhausting rebuild attempts.", }, []string{labelNamespace, labelCocoonSet}, ) From da9168afaf60e06b430c3e8c23062df987368285 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 3 Sep 2026 01:49:36 +0800 Subject: [PATCH 2/6] review: podpatch joins the repo import group; no godoc on interface methods The package created yesterday sat in its own import group in six files. Reconcile and the logr sink's Enabled implement documented interfaces. --- cocoonset/lifecycle.go | 3 +-- cocoonset/migrate.go | 3 +-- cocoonset/slotrelease.go | 3 +-- cocoonset/suspend.go | 3 +-- hibernation/hibernate.go | 3 +-- hibernation/reconciler.go | 1 - hibernation/wake.go | 3 +-- logbridge.go | 1 - podpatch/podpatch_test.go | 4 ++-- 9 files changed, 8 insertions(+), 16 deletions(-) diff --git a/cocoonset/lifecycle.go b/cocoonset/lifecycle.go index fb59a49..281fb74 100644 --- a/cocoonset/lifecycle.go +++ b/cocoonset/lifecycle.go @@ -4,11 +4,10 @@ import ( "context" "fmt" - "github.com/cocoonstack/cocoon-operator/podpatch" - corev1 "k8s.io/api/core/v1" cocoonv1 "github.com/cocoonstack/cocoon-common/apis/v1" + "github.com/cocoonstack/cocoon-operator/podpatch" ) // syncCocoonSetGeneration lets vk-cocoon echo the generation back as a skew-free completion signal. diff --git a/cocoonset/migrate.go b/cocoonset/migrate.go index ea16ba6..03e0693 100644 --- a/cocoonset/migrate.go +++ b/cocoonset/migrate.go @@ -5,8 +5,6 @@ import ( "context" "fmt" - "github.com/cocoonstack/cocoon-operator/podpatch" - "github.com/projecteru2/core/log" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -14,6 +12,7 @@ import ( cocoonv1 "github.com/cocoonstack/cocoon-common/apis/v1" "github.com/cocoonstack/cocoon-common/meta" + "github.com/cocoonstack/cocoon-operator/podpatch" "github.com/cocoonstack/cocoon-operator/snapshot" ) diff --git a/cocoonset/slotrelease.go b/cocoonset/slotrelease.go index 79ed4f1..2c6fcfa 100644 --- a/cocoonset/slotrelease.go +++ b/cocoonset/slotrelease.go @@ -6,8 +6,6 @@ import ( "maps" "slices" - "github.com/cocoonstack/cocoon-operator/podpatch" - "github.com/projecteru2/core/log" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -18,6 +16,7 @@ import ( commonk8s "github.com/cocoonstack/cocoon-common/k8s" "github.com/cocoonstack/cocoon-common/meta" "github.com/cocoonstack/cocoon-operator/metrics" + "github.com/cocoonstack/cocoon-operator/podpatch" "github.com/cocoonstack/cocoon-operator/snapshot" ) diff --git a/cocoonset/suspend.go b/cocoonset/suspend.go index 55a6835..d0e755b 100644 --- a/cocoonset/suspend.go +++ b/cocoonset/suspend.go @@ -6,8 +6,6 @@ import ( "maps" "slices" - "github.com/cocoonstack/cocoon-operator/podpatch" - "github.com/projecteru2/core/log" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -15,6 +13,7 @@ import ( cocoonv1 "github.com/cocoonstack/cocoon-common/apis/v1" "github.com/cocoonstack/cocoon-common/meta" + "github.com/cocoonstack/cocoon-operator/podpatch" "github.com/cocoonstack/cocoon-operator/snapshot" ) diff --git a/hibernation/hibernate.go b/hibernation/hibernate.go index a029b9a..c567769 100644 --- a/hibernation/hibernate.go +++ b/hibernation/hibernate.go @@ -4,13 +4,12 @@ import ( "context" "fmt" - "github.com/cocoonstack/cocoon-operator/podpatch" - corev1 "k8s.io/api/core/v1" ctrl "sigs.k8s.io/controller-runtime" cocoonv1 "github.com/cocoonstack/cocoon-common/apis/v1" "github.com/cocoonstack/cocoon-common/meta" + "github.com/cocoonstack/cocoon-operator/podpatch" "github.com/cocoonstack/cocoon-operator/snapshot" ) diff --git a/hibernation/reconciler.go b/hibernation/reconciler.go index aeee330..c740c02 100644 --- a/hibernation/reconciler.go +++ b/hibernation/reconciler.go @@ -97,7 +97,6 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) err Complete(r) } -// Reconcile drives a single hibernate or wake transition. Failed phases are recoverable. func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := log.WithFunc("hibernation.Reconciler.Reconcile") diff --git a/hibernation/wake.go b/hibernation/wake.go index 2b8d258..defe765 100644 --- a/hibernation/wake.go +++ b/hibernation/wake.go @@ -4,14 +4,13 @@ import ( "context" "fmt" - "github.com/cocoonstack/cocoon-operator/podpatch" - "github.com/projecteru2/core/log" corev1 "k8s.io/api/core/v1" ctrl "sigs.k8s.io/controller-runtime" cocoonv1 "github.com/cocoonstack/cocoon-common/apis/v1" "github.com/cocoonstack/cocoon-common/meta" + "github.com/cocoonstack/cocoon-operator/podpatch" ) func (r *Reconciler) reconcileWake(ctx context.Context, hib *cocoonv1.CocoonHibernation, pod *corev1.Pod, vmName string) (ctrl.Result, error) { diff --git a/logbridge.go b/logbridge.go index 04dbef4..b7901f5 100644 --- a/logbridge.go +++ b/logbridge.go @@ -20,7 +20,6 @@ type crSink struct { func (s *crSink) Init(logr.RuntimeInfo) {} -// Errors bypass this gate entirely (logr contract). func (s *crSink) Enabled(level int) bool { return level == 0 } func (s *crSink) Info(_ int, msg string, kvs ...any) { diff --git a/podpatch/podpatch_test.go b/podpatch/podpatch_test.go index b39cea4..4b07f45 100644 --- a/podpatch/podpatch_test.go +++ b/podpatch/podpatch_test.go @@ -21,7 +21,7 @@ func TestHibernateStateShortCircuitsNoOp(t *testing.T) { cli := newFakeClient(t, pod.DeepCopy()) if err := HibernateState(t.Context(), cli, pod, true); err != nil { - t.Fatalf("no-op PatchHibernateState must not reach the client: %v", err) + t.Fatalf("no-op HibernateState must not reach the client: %v", err) } } @@ -30,7 +30,7 @@ func TestHibernateStateSetsAnnotation(t *testing.T) { cli := newFakeClient(t, pod.DeepCopy()) if err := HibernateState(t.Context(), cli, pod, true); err != nil { - t.Fatalf("PatchHibernateState: %v", err) + t.Fatalf("HibernateState: %v", err) } var got corev1.Pod From 1be7f7d9b1d51aa02d03ddc27dabaeba8127dba0 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 3 Sep 2026 01:49:36 +0800 Subject: [PATCH 3/6] docs: package tree lists podpatch; cocoonset steps name the current symbols --- README.md | 1 + docs/architecture.md | 1 + docs/cocoonset.md | 4 ++-- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1c3c02e..64b1cb2 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,7 @@ cocoon-operator/ ├── cocoonset/ # CocoonSet reconciler, pod builders, slot release, status diff ├── hibernation/ # CocoonHibernation reconciler ├── metrics/ # Prometheus collectors both reconcilers write to +├── podpatch/ # pod annotation patchers shared by both reconcilers ├── snapshot/ # snapshot.Registry interface consumed by both reconcilers └── version/ # ldflags-injected build identity ``` diff --git a/docs/architecture.md b/docs/architecture.md index f77ce2e..486cf96 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -16,6 +16,7 @@ cocoon-operator/ ├── cocoonset/ # CocoonSet reconciler, pod builders, slot release, status diff ├── hibernation/ # CocoonHibernation reconciler ├── metrics/ # Prometheus collectors both reconcilers write to +├── podpatch/ # pod annotation patchers shared by both reconcilers ├── snapshot/ # snapshot.Registry interface consumed by both reconcilers └── version/ # ldflags-injected build identity ``` diff --git a/docs/cocoonset.md b/docs/cocoonset.md index a46430e..0196332 100644 --- a/docs/cocoonset.md +++ b/docs/cocoonset.md @@ -9,8 +9,8 @@ 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. **Seat-release wake**: `reconcileWake` short-circuits while the CocoonSet is `Waking`/`Suspended`/`Suspending` with a stashed node hint, or whenever a restore-marked main still carries one. It recreates the released main from its snapshot and clears the hint on completion. It runs before the main-agent step, whose CR-only restore intent would otherwise fresh-boot over that snapshot. -10. **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. -11. 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`. +10. **Un-suspend**: if `spec.suspend == false` and any owned pod still carries the hibernate annotation from a prior suspend, clear it via `podpatch.HibernateState(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. +11. Ensure the **main agent** (slot 0). If the existing pod has drifted from spec, delete it for recreate within the rebuild budget (below). If it is not yet `Ready`, requeue in 5 s and report `Phase=Pending`. 12. Ensure sub-agents `[1..Replicas]` (creates are fanned out via an errgroup capped at 8 concurrent pod creates so a large scale-up does not burst the apiserver); delete extras above the requested count. 13. Ensure toolboxes by name; skip creation with an error if the toolbox pod name collides with an existing non-toolbox pod (e.g. an agent). Delete extras. 14. Patch `/status` from the classification taken in step 4 (with structural diff so unchanged status patches are no-ops). From c805293227fd4522e349cfda1879855e24602311 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 3 Sep 2026 02:30:42 +0800 Subject: [PATCH 4/6] fix: a rebuild budget belongs to one generation; an exhausted name is not recreated History entries record the CocoonSet generation they were counted at, so a spec edit resets every budget instead of only the parked pod's, and a Count=4 entry left behind after the fourth delete cannot stamp a fresh generation's pod straight into dead-letter. A missing pod whose name exhausted its budget at the current generation stays absent (main reports Failed, sub-agent slots and toolboxes are skipped), so Kubernetes' terminated-pod GC removing a parked pod no longer restarts the create/fail/park cycle. Comment and doc placement corrected. --- cocoonset/agents.go | 1 + cocoonset/pods.go | 2 +- cocoonset/rebuild.go | 12 +++++++++++- cocoonset/reconciler.go | 3 +++ cocoonset/toolboxes.go | 3 +++ docs/cocoonset.md | 2 +- 6 files changed, 20 insertions(+), 3 deletions(-) diff --git a/cocoonset/agents.go b/cocoonset/agents.go index 742990d..4b35337 100644 --- a/cocoonset/agents.go +++ b/cocoonset/agents.go @@ -42,6 +42,7 @@ func (r *Reconciler) ensureSubAgents(ctx context.Context, cs *cocoonv1.CocoonSet } } + missing = slices.DeleteFunc(missing, func(slot int32) bool { return budgetExhausted(cs, agentPodName(cs.Name, slot)) }) created, err := r.createSubAgents(ctx, logger, cs, missing, mainVMName, mainNodeName, intent) changed = changed || created if err != nil { diff --git a/cocoonset/pods.go b/cocoonset/pods.go index 6a212c6..a5966f3 100644 --- a/cocoonset/pods.go +++ b/cocoonset/pods.go @@ -156,11 +156,11 @@ func buildToolboxPod(cs *cocoonv1.CocoonSet, tb cocoonv1.ToolboxSpec, scheme *ru return pod, nil } -// toolboxPodName is shared by the builder and the collision check so the two cannot diverge. func agentPodName(csName string, slot int32) string { return fmt.Sprintf("%s-%d", csName, slot) } +// toolboxPodName is shared by the builder and the collision check so the two cannot diverge. func toolboxPodName(csName, tbName string) string { return fmt.Sprintf("%s-%s", csName, tbName) } diff --git a/cocoonset/rebuild.go b/cocoonset/rebuild.go index 55acac6..03ee24a 100644 --- a/cocoonset/rebuild.go +++ b/cocoonset/rebuild.go @@ -27,10 +27,11 @@ const ( maxRebuildAttempts = 4 ) -// rebuildEntry persists in a CocoonSet annotation, keyed by pod name, so the count survives the pod delete. +// rebuildEntry persists in a CocoonSet annotation, keyed by pod name, so the count survives the pod delete; a newer generation resets it. type rebuildEntry struct { Count int `json:"count"` LastDeleted time.Time `json:"lastDeleted"` + Generation int64 `json:"generation"` } // triagePod deletes a terminal or drifted pod for recreate within its rebuild budget; a dead-lettered pod waits for a spec edit. @@ -69,6 +70,9 @@ func (r *Reconciler) triagePod(ctx context.Context, logger *log.Fields, cs *coco func (r *Reconciler) rebuildPod(ctx context.Context, logger *log.Fields, cs *cocoonv1.CocoonSet, pod *corev1.Pod, reason string) (bool, time.Duration, error) { history := readRebuildHistory(cs) entry := history[pod.Name] + if entry.Generation != cs.Generation { + entry = rebuildEntry{Generation: cs.Generation} + } if entry.Count >= maxRebuildAttempts { if err := r.patchAnnotation(ctx, pod, annotationDeadLetter, strconv.FormatInt(cs.Generation, 10)); err != nil { return false, 0, err @@ -159,6 +163,12 @@ func encodeRebuildHistory(cs *cocoonv1.CocoonSet, m map[string]rebuildEntry) (st return string(raw), nil } +// budgetExhausted reports a pod name that dead-lettered at the current generation; a missing pod with that history stays absent. +func budgetExhausted(cs *cocoonv1.CocoonSet, name string) bool { + entry := readRebuildHistory(cs)[name] + return entry.Generation == cs.Generation && entry.Count >= maxRebuildAttempts +} + func desiredPodNames(cs *cocoonv1.CocoonSet) map[string]bool { names := map[string]bool{} for slot := range cs.Spec.Agent.Replicas + 1 { diff --git a/cocoonset/reconciler.go b/cocoonset/reconciler.go index ce64526..25079dc 100644 --- a/cocoonset/reconciler.go +++ b/cocoonset/reconciler.go @@ -126,6 +126,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } intent := r.newRestoreIntent(ctx, cs.Namespace) if classified.main == nil { + if budgetExhausted(&cs, agentPodName(cs.Name, 0)) { + return ctrl.Result{RequeueAfter: requeueWaitForMain}, r.patchStatus(ctx, &cs, buildStatus(&cs, classified, cocoonv1.CocoonSetPhaseFailed)) + } return r.createMainAgent(ctx, &cs, intent) } diff --git a/cocoonset/toolboxes.go b/cocoonset/toolboxes.go index 383a34f..79a67a2 100644 --- a/cocoonset/toolboxes.go +++ b/cocoonset/toolboxes.go @@ -39,6 +39,9 @@ func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet if classified.allByName[podName] != nil && classified.toolbox[tb.Name] == nil { return changed, requeueAfter, fmt.Errorf("create toolbox %s: name collision with existing pod %s", tb.Name, podName) } + if budgetExhausted(cs, podName) { + continue + } if pod, exists := classified.toolbox[tb.Name]; exists { deleted, wait, err := r.triagePod(ctx, logger, cs, pod, podSpecMatchesToolbox(pod, cs, tb)) if err != nil { diff --git a/docs/cocoonset.md b/docs/cocoonset.md index 0196332..9d9a083 100644 --- a/docs/cocoonset.md +++ b/docs/cocoonset.md @@ -9,7 +9,7 @@ 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. **Seat-release wake**: `reconcileWake` short-circuits while the CocoonSet is `Waking`/`Suspended`/`Suspending` with a stashed node hint, or whenever a restore-marked main still carries one. It recreates the released main from its snapshot and clears the hint on completion. It runs before the main-agent step, whose CR-only restore intent would otherwise fresh-boot over that snapshot. -10. **Un-suspend**: if `spec.suspend == false` and any owned pod still carries the hibernate annotation from a prior suspend, clear it via `podpatch.HibernateState(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. **Un-suspend**: if `spec.suspend == false` and any owned pod still carries the hibernate annotation from a prior suspend, clear it via `podpatch.HibernateState(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. `podpatch.HibernateState(false)` is a no-op on pods whose annotation is already absent, so this is cheap in the common "never suspended" case. 11. Ensure the **main agent** (slot 0). If the existing pod has drifted from spec, delete it for recreate within the rebuild budget (below). If it is not yet `Ready`, requeue in 5 s and report `Phase=Pending`. 12. Ensure sub-agents `[1..Replicas]` (creates are fanned out via an errgroup capped at 8 concurrent pod creates so a large scale-up does not burst the apiserver); delete extras above the requested count. 13. Ensure toolboxes by name; skip creation with an error if the toolbox pod name collides with an existing non-toolbox pod (e.g. an agent). Delete extras. From c58333b9ffbf0b17de924c2ddee6555e46b1074b Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 3 Sep 2026 02:48:17 +0800 Subject: [PATCH 5/6] fix: park a rebuilt name explicitly instead of inferring it from the count Count reaching the budget only means the last delete happened; the replacement still has to be created. A parked flag in the history entry now marks the dead-lettered name, the missing-pod gates read that flag, an existing toolbox is always triaged, and an exhausted absent main reports Failed without a timer requeue since only a generation or pod event can change the decision. --- cocoonset/rebuild.go | 12 ++++- cocoonset/rebuild_test.go | 101 ++++++++++++++++++++++++++++++++++++++ cocoonset/reconciler.go | 2 +- cocoonset/toolboxes.go | 6 +-- docs/observability.md | 2 +- 5 files changed, 116 insertions(+), 7 deletions(-) diff --git a/cocoonset/rebuild.go b/cocoonset/rebuild.go index 03ee24a..fbb2108 100644 --- a/cocoonset/rebuild.go +++ b/cocoonset/rebuild.go @@ -32,6 +32,7 @@ type rebuildEntry struct { Count int `json:"count"` LastDeleted time.Time `json:"lastDeleted"` Generation int64 `json:"generation"` + Parked bool `json:"parked,omitempty"` } // triagePod deletes a terminal or drifted pod for recreate within its rebuild budget; a dead-lettered pod waits for a spec edit. @@ -74,6 +75,13 @@ func (r *Reconciler) rebuildPod(ctx context.Context, logger *log.Fields, cs *coc entry = rebuildEntry{Generation: cs.Generation} } if entry.Count >= maxRebuildAttempts { + if !entry.Parked { + entry.Parked = true + history[pod.Name] = entry + if err := r.patchRebuildHistory(ctx, cs, history); err != nil { + return false, 0, fmt.Errorf("persist rebuild history: %w", err) + } + } if err := r.patchAnnotation(ctx, pod, annotationDeadLetter, strconv.FormatInt(cs.Generation, 10)); err != nil { return false, 0, err } @@ -163,10 +171,10 @@ func encodeRebuildHistory(cs *cocoonv1.CocoonSet, m map[string]rebuildEntry) (st return string(raw), nil } -// budgetExhausted reports a pod name that dead-lettered at the current generation; a missing pod with that history stays absent. +// budgetExhausted reports a pod name parked at the current generation; a missing pod with that history stays absent. func budgetExhausted(cs *cocoonv1.CocoonSet, name string) bool { entry := readRebuildHistory(cs)[name] - return entry.Generation == cs.Generation && entry.Count >= maxRebuildAttempts + return entry.Generation == cs.Generation && entry.Parked } func desiredPodNames(cs *cocoonv1.CocoonSet) map[string]bool { diff --git a/cocoonset/rebuild_test.go b/cocoonset/rebuild_test.go index 7d758fb..7ae5ba1 100644 --- a/cocoonset/rebuild_test.go +++ b/cocoonset/rebuild_test.go @@ -5,6 +5,13 @@ import ( "testing" "time" + "github.com/projecteru2/core/log" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + cocoonv1 "github.com/cocoonstack/cocoon-common/apis/v1" ) @@ -95,3 +102,97 @@ func TestReadRebuildHistoryHandlesNullPayload(t *testing.T) { t.Fatalf("encodeRebuildHistory on normalized null payload: %v", err) } } + +func TestBudgetExhaustedRequiresParkedEntry(t *testing.T) { + cs := &cocoonv1.CocoonSet{} + cs.Name = "demo" + cs.Generation = 3 + cs.Spec.Agent.Replicas = 1 + cases := []struct { + name string + entry rebuildEntry + want bool + }{ + {"count at budget awaiting its replacement", rebuildEntry{Count: maxRebuildAttempts, Generation: 3}, false}, + {"parked at this generation", rebuildEntry{Count: maxRebuildAttempts, Generation: 3, Parked: true}, true}, + {"parked at an older generation", rebuildEntry{Count: maxRebuildAttempts, Generation: 2, Parked: true}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + enc, err := encodeRebuildHistory(cs, map[string]rebuildEntry{"demo-0": tc.entry}) + if err != nil { + t.Fatalf("encodeRebuildHistory: %v", err) + } + cs.Annotations = map[string]string{annotationRebuildHistory: enc} + if got := budgetExhausted(cs, "demo-0"); got != tc.want { + t.Fatalf("budgetExhausted = %v, want %v", got, tc.want) + } + }) + } +} + +func TestRebuildPodPastBudgetParksNameAndPod(t *testing.T) { + scheme := testScheme(t) + cs := newRebuildCS(t, rebuildEntry{Count: maxRebuildAttempts, Generation: 3}) + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "demo-tb", Namespace: "ns"}} + cli := ctrlfake.NewClientBuilder().WithScheme(scheme).WithObjects(cs, pod).Build() + r := &Reconciler{Client: cli, Scheme: scheme} + + deleted, wait, err := r.rebuildPod(t.Context(), log.WithFunc("test"), cs, pod, "spec drifted") + if err != nil || deleted || wait != 0 { + t.Fatalf("rebuildPod = (%v, %v, %v), want (false, 0, nil)", deleted, wait, err) + } + if !budgetExhausted(cs, "demo-tb") { + t.Fatal("parked name not exhausted in the mirrored history") + } + var stored cocoonv1.CocoonSet + if err := cli.Get(t.Context(), types.NamespacedName{Namespace: "ns", Name: "demo"}, &stored); err != nil { + t.Fatalf("get CocoonSet: %v", err) + } + if entry := readRebuildHistory(&stored)["demo-tb"]; !entry.Parked || entry.Count != maxRebuildAttempts { + t.Fatalf("persisted entry = %+v, want parked at count %d", entry, maxRebuildAttempts) + } + var got corev1.Pod + if err := cli.Get(t.Context(), types.NamespacedName{Namespace: "ns", Name: "demo-tb"}, &got); err != nil { + t.Fatalf("get pod: %v", err) + } + if got.Annotations[annotationDeadLetter] != "3" { + t.Fatalf("dead-letter annotation = %q, want 3", got.Annotations[annotationDeadLetter]) + } +} + +func TestRebuildPodFourthDeleteKeepsNameRecreatable(t *testing.T) { + scheme := testScheme(t) + cs := newRebuildCS(t, rebuildEntry{Count: maxRebuildAttempts - 1, Generation: 3, LastDeleted: time.Now().Add(-time.Minute)}) + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "demo-tb", Namespace: "ns"}} + cli := ctrlfake.NewClientBuilder().WithScheme(scheme).WithObjects(cs, pod).Build() + r := &Reconciler{Client: cli, Scheme: scheme} + + deleted, wait, err := r.rebuildPod(t.Context(), log.WithFunc("test"), cs, pod, "spec drifted") + if err != nil || !deleted || wait != 0 { + t.Fatalf("rebuildPod = (%v, %v, %v), want (true, 0, nil)", deleted, wait, err) + } + if budgetExhausted(cs, "demo-tb") { + t.Fatal("name exhausted before its final replacement was created") + } + if entry := readRebuildHistory(cs)["demo-tb"]; entry.Count != maxRebuildAttempts || entry.Parked { + t.Fatalf("entry after the fourth delete = %+v", entry) + } + var got corev1.Pod + if err := cli.Get(t.Context(), types.NamespacedName{Namespace: "ns", Name: "demo-tb"}, &got); !apierrors.IsNotFound(err) { + t.Fatalf("pod after the fourth delete: err=%v, want NotFound", err) + } +} + +func newRebuildCS(t *testing.T, entry rebuildEntry) *cocoonv1.CocoonSet { + t.Helper() + cs := &cocoonv1.CocoonSet{} + cs.Name, cs.Namespace, cs.Generation = "demo", "ns", 3 + cs.Spec.Toolboxes = []cocoonv1.ToolboxSpec{{Name: "tb"}} + enc, err := encodeRebuildHistory(cs, map[string]rebuildEntry{"demo-tb": entry}) + if err != nil { + t.Fatalf("encodeRebuildHistory: %v", err) + } + cs.Annotations = map[string]string{annotationRebuildHistory: enc} + return cs +} diff --git a/cocoonset/reconciler.go b/cocoonset/reconciler.go index 25079dc..5a182cf 100644 --- a/cocoonset/reconciler.go +++ b/cocoonset/reconciler.go @@ -127,7 +127,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu intent := r.newRestoreIntent(ctx, cs.Namespace) if classified.main == nil { if budgetExhausted(&cs, agentPodName(cs.Name, 0)) { - return ctrl.Result{RequeueAfter: requeueWaitForMain}, r.patchStatus(ctx, &cs, buildStatus(&cs, classified, cocoonv1.CocoonSetPhaseFailed)) + return ctrl.Result{}, r.patchStatus(ctx, &cs, buildStatus(&cs, classified, cocoonv1.CocoonSetPhaseFailed)) } return r.createMainAgent(ctx, &cs, intent) } diff --git a/cocoonset/toolboxes.go b/cocoonset/toolboxes.go index 79a67a2..5af4c0e 100644 --- a/cocoonset/toolboxes.go +++ b/cocoonset/toolboxes.go @@ -39,9 +39,6 @@ func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet if classified.allByName[podName] != nil && classified.toolbox[tb.Name] == nil { return changed, requeueAfter, fmt.Errorf("create toolbox %s: name collision with existing pod %s", tb.Name, podName) } - if budgetExhausted(cs, podName) { - continue - } if pod, exists := classified.toolbox[tb.Name]; exists { deleted, wait, err := r.triagePod(ctx, logger, cs, pod, podSpecMatchesToolbox(pod, cs, tb)) if err != nil { @@ -53,6 +50,9 @@ func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet } continue } + if budgetExhausted(cs, podName) { + continue + } tbPod, err := buildToolboxPod(cs, tb, r.Scheme) if err != nil { return changed, requeueAfter, fmt.Errorf("build toolbox %s: %w", tb.Name, err) diff --git a/docs/observability.md b/docs/observability.md index 38ccb91..4f4cf0f 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -38,4 +38,4 @@ elsewhere and cold-pulled from the registry. A rising `slot_release_wake_unschedulable_total` means wakes are queuing behind cluster capacity, which is the cost side of the policy. -`CocoonSet` consumes the `vm.cocoonstack.io/lifecycle-state=Failed` annotation that vk-cocoon writes on terminal failures (hibernate, wake, post-clone, SAC); the operator treats it as terminal on every owned pod role (main, sub-agent, toolbox) so reconciliation reacts immediately instead of waiting for `Pod.Status.Phase` to follow. A terminal or drifted pod (main, sub-agent or toolbox) is deleted for recreate up to four times with `0/1/5/30 s` backoff between attempts, then marked `cocoonset.cocoonstack.io/dead-letter=` and left in place, so a slot that cannot converge (a terminal image, or a pod an external defaulter such as a LimitRange keeps rewriting) stops churning VMs. A spec edit lifts the dead-letter: at a newer generation the pod is rebuilt with a fresh budget. Rebuild counts persist in the `cocoonset.cocoonstack.io/rebuild-history` annotation on the CocoonSet, keyed by pod name, so they survive the pod delete; entries for pods the spec no longer names are garbage-collected on every write. +`CocoonSet` consumes the `vm.cocoonstack.io/lifecycle-state=Failed` annotation that vk-cocoon writes on terminal failures (hibernate, wake, post-clone, SAC); the operator treats it as terminal on every owned pod role (main, sub-agent, toolbox) so reconciliation reacts immediately instead of waiting for `Pod.Status.Phase` to follow. A drifted pod (main, sub-agent or toolbox) or a terminal sub-agent or toolbox is deleted for recreate up to four times with `0/1/5/30 s` backoff between attempts, then marked `cocoonset.cocoonstack.io/dead-letter=` and left in place, so a slot that cannot converge (a terminal image, or a pod an external defaulter such as a LimitRange keeps rewriting) stops churning VMs. A terminal main that still matches its spec is not rebuilt: the CocoonSet reports `Failed` until the main is edited or recovers. A spec edit lifts the dead-letter: at a newer generation the pod is rebuilt with a fresh budget. Rebuild counts persist in the `cocoonset.cocoonstack.io/rebuild-history` annotation on the CocoonSet, keyed by pod name, so they survive the pod delete; entries for pods the spec no longer names are garbage-collected on every write. From cd43765e037f9634e3149514d0e81ee7e27807f6 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 3 Sep 2026 03:04:18 +0800 Subject: [PATCH 6/6] docs: the terminal-main short-circuit applies only to a main that still matches its spec --- docs/cocoonset.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/cocoonset.md b/docs/cocoonset.md index 9d9a083..37b2448 100644 --- a/docs/cocoonset.md +++ b/docs/cocoonset.md @@ -5,7 +5,7 @@ 3. Ensure the `cocoonset.cocoonstack.io/finalizer` is in place. 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. +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) and still matches its spec, patch `Phase=Failed` and emit `MainAgentFailed` / `PodLifecycleFailed`; a terminal main that has also drifted from spec takes the rebuild budget of step 11 instead. 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`. 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. **Seat-release wake**: `reconcileWake` short-circuits while the CocoonSet is `Waking`/`Suspended`/`Suspending` with a stashed node hint, or whenever a restore-marked main still carries one. It recreates the released main from its snapshot and clears the hint on completion. It runs before the main-agent step, whose CR-only restore intent would otherwise fresh-boot over that snapshot.