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/cocoonset/agents.go b/cocoonset/agents.go index ecde94e..4b35337 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 } @@ -47,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 { @@ -103,114 +99,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/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/pods.go b/cocoonset/pods.go index 182463f..a5966f3 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 { @@ -156,6 +156,10 @@ func buildToolboxPod(cs *cocoonv1.CocoonSet, tb cocoonv1.ToolboxSpec, scheme *ru return pod, nil } +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 18f5d93..fbb2108 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,167 @@ 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; a newer generation resets it. 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. +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.Generation != cs.Generation { + 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 + } + 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 } -func readRebuildHistory(cs *cocoonv1.CocoonSet) map[int32]rebuildEntry { - m := map[int32]rebuildEntry{} +// 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 } +// 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.Parked +} + +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..7ae5ba1 100644 --- a/cocoonset/rebuild_test.go +++ b/cocoonset/rebuild_test.go @@ -5,45 +5,60 @@ 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" ) 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 +97,102 @@ 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) } } + +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 2b9d775..5a182cf 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,15 +121,14 @@ 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 { + if budgetExhausted(&cs, agentPodName(cs.Name, 0)) { + return ctrl.Result{}, r.patchStatus(ctx, &cs, buildStatus(&cs, classified, cocoonv1.CocoonSetPhaseFailed)) + } return r.createMainAgent(ctx, &cs, intent) } @@ -147,7 +145,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 +156,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/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/cocoonset/toolboxes.go b/cocoonset/toolboxes.go index b856ed5..5af4c0e 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,54 @@ 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 + } + if budgetExhausted(cs, podName) { 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 +78,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/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..37b2448 100644 --- a/docs/cocoonset.md +++ b/docs/cocoonset.md @@ -5,12 +5,12 @@ 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. -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. `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. 14. Patch `/status` from the classification taken in step 4 (with structural diff so unchanged status patches are no-ops). diff --git a/docs/observability.md b/docs/observability.md index d72289e..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. `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 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. 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/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}, ) 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