From c533a044529cf1224f3b1bc814e5afa52309f437 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 15 Jul 2026 23:47:08 +0800 Subject: [PATCH 1/8] feat: make controller reconcile concurrency configurable Both controllers ran at controller-runtime's default of one worker while reconciles block on registry probes, so one slow request delayed every unrelated CR. Concurrency is now per-controller configurable (flag or env, default 4) and rejected below 1. --- cocoonset/reconciler.go | 8 ++++++ cocoonset/reconciler_test.go | 9 ++++++ hibernation/reconciler.go | 8 ++++++ hibernation/reconciler_test.go | 9 ++++++ main.go | 51 +++++++++++++++++++++++++--------- main_test.go | 20 +++++++++++++ 6 files changed, 92 insertions(+), 13 deletions(-) diff --git a/cocoonset/reconciler.go b/cocoonset/reconciler.go index 6964c7b..8057baa 100644 --- a/cocoonset/reconciler.go +++ b/cocoonset/reconciler.go @@ -13,6 +13,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -36,15 +37,22 @@ type Reconciler struct { Scheme *runtime.Scheme Registry snapshot.Registry Recorder record.EventRecorder + // Concurrency caps in-flight reconciles. Reconciles block on registry + // probes, so at 1 a single slow probe stalls every other CocoonSet. + Concurrency int } // SetupWithManager registers the reconciler. `For` uses GenerationChangedPredicate // to avoid status-update loops; Owns filters pod events to creation, deletion, // and readiness transitions to prevent reconcile storms from VK status churn. func (r *Reconciler) SetupWithManager(_ context.Context, mgr ctrl.Manager) error { + if r.Concurrency < 1 { + return fmt.Errorf("cocoonset concurrency must be at least 1, got %d", r.Concurrency) + } return ctrl.NewControllerManagedBy(mgr). For(&cocoonv1.CocoonSet{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Owns(&corev1.Pod{}, builder.WithPredicates(podRelevantChange{})). + WithOptions(controller.Options{MaxConcurrentReconciles: r.Concurrency}). Complete(r) } diff --git a/cocoonset/reconciler_test.go b/cocoonset/reconciler_test.go index 4aa40e3..7ac4ec3 100644 --- a/cocoonset/reconciler_test.go +++ b/cocoonset/reconciler_test.go @@ -653,6 +653,15 @@ func TestApplyUnsuspendSkipsPodHibernatedByCR(t *testing.T) { } } +// A nil manager suffices: the guard rejects before mgr is touched. +func TestSetupWithManagerRejectsInvalidConcurrency(t *testing.T) { + for _, n := range []int{0, -1} { + if err := (&Reconciler{Concurrency: n}).SetupWithManager(t.Context(), nil); err == nil { + t.Errorf("concurrency %d must be rejected", n) + } + } +} + type fakeRegistry struct { present map[string]bool probeErr error diff --git a/hibernation/reconciler.go b/hibernation/reconciler.go index 54d2c3f..20c178a 100644 --- a/hibernation/reconciler.go +++ b/hibernation/reconciler.go @@ -18,6 +18,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -56,6 +57,9 @@ type Reconciler struct { Scheme *runtime.Scheme Registry snapshot.Registry Recorder record.EventRecorder + // Concurrency caps in-flight reconciles. Reconciles block on registry + // pushes and deletes, so at 1 a single slow request stalls every other CR. + Concurrency int // observed[UID] = last recorded Ready.LastTransitionTime, dedups // phase-exit observations against controller-runtime cache lag. @@ -66,6 +70,9 @@ type Reconciler struct { // An index on spec.podRef.name lets the pod watcher fan out events to every // CR targeting a given pod, so late-arriving pods self-heal without user edits. func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error { + if r.Concurrency < 1 { + return fmt.Errorf("hibernation concurrency must be at least 1, got %d", r.Concurrency) + } if err := mgr.GetFieldIndexer().IndexField( ctx, &cocoonv1.CocoonHibernation{}, indexPodRefName, func(o client.Object) []string { @@ -89,6 +96,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) err }, )), ). + WithOptions(controller.Options{MaxConcurrentReconciles: r.Concurrency}). Complete(r) } diff --git a/hibernation/reconciler_test.go b/hibernation/reconciler_test.go index 898dbc8..5d08dd4 100644 --- a/hibernation/reconciler_test.go +++ b/hibernation/reconciler_test.go @@ -761,6 +761,15 @@ func TestReconcilePendingWhenPodMissingVMName(t *testing.T) { } } +// A nil manager suffices: the guard rejects before mgr is touched. +func TestSetupWithManagerRejectsInvalidConcurrency(t *testing.T) { + for _, n := range []int{0, -1} { + if err := (&Reconciler{Concurrency: n}).SetupWithManager(t.Context(), nil); err == nil { + t.Errorf("concurrency %d must be rejected", n) + } + } +} + func testScheme(t *testing.T) *runtime.Scheme { t.Helper() sch := runtime.NewScheme() diff --git a/main.go b/main.go index 288fe26..0a52a5b 100644 --- a/main.go +++ b/main.go @@ -9,6 +9,7 @@ import ( "fmt" "os" "os/signal" + "strconv" "syscall" "github.com/google/go-containerregistry/pkg/authn" @@ -42,19 +43,28 @@ const ( defaultMetricsAddr = ":8080" defaultProbeAddr = ":8081" leaderElectionID = "cocoon-operator.cocoonset.cocoonstack.io" + + // defaultConcurrency stays well inside the client-go QPS budget: reconciles + // block on registry probes, so the win comes from overlapping those waits, + // not from saturating the apiserver. + defaultConcurrency = 4 ) func main() { leaderDefault := commonk8s.EnvBool("LEADER_ELECT", true) var ( - metricsAddr string - probeAddr string - enableLeaderElection bool + metricsAddr string + probeAddr string + enableLeaderElection bool + cocoonSetConcurrency int + hibernationConcurrency int ) flag.StringVar(&metricsAddr, "metrics-bind-address", commonk8s.EnvOrDefault("METRICS_ADDR", defaultMetricsAddr), "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", commonk8s.EnvOrDefault("PROBE_ADDR", defaultProbeAddr), "The address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", leaderDefault, "Enable leader election so only one operator instance reconciles at a time.") + flag.IntVar(&cocoonSetConcurrency, "cocoonset-concurrency", envInt("COCOONSET_CONCURRENCY", defaultConcurrency), "Maximum concurrent CocoonSet reconciles.") + flag.IntVar(&hibernationConcurrency, "hibernation-concurrency", envInt("HIBERNATION_CONCURRENCY", defaultConcurrency), "Maximum concurrent CocoonHibernation reconciles.") flag.Parse() ctx := context.Background() @@ -109,18 +119,20 @@ func main() { recorder := broadcaster.NewRecorder(mgr.GetScheme(), corev1.EventSource{Component: "cocoon-operator"}) if err = (&cocoonset.Reconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Registry: registry, - Recorder: recorder, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Registry: registry, + Recorder: recorder, + Concurrency: cocoonSetConcurrency, }).SetupWithManager(ctx, mgr); err != nil { logger.Fatalf(ctx, err, "register cocoonset.Reconciler") } if err = (&hibernation.Reconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Registry: registry, - Recorder: recorder, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Registry: registry, + Recorder: recorder, + Concurrency: hibernationConcurrency, }).SetupWithManager(ctx, mgr); err != nil { logger.Fatalf(ctx, err, "register hibernation.Reconciler") } @@ -128,8 +140,8 @@ func main() { signalCtx, cancel := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM) defer cancel() - logger.Infof(signalCtx, "starting controller manager (metrics=%s probe=%s leader=%t)", - metricsAddr, probeAddr, enableLeaderElection) + logger.Infof(signalCtx, "starting controller manager (metrics=%s probe=%s leader=%t cocoonset-concurrency=%d hibernation-concurrency=%d)", + metricsAddr, probeAddr, enableLeaderElection, cocoonSetConcurrency, hibernationConcurrency) if err = mgr.Start(signalCtx); err != nil { logger.Fatalf(signalCtx, err, "run manager") } @@ -152,3 +164,16 @@ func buildScheme() *runtime.Scheme { utilruntime.Must(cocoonv1.AddToScheme(scheme)) return scheme } + +// envInt parses an int env var, falling back when unset or invalid. +func envInt(key string, fallback int) int { + v := os.Getenv(key) + if v == "" { + return fallback + } + n, err := strconv.Atoi(v) + if err != nil { + return fallback + } + return n +} diff --git a/main_test.go b/main_test.go index 6c82f8a..80916d9 100644 --- a/main_test.go +++ b/main_test.go @@ -21,3 +21,23 @@ func TestBuildRegistry(t *testing.T) { t.Fatal("buildRegistry with no OCI_REGISTRY: want error, got nil") } } + +func TestEnvInt(t *testing.T) { + cases := []struct { + name string + set string + want int + }{ + {"unset falls back", "", 4}, + {"valid value wins", "8", 8}, + {"invalid falls back", "many", 4}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + t.Setenv("TEST_CONCURRENCY", c.set) + if got := envInt("TEST_CONCURRENCY", 4); got != c.want { + t.Errorf("envInt = %d, want %d", got, c.want) + } + }) + } +} From 7d5a72778488f10c0e0f2a1d800a3c5576ab2796 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 15 Jul 2026 23:53:20 +0800 Subject: [PATCH 2/8] perf(cocoonset): load restore intent lazily and once per reconcile Both ensure passes eagerly listed every CocoonHibernation in the namespace before checking whether any pod was missing, so a steady reconcile paid two O(Hibernations) Lists to build a map it never read. A per-reconcile memo defers the List to the first pod that has to be built and shares it across the main, sub-agent and toolbox paths. --- cocoonset/agents.go | 98 ++++++++------- cocoonset/pods_test.go | 2 +- cocoonset/reconcile_bench_test.go | 136 +++++++++++++++++++++ cocoonset/reconciler.go | 16 +-- cocoonset/reconciler_test.go | 12 +- cocoonset/restore.go | 25 ++++ cocoonset/restore_intent_test.go | 190 ++++++++++++++++++++++++++++++ cocoonset/restore_test.go | 2 +- cocoonset/toolboxes.go | 14 +-- 9 files changed, 429 insertions(+), 66 deletions(-) create mode 100644 cocoonset/reconcile_bench_test.go create mode 100644 cocoonset/restore_intent_test.go diff --git a/cocoonset/agents.go b/cocoonset/agents.go index 75c0b2d..5ed5995 100644 --- a/cocoonset/agents.go +++ b/cocoonset/agents.go @@ -32,59 +32,69 @@ const subAgentCreateConcurrency = 8 // (non-zero when a sub-agent is in rebuild backoff and the caller should // re-reconcile when backoff elapses). Missing slots are created concurrently // so batch scale-ups do not serialize N apiserver round trips. -func (r *Reconciler) ensureSubAgents(ctx context.Context, cs *cocoonv1.CocoonSet, classified classifiedPods, mainVMName, mainNodeName string) (bool, time.Duration, error) { +func (r *Reconciler) ensureSubAgents(ctx context.Context, cs *cocoonv1.CocoonSet, classified classifiedPods, mainVMName, mainNodeName string, intent *restoreIntent) (bool, time.Duration, error) { logger := log.WithFunc("cocoonset.Reconciler.ensureSubAgents") changed := false var requeueAfter time.Duration - restorable, err := r.podsRestorableByCR(ctx, cs.Namespace) - if err != nil { - return changed, requeueAfter, err - } - - g, gctx := errgroup.WithContext(ctx) - g.SetLimit(subAgentCreateConcurrency) - var created atomic.Bool + var missing []int32 for slot := int32(1); slot <= cs.Spec.Agent.Replicas; slot++ { - if pod, exists := classified.sub[slot]; exists { - deleted, wait, err := r.triageSubAgent(ctx, logger, pod, cs, slot) - if err != nil { - return changed, requeueAfter, err - } - if deleted { - changed = true - } - if wait > 0 && (requeueAfter == 0 || wait < requeueAfter) { - requeueAfter = wait - } + pod, exists := classified.sub[slot] + if !exists { + missing = append(missing, slot) continue } - g.Go(func() error { - subPod, err := buildAgentPod(cs, slot, mainVMName, mainNodeName, r.Scheme) - if err != nil { - return fmt.Errorf("build sub-agent slot %d: %w", slot, err) - } - _, intent := restorable[subPod.Name] - if err := r.markRestoreIfHibernated(gctx, subPod, intent); err != nil { - return fmt.Errorf("mark restore sub-agent slot %d: %w", slot, err) - } - if err := r.Create(gctx, subPod); err != nil { - if apierrors.IsAlreadyExists(err) { - return nil - } - return fmt.Errorf("create sub-agent slot %d: %w", slot, err) - } - logger.Infof(gctx, "created sub-agent %s/%s", subPod.Namespace, subPod.Name) - created.Store(true) - return nil - }) - } - if err := g.Wait(); err != nil { - return changed || created.Load(), requeueAfter, err + deleted, wait, err := r.triageSubAgent(ctx, logger, pod, cs, slot) + if err != nil { + return changed, requeueAfter, err + } + if deleted { + changed = true + } + if wait > 0 && (requeueAfter == 0 || wait < requeueAfter) { + requeueAfter = wait + } } - if created.Load() { - changed = true + + if len(missing) > 0 { + // Resolved before the fan-out so the goroutines only ever read it. + restorable, err := intent.resolve(ctx) + if err != nil { + return changed, requeueAfter, err + } + g, gctx := errgroup.WithContext(ctx) + g.SetLimit(subAgentCreateConcurrency) + var created atomic.Bool + for _, slot := range missing { + g.Go(func() error { + subPod, err := buildAgentPod(cs, slot, mainVMName, mainNodeName, r.Scheme) + if err != nil { + return fmt.Errorf("build sub-agent slot %d: %w", slot, err) + } + _, wantRestore := restorable[subPod.Name] + if err := r.markRestoreIfHibernated(gctx, subPod, wantRestore); err != nil { + return fmt.Errorf("mark restore sub-agent slot %d: %w", slot, err) + } + if err := r.Create(gctx, subPod); err != nil { + if apierrors.IsAlreadyExists(err) { + return nil + } + return fmt.Errorf("create sub-agent slot %d: %w", slot, err) + } + logger.Infof(gctx, "created sub-agent %s/%s", subPod.Namespace, subPod.Name) + created.Store(true) + return nil + }) + } + err = g.Wait() + if created.Load() { + changed = true + } + if err != nil { + return changed, requeueAfter, err + } } + for _, slot := range slices.Sorted(maps.Keys(classified.sub)) { if slot <= cs.Spec.Agent.Replicas { continue diff --git a/cocoonset/pods_test.go b/cocoonset/pods_test.go index 1ada991..0adcb83 100644 --- a/cocoonset/pods_test.go +++ b/cocoonset/pods_test.go @@ -559,7 +559,7 @@ func TestPodSpecMatchesAgentIgnoresAffinity(t *testing.T) { } } -func testScheme(t *testing.T) *runtime.Scheme { +func testScheme(t testing.TB) *runtime.Scheme { t.Helper() scheme := runtime.NewScheme() utilruntime.Must(clientgoscheme.AddToScheme(scheme)) diff --git a/cocoonset/reconcile_bench_test.go b/cocoonset/reconcile_bench_test.go new file mode 100644 index 0000000..8127dc1 --- /dev/null +++ b/cocoonset/reconcile_bench_test.go @@ -0,0 +1,136 @@ +package cocoonset + +import ( + "context" + "fmt" + "slices" + "sync" + "sync/atomic" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + + cocoonv1 "github.com/cocoonstack/cocoon-common/apis/v1" +) + +const ( + benchCocoonSets = 24 + benchRegistryDelay = 5 * time.Millisecond +) + +// BenchmarkReconcileThroughput drives independent CocoonSets whose reconciles +// each block on one fixed-latency registry probe, mirroring how the controller +// worker pool overlaps those waits. Concurrency 1 is the pre-change behavior. +func BenchmarkReconcileThroughput(b *testing.B) { + for _, concurrency := range []int{1, 4, 8} { + b.Run(fmt.Sprintf("concurrency=%d", concurrency), func(b *testing.B) { + var wall time.Duration + var latencies []time.Duration + for b.Loop() { + b.StopTimer() + r, sets := newBenchFixture(b) + b.StartTimer() + + start := time.Now() + latencies = append(latencies, runReconciles(b, r, sets, concurrency)...) + wall += time.Since(start) + } + slices.Sort(latencies) + b.ReportMetric(float64(wall.Milliseconds())/float64(b.N), "wall-ms/op") + b.ReportMetric(float64(percentile(latencies, 95).Milliseconds()), "p95-ms") + }) + } +} + +// runReconciles drains sets through a pool of `concurrency` workers, the shape +// controller-runtime gives MaxConcurrentReconciles, and returns per-reconcile +// latencies. +func runReconciles(b *testing.B, r *Reconciler, sets []*cocoonv1.CocoonSet, concurrency int) []time.Duration { + b.Helper() + queue := make(chan *cocoonv1.CocoonSet, len(sets)) + for _, cs := range sets { + queue <- cs + } + close(queue) + + latencies := make([]time.Duration, len(sets)) + var idx atomic.Int32 + var wg sync.WaitGroup + for range concurrency { + wg.Go(func() { + for cs := range queue { + start := time.Now() + if _, err := r.Reconcile(context.Background(), reqFor(cs)); err != nil { + b.Errorf("Reconcile %s: %v", cs.Name, err) + } + latencies[idx.Add(1)-1] = time.Since(start) + } + }) + } + wg.Wait() + return latencies +} + +// newBenchFixture builds independent CocoonSets each missing its main agent and +// carrying a hibernated CR, so every reconcile pays exactly one registry probe. +func newBenchFixture(b *testing.B) (*Reconciler, []*cocoonv1.CocoonSet) { + b.Helper() + scheme := testScheme(b) + objs := make([]client.Object, 0, benchCocoonSets*2) + sets := make([]*cocoonv1.CocoonSet, 0, benchCocoonSets) + for i := range benchCocoonSets { + cs := &cocoonv1.CocoonSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("bench-%d", i), + Namespace: "ns", + UID: types.UID(fmt.Sprintf("uid-%d", i)), + Finalizers: []string{finalizerName}, + }, + Spec: cocoonv1.CocoonSetSpec{ + Agent: cocoonv1.AgentSpec{Image: "ghcr.io/cocoonstack/cocoon/ubuntu:24.04"}, + }, + } + sets = append(sets, cs) + objs = append(objs, cs, &cocoonv1.CocoonHibernation{ + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("h-%d", i), Namespace: "ns"}, + Spec: cocoonv1.CocoonHibernationSpec{ + PodRef: cocoonv1.HibernationPodRef{Name: cs.Name + "-0"}, + Desire: cocoonv1.HibernationDesireHibernate, + }, + Status: cocoonv1.CocoonHibernationStatus{Phase: cocoonv1.CocoonHibernationPhaseHibernated}, + }) + } + cli := ctrlfake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objs...). + WithStatusSubresource(&cocoonv1.CocoonSet{}). + Build() + return &Reconciler{Client: cli, Scheme: scheme, Registry: &slowRegistry{delay: benchRegistryDelay}}, sets +} + +func percentile(sorted []time.Duration, p int) time.Duration { + if len(sorted) == 0 { + return 0 + } + i := min(len(sorted)*p/100, len(sorted)-1) + return sorted[i] +} + +// slowRegistry models a remote registry: every probe costs a fixed round trip. +type slowRegistry struct { + delay time.Duration +} + +func (s *slowRegistry) HasManifest(_ context.Context, _, _ string) (bool, error) { + time.Sleep(s.delay) + return false, nil +} + +func (s *slowRegistry) DeleteManifest(_ context.Context, _, _ string) error { + time.Sleep(s.delay) + return nil +} diff --git a/cocoonset/reconciler.go b/cocoonset/reconciler.go index 8057baa..f887abb 100644 --- a/cocoonset/reconciler.go +++ b/cocoonset/reconciler.go @@ -130,8 +130,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } return ctrl.Result{Requeue: true}, nil } + // One lazily-loaded CocoonHibernation List, shared by every create below. + intent := r.newRestoreIntent(cs.Namespace) if classified.main == nil { - return r.createMainAgent(ctx, &cs) + return r.createMainAgent(ctx, &cs, intent) } // Sub-agents fork from main and need it live before creation. @@ -143,11 +145,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu mainVMName := meta.ParseVMSpec(classified.main).VMName mainNodeName := classified.main.Spec.NodeName - subChanged, subRequeue, err := r.ensureSubAgents(ctx, &cs, classified, mainVMName, mainNodeName) + subChanged, subRequeue, err := r.ensureSubAgents(ctx, &cs, classified, mainVMName, mainNodeName, intent) if err != nil { return ctrl.Result{}, err } - tbChanged, err := r.ensureToolboxes(ctx, &cs, classified) + tbChanged, err := r.ensureToolboxes(ctx, &cs, classified, intent) if err != nil { return ctrl.Result{}, err } @@ -165,18 +167,18 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu // restore-from-hibernate when the agent is hibernated so a cross-node recreate // restores from the :hibernate snapshot instead of booting fresh. It always // requeues so sub-agents fork off the now-created main. -func (r *Reconciler) createMainAgent(ctx context.Context, cs *cocoonv1.CocoonSet) (ctrl.Result, error) { +func (r *Reconciler) createMainAgent(ctx context.Context, cs *cocoonv1.CocoonSet, intent *restoreIntent) (ctrl.Result, error) { logger := log.WithFunc("cocoonset.Reconciler.createMainAgent") mainPod, err := buildAgentPod(cs, 0, "", "", r.Scheme) if err != nil { return ctrl.Result{}, fmt.Errorf("build main agent: %w", err) } - restorable, err := r.podsRestorableByCR(ctx, cs.Namespace) + restorable, err := intent.resolve(ctx) if err != nil { return ctrl.Result{}, err } - _, intent := restorable[mainPod.Name] - if err := r.markRestoreIfHibernated(ctx, mainPod, intent); err != nil { + _, wantRestore := restorable[mainPod.Name] + if err := r.markRestoreIfHibernated(ctx, mainPod, wantRestore); err != nil { return ctrl.Result{}, err } if err := r.Create(ctx, mainPod); err != nil { diff --git a/cocoonset/reconciler_test.go b/cocoonset/reconciler_test.go index 7ac4ec3..04321b4 100644 --- a/cocoonset/reconciler_test.go +++ b/cocoonset/reconciler_test.go @@ -120,7 +120,7 @@ func TestEnsureToolboxesCollisionReturnsError(t *testing.T) { allByName: map[string]*corev1.Pod{agentPod.Name: agentPod}, } - _, err := r.ensureToolboxes(t.Context(), cs, classified) + _, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(cs.Namespace)) if err == nil { t.Fatal("ensureToolboxes should return error on name collision with agent pod") } @@ -143,7 +143,7 @@ func TestEnsureToolboxesRejectsDuplicateNames(t *testing.T) { allByName: map[string]*corev1.Pod{}, } - _, err := r.ensureToolboxes(t.Context(), cs, classified) + _, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(cs.Namespace)) if err == nil { t.Fatal("ensureToolboxes must reject a spec with duplicate toolbox names") } @@ -169,7 +169,7 @@ func TestEnsureToolboxesIdempotentOnExistingToolbox(t *testing.T) { allByName: map[string]*corev1.Pod{}, } - changed, err := r.ensureToolboxes(t.Context(), cs, classified) + changed, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(cs.Namespace)) if err != nil { t.Fatalf("ensureToolboxes: %v", err) } @@ -280,7 +280,7 @@ func TestEnsureSubAgentsReplacesTerminalPod(t *testing.T) { allByName: map[string]*corev1.Pod{subPod.Name: subPod}, } - changed, _, err := r.ensureSubAgents(t.Context(), cs, classified, "vk-ns-demo-0", "") + changed, _, err := r.ensureSubAgents(t.Context(), cs, classified, "vk-ns-demo-0", "", r.newRestoreIntent(cs.Namespace)) if err != nil { t.Fatalf("ensureSubAgents: %v", err) } @@ -378,7 +378,7 @@ func TestEnsureSubAgentsTreatsLifecycleFailedAsTerminal(t *testing.T) { allByName: map[string]*corev1.Pod{subPod.Name: subPod}, } - changed, _, err := r.ensureSubAgents(t.Context(), cs, classified, "vk-ns-demo-0", "") + changed, _, err := r.ensureSubAgents(t.Context(), cs, classified, "vk-ns-demo-0", "", r.newRestoreIntent(cs.Namespace)) if err != nil { t.Fatalf("ensureSubAgents: %v", err) } @@ -412,7 +412,7 @@ func TestEnsureToolboxesReplacesTerminalPod(t *testing.T) { allByName: map[string]*corev1.Pod{tbPod.Name: tbPod}, } - changed, err := r.ensureToolboxes(t.Context(), cs, classified) + changed, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(cs.Namespace)) if err != nil { t.Fatalf("ensureToolboxes: %v", err) } diff --git a/cocoonset/restore.go b/cocoonset/restore.go index 7e870f6..b605304 100644 --- a/cocoonset/restore.go +++ b/cocoonset/restore.go @@ -3,6 +3,7 @@ package cocoonset import ( "context" "fmt" + "sync" "github.com/projecteru2/core/log" corev1 "k8s.io/api/core/v1" @@ -12,6 +13,30 @@ import ( "github.com/cocoonstack/cocoon-common/meta" ) +// restoreIntent memoizes the namespace's restore-intent set for one reconcile. +// The List behind it is O(CocoonHibernations in the namespace), so the steady +// path (every desired pod already present) must never pay it, and the agent and +// toolbox passes must not pay it twice. +type restoreIntent struct { + load func(context.Context) (map[string]struct{}, error) + once sync.Once + names map[string]struct{} + err error +} + +// resolve loads the set on first call; safe under the sub-agent create fan-out. +func (ri *restoreIntent) resolve(ctx context.Context) (map[string]struct{}, error) { + ri.once.Do(func() { ri.names, ri.err = ri.load(ctx) }) + return ri.names, ri.err +} + +// newRestoreIntent defers the List until a pod actually has to be built. +func (r *Reconciler) newRestoreIntent(namespace string) *restoreIntent { + return &restoreIntent{load: func(ctx context.Context) (map[string]struct{}, error) { + return r.podsRestorableByCR(ctx, namespace) + }} +} + // hibernationPodNames lists the namespace's CocoonHibernations and returns the // set of PodRef names whose CR satisfies accept. func (r *Reconciler) hibernationPodNames(ctx context.Context, namespace string, accept func(*cocoonv1.CocoonHibernation) bool) (map[string]struct{}, error) { diff --git a/cocoonset/restore_intent_test.go b/cocoonset/restore_intent_test.go new file mode 100644 index 0000000..10d9c74 --- /dev/null +++ b/cocoonset/restore_intent_test.go @@ -0,0 +1,190 @@ +package cocoonset + +import ( + "context" + "sync/atomic" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + cocoonv1 "github.com/cocoonstack/cocoon-common/apis/v1" + "github.com/cocoonstack/cocoon-common/meta" +) + +// TestReconcileSteadyStateSkipsHibernationList pins the lazy load: with every +// desired pod already present, the reconcile must not pay the namespace-wide +// CocoonHibernation List. +func TestReconcileSteadyStateSkipsHibernationList(t *testing.T) { + scheme := testScheme(t) + cs := newCocoonSet("demo", func(cs *cocoonv1.CocoonSet) { + cs.Finalizers = []string{finalizerName} + cs.Spec.Agent.Replicas = 1 + cs.Spec.Toolboxes = []cocoonv1.ToolboxSpec{ + {Name: "tb", Image: "img", Mode: cocoonv1.ToolboxModeRun}, + } + }) + mainPod := readyPod(mustBuildAgentPod(t, cs, 0, "", "", scheme)) + subPod := readyPod(mustBuildAgentPod(t, cs, 1, "vk-ns-demo-0", "", scheme)) + tbPod := readyPod(mustBuildToolboxPod(t, cs, cs.Spec.Toolboxes[0], scheme)) + + var lists atomic.Int32 + cli := ctrlfake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(cs, mainPod, subPod, tbPod). + WithStatusSubresource(&cocoonv1.CocoonSet{}). + WithInterceptorFuncs(countHibernationLists(&lists)). + Build() + r := &Reconciler{Client: cli, Scheme: scheme, Registry: &fakeRegistry{}} + + if _, err := r.Reconcile(t.Context(), reqFor(cs)); err != nil { + t.Fatalf("Reconcile: %v", err) + } + // Guards the assertion below against a vacuous pass: 0 Lists would also + // follow from an early return or from triage deleting a drifted pod. + for _, p := range []*corev1.Pod{mainPod, subPod, tbPod} { + if err := cli.Get(t.Context(), client.ObjectKeyFromObject(p), &corev1.Pod{}); err != nil { + t.Fatalf("pod %s must survive a steady reconcile: %v", p.Name, err) + } + } + if got := lists.Load(); got != 0 { + t.Errorf("steady reconcile listed CocoonHibernations %d times, want 0", got) + } +} + +// TestReconcileMissingPodsListsHibernationsOnce pins the sharing: a reconcile +// missing both a sub-agent and a toolbox loads restore intent exactly once. +func TestReconcileMissingPodsListsHibernationsOnce(t *testing.T) { + scheme := testScheme(t) + cs := newCocoonSet("demo", func(cs *cocoonv1.CocoonSet) { + cs.Finalizers = []string{finalizerName} + cs.Spec.Agent.Replicas = 2 + cs.Spec.Toolboxes = []cocoonv1.ToolboxSpec{ + {Name: "tb", Image: "img", Mode: cocoonv1.ToolboxModeRun}, + } + }) + mainPod := readyPod(mustBuildAgentPod(t, cs, 0, "", "", scheme)) + + var lists atomic.Int32 + cli := ctrlfake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(cs, mainPod). + WithStatusSubresource(&cocoonv1.CocoonSet{}). + WithInterceptorFuncs(countHibernationLists(&lists)). + Build() + r := &Reconciler{Client: cli, Scheme: scheme, Registry: &fakeRegistry{}} + + if _, err := r.Reconcile(t.Context(), reqFor(cs)); err != nil { + t.Fatalf("Reconcile: %v", err) + } + if got := lists.Load(); got != 1 { + t.Errorf("reconcile listed CocoonHibernations %d times, want exactly 1", got) + } + for _, name := range []string{"demo-1", "demo-2", toolboxPodName(cs.Name, "tb")} { + if err := cli.Get(t.Context(), types.NamespacedName{Namespace: "ns", Name: name}, &corev1.Pod{}); err != nil { + t.Errorf("pod %s should have been created: %v", name, err) + } + } +} + +// TestRestoreIntentResolvesOnce guards the memo itself: the loader must run once +// however many creates ask for the set. +func TestRestoreIntentResolvesOnce(t *testing.T) { + var loads int + intent := &restoreIntent{load: func(context.Context) (map[string]struct{}, error) { + loads++ + return map[string]struct{}{"p": {}}, nil + }} + for range 3 { + got, err := intent.resolve(t.Context()) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if _, ok := got["p"]; !ok { + t.Fatal("resolve returned the wrong set") + } + } + if loads != 1 { + t.Errorf("loader ran %d times, want 1", loads) + } +} + +// TestReconcileUnrelatedKeyProgressesWhileProbeBlocks pins what concurrency +// buys: with one CocoonSet's registry probe wedged, a second must still finish. +// At MaxConcurrentReconciles=1 the worker pool would serialize them. +func TestReconcileUnrelatedKeyProgressesWhileProbeBlocks(t *testing.T) { + scheme := testScheme(t) + blocked, wedged := newCocoonSet("blocked", withMainRestore), newCocoonSet("free", withMainRestore) + release := make(chan struct{}) + t.Cleanup(func() { close(release) }) + + cli := ctrlfake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(blocked, wedged, hibernatedFor(blocked), hibernatedFor(wedged)). + WithStatusSubresource(&cocoonv1.CocoonSet{}). + Build() + r := &Reconciler{Client: cli, Scheme: scheme, Registry: &gatedRegistry{ + block: map[string]chan struct{}{meta.VMNameForPod("ns", "blocked-0"): release}, + }} + + go func() { _, _ = r.Reconcile(context.Background(), reqFor(blocked)) }() + + done := make(chan error, 1) + go func() { _, err := r.Reconcile(context.Background(), reqFor(wedged)); done <- err }() + select { + case err := <-done: + if err != nil { + t.Fatalf("unrelated reconcile: %v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("unrelated CocoonSet was blocked by another key's registry probe") + } +} + +func withMainRestore(cs *cocoonv1.CocoonSet) { cs.Finalizers = []string{finalizerName} } + +func hibernatedFor(cs *cocoonv1.CocoonSet) *cocoonv1.CocoonHibernation { + return &cocoonv1.CocoonHibernation{ + ObjectMeta: metav1.ObjectMeta{Name: "h-" + cs.Name, Namespace: cs.Namespace}, + Spec: cocoonv1.CocoonHibernationSpec{ + PodRef: cocoonv1.HibernationPodRef{Name: cs.Name + "-0"}, + Desire: cocoonv1.HibernationDesireHibernate, + }, + Status: cocoonv1.CocoonHibernationStatus{Phase: cocoonv1.CocoonHibernationPhaseHibernated}, + } +} + +// gatedRegistry wedges the probe for the named VMs until their channel closes. +type gatedRegistry struct { + block map[string]chan struct{} +} + +func (g *gatedRegistry) HasManifest(_ context.Context, name, _ string) (bool, error) { + if ch, ok := g.block[name]; ok { + <-ch + } + return false, nil +} + +func (g *gatedRegistry) DeleteManifest(context.Context, string, string) error { return nil } + +func countHibernationLists(counter *atomic.Int32) interceptor.Funcs { + return interceptor.Funcs{ + List: func(ctx context.Context, cli client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + if _, ok := list.(*cocoonv1.CocoonHibernationList); ok { + counter.Add(1) + } + return cli.List(ctx, list, opts...) + }, + } +} + +func reqFor(cs *cocoonv1.CocoonSet) ctrl.Request { + return ctrl.Request{NamespacedName: types.NamespacedName{Namespace: cs.Namespace, Name: cs.Name}} +} diff --git a/cocoonset/restore_test.go b/cocoonset/restore_test.go index 17816a7..35735ae 100644 --- a/cocoonset/restore_test.go +++ b/cocoonset/restore_test.go @@ -122,7 +122,7 @@ func TestEnsureToolboxesRestoresHibernated(t *testing.T) { Registry: &fakeRegistry{present: map[string]bool{tbVMName + ":hibernate": true}}, } - changed, err := r.ensureToolboxes(t.Context(), cs, classifyPods(nil)) + changed, err := r.ensureToolboxes(t.Context(), cs, classifyPods(nil), r.newRestoreIntent(cs.Namespace)) if err != nil { t.Fatalf("ensureToolboxes: %v", err) } diff --git a/cocoonset/toolboxes.go b/cocoonset/toolboxes.go index 52152f1..9b45dd7 100644 --- a/cocoonset/toolboxes.go +++ b/cocoonset/toolboxes.go @@ -16,7 +16,7 @@ import ( "github.com/cocoonstack/cocoon-common/meta" ) -func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet, classified classifiedPods) (bool, error) { +func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet, classified classifiedPods, intent *restoreIntent) (bool, error) { logger := log.WithFunc("cocoonset.Reconciler.ensureToolboxes") // Defense in depth: webhook should already reject duplicates. Validate // upfront before any Create/Delete so a bypass can't leave partial state. @@ -27,10 +27,6 @@ func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet } desired[tb.Name] = true } - restorable, err := r.podsRestorableByCR(ctx, cs.Namespace) - if err != nil { - return false, err - } changed := false for _, tb := range cs.Spec.Toolboxes { podName := toolboxPodName(cs.Name, tb.Name) @@ -51,8 +47,12 @@ func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet if err != nil { return changed, fmt.Errorf("build toolbox %s: %w", tb.Name, err) } - _, intent := restorable[tbPod.Name] - if err := r.markRestoreIfHibernated(ctx, tbPod, intent); err != nil { + restorable, err := intent.resolve(ctx) + if err != nil { + return changed, err + } + _, wantRestore := restorable[tbPod.Name] + if err := r.markRestoreIfHibernated(ctx, tbPod, wantRestore); err != nil { return changed, fmt.Errorf("mark restore toolbox %s: %w", tb.Name, err) } if err := r.Create(ctx, tbPod); err != nil { From c89808ccd1d155c54a311176b7490b8bc2ab6dfb Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 16 Jul 2026 00:01:26 +0800 Subject: [PATCH 3/8] review: /code + /simplify round - extract createSubAgents so the fan-out stops nesting inside ensureSubAgents and its goroutines no longer shadow a live outer err (nestif + govet shadow) - replace the hand-rolled sync.Once memo with stdlib sync.OnceValues - drop the dead empty-string branch in envInt; strconv.Atoi("") already errors - fold the benchmark and blocking-probe registry fakes into fakeRegistry --- cocoonset/agents.go | 84 +++++++++++++++++-------------- cocoonset/reconcile_bench_test.go | 26 +--------- cocoonset/reconciler.go | 6 +-- cocoonset/reconciler_test.go | 25 ++++++--- cocoonset/restore.go | 29 ++++------- cocoonset/restore_intent_test.go | 38 +------------- cocoonset/restore_test.go | 2 +- cocoonset/toolboxes.go | 4 +- main.go | 6 +-- 9 files changed, 82 insertions(+), 138 deletions(-) diff --git a/cocoonset/agents.go b/cocoonset/agents.go index 5ed5995..d176c15 100644 --- a/cocoonset/agents.go +++ b/cocoonset/agents.go @@ -32,7 +32,7 @@ const subAgentCreateConcurrency = 8 // (non-zero when a sub-agent is in rebuild backoff and the caller should // re-reconcile when backoff elapses). Missing slots are created concurrently // so batch scale-ups do not serialize N apiserver round trips. -func (r *Reconciler) ensureSubAgents(ctx context.Context, cs *cocoonv1.CocoonSet, classified classifiedPods, mainVMName, mainNodeName string, intent *restoreIntent) (bool, time.Duration, error) { +func (r *Reconciler) ensureSubAgents(ctx context.Context, cs *cocoonv1.CocoonSet, classified classifiedPods, mainVMName, mainNodeName string, intent restoreIntent) (bool, time.Duration, error) { logger := log.WithFunc("cocoonset.Reconciler.ensureSubAgents") changed := false var requeueAfter time.Duration @@ -56,43 +56,12 @@ func (r *Reconciler) ensureSubAgents(ctx context.Context, cs *cocoonv1.CocoonSet } } - if len(missing) > 0 { - // Resolved before the fan-out so the goroutines only ever read it. - restorable, err := intent.resolve(ctx) - if err != nil { - return changed, requeueAfter, err - } - g, gctx := errgroup.WithContext(ctx) - g.SetLimit(subAgentCreateConcurrency) - var created atomic.Bool - for _, slot := range missing { - g.Go(func() error { - subPod, err := buildAgentPod(cs, slot, mainVMName, mainNodeName, r.Scheme) - if err != nil { - return fmt.Errorf("build sub-agent slot %d: %w", slot, err) - } - _, wantRestore := restorable[subPod.Name] - if err := r.markRestoreIfHibernated(gctx, subPod, wantRestore); err != nil { - return fmt.Errorf("mark restore sub-agent slot %d: %w", slot, err) - } - if err := r.Create(gctx, subPod); err != nil { - if apierrors.IsAlreadyExists(err) { - return nil - } - return fmt.Errorf("create sub-agent slot %d: %w", slot, err) - } - logger.Infof(gctx, "created sub-agent %s/%s", subPod.Namespace, subPod.Name) - created.Store(true) - return nil - }) - } - err = g.Wait() - if created.Load() { - changed = true - } - if err != nil { - return changed, requeueAfter, err - } + created, err := r.createSubAgents(ctx, logger, cs, missing, mainVMName, mainNodeName, intent) + if created { + changed = true + } + if err != nil { + return changed, requeueAfter, err } for _, slot := range slices.Sorted(maps.Keys(classified.sub)) { @@ -112,6 +81,45 @@ func (r *Reconciler) ensureSubAgents(ctx context.Context, cs *cocoonv1.CocoonSet return changed, requeueAfter, nil } +// createSubAgents builds the missing slots concurrently so a batch scale-up does +// not serialize N apiserver round trips. Reports whether any pod was created. +func (r *Reconciler) createSubAgents(ctx context.Context, logger *log.Fields, cs *cocoonv1.CocoonSet, missing []int32, mainVMName, mainNodeName string, intent restoreIntent) (bool, error) { + if len(missing) == 0 { + return false, nil + } + // Resolved before the fan-out so the goroutines only ever read it. + restorable, err := intent() + if err != nil { + return false, err + } + g, gctx := errgroup.WithContext(ctx) + g.SetLimit(subAgentCreateConcurrency) + var created atomic.Bool + for _, slot := range missing { + g.Go(func() error { + subPod, err := buildAgentPod(cs, slot, mainVMName, mainNodeName, r.Scheme) + if err != nil { + return fmt.Errorf("build sub-agent slot %d: %w", slot, err) + } + _, wantRestore := restorable[subPod.Name] + if err := r.markRestoreIfHibernated(gctx, subPod, wantRestore); err != nil { + return fmt.Errorf("mark restore sub-agent slot %d: %w", slot, err) + } + if err := r.Create(gctx, subPod); err != nil { + if apierrors.IsAlreadyExists(err) { + return nil + } + return fmt.Errorf("create sub-agent slot %d: %w", slot, err) + } + logger.Infof(gctx, "created sub-agent %s/%s", subPod.Namespace, subPod.Name) + created.Store(true) + return nil + }) + } + waitErr := g.Wait() + return created.Load(), waitErr +} + // triageSubAgent deletes pod when it is terminal or has drifted from spec. // requeueAfter > 0 means the slot is in rebuild backoff and the caller should // re-reconcile when the window elapses; deleted=false with requeueAfter=0 diff --git a/cocoonset/reconcile_bench_test.go b/cocoonset/reconcile_bench_test.go index 8127dc1..777b854 100644 --- a/cocoonset/reconcile_bench_test.go +++ b/cocoonset/reconcile_bench_test.go @@ -95,21 +95,14 @@ func newBenchFixture(b *testing.B) (*Reconciler, []*cocoonv1.CocoonSet) { }, } sets = append(sets, cs) - objs = append(objs, cs, &cocoonv1.CocoonHibernation{ - ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("h-%d", i), Namespace: "ns"}, - Spec: cocoonv1.CocoonHibernationSpec{ - PodRef: cocoonv1.HibernationPodRef{Name: cs.Name + "-0"}, - Desire: cocoonv1.HibernationDesireHibernate, - }, - Status: cocoonv1.CocoonHibernationStatus{Phase: cocoonv1.CocoonHibernationPhaseHibernated}, - }) + objs = append(objs, cs, hibernatedFor(cs)) } cli := ctrlfake.NewClientBuilder(). WithScheme(scheme). WithObjects(objs...). WithStatusSubresource(&cocoonv1.CocoonSet{}). Build() - return &Reconciler{Client: cli, Scheme: scheme, Registry: &slowRegistry{delay: benchRegistryDelay}}, sets + return &Reconciler{Client: cli, Scheme: scheme, Registry: &fakeRegistry{delay: benchRegistryDelay}}, sets } func percentile(sorted []time.Duration, p int) time.Duration { @@ -119,18 +112,3 @@ func percentile(sorted []time.Duration, p int) time.Duration { i := min(len(sorted)*p/100, len(sorted)-1) return sorted[i] } - -// slowRegistry models a remote registry: every probe costs a fixed round trip. -type slowRegistry struct { - delay time.Duration -} - -func (s *slowRegistry) HasManifest(_ context.Context, _, _ string) (bool, error) { - time.Sleep(s.delay) - return false, nil -} - -func (s *slowRegistry) DeleteManifest(_ context.Context, _, _ string) error { - time.Sleep(s.delay) - return nil -} diff --git a/cocoonset/reconciler.go b/cocoonset/reconciler.go index f887abb..b1bb859 100644 --- a/cocoonset/reconciler.go +++ b/cocoonset/reconciler.go @@ -131,7 +131,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{Requeue: true}, nil } // One lazily-loaded CocoonHibernation List, shared by every create below. - intent := r.newRestoreIntent(cs.Namespace) + intent := r.newRestoreIntent(ctx, cs.Namespace) if classified.main == nil { return r.createMainAgent(ctx, &cs, intent) } @@ -167,13 +167,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu // restore-from-hibernate when the agent is hibernated so a cross-node recreate // restores from the :hibernate snapshot instead of booting fresh. It always // requeues so sub-agents fork off the now-created main. -func (r *Reconciler) createMainAgent(ctx context.Context, cs *cocoonv1.CocoonSet, intent *restoreIntent) (ctrl.Result, error) { +func (r *Reconciler) createMainAgent(ctx context.Context, cs *cocoonv1.CocoonSet, intent restoreIntent) (ctrl.Result, error) { logger := log.WithFunc("cocoonset.Reconciler.createMainAgent") mainPod, err := buildAgentPod(cs, 0, "", "", r.Scheme) if err != nil { return ctrl.Result{}, fmt.Errorf("build main agent: %w", err) } - restorable, err := intent.resolve(ctx) + restorable, err := intent() if err != nil { return ctrl.Result{}, err } diff --git a/cocoonset/reconciler_test.go b/cocoonset/reconciler_test.go index 04321b4..19200c9 100644 --- a/cocoonset/reconciler_test.go +++ b/cocoonset/reconciler_test.go @@ -6,6 +6,7 @@ import ( "slices" "sync" "testing" + "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -120,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(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 +144,7 @@ func TestEnsureToolboxesRejectsDuplicateNames(t *testing.T) { allByName: map[string]*corev1.Pod{}, } - _, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(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") } @@ -169,7 +170,7 @@ func TestEnsureToolboxesIdempotentOnExistingToolbox(t *testing.T) { allByName: map[string]*corev1.Pod{}, } - changed, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(cs.Namespace)) + changed, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) if err != nil { t.Fatalf("ensureToolboxes: %v", err) } @@ -280,7 +281,7 @@ func TestEnsureSubAgentsReplacesTerminalPod(t *testing.T) { allByName: map[string]*corev1.Pod{subPod.Name: subPod}, } - changed, _, err := r.ensureSubAgents(t.Context(), cs, classified, "vk-ns-demo-0", "", r.newRestoreIntent(cs.Namespace)) + changed, _, err := r.ensureSubAgents(t.Context(), cs, classified, "vk-ns-demo-0", "", r.newRestoreIntent(t.Context(), cs.Namespace)) if err != nil { t.Fatalf("ensureSubAgents: %v", err) } @@ -378,7 +379,7 @@ func TestEnsureSubAgentsTreatsLifecycleFailedAsTerminal(t *testing.T) { allByName: map[string]*corev1.Pod{subPod.Name: subPod}, } - changed, _, err := r.ensureSubAgents(t.Context(), cs, classified, "vk-ns-demo-0", "", r.newRestoreIntent(cs.Namespace)) + changed, _, err := r.ensureSubAgents(t.Context(), cs, classified, "vk-ns-demo-0", "", r.newRestoreIntent(t.Context(), cs.Namespace)) if err != nil { t.Fatalf("ensureSubAgents: %v", err) } @@ -412,7 +413,7 @@ func TestEnsureToolboxesReplacesTerminalPod(t *testing.T) { allByName: map[string]*corev1.Pod{tbPod.Name: tbPod}, } - changed, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(cs.Namespace)) + changed, err := r.ensureToolboxes(t.Context(), cs, classified, r.newRestoreIntent(t.Context(), cs.Namespace)) if err != nil { t.Fatalf("ensureToolboxes: %v", err) } @@ -663,8 +664,12 @@ func TestSetupWithManagerRejectsInvalidConcurrency(t *testing.T) { } type fakeRegistry struct { - present map[string]bool - probeErr error + present map[string]bool + probeErr error + // delay models a remote round trip; block wedges the named VM's probe + // until its channel closes. + delay time.Duration + block map[string]chan struct{} deletedMu sync.Mutex deleted []string } @@ -673,6 +678,10 @@ func (f *fakeRegistry) HasManifest(_ context.Context, name, tag string) (bool, e if f.probeErr != nil { return false, f.probeErr } + if ch, ok := f.block[name]; ok { + <-ch + } + time.Sleep(f.delay) return f.present[name+":"+tag], nil } diff --git a/cocoonset/restore.go b/cocoonset/restore.go index b605304..7d2124e 100644 --- a/cocoonset/restore.go +++ b/cocoonset/restore.go @@ -13,28 +13,17 @@ import ( "github.com/cocoonstack/cocoon-common/meta" ) -// restoreIntent memoizes the namespace's restore-intent set for one reconcile. -// The List behind it is O(CocoonHibernations in the namespace), so the steady -// path (every desired pod already present) must never pay it, and the agent and -// toolbox passes must not pay it twice. -type restoreIntent struct { - load func(context.Context) (map[string]struct{}, error) - once sync.Once - names map[string]struct{} - err error -} - -// resolve loads the set on first call; safe under the sub-agent create fan-out. -func (ri *restoreIntent) resolve(ctx context.Context) (map[string]struct{}, error) { - ri.once.Do(func() { ri.names, ri.err = ri.load(ctx) }) - return ri.names, ri.err -} +// restoreIntent returns the namespace's restore-intent set, loading it at most +// once however many pods ask. +type restoreIntent func() (map[string]struct{}, error) -// newRestoreIntent defers the List until a pod actually has to be built. -func (r *Reconciler) newRestoreIntent(namespace string) *restoreIntent { - return &restoreIntent{load: func(ctx context.Context) (map[string]struct{}, error) { +// newRestoreIntent defers the List until a pod actually has to be built: it is +// O(CocoonHibernations in the namespace), so the steady path must not pay it and +// the agent and toolbox passes must not pay it twice. +func (r *Reconciler) newRestoreIntent(ctx context.Context, namespace string) restoreIntent { + return sync.OnceValues(func() (map[string]struct{}, error) { return r.podsRestorableByCR(ctx, namespace) - }} + }) } // hibernationPodNames lists the namespace's CocoonHibernations and returns the diff --git a/cocoonset/restore_intent_test.go b/cocoonset/restore_intent_test.go index 10d9c74..318eec8 100644 --- a/cocoonset/restore_intent_test.go +++ b/cocoonset/restore_intent_test.go @@ -93,28 +93,6 @@ func TestReconcileMissingPodsListsHibernationsOnce(t *testing.T) { } } -// TestRestoreIntentResolvesOnce guards the memo itself: the loader must run once -// however many creates ask for the set. -func TestRestoreIntentResolvesOnce(t *testing.T) { - var loads int - intent := &restoreIntent{load: func(context.Context) (map[string]struct{}, error) { - loads++ - return map[string]struct{}{"p": {}}, nil - }} - for range 3 { - got, err := intent.resolve(t.Context()) - if err != nil { - t.Fatalf("resolve: %v", err) - } - if _, ok := got["p"]; !ok { - t.Fatal("resolve returned the wrong set") - } - } - if loads != 1 { - t.Errorf("loader ran %d times, want 1", loads) - } -} - // TestReconcileUnrelatedKeyProgressesWhileProbeBlocks pins what concurrency // buys: with one CocoonSet's registry probe wedged, a second must still finish. // At MaxConcurrentReconciles=1 the worker pool would serialize them. @@ -129,7 +107,7 @@ func TestReconcileUnrelatedKeyProgressesWhileProbeBlocks(t *testing.T) { WithObjects(blocked, wedged, hibernatedFor(blocked), hibernatedFor(wedged)). WithStatusSubresource(&cocoonv1.CocoonSet{}). Build() - r := &Reconciler{Client: cli, Scheme: scheme, Registry: &gatedRegistry{ + r := &Reconciler{Client: cli, Scheme: scheme, Registry: &fakeRegistry{ block: map[string]chan struct{}{meta.VMNameForPod("ns", "blocked-0"): release}, }} @@ -160,20 +138,6 @@ func hibernatedFor(cs *cocoonv1.CocoonSet) *cocoonv1.CocoonHibernation { } } -// gatedRegistry wedges the probe for the named VMs until their channel closes. -type gatedRegistry struct { - block map[string]chan struct{} -} - -func (g *gatedRegistry) HasManifest(_ context.Context, name, _ string) (bool, error) { - if ch, ok := g.block[name]; ok { - <-ch - } - return false, nil -} - -func (g *gatedRegistry) DeleteManifest(context.Context, string, string) error { return nil } - func countHibernationLists(counter *atomic.Int32) interceptor.Funcs { return interceptor.Funcs{ List: func(ctx context.Context, cli client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { diff --git a/cocoonset/restore_test.go b/cocoonset/restore_test.go index 35735ae..d246a70 100644 --- a/cocoonset/restore_test.go +++ b/cocoonset/restore_test.go @@ -122,7 +122,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(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 9b45dd7..c0ddf3c 100644 --- a/cocoonset/toolboxes.go +++ b/cocoonset/toolboxes.go @@ -16,7 +16,7 @@ import ( "github.com/cocoonstack/cocoon-common/meta" ) -func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet, classified classifiedPods, intent *restoreIntent) (bool, error) { +func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet, classified classifiedPods, intent restoreIntent) (bool, error) { logger := log.WithFunc("cocoonset.Reconciler.ensureToolboxes") // Defense in depth: webhook should already reject duplicates. Validate // upfront before any Create/Delete so a bypass can't leave partial state. @@ -47,7 +47,7 @@ func (r *Reconciler) ensureToolboxes(ctx context.Context, cs *cocoonv1.CocoonSet if err != nil { return changed, fmt.Errorf("build toolbox %s: %w", tb.Name, err) } - restorable, err := intent.resolve(ctx) + restorable, err := intent() if err != nil { return changed, err } diff --git a/main.go b/main.go index 0a52a5b..1c7479e 100644 --- a/main.go +++ b/main.go @@ -167,11 +167,7 @@ func buildScheme() *runtime.Scheme { // envInt parses an int env var, falling back when unset or invalid. func envInt(key string, fallback int) int { - v := os.Getenv(key) - if v == "" { - return fallback - } - n, err := strconv.Atoi(v) + n, err := strconv.Atoi(os.Getenv(key)) if err != nil { return fallback } From bc5a769126acffb3e7931e3ef8ec80ea94754d9f Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 16 Jul 2026 00:02:58 +0800 Subject: [PATCH 4/8] review: tighten comments to the load-bearing why Drop the ones restating a return value or repeating an adjacent godoc, and fold the rest to the one non-obvious constraint each carries. --- cocoonset/agents.go | 2 +- cocoonset/reconcile_bench_test.go | 10 ++++------ cocoonset/reconciler.go | 5 ++--- cocoonset/reconciler_test.go | 3 +-- cocoonset/restore.go | 6 ++---- cocoonset/restore_intent_test.go | 6 ++---- hibernation/reconciler.go | 4 ++-- main.go | 4 +--- 8 files changed, 15 insertions(+), 25 deletions(-) diff --git a/cocoonset/agents.go b/cocoonset/agents.go index d176c15..542b014 100644 --- a/cocoonset/agents.go +++ b/cocoonset/agents.go @@ -82,7 +82,7 @@ func (r *Reconciler) ensureSubAgents(ctx context.Context, cs *cocoonv1.CocoonSet } // createSubAgents builds the missing slots concurrently so a batch scale-up does -// not serialize N apiserver round trips. Reports whether any pod was created. +// not serialize N apiserver round trips. func (r *Reconciler) createSubAgents(ctx context.Context, logger *log.Fields, cs *cocoonv1.CocoonSet, missing []int32, mainVMName, mainNodeName string, intent restoreIntent) (bool, error) { if len(missing) == 0 { return false, nil diff --git a/cocoonset/reconcile_bench_test.go b/cocoonset/reconcile_bench_test.go index 777b854..07e553c 100644 --- a/cocoonset/reconcile_bench_test.go +++ b/cocoonset/reconcile_bench_test.go @@ -22,9 +22,8 @@ const ( benchRegistryDelay = 5 * time.Millisecond ) -// BenchmarkReconcileThroughput drives independent CocoonSets whose reconciles -// each block on one fixed-latency registry probe, mirroring how the controller -// worker pool overlaps those waits. Concurrency 1 is the pre-change behavior. +// BenchmarkReconcileThroughput drives independent CocoonSets that each block on +// one fixed-latency registry probe; concurrency 1 is the pre-change behavior. func BenchmarkReconcileThroughput(b *testing.B) { for _, concurrency := range []int{1, 4, 8} { b.Run(fmt.Sprintf("concurrency=%d", concurrency), func(b *testing.B) { @@ -46,9 +45,8 @@ func BenchmarkReconcileThroughput(b *testing.B) { } } -// runReconciles drains sets through a pool of `concurrency` workers, the shape -// controller-runtime gives MaxConcurrentReconciles, and returns per-reconcile -// latencies. +// runReconciles drains sets through a worker pool, the shape controller-runtime +// gives MaxConcurrentReconciles. func runReconciles(b *testing.B, r *Reconciler, sets []*cocoonv1.CocoonSet, concurrency int) []time.Duration { b.Helper() queue := make(chan *cocoonv1.CocoonSet, len(sets)) diff --git a/cocoonset/reconciler.go b/cocoonset/reconciler.go index b1bb859..a72e5e6 100644 --- a/cocoonset/reconciler.go +++ b/cocoonset/reconciler.go @@ -37,8 +37,8 @@ type Reconciler struct { Scheme *runtime.Scheme Registry snapshot.Registry Recorder record.EventRecorder - // Concurrency caps in-flight reconciles. Reconciles block on registry - // probes, so at 1 a single slow probe stalls every other CocoonSet. + // Concurrency caps in-flight reconciles; at 1 one slow registry probe + // stalls every other CocoonSet. Concurrency int } @@ -130,7 +130,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } return ctrl.Result{Requeue: true}, nil } - // One lazily-loaded CocoonHibernation List, shared by every create below. intent := r.newRestoreIntent(ctx, cs.Namespace) if classified.main == nil { return r.createMainAgent(ctx, &cs, intent) diff --git a/cocoonset/reconciler_test.go b/cocoonset/reconciler_test.go index 19200c9..ba18fb7 100644 --- a/cocoonset/reconciler_test.go +++ b/cocoonset/reconciler_test.go @@ -666,8 +666,7 @@ func TestSetupWithManagerRejectsInvalidConcurrency(t *testing.T) { type fakeRegistry struct { present map[string]bool probeErr error - // delay models a remote round trip; block wedges the named VM's probe - // until its channel closes. + // delay models a remote round trip; block wedges the named VM's probe. delay time.Duration block map[string]chan struct{} deletedMu sync.Mutex diff --git a/cocoonset/restore.go b/cocoonset/restore.go index 7d2124e..01d25ea 100644 --- a/cocoonset/restore.go +++ b/cocoonset/restore.go @@ -13,13 +13,11 @@ import ( "github.com/cocoonstack/cocoon-common/meta" ) -// restoreIntent returns the namespace's restore-intent set, loading it at most -// once however many pods ask. +// restoreIntent returns the namespace's restore-intent set, loaded at most once. type restoreIntent func() (map[string]struct{}, error) // newRestoreIntent defers the List until a pod actually has to be built: it is -// O(CocoonHibernations in the namespace), so the steady path must not pay it and -// the agent and toolbox passes must not pay it twice. +// O(CocoonHibernations in the namespace) and the steady path must not pay it. func (r *Reconciler) newRestoreIntent(ctx context.Context, namespace string) restoreIntent { return sync.OnceValues(func() (map[string]struct{}, error) { return r.podsRestorableByCR(ctx, namespace) diff --git a/cocoonset/restore_intent_test.go b/cocoonset/restore_intent_test.go index 318eec8..81060e4 100644 --- a/cocoonset/restore_intent_test.go +++ b/cocoonset/restore_intent_test.go @@ -19,8 +19,7 @@ import ( ) // TestReconcileSteadyStateSkipsHibernationList pins the lazy load: with every -// desired pod already present, the reconcile must not pay the namespace-wide -// CocoonHibernation List. +// desired pod present, the reconcile must not pay the CocoonHibernation List. func TestReconcileSteadyStateSkipsHibernationList(t *testing.T) { scheme := testScheme(t) cs := newCocoonSet("demo", func(cs *cocoonv1.CocoonSet) { @@ -94,8 +93,7 @@ func TestReconcileMissingPodsListsHibernationsOnce(t *testing.T) { } // TestReconcileUnrelatedKeyProgressesWhileProbeBlocks pins what concurrency -// buys: with one CocoonSet's registry probe wedged, a second must still finish. -// At MaxConcurrentReconciles=1 the worker pool would serialize them. +// buys: with one CocoonSet's probe wedged, a second must still finish. func TestReconcileUnrelatedKeyProgressesWhileProbeBlocks(t *testing.T) { scheme := testScheme(t) blocked, wedged := newCocoonSet("blocked", withMainRestore), newCocoonSet("free", withMainRestore) diff --git a/hibernation/reconciler.go b/hibernation/reconciler.go index 20c178a..ef6c9a2 100644 --- a/hibernation/reconciler.go +++ b/hibernation/reconciler.go @@ -57,8 +57,8 @@ type Reconciler struct { Scheme *runtime.Scheme Registry snapshot.Registry Recorder record.EventRecorder - // Concurrency caps in-flight reconciles. Reconciles block on registry - // pushes and deletes, so at 1 a single slow request stalls every other CR. + // Concurrency caps in-flight reconciles; at 1 one slow registry request + // stalls every other CR. Concurrency int // observed[UID] = last recorded Ready.LastTransitionTime, dedups diff --git a/main.go b/main.go index 1c7479e..5392374 100644 --- a/main.go +++ b/main.go @@ -44,9 +44,7 @@ const ( defaultProbeAddr = ":8081" leaderElectionID = "cocoon-operator.cocoonset.cocoonstack.io" - // defaultConcurrency stays well inside the client-go QPS budget: reconciles - // block on registry probes, so the win comes from overlapping those waits, - // not from saturating the apiserver. + // defaultConcurrency overlaps registry waits without straining client-go QPS. defaultConcurrency = 4 ) From 5c8518fcf95fddb3c03bf8146faf4be5c95aa8b8 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 16 Jul 2026 00:21:44 +0800 Subject: [PATCH 5/8] fix(hibernation): serialize CRs targeting one pod before raising concurrency Nothing rejects two CocoonHibernation CRs naming the same podRef, and the pod watcher fans an event out to every CR that targets it, so opposing Hibernate/Wake desires can both be live. Above one worker they interleave their patch of the shared hibernate annotation and their HasManifest / DeleteManifest of the one :hibernate tag, so a CR can settle Hibernated after the tag was already deleted. A per-podRef lock restores the ordering the single worker gave for free; distinct pods stay concurrent. reconcileDelete drops its always-zero ctrl.Result while here. Also document the two concurrency knobs and make the wedged-probe test wait for the probe to actually block, so it cannot pass under serialization. --- cocoonset/reconciler_test.go | 7 ++- cocoonset/restore_intent_test.go | 11 ++++- docs/configuration.md | 6 ++- hibernation/reconciler.go | 28 +++++++++-- hibernation/reconciler_test.go | 82 +++++++++++++++++++++++++++++++- 5 files changed, 125 insertions(+), 9 deletions(-) diff --git a/cocoonset/reconciler_test.go b/cocoonset/reconciler_test.go index ba18fb7..269999e 100644 --- a/cocoonset/reconciler_test.go +++ b/cocoonset/reconciler_test.go @@ -666,9 +666,11 @@ func TestSetupWithManagerRejectsInvalidConcurrency(t *testing.T) { type fakeRegistry struct { present map[string]bool probeErr error - // delay models a remote round trip; block wedges the named VM's probe. + // delay models a remote round trip; block wedges the named VM's probe and + // entered reports that the probe is actually wedged. delay time.Duration block map[string]chan struct{} + entered chan string deletedMu sync.Mutex deleted []string } @@ -678,6 +680,9 @@ func (f *fakeRegistry) HasManifest(_ context.Context, name, tag string) (bool, e return false, f.probeErr } if ch, ok := f.block[name]; ok { + if f.entered != nil { + f.entered <- name + } <-ch } time.Sleep(f.delay) diff --git a/cocoonset/restore_intent_test.go b/cocoonset/restore_intent_test.go index 81060e4..081bf68 100644 --- a/cocoonset/restore_intent_test.go +++ b/cocoonset/restore_intent_test.go @@ -105,11 +105,20 @@ func TestReconcileUnrelatedKeyProgressesWhileProbeBlocks(t *testing.T) { WithObjects(blocked, wedged, hibernatedFor(blocked), hibernatedFor(wedged)). WithStatusSubresource(&cocoonv1.CocoonSet{}). Build() + entered := make(chan string, 1) r := &Reconciler{Client: cli, Scheme: scheme, Registry: &fakeRegistry{ - block: map[string]chan struct{}{meta.VMNameForPod("ns", "blocked-0"): release}, + block: map[string]chan struct{}{meta.VMNameForPod("ns", "blocked-0"): release}, + entered: entered, }} go func() { _, _ = r.Reconcile(context.Background(), reqFor(blocked)) }() + // Wait until the probe is genuinely wedged; otherwise the second reconcile + // could finish first and pass even under full serialization. + select { + case <-entered: + case <-time.After(5 * time.Second): + t.Fatal("blocked reconcile never reached the registry probe") + } done := make(chan error, 1) go func() { _, err := r.Reconcile(context.Background(), reqFor(wedged)); done <- err }() diff --git a/docs/configuration.md b/docs/configuration.md index a75bf6e..8410146 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -8,5 +8,9 @@ | `METRICS_ADDR` | `:8080` | Prometheus listener | | `PROBE_ADDR` | `:8081` | healthz / readyz listener | | `LEADER_ELECT` | `true` | Enable leader election so only one replica reconciles | +| `COCOONSET_CONCURRENCY` | `4` | Maximum concurrent CocoonSet reconciles. Must be at least 1. | +| `HIBERNATION_CONCURRENCY` | `4` | Maximum concurrent CocoonHibernation reconciles. Must be at least 1. | -CLI flags (`--metrics-bind-address`, `--health-probe-bind-address`, `--leader-elect`) override the corresponding env var. +CLI flags (`--metrics-bind-address`, `--health-probe-bind-address`, `--leader-elect`, `--cocoonset-concurrency`, `--hibernation-concurrency`) override the corresponding env var. + +Reconciles block on registry round trips, so concurrency above 1 overlaps those waits across unrelated resources. Reconciles of one resource are never concurrent, and CocoonHibernation CRs that target the same pod are serialized against each other. diff --git a/hibernation/reconciler.go b/hibernation/reconciler.go index ef6c9a2..f681b8e 100644 --- a/hibernation/reconciler.go +++ b/hibernation/reconciler.go @@ -64,6 +64,13 @@ type Reconciler struct { // observed[UID] = last recorded Ready.LastTransitionTime, dedups // phase-exit observations against controller-runtime cache lag. observed sync.Map + + // podLocks[namespace/podRef] serializes the distinct CRs that may target one + // pod: controller-runtime only serializes a single key, so above one worker + // opposing Hibernate/Wake desires would interleave their patch of the shared + // hibernate annotation and their HasManifest/DeleteManifest of the one + // :hibernate tag. Different pods stay concurrent. + podLocks sync.Map } // SetupWithManager registers the reconciler with the controller manager. @@ -113,7 +120,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } if !hib.DeletionTimestamp.IsZero() { - return r.reconcileDelete(ctx, &hib) + return ctrl.Result{}, r.reconcileDelete(ctx, &hib) } if !controllerutil.ContainsFinalizer(&hib, finalizerName) { controllerutil.AddFinalizer(&hib, finalizerName) @@ -126,6 +133,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if hib.Spec.PodRef.Name == "" { return ctrl.Result{}, r.markFailed(ctx, &hib, "spec.podRef.name is required") } + defer r.lockPod(hib.Namespace, hib.Spec.PodRef.Name)() + var pod corev1.Pod err := r.Get(ctx, types.NamespacedName{Namespace: hib.Namespace, Name: hib.Spec.PodRef.Name}, &pod) if err != nil { @@ -155,8 +164,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } } +// lockPod locks the CR's target pod and returns its release. Entries are keyed +// by pod name, which is stable and bounded by the CocoonSet's pods, so they are +// never evicted: a live holder would otherwise keep a mutex that a later CR no +// longer shares. +func (r *Reconciler) lockPod(namespace, podName string) func() { + lock, _ := r.podLocks.LoadOrStore(meta.PodKey(namespace, podName), &sync.Mutex{}) + mu := lock.(*sync.Mutex) + mu.Lock() + return mu.Unlock +} + // reconcileDelete clears the :hibernate tag (if Status.VMName is set) and removes the finalizer. -func (r *Reconciler) reconcileDelete(ctx context.Context, hib *cocoonv1.CocoonHibernation) (ctrl.Result, error) { +func (r *Reconciler) reconcileDelete(ctx context.Context, hib *cocoonv1.CocoonHibernation) error { logger := log.WithFunc("hibernation.Reconciler.reconcileDelete") if r.Registry != nil && hib.Status.VMName != "" { if err := r.Registry.DeleteManifest(ctx, hib.Status.VMName, meta.HibernateSnapshotTag); err != nil { @@ -167,10 +187,10 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, hib *cocoonv1.CocoonHi if controllerutil.ContainsFinalizer(hib, finalizerName) { controllerutil.RemoveFinalizer(hib, finalizerName) if err := r.Update(ctx, hib); err != nil { - return ctrl.Result{}, fmt.Errorf("remove finalizer: %w", err) + return fmt.Errorf("remove finalizer: %w", err) } } - return ctrl.Result{}, nil + return nil } // hibernationsTargetingPod returns reconcile requests for every CocoonHibernation diff --git a/hibernation/reconciler_test.go b/hibernation/reconciler_test.go index 5d08dd4..da237f7 100644 --- a/hibernation/reconciler_test.go +++ b/hibernation/reconciler_test.go @@ -3,6 +3,8 @@ package hibernation import ( "context" "errors" + "sync" + "sync/atomic" "testing" "time" @@ -143,7 +145,7 @@ func TestReconcileDeleteClearsHibernateTagAndFinalizer(t *testing.T) { reg := &fakeRegistry{} r := &Reconciler{Client: cli, Scheme: scheme, Registry: reg} - if _, err := r.reconcileDelete(t.Context(), hib); err != nil { + if err := r.reconcileDelete(t.Context(), hib); err != nil { t.Fatalf("reconcileDelete: %v", err) } if !reg.deleteCalled { @@ -179,7 +181,7 @@ func TestReconcileDeleteSkipsTagWhenVMNameMissing(t *testing.T) { reg := &fakeRegistry{} r := &Reconciler{Client: cli, Scheme: scheme, Registry: reg} - if _, err := r.reconcileDelete(t.Context(), hib); err != nil { + if err := r.reconcileDelete(t.Context(), hib); err != nil { t.Fatalf("reconcileDelete: %v", err) } if reg.deleteCalled { @@ -761,6 +763,82 @@ func TestReconcilePendingWhenPodMissingVMName(t *testing.T) { } } +// TestReconcileSerializesCRsTargetingOnePod pins the pod lock: nothing stops two +// CRs from naming one pod with opposing desires, and above one worker they would +// otherwise interleave the shared hibernate annotation and :hibernate tag. +func TestReconcileSerializesCRsTargetingOnePod(t *testing.T) { + hib := func(name string, desire cocoonv1.HibernationDesire) *cocoonv1.CocoonHibernation { + return &cocoonv1.CocoonHibernation{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "ns", Finalizers: []string{finalizerName}}, + Spec: cocoonv1.CocoonHibernationSpec{ + Desire: desire, + PodRef: cocoonv1.HibernationPodRef{Name: "demo-0"}, + }, + } + } + // Running + a VMID is what drives the wake path all the way to DeleteManifest, + // so both desires actually reach the registry and can be observed racing. + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo-0", Namespace: "ns", + Annotations: map[string]string{ + meta.AnnotationVMName: "vk-ns-demo-0", + meta.AnnotationVMID: "vmid-1", + }, + }, + Status: corev1.PodStatus{ContainerStatuses: []corev1.ContainerStatus{ + {State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}}, + }}, + } + scheme := testScheme(t) + cli := ctrlfake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(hib("a", cocoonv1.HibernationDesireHibernate), hib("b", cocoonv1.HibernationDesireWake), pod). + WithStatusSubresource(&cocoonv1.CocoonHibernation{}). + Build() + reg := &concurrencyProbe{} + r := &Reconciler{Client: cli, Scheme: scheme, Registry: reg} + + var wg sync.WaitGroup + for _, name := range []string{"a", "b"} { + wg.Go(func() { + _, _ = r.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{Namespace: "ns", Name: name}, + }) + }) + } + wg.Wait() + if reg.maxInFlight.Load() > 1 { + t.Errorf("CRs targeting one pod ran %d registry calls in flight, want serialized", reg.maxInFlight.Load()) + } +} + +// concurrencyProbe records the peak number of registry calls in flight. +type concurrencyProbe struct { + inFlight atomic.Int32 + maxInFlight atomic.Int32 +} + +func (c *concurrencyProbe) HasManifest(context.Context, string, string) (bool, error) { + c.enter() + defer c.inFlight.Add(-1) + time.Sleep(20 * time.Millisecond) + return false, nil +} + +func (c *concurrencyProbe) DeleteManifest(context.Context, string, string) error { + c.enter() + defer c.inFlight.Add(-1) + time.Sleep(20 * time.Millisecond) + return nil +} + +func (c *concurrencyProbe) enter() { + if n := c.inFlight.Add(1); n > c.maxInFlight.Load() { + c.maxInFlight.Store(n) + } +} + // A nil manager suffices: the guard rejects before mgr is touched. func TestSetupWithManagerRejectsInvalidConcurrency(t *testing.T) { for _, n := range []int{0, -1} { From acde9a23446c8d671e4452d5842cf009df7bc4ee Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 16 Jul 2026 00:28:26 +0800 Subject: [PATCH 6/8] fix(hibernation): lock the pod ahead of the delete path, stripe the table reconcileDelete drops the same :hibernate tag a live CR on that pod probes, but the lock sat below the deletion fast path, so a deleting CR skipped it. Stripe podLocks into a fixed table: pod names come and go with CocoonSets, so a per-name map grows for the process lifetime; a stripe collision only costs two unrelated pods an occasional serialization. --- hibernation/reconciler.go | 36 ++++++++++++-------- hibernation/reconciler_test.go | 60 ++++++++++++++++++++++++++++++++-- 2 files changed, 81 insertions(+), 15 deletions(-) diff --git a/hibernation/reconciler.go b/hibernation/reconciler.go index f681b8e..f372c4f 100644 --- a/hibernation/reconciler.go +++ b/hibernation/reconciler.go @@ -4,6 +4,7 @@ package hibernation import ( "context" "fmt" + "hash/fnv" "sync" "time" @@ -45,6 +46,10 @@ const ( // finalizerName keeps the CR alive long enough to clear its :hibernate tag from the registry. finalizerName = "cocoonhibernation.cocoonset.cocoonstack.io/finalizer" + // podLockStripes bounds the per-pod lock table; 64 keeps collisions rare + // against realistic per-namespace pod counts. + podLockStripes = 64 + conditionReasonPending = "Pending" conditionReasonDone = "Done" conditionReasonFailed = "Failed" @@ -65,12 +70,11 @@ type Reconciler struct { // phase-exit observations against controller-runtime cache lag. observed sync.Map - // podLocks[namespace/podRef] serializes the distinct CRs that may target one - // pod: controller-runtime only serializes a single key, so above one worker - // opposing Hibernate/Wake desires would interleave their patch of the shared - // hibernate annotation and their HasManifest/DeleteManifest of the one - // :hibernate tag. Different pods stay concurrent. - podLocks sync.Map + // podLocks serializes the distinct CRs that may target one pod: controller- + // runtime only serializes a single key, so above one worker opposing + // Hibernate/Wake desires would interleave their patch of the shared hibernate + // annotation and their HasManifest/DeleteManifest of the one :hibernate tag. + podLocks [podLockStripes]sync.Mutex } // SetupWithManager registers the reconciler with the controller manager. @@ -119,6 +123,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, fmt.Errorf("get hibernation %s: %w", req.NamespacedName, err) } + // Ahead of the delete fast path: reconcileDelete drops the same :hibernate + // tag another CR on this pod may be probing. + if hib.Spec.PodRef.Name != "" { + defer r.lockPod(hib.Namespace, hib.Spec.PodRef.Name)() + } + if !hib.DeletionTimestamp.IsZero() { return ctrl.Result{}, r.reconcileDelete(ctx, &hib) } @@ -133,7 +143,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if hib.Spec.PodRef.Name == "" { return ctrl.Result{}, r.markFailed(ctx, &hib, "spec.podRef.name is required") } - defer r.lockPod(hib.Namespace, hib.Spec.PodRef.Name)() var pod corev1.Pod err := r.Get(ctx, types.NamespacedName{Namespace: hib.Namespace, Name: hib.Spec.PodRef.Name}, &pod) @@ -164,13 +173,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } } -// lockPod locks the CR's target pod and returns its release. Entries are keyed -// by pod name, which is stable and bounded by the CocoonSet's pods, so they are -// never evicted: a live holder would otherwise keep a mutex that a later CR no -// longer shares. +// lockPod locks the CR's target pod and returns its release. Striping keeps the +// table fixed-size: pod names come and go with CocoonSets, so a per-name map +// would grow for the process lifetime, and a collision only costs two unrelated +// pods an occasional serialization. func (r *Reconciler) lockPod(namespace, podName string) func() { - lock, _ := r.podLocks.LoadOrStore(meta.PodKey(namespace, podName), &sync.Mutex{}) - mu := lock.(*sync.Mutex) + h := fnv.New32a() + _, _ = h.Write([]byte(meta.PodKey(namespace, podName))) + mu := &r.podLocks[h.Sum32()%podLockStripes] mu.Lock() return mu.Unlock } diff --git a/hibernation/reconciler_test.go b/hibernation/reconciler_test.go index da237f7..2cb2f6f 100644 --- a/hibernation/reconciler_test.go +++ b/hibernation/reconciler_test.go @@ -813,6 +813,56 @@ func TestReconcileSerializesCRsTargetingOnePod(t *testing.T) { } } +// TestReconcileSerializesDeletingCRAgainstLiveCR covers the delete fast path: +// reconcileDelete drops the same :hibernate tag a live CR on that pod probes, so +// it must take the pod lock too rather than return ahead of it. +func TestReconcileSerializesDeletingCRAgainstLiveCR(t *testing.T) { + deleting := &cocoonv1.CocoonHibernation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "a", Namespace: "ns", + Finalizers: []string{finalizerName}, + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + }, + Spec: cocoonv1.CocoonHibernationSpec{ + Desire: cocoonv1.HibernationDesireHibernate, + PodRef: cocoonv1.HibernationPodRef{Name: "demo-0"}, + }, + Status: cocoonv1.CocoonHibernationStatus{VMName: "vk-ns-demo-0"}, + } + live := &cocoonv1.CocoonHibernation{ + ObjectMeta: metav1.ObjectMeta{Name: "b", Namespace: "ns", Finalizers: []string{finalizerName}}, + Spec: cocoonv1.CocoonHibernationSpec{ + Desire: cocoonv1.HibernationDesireHibernate, + PodRef: cocoonv1.HibernationPodRef{Name: "demo-0"}, + }, + } + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Name: "demo-0", Namespace: "ns", + Annotations: map[string]string{meta.AnnotationVMName: "vk-ns-demo-0"}, + }} + scheme := testScheme(t) + cli := ctrlfake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(deleting, live, pod). + WithStatusSubresource(&cocoonv1.CocoonHibernation{}). + Build() + reg := &concurrencyProbe{} + r := &Reconciler{Client: cli, Scheme: scheme, Registry: reg} + + var wg sync.WaitGroup + for _, name := range []string{"a", "b"} { + wg.Go(func() { + _, _ = r.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{Namespace: "ns", Name: name}, + }) + }) + } + wg.Wait() + if reg.maxInFlight.Load() > 1 { + t.Errorf("deleting CR ran %d registry calls in flight with a live CR on the same pod, want serialized", reg.maxInFlight.Load()) + } +} + // concurrencyProbe records the peak number of registry calls in flight. type concurrencyProbe struct { inFlight atomic.Int32 @@ -833,9 +883,15 @@ func (c *concurrencyProbe) DeleteManifest(context.Context, string, string) error return nil } +// enter raises the peak by CAS: a plain load-then-store lets a later, smaller +// caller overwrite a peak of 2 with 1 and pass the test falsely. func (c *concurrencyProbe) enter() { - if n := c.inFlight.Add(1); n > c.maxInFlight.Load() { - c.maxInFlight.Store(n) + n := c.inFlight.Add(1) + for { + peak := c.maxInFlight.Load() + if n <= peak || c.maxInFlight.CompareAndSwap(peak, n) { + return + } } } From b3414fe22048b6b9c0ca8747bf511c63692ff8a2 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 16 Jul 2026 00:36:56 +0800 Subject: [PATCH 7/8] review: tighten the pod-lock comments to their one constraint --- hibernation/reconciler.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/hibernation/reconciler.go b/hibernation/reconciler.go index f372c4f..f0dba74 100644 --- a/hibernation/reconciler.go +++ b/hibernation/reconciler.go @@ -70,10 +70,9 @@ type Reconciler struct { // phase-exit observations against controller-runtime cache lag. observed sync.Map - // podLocks serializes the distinct CRs that may target one pod: controller- - // runtime only serializes a single key, so above one worker opposing - // Hibernate/Wake desires would interleave their patch of the shared hibernate - // annotation and their HasManifest/DeleteManifest of the one :hibernate tag. + // podLocks serializes the distinct CRs that may target one pod: above one + // worker their opposing desires would race the shared hibernate annotation + // and the one :hibernate tag. podLocks [podLockStripes]sync.Mutex } @@ -173,10 +172,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } } -// lockPod locks the CR's target pod and returns its release. Striping keeps the -// table fixed-size: pod names come and go with CocoonSets, so a per-name map -// would grow for the process lifetime, and a collision only costs two unrelated -// pods an occasional serialization. +// lockPod locks the CR's target pod and returns its release. Striping bounds the +// table because pod names come and go with CocoonSets; a collision only costs two +// unrelated pods an occasional serialization. func (r *Reconciler) lockPod(namespace, podName string) func() { h := fnv.New32a() _, _ = h.Write([]byte(meta.PodKey(namespace, podName))) From 4f46722ac938c58ce5deff982160514db54da824 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 16 Jul 2026 01:08:39 +0800 Subject: [PATCH 8/8] fix(hibernation): key the lock on the VM, not on the mutable podRef spec.podRef.name carries no immutability constraint (the CRD has only minLength), and markPending never rewrites Status.VMName, so a retargeted CR keeps naming its old VM in status. reconcileDelete drops the tag of that stale VM, so keying the lock on spec.podRef locked the new pod while touching the old VM's tag, leaving a live CR on the old pod free to probe the tag being deleted. Lock the VM whose tag and pod annotation are actually being touched: the resolved vmName on the live paths, Status.VMName on the delete path. --- hibernation/reconciler.go | 39 +++++++++++++------------ hibernation/reconciler_test.go | 53 ++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 19 deletions(-) diff --git a/hibernation/reconciler.go b/hibernation/reconciler.go index f0dba74..6558cd3 100644 --- a/hibernation/reconciler.go +++ b/hibernation/reconciler.go @@ -46,9 +46,9 @@ const ( // finalizerName keeps the CR alive long enough to clear its :hibernate tag from the registry. finalizerName = "cocoonhibernation.cocoonset.cocoonstack.io/finalizer" - // podLockStripes bounds the per-pod lock table; 64 keeps collisions rare - // against realistic per-namespace pod counts. - podLockStripes = 64 + // vmLockStripes bounds the per-VM lock table; 64 keeps collisions rare + // against realistic per-namespace VM counts. + vmLockStripes = 64 conditionReasonPending = "Pending" conditionReasonDone = "Done" @@ -70,10 +70,10 @@ type Reconciler struct { // phase-exit observations against controller-runtime cache lag. observed sync.Map - // podLocks serializes the distinct CRs that may target one pod: above one - // worker their opposing desires would race the shared hibernate annotation - // and the one :hibernate tag. - podLocks [podLockStripes]sync.Mutex + // vmLocks serializes the distinct CRs that may reach one VM: above one worker + // their opposing desires would race that VM's :hibernate tag and its pod's + // hibernate annotation. + vmLocks [vmLockStripes]sync.Mutex } // SetupWithManager registers the reconciler with the controller manager. @@ -122,12 +122,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, fmt.Errorf("get hibernation %s: %w", req.NamespacedName, err) } - // Ahead of the delete fast path: reconcileDelete drops the same :hibernate - // tag another CR on this pod may be probing. - if hib.Spec.PodRef.Name != "" { - defer r.lockPod(hib.Namespace, hib.Spec.PodRef.Name)() - } - if !hib.DeletionTimestamp.IsZero() { return ctrl.Result{}, r.reconcileDelete(ctx, &hib) } @@ -159,6 +153,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu // VMName is filled by vk-cocoon once the VM is provisioned; wait. return ctrl.Result{RequeueAfter: requeueInterval}, r.markPending(ctx, &hib, fmt.Sprintf("pod %s/%s has no %s annotation yet", pod.Namespace, pod.Name, meta.AnnotationVMName)) } + defer r.lockVM(vmName)() logger.Debugf(ctx, "reconcile hibernation %s/%s desire=%s vm=%s", hib.Namespace, hib.Name, hib.Spec.Desire, vmName) @@ -172,13 +167,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } } -// lockPod locks the CR's target pod and returns its release. Striping bounds the -// table because pod names come and go with CocoonSets; a collision only costs two -// unrelated pods an occasional serialization. -func (r *Reconciler) lockPod(namespace, podName string) func() { +// lockVM locks the VM whose :hibernate tag and pod annotation are about to be +// touched, and returns its release. Striping bounds the table because VM names +// come and go with CocoonSets; a collision only costs two unrelated VMs an +// occasional serialization. +func (r *Reconciler) lockVM(vmName string) func() { h := fnv.New32a() - _, _ = h.Write([]byte(meta.PodKey(namespace, podName))) - mu := &r.podLocks[h.Sum32()%podLockStripes] + _, _ = h.Write([]byte(vmName)) + mu := &r.vmLocks[h.Sum32()%vmLockStripes] mu.Lock() return mu.Unlock } @@ -186,6 +182,11 @@ func (r *Reconciler) lockPod(namespace, podName string) func() { // reconcileDelete clears the :hibernate tag (if Status.VMName is set) and removes the finalizer. func (r *Reconciler) reconcileDelete(ctx context.Context, hib *cocoonv1.CocoonHibernation) error { logger := log.WithFunc("hibernation.Reconciler.reconcileDelete") + // spec.podRef is mutable, so a retargeted CR still owns the tag its status + // names: lock that VM, not whichever pod spec points at now. + if hib.Status.VMName != "" { + defer r.lockVM(hib.Status.VMName)() + } if r.Registry != nil && hib.Status.VMName != "" { if err := r.Registry.DeleteManifest(ctx, hib.Status.VMName, meta.HibernateSnapshotTag); err != nil { logger.Errorf(ctx, err, "delete hibernate snapshot %s", hib.Status.VMName) diff --git a/hibernation/reconciler_test.go b/hibernation/reconciler_test.go index 2cb2f6f..c36d893 100644 --- a/hibernation/reconciler_test.go +++ b/hibernation/reconciler_test.go @@ -863,6 +863,59 @@ func TestReconcileSerializesDeletingCRAgainstLiveCR(t *testing.T) { } } +// TestReconcileSerializesRetargetedCRAgainstItsStaleVM covers the podRef/status +// split: spec.podRef is mutable and markPending never rewrites Status.VMName, so +// a CR retargeted to demo-1 still deletes demo-0's tag on the way out. Keying the +// lock on spec.podRef would lock demo-1 while touching demo-0's tag, leaving a +// live CR on demo-0 free to probe the tag being deleted. +func TestReconcileSerializesRetargetedCRAgainstItsStaleVM(t *testing.T) { + retargeted := &cocoonv1.CocoonHibernation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "a", Namespace: "ns", + Finalizers: []string{finalizerName}, + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + }, + Spec: cocoonv1.CocoonHibernationSpec{ + Desire: cocoonv1.HibernationDesireHibernate, + PodRef: cocoonv1.HibernationPodRef{Name: "demo-1"}, + }, + // Status still names demo-0's VM: the tag this CR will delete. + Status: cocoonv1.CocoonHibernationStatus{VMName: "vk-ns-demo-0"}, + } + live := &cocoonv1.CocoonHibernation{ + ObjectMeta: metav1.ObjectMeta{Name: "b", Namespace: "ns", Finalizers: []string{finalizerName}}, + Spec: cocoonv1.CocoonHibernationSpec{ + Desire: cocoonv1.HibernationDesireHibernate, + PodRef: cocoonv1.HibernationPodRef{Name: "demo-0"}, + }, + } + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{ + Name: "demo-0", Namespace: "ns", + Annotations: map[string]string{meta.AnnotationVMName: "vk-ns-demo-0"}, + }} + scheme := testScheme(t) + cli := ctrlfake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(retargeted, live, pod). + WithStatusSubresource(&cocoonv1.CocoonHibernation{}). + Build() + reg := &concurrencyProbe{} + r := &Reconciler{Client: cli, Scheme: scheme, Registry: reg} + + var wg sync.WaitGroup + for _, name := range []string{"a", "b"} { + wg.Go(func() { + _, _ = r.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{Namespace: "ns", Name: name}, + }) + }) + } + wg.Wait() + if reg.maxInFlight.Load() > 1 { + t.Errorf("retargeted CR ran %d registry calls in flight against its stale VM, want serialized", reg.maxInFlight.Load()) + } +} + // concurrencyProbe records the peak number of registry calls in flight. type concurrencyProbe struct { inFlight atomic.Int32