Preserve external annotations on operator Services

[blkt]

Summary

A VirtualMCPServer's backing vmcp-<name> Service (and the equivalent Services for MCPServer, MCPRemoteProxy, and EmbeddingServer) is co-owned by external controllers on some platforms. On GKE with the Gateway API, the NEG/Gateway controllers continuously write cloud.google.com/* annotations onto it. The operator's Service reconcile did not tolerate this:

• serviceNeedsUpdate compared annotations with maps.Equal against an operator-only expected set, so any externally-added annotation read as drift and the check returned true on every reconcile.
• The update path then did service.Annotations = newService.Annotations, wholesale-replacing the map and stripping the external annotations.

The result was a tight hot-loop of Update calls racing the concurrent cloud controller, each losing the resourceVersion race with the object has been modified and requeuing immediately (~700-835 conflict log lines/minute observed). Serving was unaffected, but it drove continuous kube-apiserver churn and ~1M+ log lines/day per workload.

This is the Service-object sibling of #5616 (which fixed the same class of build/diff asymmetry on the Deployment). The fix reuses helpers that already exist in the repo rather than introducing Server-Side Apply (which .claude/rules/operator.md explicitly disallows as a one-off):

• Detection: compare operator-owned labels/annotations with ctrlutil.MapIsSubset instead of maps.Equal, so externally-managed keys are ignored and the reconcile is a genuine no-op once converged.
• Write: merge rather than replace labels/annotations via ctrlutil.MergeLabels / ctrlutil.MergeAnnotations (operator-owned values win, external keys preserved), matching what the Deployment path already does.

Applied consistently to all four controllers that reconcile a proxy Service: VirtualMCPServer, MCPServer, MCPRemoteProxy, and EmbeddingServer. EmbeddingServer already tolerated external annotations in detection but still stripped them on write, so it would flap the NEG annotations on every legitimate Service update; it is fixed here too.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Changes

File	Change
virtualmcpserver_controller.go	serviceNeedsUpdate subset check; merge-on-write for labels/annotations
mcpserver_controller.go	Same fix
mcpremoteproxy_controller.go	Same fix
embeddingserver_controller.go	Merge-on-write fix; detection loops replaced with MapIsSubset for consistency
*_test.go (4 files)	Detection regression cases for all four controllers, plus write-path tests proving external annotations survive an Update

Test plan

• Unit tests pass (cmd/thv-operator/controllers and pkg/controllerutil)
• Detection regression tests: a cloud.google.com/neg-status annotation must not trigger an update (all four controllers)
• Write-path regression tests: when a genuine operator-owned change triggers an update, the external annotation must survive and the operator-owned field must still be corrected (all four controllers)
• task lint-fix (no new findings on changed files)

Does this introduce a user-facing change?

No. Behavior is unchanged for Services with no external co-owner; the fix only stops spurious Update/conflict churn when another controller writes annotations on the operator's Service.

Special notes for reviewers

• Not addressing the issue's "backoff on conflict" suggestion: controller-runtime already backs off on returned errors, and once the write stops firing there are no conflicts left to back off from.
• Known limitation (shared with the existing merge-patch approach): removing an operator-owned annotation from spec won't delete it from an existing Service, since the subset check can't distinguish "operator no longer wants this key" from "external controller owns this key". Full removal semantics would require Server-Side Apply, which is out of scope here.

Fixes #5730

[codecov]

Codecov Report

❌ Patch coverage is 44.44444% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.69%. Comparing base (69826e9) to head (3f6125f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...operator/controllers/embeddingserver_controller.go	33.33%	2 Missing and 2 partials ⚠️
...-operator/controllers/mcpremoteproxy_controller.go	50.00%	0 Missing and 2 partials ⚠️
...d/thv-operator/controllers/mcpserver_controller.go	50.00%	0 Missing and 2 partials ⚠️
...perator/controllers/virtualmcpserver_controller.go	50.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5731      +/-   ##
==========================================
+ Coverage   70.65%   70.69%   +0.04%     
==========================================
  Files         682      682              
  Lines       68854    68852       -2     
==========================================
+ Hits        48646    48675      +29     
+ Misses      16662    16619      -43     
- Partials     3546     3558      +12     

☔ 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.

[aponcedeleonch]

Nice fix, the subset-check + merge approach is a clean reuse of the existing Deployment pattern and it clearly kills the hot-loop. Left two non-blocking comments inline.