Stop VirtualMCPServer hot-reconcile on cleared podTemplateSpec

[jhrozek]

Summary

Clearing spec.podTemplateSpec on a VirtualMCPServer (or on an MCPRegistry with a registry-api deployment) left the operator writing a no-op Deployment update on every reconcile, incrementing metadata.generation indefinitely. On a kind cluster this saturates etcd write bandwidth and can starve leader-election lease renewal, causing the operator to lose leadership and restart. Same bug class as #5817 (imagePullSecrets), now fixed for podTemplateSpec:

• Made podTemplateSpecNeedsUpdate a plain stored != expected hash comparison instead of the previous asymmetric presence check that always reported drift once a hash annotation existed.
• Extended mergeDeploymentAnnotations to prune podTemplateSpecHashAnnotation alongside imagePullRefsHashAnnotation when desired no longer wants it, collapsing the two prune checks into one loop.
• Fixed the identical bug in the MCPRegistry registry-api deployment path (cmd/thv-operator/pkg/registryapi/deployment.go), where the same stale annotation could survive an additive-only annotation merge.
• Added regression tests for both controllers, each reconciling twice from a stale-annotation fixture and asserting the second pass is a true no-op.
• Simplified the surrounding VirtualMCPServer Deployment test suite: merged a duplicate create-deployment test, consolidated two near-identical stale-annotation regression tests and two near-identical replica-sync tests into table-driven tests, filled a coverage gap in TestVirtualMCPServerDeploymentNeedsUpdate (previously only exercised 3 of the 5 branches it delegates to), and added a direct unit test for mergeDeploymentAnnotations.

Fixes #5818

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Changes

File	Change
cmd/thv-operator/controllers/virtualmcpserver_controller.go	Symmetric podTemplateSpecNeedsUpdate; mergeDeploymentAnnotations prunes both hash annotations
cmd/thv-operator/controllers/virtualmcpserver_controller_test.go	Regression tests + test suite simplification (see summary)
cmd/thv-operator/pkg/registryapi/deployment.go	Same stale-annotation prune fix for the registry-api deployment path
cmd/thv-operator/pkg/registryapi/deployment_test.go	Regression test for the registry-api fix

Does this introduce a user-facing change?

Yes — clearing spec.podTemplateSpec (VirtualMCPServer or MCPRegistry) no longer causes a runaway reconcile loop that repeatedly updates the managed Deployment.

Special notes for reviewers

Both fixes are required together: the symmetric comparison alone doesn't stop the loop, since the write path would still never remove the stale annotation via MergeAnnotations. Only pruning it explicitly at the write site converges to a steady state — same pattern as the imagePullSecrets fix in #5821.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.11%. Comparing base (d76d4e3) to head (1783d87).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...perator/controllers/virtualmcpserver_controller.go	70.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5846      +/-   ##
==========================================
+ Coverage   71.06%   71.11%   +0.05%     
==========================================
  Files         686      686              
  Lines       69909    69912       +3     
==========================================
+ Hits        49680    49719      +39     
+ Misses      16599    16543      -56     
- Partials     3630     3650      +20     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JAORMX]

LGTM — correct fix, and it fully stops the hot-reconcile loop. ✅

Two independent lenses (K8s reconcile-correctness + Go/duplication/reuse) traced both halves on the real code: the symmetric stored != expected hash compare plus the write-path annotation prune converge to a true no-op on the second reconcile, and the regression tests assert exactly that via resourceVersion equality — the right shape for this bug class (a presence-only assertion would have missed it). The registry-api path is genuinely equivalent, not half-fixed (its deploymentNeedsUpdate was already symmetric; this PR adds the missing write-path prune). No remaining asymmetric-compare / no-op-write path in either deployment reconcile. CI is green.

One non-blocking finding (inline on registryapi/deployment.go): the merge-and-prune logic now exists in three diverging idioms across virtualmcpserver_controller, registryapi/deployment, and mcpremoteproxy_controller. Worth noting that mcpremoteproxy_controller.go:1767 still uses the old asymmetric hadPrevious presence check — safe today only because its write path deletes the annotation (line 412), but it's the exact #5818 anti-pattern kept alive. Extracting a shared ctrlutil helper (MergeAnnotations already lives at controllerutil/resources.go:155) and aligning mcpremoteproxy on it would collapse all three onto one correct path. Non-blocking — fine to track as follow-up.

Strengths: minimal, targeted, both halves matched per path; externally-managed annotations are preserved while only operator-owned keys are pruned (explicitly tested); good table-driven test consolidation covering both imagePullSecrets and podTemplateSpec; comments cite #5817/#5818.

Approving; recommend tracking the shared-helper extraction (and the mcpremoteproxy alignment) as a follow-up.

_{🤖 AI-assisted panel review via Claude Code (kubernetes-expert · Go/reuse lenses). Line numbers against 1783d87.}

[ChrisJBurns]

Thanks for this, I believe there was a similar community contribution for this that got auto closed due to staleness!