feat(operator): allow pod scheduling on the proxy Deployment via resourceOverrides

[talshechanovitz]

Closes #5879.

What

Adds nodeSelector, tolerations and affinity to resourceOverrides.proxyDeployment and applies them to the proxy pod, so the proxy can be steered onto the same nodes as the MCP server pod.

Without it the proxy is repelled by a dedicated pre-warmed pool's taint, lands on a cold general node, and the ~40s node boot is paid anyway — the server is warm but the proxy isn't.

Diff shape: +335 hand-written, +7,827 generated. All the generated volume is corev1.Affinity's schema expanding into four CRD files; regenerating on a clean tree produces no churn, so it's all attributable here.

Scope — please sanity-check this call

@ChrisJBurns asked to keep this to MCPServer. That turns out not to be cleanly possible: ProxyDeploymentOverrides is reached via ResourceOverrides, which MCPRemoteProxy also embeds (mcpremoteproxy_types.go:153), so the fields land on both CRDs regardless.

This PR wires both, rather than leaving MCPRemoteProxy accepting fields it ignores.

One wrinkle worth knowing before you weigh in — the two resources aren't symmetric:

	Can it already schedule its proxy?	Effect of these fields
MCPServer	No — podTemplateSpec is marshalled into a runtime patch for the MCP pod and never reaches the proxy PodSpec	the actual fix
MCPRemoteProxy	Yes — podTemplateSpec is applied to its proxy Deployment directly	a second route to the same field

And where both are set on MCPRemoteProxy, podTemplateSpec wins silently (verified: nodeSelector from podTemplateSpec overwrites the override).

So there's a case for splitting the type instead, giving MCPServer its own overrides type so scheduling never appears on MCPRemoteProxy. I didn't, for one reason: it needs two new API types plus retyping ~24 ResourceOverrides literals across ~10 files, which makes the review considerably larger than the alternative you were trying to avoid. Happy to do it if you'd rather have the clean separation — just say and I'll push it.

How

• Three fields on ProxyDeploymentOverrides (mcpserver_types.go)
• proxyDeploymentScheduling(*ResourceOverrides) — shared helper, so both controllers behave identically from one implementation
• Applied to the proxy PodSpec in deploymentForMCPServer and deploymentForMCPRemoteProxy
• proxySchedulingNeedsUpdate — shared drift predicate, used by both deploymentNeedsUpdate paths
• task operator-generate + task operator-manifests

Simpler than I sketched on the issue: both operators build their proxy PodSpec directly, so no runtime or applyPodTemplatePatch change is needed.

The overrides are authoritative, not merged — neither controller sets proxy scheduling of its own today (every NodeSelector/Tolerations/Affinity match under cmd/thv-operator is SessionAffinity, a Service field).

Drift detection

Worth calling out because the first version of this PR got it wrong: deploymentNeedsUpdate compared no scheduling fields, so editing the overrides on an existing CR silently did nothing until an unrelated field changed. Create worked, update didn't.

Both controllers now compare, following the ImagePullSecrets precedent (equality.Semantic.DeepEqual, so nil ≡ empty and an unset override isn't perpetual drift).

The comparison has to differ per resource, which TestMCPRemoteProxyPodTemplateSpecDriftDetection caught immediately: for MCPRemoteProxy scheduling may come from podTemplateSpec, so it compares against the rebuilt Deployment; comparing against the overrides alone reported false drift — an infinite reconcile loop. MCPServer needs no equivalent, since its podTemplateSpec never touches the proxy PodSpec.

Tests

• TestDeploymentForMCPServer_ProxyDeploymentScheduling — each field alone, all three together, and no overrides asserting scheduling stays unset so the default path is provably unchanged
• TestMCPServerDeploymentNeedsUpdate_ProxySchedulingDrift — spec-gains-value, spec-cleared-with-stale-deployment, nil-vs-empty-is-not-drift
• TestMCPRemoteProxySchedulingOverridesAndPodTemplateSpec — overrides applied and stable, clearing them detected, and podTemplateSpec-supplied scheduling not mistaken for drift

go test ./cmd/thv-operator/... — 21 packages pass. The test-integration/... suites fail locally on missing envtest binaries (kubebuilder/bin/etcd): 23 failures both with this change and on a clean main, so environmental here. I couldn't run golangci-lint locally either — the installed build targets go1.23 and the repo is on 1.26 — so CI will be the first real lint pass; gofmt is clean.

Follow-up

VirtualMCPServer, as agreed on the issue.

[amirejaz]

Thanks for this — it's a well-put-together change, and I appreciate how thoroughly you traced the plumbing on the issue before writing code. I went through the hand-written portion carefully (the drift detection especially) and it holds up. A few notes, plus one scoping point I want to sync with @ChrisJBurns on since he'd framed the original scope.

Verified as correct

• Drift detection handles the asymmetry properly. On MCPRemoteProxy, scheduling can come from either podTemplateSpec or the overrides, and the two comparisons are in mutually-exclusive branches of podSpecNeedsUpdate — podTemplateSpec present compares against the rebuilt expectedDeployment (so a podTemplateSpec-supplied nodeSelector isn't read as drift against empty overrides), absent uses proxySchedulingNeedsUpdate against the overrides alone. MCPServer uses the unconditional override comparison, which is right because its podTemplateSpec becomes a runtime patch for the MCP pod and never reaches the proxy PodSpec. That's the subtle part and it's correct.
• API compatibility: the three fields are all optional, so this is v1beta1-safe.
• Generated artifacts are complete: exactly the two CRDs that embed the shared ResourceOverrides (mcpservers + mcpremoteproxies), and equality.Semantic.DeepEqual gives you nil ≡ empty so an unset override doesn't read as perpetual drift — good call following the ImagePullSecrets precedent.

One required change

• docs/operator/crd-api.md needs regenerating. It still lists only the pre-existing fields for ProxyDeploymentOverrides; the three new ones aren't there. Running task crdref-gen from the repo root will pick them up (it's a separate step from operator-manifests, which is why the CRD YAMLs updated but the reference doc didn't).

Scope — @ChrisJBurns let's sync on this

Chris, you'd originally scoped this to MCPServer with MCPRemoteProxy as a follow-up. @talshechanovitz found that isn't cleanly separable — ProxyDeploymentOverrides is reached through the shared ResourceOverrides, which MCPRemoteProxy also embeds, so the fields show up on its CRD schema either way — and wired both (option 2), with a +1 from @oryanomer1 who runs MCPRemoteProxy and would hit the inert-field case directly.

My read is that option 2 is the better of the two: shipping an inert surface (a field that validates then silently does nothing) is the kind of thing that quietly burns someone an afternoon. But since you set the original scope I didn't want to just wave it through — figured it's worth a quick sync on whether you're happy with wiring both, or would rather keep the boundary clean with option 1 (a dedicated MCPServer overrides type so scheduling never lands on MCPRemoteProxy — bigger diff, cleaner separation).

One input for that call: MCPRemoteProxy can already schedule its proxy via spec.podTemplateSpec, so wiring the overrides there gives it two ways to do the same thing, with podTemplateSpec silently winning when both are set. It's documented in the PR but unguarded — if we keep option 2, a godoc note on the fields calling out the precedence (or a validation warning when both are set) would save the head-scratch.

Non-blocking

• Our operator guidance has a "CRD vs PodTemplateSpec" rule of thumb that puts nodeSelector/affinity/tolerations in PodTemplateSpec rather than CRD attributes. Explicit fields here are a reasonable deviation given the proxy has no PodTemplateSpec of its own on MCPServer and the explicit-fields choice was deliberate — just noting it so it's on the record.
• Tiny test gap: the "both podTemplateSpec and overrides set, podTemplateSpec wins" case on MCPRemoteProxy is asserted in the PR description but not in a test. Low value, optional.

Net: the implementation is solid. Once crd-api.md is regenerated and Chris and I have settled the MCPRemoteProxy scope, I'm happy to approve.

[ChrisJBurns]

Thanks both — and thanks @talshechanovitz for digging into the shared-struct problem before writing code rather than after.

Happy with option 2. My original "MCPServer only" scoping assumed the two resources could be separated cleanly, and they can't: ProxyDeploymentOverrides hangs off the shared ResourceOverrides that MCPRemoteProxy also embeds, so the fields land on both CRD schemas whether or not we wire the second controller. Given that, wiring both is the honest option — I'd rather have a slightly bigger review than ship a field that validates and then silently does nothing.

Option 1 would give cleaner separation, but two new API types plus retyping ~24 ResourceOverrides literals across ~10 files makes the review bigger than the one I was trying to keep small. Not worth it.

One thing I do want before this goes in: MCPRemoteProxy can already schedule its proxy through spec.podTemplateSpec, so after this it has two ways to set the same field, with podTemplateSpec silently winning when both are set. Please add a godoc note on the three new fields calling that out — something like "on MCPRemoteProxy, spec.podTemplateSpec takes precedence over these fields if both are set" — so it shows up in the generated CRD docs. A note is enough; no need for validation.

Otherwise: agree with @amirejaz that docs/operator/crd-api.md needs regenerating (task crdref-gen). The test for the precedence case is nice-to-have, not a blocker.

VirtualMCPServer stays a follow-up as agreed.