Executor status: concurrent writes from multiple replicas (no leader election, no predicate)

[fseldow]

Summary

In Ratify v2, the only writer of Executor.status is ExecutorReconciler.updateStatus (internal/controller/executor_controller.go), invoked at the end of every Reconcile. The controller manager is started without leader election and the watch is a bare For(&configv2alpha1.Executor{}).Complete(r) (no predicates). This is safe at the default replicas: 1, but becomes incorrect as soon as the deployment is scaled to multiple replicas: every pod runs its own ExecutorReconciler and they all write the same Executor.status concurrently.

Current behavior (confirmed in code, main)

• internal/manager/manager.go: ctrl.NewManager(..., ctrl.Options{Scheme: scheme}) — LeaderElection defaults to false. A repo-wide grep for leaderelection returns no hits.
• setupCRDControllers unconditionally sets up ExecutorReconciler on every pod (unless --disable-crd-manager, off by default).
• SetupWithManager: For(&Executor{}).Complete(r) — no GenerationChangedPredicate, no Owns, no Watches.
• updateStatus: on conflict, r.Status().Update error is only logged — not requeued/retried.
• ExecutorStatus is a single flat record (Succeeded, Error, BriefError); BriefError is currently never populated.
• Deployment defaults: config/manager/manager.yaml and Helm chart both default to replicas: 1.

Problems when replicaCount > 1

1. Concurrent status writers / write conflicts. N pods issue Status().Update() on the same object. apiserver optimistic concurrency (resourceVersion) lets one win; the others get HTTP 409. Because 409 is swallowed (no retry), those updates are silently lost.
2. Feedback loop, amplified ×N. With no GenerationChangedPredicate, each successful status write is itself an update event that re-triggers Reconcile on all pods → rebuild → write again. Multiple replicas multiply the write amplification.
3. Rebuild storms hit external providers. Every re-triggered reconcile calls refreshExecutor() → recreates key providers. The Azure Key Vault provider eagerly fetches certificates on init, so the loop repeatedly calls AKV (throttling / auth pressure).
4. Single field cannot represent N pods. succeeded/error is last-writer-wins. If pod-2 can't reach AKV but pod-1/3 can, status flaps between succeeded:true and succeeded:false+error depending on who wrote last — you cannot tell "2 of 5 replicas are unhealthy".

Note: deterministic config errors (e.g. the leaf-cert rejection added in #2723) are identical across pods and don't flap. The flapping/races come from (a) the feedback loop and (b) environment-dependent runtime failures (e.g. AKV reachability).

What triggers a status write today

Scenario	Should write?	Currently writes?	Multi-replica impact
Executor spec change (kubectl apply)	✅ yes	✅ yes	×N writers, 409 races
CR create	✅ (initial)	✅ yes	×N
CR delete	n/a (object gone)	❌ no (deleteExecutor path returns before updateStatus)	—
Status-update feedback loop (no predicate)	❌ no	⚠️ yes	×N amplification
Informer resync (~10h default)	❌ no	⚠️ yes	×N periodic rewrite
Pod start / restart / rollout / scale-up	❌ no	⚠️ yes	each new pod rewrites
Data-plane verify request	❌ no	❌ no (status never reflects verify results)	—
AKV cert rotation / expiry	(arguably yes)	❌ no (providers fetch once at init, never refresh)	—

Only the first two rows are "meaningful"; the rest are noise that multiplies with replicas.

Why it hasn't surfaced

Everything ships with replicas: 1, so there is exactly one writer — no conflicts, no flapping. The issue only manifests on horizontal scale-out.

Prior art: Gatekeeper's per-pod status pattern

Gatekeeper solves exactly this. Instead of many pods writing one shared status, each pod writes its own dedicated *PodStatus object, and a status controller aggregates them back into the parent's status.byPod[]:

• Per-pod CRDs: ConstraintPodStatus, ConstraintTemplatePodStatus, ConfigPodStatus, etc.
• Object name embeds pod identity (DashPacker(podName, resourceName)) → no two pods share an object → no write conflict.
• SetOwnerReference(pod, podStatus) → when a pod is deleted, its PodStatus is garbage-collected automatically.
• Per-pod status carries id (pod), observedGeneration, operations, errors.
• A status controller watches all PodStatus, lists-by-label, and rebuilds status.byPod[] on the parent (full rebuild is idempotent; a deleted pod's entry simply disappears). Conflicts on the parent update are requeued, not swallowed.

When a pod is deleted, the deleted pod does nothing — cleanup is (1) owner-ref GC removing its PodStatus, (2) a surviving controller re-aggregating byPod.

Proposed remediation (incremental)

1. Minimal / immediate: add builder.WithPredicates(predicate.GenerationChangedPredicate{}) to the For(&Executor{}) watch so status-only updates stop re-triggering reconcile. Cuts the feedback loop (and the AKV rebuild storm) with the smallest change.
2. Fix conflict handling: wrap Status().Update with retry.RetryOnConflict instead of logging-and-dropping.
3. Separate control-plane vs data-plane status responsibility: each pod still builds its in-memory executor (data plane needs it), but the writing of CR status should be leader-gated (enable leader election, or a leader-only runnable) so there's a single writer.
4. Per-pod health → readiness probe + metrics, not CR status. A pod that can't build its executor (invalid cert / AKV unreachable) should fail readiness and drop out of Service endpoints (readiness/liveness probes were just added for the gatekeeper provider in feat: add liveness and readiness probes to gatekeeper provider  #2772). Reserve CR status for deterministic config validity (with observedGeneration + conditions), which is pod-independent.
5. Longer term (if per-replica health must be surfaced in the API): adopt the Gatekeeper pattern — an ExecutorPodStatus CRD (name <pod>-<executor>, owner-ref → pod) written only by its own pod, aggregated into Executor.status.byPod[] by a status controller.

Environment

• Repo: notaryproject/ratify (v2, main)
• Files: internal/controller/executor_controller.go, internal/manager/manager.go, api/v2alpha1/executor_types.go