fix(pylon): probe /v1/health/ready when the upstream serves no /health by Max-NV · Pull Request #907 · NVIDIA/nvcf

Max-NV · 2026-08-17T15:25:43Z

TL;DR

Pylon gated startup on a hardcoded GET <upstream>/health and exited when it did not answer, so engines that serve only /v1/health/ready crash-looped next to a healthy inference container. Pylon now probes an ordered candidate list, and icms-translate passes down the health endpoint the function already declares.

Additional Details

Pylon side:

UpstreamHealthPaths (new crates/pylon-lib/src/upstream_health.rs) holds the candidate list and the resolved index. Configured paths are probed first, followed by the built-in /health and /v1/health/ready, so a path that does not answer falls back instead of stranding a healthy upstream.
check_upstream_health probes the resolved path first, falls back to the rest, and records the winner.
Stargate's forwarded /health RTT probe is rewritten to the resolved upstream path inside pylon. The tunnel wire path stays /health, so mixed router/pylon versions need no lockstep rollout.
Startup retries the probe every 500ms for --upstream-health-wait-ms (default 60s) before failing, which also removes the restarts seen while an engine is still loading. 0 keeps the old probe-once behavior.
New flags: repeatable --upstream-health-path and --upstream-health-wait-ms. Both default to working behavior.

Translate side:

The llm-worker container already receives INFERENCE_HEALTH_ENDPOINT from the function's health URI, but pylon takes CLI args only, so the value was never used. newLLMRouterClientContainer now forwards it as --upstream-health-path.
It is skipped when the health check is gRPC or bound to a port other than the inference port, since pylon probes over HTTP against the inference port. Those cases fall back to the built-in candidates.
Regenerated the two LLM icms-translate goldens and synced the vendored copy under src/compute-plane-services/nvca/vendor.

Rollout order

--upstream-health-path is a new flag, and pylon rejects unknown arguments, so the pylon image must roll out with or before the translate change. A worker still pinned to an older pylon exits at startup with unexpected argument '--upstream-health-path' found. Only functionType: LLM workloads run this container.

For the Reviewer

crates/pylon-lib/src/quic_http_tunnel/core.rs is the part worth close reading: the health rewrite happens before send_upstream_request, so only requests matched by is_health_request_path are affected.

For QA

cargo test --workspace, cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings, and go test ./pkg/icms-translate/... pass. Two pre-existing local failures reproduce unchanged on main in the same environment and are unrelated to this change: stargate::tests::occupied_metrics_port_fails_before_runtime_construction, and cmd/icms-translate TestRun, which compares generator output against goldens that carry an SPDX header the generator does not emit (39 subtests before and after).

New tests cover the fallback probe, resolved-path reuse, the no-candidate case, configured-path precedence, fallback when a configured path is wrong, the tunnel probe rewrite with and without a query string, the startup wait, the flag-to-config wiring, and the four translate cases (declared endpoint, divergent health port, gRPC protocol, absent endpoint).

QA needed: deploy an LLM function on an engine image that serves only /v1/health/ready and confirm the worker pod reaches full readiness.

Issues

Closes #906

Checklist

I am familiar with the Contributing Guidelines.
I have signed off my commits for Developer Certificate of Origin (DCO) compliance.
New or existing tests cover these changes.
The documentation is up to date with these changes.

Summary by CodeRabbit

New Features
- Added configurable upstream health-probe paths with /health and /v1/health/ready fallbacks.
- Successful health paths are reused for subsequent checks and tunnel health requests.
- Added startup health verification with configurable retry duration, including fail-fast behavior.
- Added support for forwarding custom health paths while preserving query parameters.
- LLM router health endpoints are now forwarded only when compatible with the inference protocol and port.
Documentation
- Documented health-probe order, fallback behavior, path reuse, and startup wait settings.

Pylon gated startup on a hardcoded GET /health against the local inference server and exited when it did not answer, so an OpenAI-style engine that serves only /v1/health/ready never completed bringup and crash-looped on a healthy inference container. Probe an ordered candidate list instead, defaulting to /health then /v1/health/ready, remember the path that answers, and forward Stargate's /health RTT probe to that same path so the routing signal follows. Startup now retries the probe for --upstream-health-wait-ms (default 60s) before failing, which also removes the restarts that happen while the engine is still loading. New flags: repeatable --upstream-health-path and --upstream-health-wait-ms. Signed-off-by: Max Xing <mxing@nvidia.com>

coderabbitai · 2026-08-17T15:26:45Z

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b3dc4c98-b629-4271-aece-28d878738b5a

📥 Commits

Reviewing files that changed from the base of the PR and between 1be91b8 and 171c4df.

⛔ Files ignored due to path filters (3)

src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/icms-translate/translate/function/llm.go is excluded by !**/vendor/**
src/libraries/go/lib/testdata/icms-translate/function/container/llm/exp.yaml is excluded by !**/testdata/**
src/libraries/go/lib/testdata/icms-translate/function/helmchart/llm/exp.yaml is excluded by !**/testdata/**

📒 Files selected for processing (16)

src/libraries/go/lib/pkg/icms-translate/translate/function/llm.go
src/libraries/go/lib/pkg/icms-translate/translate/function/llm_test.go
src/libraries/rust/stargate/README.md
src/libraries/rust/stargate/crates/pylon-lib/src/bringup.rs
src/libraries/rust/stargate/crates/pylon-lib/src/bringup/lifecycle.rs
src/libraries/rust/stargate/crates/pylon-lib/src/bringup/upstream.rs
src/libraries/rust/stargate/crates/pylon-lib/src/lib.rs
src/libraries/rust/stargate/crates/pylon-lib/src/model_lifecycle.rs
src/libraries/rust/stargate/crates/pylon-lib/src/quic_http_tunnel/core.rs
src/libraries/rust/stargate/crates/pylon-lib/src/quic_http_tunnel/raw_quic.rs
src/libraries/rust/stargate/crates/pylon-lib/src/quic_http_tunnel/tests.rs
src/libraries/rust/stargate/crates/pylon-lib/src/upstream_health.rs
src/libraries/rust/stargate/crates/pylon/src/main.rs
src/libraries/rust/stargate/crates/pylon/src/startup.rs
src/libraries/rust/stargate/crates/stargate/tests/common/mod.rs
src/libraries/rust/stargate/crates/stargate/tests/suite/lifecycle.rs

 _________________________________________________________________________________
< CodeRabbi is my uncle. He's a bit more orthodox. Feel lucky you got me instead. >
 ---------------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/pylon-upstream-health-path

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-08-17T15:28:45Z

🛡️ CodeQL Analysis

🚨 Found 2 issue(s)

Severity Breakdown:

🔴 Errors: 0
🟡 Warnings: 0
🔵 Notes: 0

📋 Top Issues

🔗 View full details in Security tab

_{🕐 Last updated: 2026-08-17 15:28:43 UTC | Commit: b560a2e}

FamousDirector · 2026-08-17T17:10:14Z

Shouldn't we be passing down that health endpoint value that we set in the NVCF API?

The llm-worker container already receives INFERENCE_HEALTH_ENDPOINT, sourced from the function's health URI, but pylon is configured by CLI args only, so the value went unused and pylon guessed the path instead. Pass it through as --upstream-health-path when the function keeps the health check on HTTP and on the inference port, which is what pylon actually probes. Configured paths now sit ahead of pylon's built-in candidates rather than replacing them, so a health URI that does not answer still falls back instead of stranding a healthy upstream. Signed-off-by: Max Xing <mxing@nvidia.com>

Max-NV · 2026-08-17T17:42:49Z

Shouldn't we be passing down that health endpoint value that we set in the NVCF API?

newLLMRouterClientContainer now forwards INFERENCE_HEALTH_ENDPOINT as --upstream-health-path, so pylon probes the endpoint the function declares instead of guessing. It is skipped when the health check is gRPC or on a port other than the inference port, since pylon probes over HTTP against the inference port. Configured paths are also tried ahead of pylon's built-in /health and /v1/health/ready rather than replacing them, so a health URI that does not answer still falls back instead of stranding a healthy upstream.

balajinvda · 2026-08-17T18:17:50Z

🎉 This PR is included in version stargate-v0.9.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Max-NV requested a review from a team as a code owner August 17, 2026 15:25

Max-NV requested a review from sanjay-saxena August 17, 2026 15:25

Max-NV self-assigned this Aug 17, 2026

Max-NV requested review from a team as code owners August 17, 2026 17:33

FamousDirector approved these changes Aug 17, 2026

View reviewed changes

Max-NV added this pull request to the merge queue Aug 17, 2026

Merged via the queue into main with commit 1e233f3 Aug 17, 2026
22 of 23 checks passed

Max-NV deleted the fix/pylon-upstream-health-path branch August 17, 2026 18:11

balajinvda added the released label Aug 17, 2026

coderabbitai Bot mentioned this pull request Aug 17, 2026

test(byoo): enforce collector startup health #921

Merged

4 tasks

Max-NV mentioned this pull request Aug 21, 2026

fix(nvca): backport pylon upstream health path to 3.2 #1069

Open

Navigation Menu

Uh oh!

fix(pylon): probe /v1/health/ready when the upstream serves no /health - #907

fix(pylon): probe /v1/health/ready when the upstream serves no /health#907
Max-NV merged 2 commits into
mainfrom
fix/pylon-upstream-health-path

Max-NV commented Aug 17, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

FamousDirector commented Aug 17, 2026

Uh oh!

Max-NV commented Aug 17, 2026

Uh oh!

Uh oh!

balajinvda commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Max-NV commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TL;DR

Additional Details

Rollout order

For the Reviewer

For QA

Issues

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 17, 2026

🛡️ CodeQL Analysis

Uh oh!

FamousDirector commented Aug 17, 2026

Uh oh!

Max-NV commented Aug 17, 2026

Uh oh!

Uh oh!

balajinvda commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Max-NV commented Aug 17, 2026 •

edited

Loading

coderabbitai Bot commented Aug 17, 2026 •

edited

Loading