Skip to content

kubernetes-specialist: add read-after-write cache-lag race to review checklist #46

Description

@bdchatham

Background

While reviewing sei-k8s-controller PR #216 (replace-pod task for chain-upgrade rollouts), the kubernetes-specialist agent did two full review rounds without catching a cache-lag race that Cursor Bugbot subsequently flagged on its first pass.

The race:

  1. apply-statefulset task SSA-writes the StatefulSet.
  2. Same reconcile, replace-pod (and later observe-image) reads the StatefulSet via the cached client (mgr.GetClient()).
  3. The controller-runtime informer cache lags the apiserver by milliseconds-to-unbounded; the read returns the pre-apply spec/status.
  4. replace-pod sees CurrentRevision == UpdateRevision, no-ops, marks itself complete; rollout deadlocks.

This is a high-severity correctness bug in any executor-driven plan where one task writes a resource and a later task in the same reconcile reads it back. The fix was to plumb mgr.GetAPIReader() through ExecutionConfig and use it for read-after-write reads.

Gap

The agent's current heuristics catch ownership refs, RBAC verbs, generation/observed-generation freshness, multi-replica safety, idempotency. It does not explicitly look for read-after-write patterns through the cached client.

This is a class of bug, not a one-off:

  • Any executor / plan runner / saga where step N+1 reads what step N just wrote.
  • Any reconcile loop that issues SSA then re-reads to make a decision.
  • Any handler that calls client.Update/Patch then immediately client.Get to observe the result.

The cached client is the right default for top-of-reconcile reads (the watch event woke us up, so the cache is already authoritative for that object). It is the wrong choice for reads of objects we just mutated in the same reconcile pass.

Proposed update to .claude/agents/kubernetes-specialist.md

Add a review-checklist section (or extend Domain Expertise) that calls out the two-client model:

controller-runtime client semanticsmgr.GetClient() reads from the informer cache (eventually consistent; lags writes). mgr.GetAPIReader() reads strongly from the apiserver. When reviewing a reconciler or task executor, identify every Get/List. For each one, ask: is this read of a resource that this same reconcile pass already wrote (via Create/Update/Patch/Apply)? If yes, the cached client is unsafe — the read can return the pre-write state. Require APIReader for read-after-write in the same reconcile, or restructure so the read happens on a subsequent reconcile (after the watch event fires).

Concrete checks the agent should run when reviewing a controller PR:

  1. Grep the diff for Apply, Patch, Update, Create, Delete calls — these mark write points.
  2. For each write target, grep for Get/List calls on the same kind in tasks that run after the write within the same reconcile (look at planner ordering, executor loop structure).
  3. Flag any such read-after-write that uses cfg.KubeClient / r.Client rather than an APIReader.
  4. Note that fake-client unit tests cannot catch this — sigs.k8s.io/controller-runtime/pkg/client/fake is synchronous and has no informer cache. Recommend envtest coverage when the fix involves cache semantics.

Evidence trail

  • PR: feat(planner): add replace-pod task to NodeUpdate plan (closes #211) sei-k8s-controller#216
  • Bugbot finding: race condition where replace-pod always no-ops due to stale STS status from cached client
  • Fix commit: plumbed APIReader through ExecutionConfig, switched replace-pod and observe-image STS reads to APIReader.Get
  • Two prior agent reviews (rounds 2 and 3 of 4) did not surface this; round 4 confirmed the fix only after the user explicitly raised it

Out of scope

  • Generic stale-cache discussions in pure-watch reconcilers (the cache is correct there by design).
  • Optimistic-concurrency patches on .status (already covered by the sei-k8s-controller CLAUDE.md).

Acceptance

A future review of a same-reconcile read-after-write through mgr.GetClient() is flagged by the agent on the first pass, with a recommendation to use APIReader or defer the read to the next reconcile.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions