shouldSkipStatefulSetApply generation gate defeated by live-updating RunConfig ConfigMap during proxy runner rolling update (regression in v0.28.1)

[aryanxsk22]

Summary

After upgrading to v0.28.1, MCP workload pods consistently end up running stale images/config after a helm
upgrade. The generation gate introduced in #5024 does not prevent this because the RunConfig ConfigMap is a
live-updating volume mount — all proxy runner pods (old and new) converge on the same generation value,
neutralising the gate.

Environment

• Toolhive operator: ghcr.io/stacklok/toolhive/operator:v0.28.1
• Toolhive proxyrunner: ghcr.io/stacklok/toolhive/proxyrunner:v0.28.1
• Kubernetes: v1.32.2 (k3s)
• Helm chart: toolhive-operator 0.28.1

Steps to Reproduce

1. Deploy an MCPServer via helm. Confirm everything is healthy (MCPServer generation=1, StatefulSet
   mcpserver-generation annotation=1, workload pod running).
2. Perform a helm upgrade that changes the MCPServer spec.image to a new tag.
3. Observe the proxy runner Deployment begin a rolling update (old RS pod and new RS pod coexist briefly).
4. Observe the workload StatefulSet cycling between ControllerRevisions — alternating between the new image
   and the old image — until the workload pod lands in ImagePullBackOff or ErrImagePull on whichever revision
   lost the final race.

Root Cause

In v0.28.1, mcpserver_generation is read from the RunConfig ConfigMap mounted at
/etc/runconfig/runconfig.json. Kubernetes ConfigMap volume mounts are live-updated — within ~30–60 seconds all
pods mounting the same ConfigMap see the updated contents.

shouldSkipStatefulSetApply skips if theirsGen > ourGen. Since both pods now read the same generation N from
the live-updated ConfigMap, N > N is false — neither pod is gated. They race:

• New proxy runner applies the StatefulSet with new-image-tag + generation annotation N
• Old proxy runner (still reading gen=N, still holding old-image-tag in its CLI arg) overwrites it back to
  old-image-tag + generation annotation N

The result is rapid alternating ControllerRevisions. The workload pod gets stuck on whichever revision it was
last assigned.

This was confirmed live by inspecting two consecutive ControllerRevisions immediately after a spec.image
patch:

ControllerRevision 7 → image: new-image-tag (new proxy runner won)
ControllerRevision 8 → image: old-image-tag (old proxy runner overwrote it)

The mcpserver-generation annotation on both revisions was identical (3), confirming neither pod was gated
against the other.

Why the Fix in #5024 Does Not Help Here

PR #5024 was designed to prevent the old proxy runner from clobbering the new runner's apply when the old pod
has a lower mcpserver_generation. That design was sound for the previous architecture where generation was a
fixed CLI arg. In v0.28.1, generation comes from a shared live-updating ConfigMap, so the old proxy runner's
generation is updated out from under it, making it an equal peer rather than a lower-generation stale actor.

Expected Behaviour

A proxy runner pod that was created before a helm upgrade should not be able to overwrite a StatefulSet spec
that a newer proxy runner pod has applied with updated configuration.

Suggested Fix

The underlying issue is that the proxy runner uses the CLI positional arg as the authoritative source for the
container image, but the RunConfig ConfigMap as the authoritative source for mcpserver_generation. These are
now decoupled — old pods can get a new generation without getting the new image.

The fix is to make the RunConfig ConfigMap the single authoritative source for all fields including the
container image. If old proxy runner pods apply the image from RunConfig (not from their stale CLI arg), both
old and new pods would apply the same correct image during a rolling update, and the race becomes harmless.

Alternatively: snapshot mcpserver_generation into a source that does not live-update during the pod's lifetime
(e.g. a pod annotation set via the downward API at creation time), so old pods cannot inadvertently acquire a
generation value they were not started with.

[truffle-dev]

The trace lines up with the code at HEAD 4c2e32c5. The asymmetry sits across two files.

Proxy runner side:

• cmd/thv-proxyrunner/app/run.go:176 overrides config.Image = mcpServerImage (the positional CLI arg, frozen at pod creation).
• cmd/thv-proxyrunner/app/run.go:122-127 reads the rest of RunConfig from /etc/runconfig/runconfig.json, which cmd/thv-operator/controllers/mcpserver_controller.go:1046-1062 mounts without subPath, so the volume is live.
• pkg/runner/runner.go:364 threads r.Config.MCPServerGeneration into runtime.Setup, and pkg/container/kubernetes/client.go:530 gates with if theirsGen > ourGen (strict).

Operator side:

• cmd/thv-operator/controllers/mcpserver_runconfig.go:147 stamps the generation into RunConfig via runner.WithMCPServerGeneration(m.Generation).
• cmd/thv-operator/controllers/mcpserver_controller.go:1265 adds the runconfig checksum to the Deployment pod-template annotations, which is what normally forces a fresh ReplicaSet on a generation bump.

That checksum annotation covers the happy-path rollout. The race opens when an old-ReplicaSet pod restarts in place (OOM, node drain, the attach-failure c.exitFunc(1) at pkg/container/kubernetes/client.go:265): it re-reads the now-updated ConfigMap and picks up gen=N, but its args[0] still points at the old image. Both that restarted-old pod and the new-ReplicaSet pod call applyStatefulSet with ourGen=N, and the strict-greater-than predicate cannot break the tie. The alternating ControllerRevisions you observed line up with that race.

Between the two sketches in the report:

(a) Routing the image through RunConfig closes the predicate race in one direction, but it introduces a worse one. With a live ConfigMap, an old proxy whose pod has not yet been replaced will read the new image and reapply the backend StatefulSet under it. The old proxy is then talking to a backend it did not deploy, with a transport contract it did not agree to. Each proxy needs to keep managing its own backend at its own image.

(b) Snapshotting mcpserver_generation via the downward API makes generation pod-scoped at pod creation, symmetric with image. Concrete shape: add toolhive.stacklok.dev/mcpserver-generation: <gen> to deploymentTemplateAnnotations next to the checksum at controller line 1265, expose it as a downward-API env var, and override config.MCPServerGeneration from that env at run.go:176 right next to the config.Image override. An old-pod restart inside the old ReplicaSet then sees its own (frozen) N-1, the new pod sees N, and the gate fires correctly.

A third option, subPath mount on the ConfigMap, would freeze every field at pod creation including audit, telemetry, secrets refs, and webhook env vars. That is a much bigger semantic change than the bug warrants.

(b) reads as the right tradeoff to me, but the call belongs to the team. Happy to take it if assigned.

[aryanxsk22]

please fix it 🙏

[ChrisJBurns]

Hey @aryanxsk22 I've got a draft PR up here awaiting review from some maintainers - hopefully we can get this fix out ASAP!