fix(operator): skip no-op VirtualMCPServer Deployment updates

[RaviTharuma]

Cross-links

• Issue: bug(operator): VirtualMCPServer statusReportingInterval requeue Updates the Deployment every interval (no rollout) #6340
• Maintainer direction: bug(operator): VirtualMCPServer statusReportingInterval requeue Updates the Deployment every interval (no rollout) #6340 (comment)
• Follow-up (CRD default 30s): bug(operator): VirtualMCPServer statusReportingInterval requeue Updates the Deployment every interval (no rollout) #6340 (comment)
• PR announcement: bug(operator): VirtualMCPServer statusReportingInterval requeue Updates the Deployment every interval (no rollout) #6340 (comment)

Summary

VirtualMCPServer statusReportingInterval requeues (including the CRD 30s
default) called Update on the Deployment even when the pod template was
unchanged. That bumped metadata.generation and emitted DeploymentUpdated
with no new ReplicaSet — and only for vMCPs that had a unique
podTemplateSpec.

Root cause: podTemplateMetadataNeedsUpdate used maps.Equal on the full
label/annotation maps. User PodTemplateSpec merge leaves extra keys on the
live template, so every status tick looked like drift.

• Compare pod-template metadata with MapIsSubset (expected ⊆ live)
• Compare container env with equality.Semantic.DeepEqual (K8s defaulting)
• Skip the Deployment write when the computed template, labels, merged
  annotations, and replicas are unchanged

Fixes #6340

Type of change

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

Test plan

• go test -ldflags=-extldflags=-Wl,-w ./cmd/thv-operator/controllers/ -run 'TestVirtualMCPServerEnsureDeployment_PodTemplateSpecSteadyState|TestVirtualMCPServerPodTemplateSpec'
• CI unit / operator tests
• After merge: a VirtualMCPServer with a unique podTemplateSpec should
  not emit DeploymentUpdated on every statusReportingInterval tick

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.

No CRD schema change.

Does this introduce a user-facing change?

Yes. VirtualMCPServer Deployments no longer get a no-op Update (and a
generation bump) on every status-interval reconcile.

Special notes for reviewers

Matches the suggested fix on #6340: do not treat the status requeue itself as
drift. The subset check is what stops podTemplateSpec extras from looking
like a spec change; the skip-write is a second line of defense if
deploymentNeedsUpdate is still true for a non-template reason.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[RaviTharuma]

Implements #6340 per maintainer direction.

Cross-links:

• Issue: bug(operator): VirtualMCPServer statusReportingInterval requeue Updates the Deployment every interval (no rollout) #6340
• Maintainer comment: bug(operator): VirtualMCPServer statusReportingInterval requeue Updates the Deployment every interval (no rollout) #6340 (comment)
• Follow-up (CRD default 30s): bug(operator): VirtualMCPServer statusReportingInterval requeue Updates the Deployment every interval (no rollout) #6340 (comment)
• Issue PR note: bug(operator): VirtualMCPServer statusReportingInterval requeue Updates the Deployment every interval (no rollout) #6340 (comment)

[codecov]

Codecov Report

❌ Patch coverage is 69.38776% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.70%. Comparing base (b48013e) to head (9380995).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...perator/controllers/virtualmcpserver_controller.go	69.38%	15 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6377   +/-   ##
=======================================
  Coverage   77.69%   77.70%           
=======================================
  Files         752      756    +4     
  Lines       72717    72800   +83     
=======================================
+ Hits        56500    56569   +69     
- Misses      16212    16226   +14     
  Partials        5        5           

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

[reyortiz3]

Nice fix for the root cause — the subset check + semantic env comparison directly address the drift false-positive from #6340, and the regression test follows the reconcile-twice/assert-ResourceVersion-unchanged pattern well.

One question on the "second line of defense" skip-write gate added in syncExistingVirtualMCPServerDeployment:

if equality.Semantic.DeepEqual(deployment.Spec.Template, newDeployment.Spec.Template) &&
    maps.Equal(deployment.Labels, newDeployment.Labels) &&
    maps.Equal(deployment.Annotations, mergedAnnotations) &&
    replicasUnchanged {
    return ctrl.Result{}, nil
}

newDeployment comes from deploymentForVirtualMCPServer, which never sets RestartPolicy, DNSPolicy, SchedulerName, etc. on the PodSpec. On a real cluster the live deployment.Spec.Template.Spec will have those API-server-defaulted, so this full-template DeepEqual will differ from newDeployment's essentially every time — meaning this gate won't actually fire in production once deploymentNeedsUpdate is true for any reason. The fake client used in TestVirtualMCPServerEnsureDeployment_PodTemplateSpecSteadyState doesn't apply that defaulting, so the test can pass without this path ever proving itself against real API-server behavior.

Since the actual fix already lives in podTemplateMetadataNeedsUpdate (subset check) and containerNeedsUpdate (semantic env compare) — both of which gate the call into this function in the first place — is this extra check pulling its weight? If it's meant as genuine defense-in-depth, could we validate it with an envtest (real API server, real defaulting) instead of/in addition to the fake-client test? Otherwise I'd lean toward dropping it to keep syncExistingVirtualMCPServerDeployment simpler, since as written it's effectively unreachable.