-
Notifications
You must be signed in to change notification settings - Fork 170
Add K8s manager infrastructure for vMCP dynamic backend discovery #3192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Large PR Detected
This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.
How to unblock this PR:
Add a section to your PR description with the following format:
## Large PR Justification
[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformationAlternative:
Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.
See our Contributing Guidelines for more details.
This review will be automatically dismissed once you add the justification section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements the Kubernetes manager infrastructure for VirtualMCP servers to support dynamic backend discovery. It introduces a controller-runtime manager wrapper that enables live backend updates without pod restarts when running in Kubernetes with outgoingAuth.source: discovered mode.
Key changes include:
- New K8s manager package wrapping controller-runtime for dynamic mode
- Readiness probe endpoint (
/readyz) with cache sync gating for gradual rollout safety - Integration of K8s manager lifecycle into the vMCP server command
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/vmcp/k8s/manager.go |
New K8s manager wrapper that provides cache sync capabilities and lifecycle management for dynamic backend discovery |
pkg/vmcp/server/server.go |
Adds K8sManager config field and implements /readyz endpoint that gates readiness on cache sync status |
pkg/vmcp/server/readiness_test.go |
Unit tests for readiness endpoint covering static mode, dynamic mode with synced cache, and dynamic mode with unsynced cache |
pkg/vmcp/k8s/manager_test.go |
Unit tests for K8s manager covering validation, lifecycle, cache sync, and error scenarios |
cmd/vmcp/app/commands.go |
Integrates K8s manager creation and startup when running in dynamic discovery mode |
test/e2e/thv-operator/virtualmcp/virtualmcp_lifecycle_test.go |
E2E tests verifying K8s manager infrastructure, readiness probes, health endpoints, and status endpoint behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e5310ce to
bfd42ba
Compare
|
✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review. |
Large PR justification has been provided. Thank you!
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3192 +/- ##
==========================================
+ Coverage 57.16% 57.20% +0.04%
==========================================
Files 343 347 +4
Lines 34384 34438 +54
==========================================
+ Hits 19656 19701 +45
- Misses 13091 13097 +6
- Partials 1637 1640 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bfd42ba to
9947ab9
Compare
Implements controller-runtime manager wrapper for VirtualMCP servers running in Kubernetes with dynamic backend discovery mode. This provides the foundation for live backend updates without pod restarts. Closes: #3000
9947ab9 to
e337160
Compare
2f7929d to
8eb0323
Compare
8eb0323 to
167a877
Compare
167a877 to
0d90143
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0d90143 to
2dd2e91
Compare
The "discovered auth behavior with real MCP requests" test was failing intermittently on k8s 1.34.3 with "connection reset by peer" errors. Root cause: The nested Context's BeforeAll only checked that the Service had a NodePort assigned, but did not verify the VirtualMCPServer pod was still running and ready. Between the outer BeforeAll completing and the nested test executing, the pod could have crashed or restarted, especially on k8s 1.34.3. Fix: Add explicit pod readiness checks in the nested BeforeAll to ensure: 1. VirtualMCPServer status is Ready 2. vMCP pods are running and ready 3. Only then proceed to get NodePort and run tests This prevents the test from attempting HTTP connections to pods that may have crashed or are in CrashLoopBackOff state. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
f8c37b5 to
681ac6c
Compare
681ac6c to
390fff9
Compare
390fff9 to
c83366c
Compare
Implements controller-runtime manager wrapper for VirtualMCP servers running in Kubernetes with dynamic backend discovery mode. This provides the foundation for live backend updates without pod restarts.
Large PR Justification
This is an atomic PR that covers a single and isolated functionality. It also contains tests and can't be split.
Closes: #3000