Skip to content

Conversation

@yrobla
Copy link
Contributor

@yrobla yrobla commented Jan 5, 2026

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

@yrobla yrobla requested a review from Copilot January 5, 2026 14:06
@github-actions github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Jan 5, 2026
Copy link
Contributor

@github-actions github-actions bot left a 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 transformation

Alternative:

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.

Copy link
Contributor

Copilot AI left a 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.

@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 5, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2026

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

@github-actions github-actions bot dismissed their stale review January 5, 2026 14:45

Large PR justification has been provided. Thank you!

@codecov
Copy link

codecov bot commented Jan 5, 2026

Codecov Report

❌ Patch coverage is 58.62069% with 48 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.20%. Comparing base (674e321) to head (c83366c).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
cmd/vmcp/app/commands.go 0.00% 23 Missing ⚠️
pkg/vmcp/k8s/manager.go 69.84% 16 Missing and 3 partials ⚠️
pkg/vmcp/server/server.go 80.00% 3 Missing and 3 partials ⚠️
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.
📢 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.

Base automatically changed from feat/issue-2999 to main January 5, 2026 15:16
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
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 5, 2026
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 7, 2026
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 7, 2026
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 7, 2026
Copy link
Contributor

Copilot AI left a 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.

@yrobla yrobla requested review from JAORMX, dmjb and jhrozek January 7, 2026 07:32
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 7, 2026
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]>
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 7, 2026
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 7, 2026
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 7, 2026
@yrobla yrobla requested a review from dmjb January 7, 2026 12:07
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 7, 2026
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 7, 2026
@yrobla yrobla merged commit d1a13af into main Jan 7, 2026
42 of 43 checks passed
@yrobla yrobla deleted the feat/issue-3000 branch January 7, 2026 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

K8s Manager wrapper for vMCP with server integration and readiness

4 participants