feat(mcp): add /healthz and /readyz probe endpoints - #1339
Conversation
The server ships a Dockerfile that serves streamable HTTP on 0.0.0.0:8000, so it is deployed as a long-running network service, but it exposed no probe endpoints. A Deployment therefore had no way to tell a wedged replica from a healthy one, or to keep traffic away from one that cannot reach the Kubernetes API. Add the two-endpoint split sandbox-router already uses (sandbox-router/server/server.go): - /healthz -- liveness. Returns 200 as soon as the ASGI app routes the request, and never contacts the Kubernetes API. Wiring liveness to an external dependency would turn a transient control-plane blip into a rolling restart of every replica, which is worse than briefly serving no traffic. A test asserts it makes no Kubernetes call. - /readyz -- readiness. Lists SandboxClaims in a configurable probe_namespace, which exercises everything a tool call needs: credential loading, client construction and one real apiserver request. Returns 503 while starting and when the API is unreachable, with a generic reason so the underlying exception is not exposed to an unauthenticated caller. Two implementation notes. The readiness check deliberately calls the public list_sandbox_claims rather than the private _ensure_initialized: the latter only loads credentials without reaching the apiserver, and nothing else in the repo depends on SDK internals. And the lifespan's yielded mapping reaches tools through ctx.lifespan_context but is not published on the ASGI app, so the client is stashed on the FastMCP server object, which routes reach as request.app.state.fastmcp_server. httpx is now declared in the test extra. The probe tests drive the ASGI app with it, and it was only available transitively through fastmcp.
✅ Deploy Preview for agent-sandbox canceled.
|
|
Hi @yuzhiquan. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe MCP server now exposes ChangesMCP server health probes
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ProbeRequest
participant MCPServer
participant AsyncSandboxClient
participant KubernetesAPI
ProbeRequest->>MCPServer: GET /healthz or GET /readyz
MCPServer->>MCPServer: Return liveness status
MCPServer->>AsyncSandboxClient: List SandboxClaims in probe_namespace
AsyncSandboxClient->>KubernetesAPI: Namespace-scoped SandboxClaim list
KubernetesAPI-->>AsyncSandboxClient: API response or failure
AsyncSandboxClient-->>MCPServer: Readiness result
MCPServer-->>ProbeRequest: HTTP 200 or HTTP 503 response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds Kubernetes-friendly liveness/readiness probe endpoints to the MCP server so it can be safely run as a long-lived HTTP service behind a Deployment/Service, with readiness depending on Kubernetes API reachability while liveness remains dependency-free.
Changes:
- Introduces
GET /healthz(liveness) andGET /readyz(readiness) routes on the MCP server ASGI app. - Adds
probe_namespacesetting to control which namespace/readyzqueries. - Adds probe-focused unit tests and declares
httpxin thetestextra to drive ASGI requests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| clients/integrations/mcp-server/tests/unit/test_probes.py | Adds unit tests covering health/ready behavior, startup state, and namespace selection. |
| clients/integrations/mcp-server/README.md | Documents probe endpoints and recommended Kubernetes probe configuration. |
| clients/integrations/mcp-server/pyproject.toml | Adds httpx to the test optional dependency set for ASGI probe testing. |
| clients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/settings.py | Adds probe_namespace setting (env-backed) for readiness checks. |
| clients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/server.py | Wires probe routes and stashes client/settings for route access. |
| clients/integrations/mcp-server/k8s_agent_sandbox_mcp_server/probes.py | Implements /healthz and /readyz endpoints. |
The README said /readyz returns 503 with {"ready": false}, but the
handler also returns a "reason" field. Document the real shape,
including both reason values, and note that the reason is generic on
purpose because the endpoint is unauthenticated.
Also assert the full body in the failure test rather than just the
"ready" key, so the documented contract cannot drift again silently --
it already had.
|
Thanks — addressed in a1575d1, applied locally (not via "Commit suggestion"). README/
Cancellation already propagates. Happy to add an explicit re-raise if you'd still prefer it as documentation of intent, but it would be a no-op. |
|
/ok-to-test |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: aditya-shantanu, yuzhiquan The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/lgtm |
What this PR does / why we need it:
The MCP server ships a Dockerfile that serves streamable HTTP on
0.0.0.0:8000, so it isdeployed as a long-running network service — but it exposes no probe endpoints. A Deployment
has no way to tell a wedged replica from a healthy one, or to keep traffic off one that cannot
reach the Kubernetes API.
Adds the two-endpoint split
sandbox-routeralready uses (sandbox-router/server/server.go:41):GET /healthzGET /readyzprobe_namespace.503while starting or when the API is unreachable.Keeping liveness independent of the Kubernetes API is the deliberate part: wiring it to an
external dependency would turn a transient control-plane blip into a rolling restart of every
replica, which is worse than briefly serving no traffic. A test asserts
/healthzmakes noKubernetes call.
/readyzreturns a generic reason string rather than the underlying exception, since theendpoint is unauthenticated.
Two implementation notes for reviewers:
/readyzcalls the publiclist_sandbox_claims, not_ensure_initialized. The latter onlyloads credentials and builds API objects without contacting the apiserver, so it would report
ready while the cluster was unreachable. It is also private, and nothing else in the repo
depends on SDK internals.
FastMCPserver object. The lifespan's yielded mapping reachestools via
ctx.lifespan_contextbut is not published on the ASGI app, so routes read it asrequest.app.state.fastmcp_server.httpxis now declared in thetestextra — the probe tests drive the ASGI app with it, and itwas previously only available transitively through
fastmcp.Testing
Six new tests:
/healthz200,/healthzmakes no Kubernetes call,/readyzready,/readyz503 when the API fails (asserting the exception text is not leaked),/readyzreports
startingbefore the lifespan runs, and the configured namespace is honoured.Suite 19 → 25.
dev/tools/test-unitgreen across all suites locally.Suggested probe config is documented in the README; because
/readyzmakes a real API call,it recommends a modest
periodSecondsrather than aggressive polling.Which issue(s) this PR is related to:
Follow-up hardening for the MCP server added in #1141
Release Note
Summary by CodeRabbit
/healthzliveness and/readyzreadiness endpoints.