Skip to content

Conversation

@JAORMX
Copy link
Collaborator

@JAORMX JAORMX commented Dec 4, 2025

Summary

  • Fix VirtualMCPServer controller to check PodReady condition instead of PodRunning phase
  • This ensures the CR is only marked Ready when pods have actually passed their readiness probes
  • Fixes flaky E2E tests caused by race condition between CR status and actual server readiness

Problem

The VirtualMCPServer controller was marking the CR as Ready based on pod.Status.Phase == PodRunning, but this doesn't guarantee the pod is actually ready to serve traffic. A pod can be in Running phase while still waiting for its readiness probe to pass.

This caused flaky E2E tests where:

  1. Pod starts → Phase becomes Running
  2. Controller sees Running → marks CR as Ready
  3. Test sees CR Ready → tries to connect
  4. Pod hasn't passed readiness probe yet → EOF error

Solution

Now the controller checks the PodReady condition in pod.Status.Conditions, which is the authoritative signal that a pod can receive traffic. This aligns with Kubernetes semantics.

Pod State Counted As Result
PodReady condition = True ready Ready
PodFailed phase failed Failed
Everything else pending Pending

Test plan

  • Unit tests pass (go test ./cmd/thv-operator/controllers/... -run VirtualMCPServer)
  • Linting passes (task lint)
  • E2E tests pass (CI will verify)

🤖 Generated with Claude Code

The VirtualMCPServer controller was marking the CR as Ready based on
pod.Status.Phase == PodRunning, but this doesn't guarantee the pod is
actually ready to serve traffic. A pod can be in Running phase while
still waiting for its readiness probe to pass.

This caused flaky E2E tests where:
1. Pod starts → Phase becomes Running
2. Controller sees Running → marks CR as Ready
3. Test sees CR Ready → tries to connect
4. Pod hasn't passed readiness probe → EOF error

Now the controller checks the PodReady condition in pod.Status.Conditions,
which is the authoritative signal that a pod can receive traffic. This
aligns with Kubernetes semantics and ensures the VirtualMCPServer is only
marked Ready when the underlying HTTP server is actually responding.

Changes:
- Check PodReady condition instead of PodRunning phase
- Rename 'running' counter to 'ready' for clarity
- Add test case for "running but not ready" pods
- Update existing test to set PodReady condition

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions bot added the size/XS Extra small PR: < 100 lines changed label Dec 4, 2025
@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.42%. Comparing base (7906ead) to head (7161d8b).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2901      +/-   ##
==========================================
+ Coverage   56.41%   56.42%   +0.01%     
==========================================
  Files         322      322              
  Lines       31638    31644       +6     
==========================================
+ Hits        17848    17856       +8     
+ Misses      12252    12250       -2     
  Partials     1538     1538              

☔ View full report in Codecov by Sentry.
📢 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 JAORMX merged commit 55d0cee into main Dec 4, 2025
33 checks passed
@JAORMX JAORMX deleted the fix-vmcp-controller-readiness-check branch December 4, 2025 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants