feat(llm): add multi-replica worker routing - #585
Open
mikeyrcamp wants to merge 1 commit into
Open
Conversation
4 tasks
📝 WalkthroughWalkthroughThe PR adds optional LLM worker TCP and UDP Gateway routes, an optional backend-router workload with validation and observability, runtime image integration, default router address handling, initialized router metrics, and duplicate repository detection during image publishing. ChangesLLM worker Gateway routes
Backend router Helm deployment
Runtime router integration
Image publishing validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GatewayAPI
participant BackendRouterService
participant BackendRouterDeployment
participant LLMWorker
GatewayAPI->>BackendRouterService: Forward gRPC and QUIC traffic
BackendRouterService->>BackendRouterDeployment: Select backend-router pods
BackendRouterDeployment->>LLMWorker: Route worker registration and reverse-tunnel traffic
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Contributor
🛡️ CodeQL Analysis🚨 Found 2 issue(s) Severity Breakdown:
📋 Top Issues🔗 View full details in Security tab 🕐 Last updated: 2026-07-30 20:55:05 UTC | Commit: b42a0ff |
mikeyrcamp
force-pushed
the
codex/fix/self-managed-llm-router-reachability
branch
from
July 31, 2026 14:59
b42a0ff to
e96e684
Compare
mikeyrcamp
force-pushed
the
codex/fix/self-managed-llm-router-reachability
branch
from
July 31, 2026 16:00
ee2b245 to
2eb0b27
Compare
mikeyrcamp
marked this pull request as ready for review
July 31, 2026 16:01
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
deploy/helm/gateway-routes/chart/templates/referencegrant-llm-worker.yaml (1)
20-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the
ReferenceGranttarget scope consistent.Line 23 restricts access to one Service. Remove
spec.to[].nameso this template grants Service access by group and kind in the configured backend namespace, consistent with the chart convention.Proposed fix
to: - group: "" kind: Service - name: {{ .Values.nvcfGatewayRoutes.routes.llmWorker.backend.name }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/gateway-routes/chart/templates/referencegrant-llm-worker.yaml` around lines 20 - 23, Remove the spec.to[].name field from the ReferenceGrant template, leaving the target scoped only by group and kind so Service access applies within the configured backend namespace. Preserve the existing group and kind values.Source: Learnings
src/libraries/rust/stargate/crates/stargate/BUILD.bazel (1)
118-118: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression check for the bundled router binary.
The existing image test checks only
/usr/local/bin/stargate. Extend it or add a layer-content test that verifies/usr/local/bin/stargate-k8s-routeris present in the composite image.As per coding guidelines, code changes must include tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/libraries/rust/stargate/crates/stargate/BUILD.bazel` at line 118, Extend the existing image-content test for the composite image built by the Stargate build configuration to also assert that /usr/local/bin/stargate-k8s-router is present, alongside the existing /usr/local/bin/stargate check. If no suitable test exists, add a focused layer-content test targeting the image assembled with extra_layers, and ensure the regression test runs as part of the standard test target.Source: Coding guidelines
src/libraries/rust/stargate/crates/stargate-k8s-router/src/metrics.rs (1)
104-119: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert every declared outcome in the zero-series test.
The test checks only two labels. Iterate over
QUIC_CONNECTION_OUTCOMESandWEBTRANSPORT_SESSION_OUTCOMESand verify that every label is exported with value0.Proposed test update
- assert!( - body.contains(r#"stargate_k8s_router_quic_connections_total{outcome="accepted"} 0"# - ) - ); - assert!( - body.contains( - r#"stargate_k8s_router_webtransport_sessions_total{outcome="completed"} 0"# - ) - ); + for &outcome in QUIC_CONNECTION_OUTCOMES { + assert!(body.contains(&format!( + r#"stargate_k8s_router_quic_connections_total{{outcome="{}"}} 0"#, + outcome + ))); + } + for &outcome in WEBTRANSPORT_SESSION_OUTCOMES { + assert!(body.contains(&format!( + r#"stargate_k8s_router_webtransport_sessions_total{{outcome="{}"}} 0"#, + outcome + ))); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/libraries/rust/stargate/crates/stargate-k8s-router/src/metrics.rs` around lines 104 - 119, Update the metrics_export_known_outcomes_before_traffic test to iterate over every label in QUIC_CONNECTION_OUTCOMES and WEBTRANSPORT_SESSION_OUTCOMES, asserting that each corresponding metric series is exported with value 0. Replace the two hardcoded assertions while preserving the existing metric names and zero-series behavior.deploy/helm/llm-request-router/llm-request-router/values.yaml (1)
71-97: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a safer default for
backendRouter.replicaCount.
backendRouter.replicaCountdefaults to1. The comment on lines 67-70 states this component exists to route worker traffic whenreplicaCountfor the main router is greater than one. A single backend-router replica becomes a new single point of failure for a feature whose purpose is multi-replica availability. Consider defaulting to more than one replica, or add a comment next toreplicaCount: 1calling out this trade-off explicitly (beyond the general "override defaults for production" note in the README).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/llm-request-router/llm-request-router/values.yaml` around lines 71 - 97, Update backendRouter.replicaCount to a safer multi-replica default greater than one, or document the single-replica availability trade-off directly beside the setting if retaining 1. Keep the existing backendRouter configuration unchanged otherwise.deploy/helm/llm-request-router/llm-request-router/templates/_helpers.tpl (1)
156-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the cert/key directory check into a shared helper to avoid divergence.
validateBackendRouterTls(lines 170-172) duplicates the same directory-mismatch check and fail message already present indeployment.yaml(the unchanged{{- if ne (dir .Values.llmRequestRouter.tls.certPath) (dir .Values.llmRequestRouter.tls.keyPath) }}block). Both checks must stay in sync manually. Extract this into a single named template (for examplellm-request-router.validateTlsCertKeyDir) and call it from bothdeployment.yamlandvalidateBackendRouterTlsto prevent the checks from drifting apart in future changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/llm-request-router/llm-request-router/templates/_helpers.tpl` around lines 156 - 176, Extract the certificate/key directory comparison and its existing failure message from validateBackendRouterTls into a shared helper such as llm-request-router.validateTlsCertKeyDir. Replace the inline check in validateBackendRouterTls and the corresponding deployment.yaml block with calls to this helper, preserving the existing validation behavior and message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/image-push-manual.yml:
- Line 170: Move target-to-repository derivation and collision validation out of
the publishing loop using seen_repos, validating every mapping before any bazel
run or registry update occurs. Abort the workflow on any duplicate so no target
is published and latest-dispatch remains unchanged. Add a regression test
confirming that a duplicate mapping prevents all push commands from running.
In
`@deploy/helm/llm-request-router/llm-request-router/templates/backend-router.yaml`:
- Line 58: Introduce a dedicated backend-router ServiceAccount name helper, such
as llm-request-router.backendRouterServiceAccountName, and use it in the
backend-router workload and backend-router-rbac resources instead of
llm-request-router.serviceAccountName. Ensure the backend-router ServiceAccount
is created and the RoleBinding targets it, while leaving the main router
ServiceAccount and permissions unchanged.
- Around line 9-31: The backend-router service namespace must match the
namespace consumed by gateway routes. Update the backend-router namespace
configuration, including the llmWorker.backend.namespace value or its source
symbol, to use the release namespace or llmRequestRouter.namespace instead of a
hard-coded nvcf value, while preserving the existing service name and ports.
In
`@deploy/helm/llm-request-router/llm-request-router/templates/certificate.yaml`:
- Around line 19-20: Update the certificate template’s dnsNames handling to add
the backend-router wildcard before validation, safely reading the flag with dig
"backendRouter" "enabled" false .Values.llmRequestRouter. Replace the current
required-only validation with an empty check that fails when the expanded
dnsNames list remains empty, including the existing required message.
---
Nitpick comments:
In `@deploy/helm/gateway-routes/chart/templates/referencegrant-llm-worker.yaml`:
- Around line 20-23: Remove the spec.to[].name field from the ReferenceGrant
template, leaving the target scoped only by group and kind so Service access
applies within the configured backend namespace. Preserve the existing group and
kind values.
In `@deploy/helm/llm-request-router/llm-request-router/templates/_helpers.tpl`:
- Around line 156-176: Extract the certificate/key directory comparison and its
existing failure message from validateBackendRouterTls into a shared helper such
as llm-request-router.validateTlsCertKeyDir. Replace the inline check in
validateBackendRouterTls and the corresponding deployment.yaml block with calls
to this helper, preserving the existing validation behavior and message.
In `@deploy/helm/llm-request-router/llm-request-router/values.yaml`:
- Around line 71-97: Update backendRouter.replicaCount to a safer multi-replica
default greater than one, or document the single-replica availability trade-off
directly beside the setting if retaining 1. Keep the existing backendRouter
configuration unchanged otherwise.
In `@src/libraries/rust/stargate/crates/stargate-k8s-router/src/metrics.rs`:
- Around line 104-119: Update the metrics_export_known_outcomes_before_traffic
test to iterate over every label in QUIC_CONNECTION_OUTCOMES and
WEBTRANSPORT_SESSION_OUTCOMES, asserting that each corresponding metric series
is exported with value 0. Replace the two hardcoded assertions while preserving
the existing metric names and zero-series behavior.
In `@src/libraries/rust/stargate/crates/stargate/BUILD.bazel`:
- Line 118: Extend the existing image-content test for the composite image built
by the Stargate build configuration to also assert that
/usr/local/bin/stargate-k8s-router is present, alongside the existing
/usr/local/bin/stargate check. If no suitable test exists, add a focused
layer-content test targeting the image assembled with extra_layers, and ensure
the regression test runs as part of the standard test target.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 66638384-9a8e-4577-ae5c-b54f3ca789f0
⛔ Files ignored due to path filters (2)
src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/icms-translate/translate/function/llm.gois excluded by!**/vendor/**src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/icms-translate/translate/function/translate.gois excluded by!**/vendor/**
📒 Files selected for processing (26)
.github/workflows/image-push-manual.ymldeploy/helm/gateway-routes/README.mddeploy/helm/gateway-routes/chart/templates/referencegrant-llm-worker.yamldeploy/helm/gateway-routes/chart/templates/tcproute-llm-worker.yamldeploy/helm/gateway-routes/chart/templates/udproute-llm-worker.yamldeploy/helm/gateway-routes/chart/values.yamldeploy/helm/gateway-routes/scripts/check-llm-worker-routes.shdeploy/helm/llm-request-router/README.mddeploy/helm/llm-request-router/llm-request-router/templates/_helpers.tpldeploy/helm/llm-request-router/llm-request-router/templates/backend-router-rbac.yamldeploy/helm/llm-request-router/llm-request-router/templates/backend-router-servicemonitor.yamldeploy/helm/llm-request-router/llm-request-router/templates/backend-router.yamldeploy/helm/llm-request-router/llm-request-router/templates/certificate.yamldeploy/helm/llm-request-router/llm-request-router/templates/deployment.yamldeploy/helm/llm-request-router/llm-request-router/values.yamldeploy/helm/llm-request-router/scripts/check-backend-router-render.shsrc/compute-plane-services/nvca/pkg/nvca/cli.gosrc/libraries/go/lib/pkg/icms-translate/translate/function/llm.gosrc/libraries/go/lib/pkg/icms-translate/translate/function/llm_test.gosrc/libraries/go/lib/pkg/icms-translate/translate/function/translate.gosrc/libraries/rust/stargate/Dockerfilesrc/libraries/rust/stargate/README.mdsrc/libraries/rust/stargate/crates/pylon/BUILD.bazelsrc/libraries/rust/stargate/crates/stargate-k8s-router/BUILD.bazelsrc/libraries/rust/stargate/crates/stargate-k8s-router/src/metrics.rssrc/libraries/rust/stargate/crates/stargate/BUILD.bazel
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
| spec: | ||
| serviceAccountName: {{ include "llm-request-router.serviceAccountName" . }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Backend-router shares the main router's ServiceAccount, over-provisioning EndpointSlice access.
serviceAccountName on line 58 resolves to the same llm-request-router.serviceAccountName helper used by the main router StatefulSet (deployment.yaml line 56). backend-router-rbac.yaml grants that ServiceAccount get/list/watch on discovery.k8s.io/endpointslices. Because values.yaml has no backendRouter.serviceAccount override, enabling backendRouter grants every main-router replica pod EndpointSlice read access too, even though only the backend-router pods use it. Give backendRouter its own ServiceAccount (and scope the Role/RoleBinding to it) to keep the main router pods at least privilege.
🔒 Proposed direction
+ backendRouter:
+ serviceAccount:
+ create: true
+ name: ""Then reference a dedicated llm-request-router.backendRouterServiceAccountName helper from both backend-router.yaml and backend-router-rbac.yaml, instead of reusing llm-request-router.serviceAccountName.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@deploy/helm/llm-request-router/llm-request-router/templates/backend-router.yaml`
at line 58, Introduce a dedicated backend-router ServiceAccount name helper,
such as llm-request-router.backendRouterServiceAccountName, and use it in the
backend-router workload and backend-router-rbac resources instead of
llm-request-router.serviceAccountName. Ensure the backend-router ServiceAccount
is created and the RoleBinding targets it, while leaving the main router
ServiceAccount and permissions unchanged.
Max-NV
approved these changes
Jul 31, 2026
Signed-off-by: Mike Camp <mcamp@nvidia.com>
mikeyrcamp
force-pushed
the
codex/fix/self-managed-llm-router-reachability
branch
from
July 31, 2026 21:04
2eb0b27 to
cea375d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
deploy/helm/gateway-routes/chart/templates/referencegrant-llm-worker.yaml (1)
21-24: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRestore the chart Service-grant model.
Remove
spec.to[].name. Other ReferenceGrant templates grant access by Service group and kind within the configured backend namespace.Proposed change
to: - group: "" kind: Service - name: {{ .Values.nvcfGatewayRoutes.routes.llmWorker.backend.name }}Based on learnings, ReferenceGrant templates should intentionally omit
spec.to[].nameunless the chart access model changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/gateway-routes/chart/templates/referencegrant-llm-worker.yaml` around lines 21 - 24, Update the ReferenceGrant template’s spec.to entry to remove the backend-specific name while retaining the empty group and Service kind, matching the chart’s namespace-wide Service-grant model used by the other ReferenceGrant templates.Source: Learnings
deploy/helm/gateway-routes/scripts/check-llm-worker-routes.sh (2)
71-74: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest the chart default disabled state.
This render explicitly sets
llmWorker.enabled=false. It passes even if the chart default changes totrue. Add a render without theenabledoverride and assert that the route resources are absent.The PR objectives require all new routing resources to be disabled by default.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/gateway-routes/scripts/check-llm-worker-routes.sh` around lines 71 - 74, Add a separate Helm render in the route test script without setting nvcfGatewayRoutes.routes.llmWorker.enabled, then assert that the generated output contains no LLM worker route resources. Keep the existing explicitly disabled render coverage, and ensure the new check verifies the chart default is disabled.
23-60: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftScope assertions to each rendered resource.
assert_containssearches the complete YAML output. Theawkcheck and namespace count are also aggregate checks. A route can use the wrong backend, listener, port, or namespace and still pass when the expected strings occur in another resource. AReferenceGrantcan also target the wrong namespace or kind.Parse each YAML document or use resource-scoped checks for both routes and the
ReferenceGrant.This protects the TCP and UDP route contract described in the PR objectives.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/gateway-routes/scripts/check-llm-worker-routes.sh` around lines 23 - 60, Replace the aggregate string checks in the script with resource-scoped validation for the TCPRoute, UDPRoute, and ReferenceGrant documents. Ensure each route independently validates its expected backend name, namespace, listener section, and protocol-specific configuration, while the ReferenceGrant validates its name, target service, namespace, and kind. Update the awk/grep logic around assert_contains, reference_grant_service_name, and backend_namespace_references so matching fields from unrelated rendered resources cannot satisfy the checks.deploy/helm/llm-request-router/llm-request-router/templates/_helpers.tpl (1)
118-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate registry/repository/tag construction with
llm-request-router.image.
llm-request-router.backendRouterImagerepeats the same registry-fallback-then-printf pattern already defined inllm-request-router.image(Lines 92-100), differing only in the tag-resolution rule (requiredversusdefault .Chart.AppVersion). Extract a shared helper that takes the resolved tag as an argument to avoid maintaining two copies of the image-string assembly logic.♻️ Proposed refactor to share image assembly logic
+{{- define "llm-request-router.buildImage" -}} +{{- $registry := index . 0 -}} +{{- $repository := index . 1 -}} +{{- $tag := index . 2 -}} +{{- if $registry -}} +{{- printf "%s/%s:%s" $registry $repository $tag -}} +{{- else -}} +{{- printf "%s:%s" $repository $tag -}} +{{- end -}} +{{- end }} + {{- define "llm-request-router.backendRouterImage" -}} {{- $image := .Values.llmRequestRouter.backendRouter.image -}} {{- $registry := default .Values.llmRequestRouter.image.registry $image.registry -}} {{- $repository := default .Values.llmRequestRouter.image.repository $image.repository -}} {{- $tag := required "llmRequestRouter.backendRouter.image.tag is required when backendRouter.enabled is true" $image.tag -}} -{{- if $registry -}} -{{- printf "%s/%s:%s" $registry $repository $tag -}} -{{- else -}} -{{- printf "%s:%s" $repository $tag -}} -{{- end -}} +{{- include "llm-request-router.buildImage" (list $registry $repository $tag) -}} {{- end }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/llm-request-router/llm-request-router/templates/_helpers.tpl` around lines 118 - 129, Extract the duplicated registry/repository image-string assembly from llm-request-router.image and llm-request-router.backendRouterImage into a shared helper that accepts the resolved tag. Update both helpers to resolve their existing tags independently—preserving required for backendRouterImage and the existing default for the primary image—then delegate formatting to the shared helper.deploy/helm/llm-request-router/llm-request-router/templates/deployment.yaml (1)
34-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing negative test for the new
reverseTunnelListenAddrrequirement.This adds a new
failguard requiringllmRequestRouter.transport.reverseTunnelListenAddrwhenbackendRouter.enabledis true.check-backend-router-render.shdoes not include a case that unsetstransport.reverseTunnelListenAddrwhile enablingbackendRouterto confirm this guard actually triggers.
As per coding guidelines, "Code changes must include tests; if tests are omitted, explain why in the Pull Request description." Add a negative-render assertion for this path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/llm-request-router/llm-request-router/templates/deployment.yaml` around lines 34 - 38, The backend-router rendering tests lack coverage for the new reverse tunnel address requirement. Update check-backend-router-render.sh to enable backendRouter while unsetting llmRequestRouter.transport.reverseTunnelListenAddr, render the chart, and assert that rendering fails with the expected requirement message; keep existing positive cases unchanged.Source: Coding guidelines
deploy/helm/llm-request-router/scripts/check-backend-router-render.sh (1)
43-55: 📐 Maintainability & Code Quality | 🔵 Trivial
assert_backend_router_replicasdoes not reset state at document boundaries.The awk script never resets
in_deployment/backend_routerat a---document separator. With only oneDeploymentmanifest in the rendered output today this works, but if a secondDeploymentis ever added to the chart, stale flag state from an earlier document could cause a false match. Reset both flags on$0 == "---"for robustness.♻️ Proposed defensive reset
actual="$(awk ' + $0 == "---" { in_deployment = 0; backend_router = 0 } $0 == "kind: Deployment" { in_deployment = 1; backend_router = 0 } in_deployment && $1 == "name:" && $2 == "llm-request-router-backend-router" { backend_router = 1 } backend_router && $1 == "replicas:" { print $2; exit } ' "$rendered")"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/llm-request-router/scripts/check-backend-router-render.sh` around lines 43 - 55, Update the awk script inside assert_backend_router_replicas to reset both in_deployment and backend_router when the input line equals the YAML document separator "---", preventing state from leaking between manifests while preserving the existing Deployment matching behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/helm/gateway-routes/README.md`:
- Line 24: Update the README text to hyphenate the compound adjective, changing
“Gateway API compatible controller” to “Gateway API-compatible controller.”
In `@src/libraries/rust/stargate/tools/ci/oci_image_contains_path_test.sh`:
- Line 1: Rename the test entrypoint from oci_image_contains_path_test.sh to the
hyphenated test-oci-image-contains-path.sh form, then update all references in
the tools/ci BUILD target and the downstream image test definition in the
stargate BUILD target to use the new filename.
- Around line 20-32: Replace the all-blob scan in the image-path test with OCI
graph traversal: parse index.json, resolve each platform manifest, follow its
ordered layers, and materialize each platform’s filesystem while applying OCI
whiteouts before checking for /usr/local/bin/stargate-k8s-router. Require the
path in the final filesystem for every platform, and add regression fixtures
covering stale unreferenced layers, cross-platform layers, and entries removed
by later whiteouts.
---
Nitpick comments:
In `@deploy/helm/gateway-routes/chart/templates/referencegrant-llm-worker.yaml`:
- Around line 21-24: Update the ReferenceGrant template’s spec.to entry to
remove the backend-specific name while retaining the empty group and Service
kind, matching the chart’s namespace-wide Service-grant model used by the other
ReferenceGrant templates.
In `@deploy/helm/gateway-routes/scripts/check-llm-worker-routes.sh`:
- Around line 71-74: Add a separate Helm render in the route test script without
setting nvcfGatewayRoutes.routes.llmWorker.enabled, then assert that the
generated output contains no LLM worker route resources. Keep the existing
explicitly disabled render coverage, and ensure the new check verifies the chart
default is disabled.
- Around line 23-60: Replace the aggregate string checks in the script with
resource-scoped validation for the TCPRoute, UDPRoute, and ReferenceGrant
documents. Ensure each route independently validates its expected backend name,
namespace, listener section, and protocol-specific configuration, while the
ReferenceGrant validates its name, target service, namespace, and kind. Update
the awk/grep logic around assert_contains, reference_grant_service_name, and
backend_namespace_references so matching fields from unrelated rendered
resources cannot satisfy the checks.
In `@deploy/helm/llm-request-router/llm-request-router/templates/_helpers.tpl`:
- Around line 118-129: Extract the duplicated registry/repository image-string
assembly from llm-request-router.image and llm-request-router.backendRouterImage
into a shared helper that accepts the resolved tag. Update both helpers to
resolve their existing tags independently—preserving required for
backendRouterImage and the existing default for the primary image—then delegate
formatting to the shared helper.
In `@deploy/helm/llm-request-router/llm-request-router/templates/deployment.yaml`:
- Around line 34-38: The backend-router rendering tests lack coverage for the
new reverse tunnel address requirement. Update check-backend-router-render.sh to
enable backendRouter while unsetting
llmRequestRouter.transport.reverseTunnelListenAddr, render the chart, and assert
that rendering fails with the expected requirement message; keep existing
positive cases unchanged.
In `@deploy/helm/llm-request-router/scripts/check-backend-router-render.sh`:
- Around line 43-55: Update the awk script inside assert_backend_router_replicas
to reset both in_deployment and backend_router when the input line equals the
YAML document separator "---", preventing state from leaking between manifests
while preserving the existing Deployment matching behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8ee6688c-cca3-4c57-9a80-d85b3afd4917
⛔ Files ignored due to path filters (2)
src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/icms-translate/translate/function/llm.gois excluded by!**/vendor/**src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/icms-translate/translate/function/translate.gois excluded by!**/vendor/**
📒 Files selected for processing (32)
.github/workflows/bazel.yml.github/workflows/image-push-manual.ymldeploy/helm/gateway-routes/README.mddeploy/helm/gateway-routes/chart/templates/_helpers.tpldeploy/helm/gateway-routes/chart/templates/referencegrant-llm-worker.yamldeploy/helm/gateway-routes/chart/templates/tcproute-llm-worker.yamldeploy/helm/gateway-routes/chart/templates/udproute-llm-worker.yamldeploy/helm/gateway-routes/chart/values.yamldeploy/helm/gateway-routes/scripts/check-llm-worker-routes.shdeploy/helm/llm-request-router/README.mddeploy/helm/llm-request-router/llm-request-router/templates/_helpers.tpldeploy/helm/llm-request-router/llm-request-router/templates/backend-router-rbac.yamldeploy/helm/llm-request-router/llm-request-router/templates/backend-router-serviceaccount.yamldeploy/helm/llm-request-router/llm-request-router/templates/backend-router-servicemonitor.yamldeploy/helm/llm-request-router/llm-request-router/templates/backend-router.yamldeploy/helm/llm-request-router/llm-request-router/templates/certificate.yamldeploy/helm/llm-request-router/llm-request-router/templates/deployment.yamldeploy/helm/llm-request-router/llm-request-router/values.yamldeploy/helm/llm-request-router/scripts/check-backend-router-render.shsrc/compute-plane-services/nvca/pkg/nvca/cli.gosrc/libraries/go/lib/pkg/icms-translate/translate/function/llm.gosrc/libraries/go/lib/pkg/icms-translate/translate/function/llm_test.gosrc/libraries/go/lib/pkg/icms-translate/translate/function/translate.gosrc/libraries/rust/stargate/Dockerfilesrc/libraries/rust/stargate/README.mdsrc/libraries/rust/stargate/crates/pylon/BUILD.bazelsrc/libraries/rust/stargate/crates/stargate-k8s-router/BUILD.bazelsrc/libraries/rust/stargate/crates/stargate-k8s-router/src/metrics.rssrc/libraries/rust/stargate/crates/stargate/BUILD.bazelsrc/libraries/rust/stargate/tools/ci/BUILD.bazelsrc/libraries/rust/stargate/tools/ci/oci_image_contains_path_test.shtools/ci/test-image-push-manual
🚧 Files skipped from review as they are similar to previous changes (15)
- src/compute-plane-services/nvca/pkg/nvca/cli.go
- src/libraries/go/lib/pkg/icms-translate/translate/function/translate.go
- src/libraries/rust/stargate/crates/pylon/BUILD.bazel
- src/libraries/go/lib/pkg/icms-translate/translate/function/llm.go
- src/libraries/rust/stargate/Dockerfile
- src/libraries/go/lib/pkg/icms-translate/translate/function/llm_test.go
- deploy/helm/gateway-routes/chart/values.yaml
- src/libraries/rust/stargate/crates/stargate/BUILD.bazel
- deploy/helm/llm-request-router/README.md
- src/libraries/rust/stargate/README.md
- .github/workflows/image-push-manual.yml
- deploy/helm/llm-request-router/llm-request-router/templates/certificate.yaml
- src/libraries/rust/stargate/crates/stargate-k8s-router/src/metrics.rs
- src/libraries/rust/stargate/crates/stargate-k8s-router/BUILD.bazel
- deploy/helm/llm-request-router/llm-request-router/values.yaml
| - Kubernetes cluster | ||
| - Helm 3.x | ||
| - `kubectl` | ||
| - A Gateway API compatible controller installed in the cluster |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Hyphenate the compound adjective.
Change Gateway API compatible controller to Gateway API-compatible controller.
🧰 Tools
🪛 LanguageTool
[grammar] ~24-~24: Use a hyphen to join words.
Context: ...r - Helm 3.x - kubectl - A Gateway API compatible controller installed in the c...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/helm/gateway-routes/README.md` at line 24, Update the README text to
hyphenate the compound adjective, changing “Gateway API compatible controller”
to “Gateway API-compatible controller.”
Source: Linters/SAST tools
| @@ -0,0 +1,35 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Rename the test entrypoint to match the repository naming rule.
oci_image_contains_path_test.sh uses underscores and does not use the required test-<tool-name> form. Rename it to a hyphenated name, such as test-oci-image-contains-path.sh. Update src/libraries/rust/stargate/tools/ci/BUILD.bazel, Line 7, and the downstream image test in src/libraries/rust/stargate/crates/stargate/BUILD.bazel, Lines 111-146.
As per coding guidelines, files under tools/ must use hyphens, and test entrypoints must use the test-<tool-name> form.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/libraries/rust/stargate/tools/ci/oci_image_contains_path_test.sh` at line
1, Rename the test entrypoint from oci_image_contains_path_test.sh to the
hyphenated test-oci-image-contains-path.sh form, then update all references in
the tools/ci BUILD target and the downstream image test definition in the
stargate BUILD target to use the new filename.
Source: Coding guidelines
Comment on lines
+20
to
+32
| while IFS= read -r -d '' blob; do | ||
| if entries="$(tar -tf "${blob}" 2>/dev/null)"; then | ||
| while IFS= read -r entry; do | ||
| entry="${entry#./}" | ||
| entry="${entry#/}" | ||
| if [[ "${entry}" == "${image_path}" ]]; then | ||
| exit 0 | ||
| fi | ||
| done <<< "${entries}" | ||
| fi | ||
| # rules_oci may symlink blob files to their source layers. Select those links | ||
| # directly without following symlinked directories outside the blob tree. | ||
| done < <(find "${image_layout}/blobs/sha256" \( -type f -o -type l \) -print0) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
cat -n src/libraries/rust/stargate/tools/ci/oci_image_contains_path_test.sh
printf '%s\n' '--- related files ---'
git ls-files | rg '(^|/)(oci_image_contains_path_test\.sh|.*oci.*test.*|index\.json|image-layout\.md|rules_oci|stargate.*router)'
printf '%s\n' '--- references to the test and image_path ---'
rg -n -C 3 'oci_image_contains_path_test|image_path|stargate-k8s-router|index\.json|whiteout' src/libraries/rust/stargate .github 2>/dev/null || trueRepository: NVIDIA/nvcf
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -eu
log=/tmp/coderabbit-shell-logs/shell-output-IxMA9z
printf '%s\n' '--- target-file section ---'
rg -n -A 80 -B 10 'oci_image_contains_path_test\.sh|while IFS= read -r -d' "$log" | head -n 180
printf '%s\n' '--- relevant references ---'
rg -n -i -C 3 'stargate-k8s-router|image_path|index\.json|whiteout|oci_image_contains' "$log" | head -n 240Repository: NVIDIA/nvcf
Length of output: 36496
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- image BUILD definitions and test call sites ---'
rg -n -C 12 'oci_image_contains_path_test|stargate-k8s-router-image|oci_load|oci_tarball|image_index' src/libraries/rust/stargate --glob 'BUILD*' --glob '*.bzl' --glob '*.sh'
printf '%s\n' '--- comparable OCI index tests ---'
cat -n src/control-plane-services/function-autoscaler/rules/oci/private/image_index_platforms_test.sh
cat -n src/control-plane-services/function-autoscaler/rules/oci/private/test_image_index_platforms.sh
printf '%s\n' '--- target script metadata ---'
cat -n src/libraries/rust/stargate/tools/ci/BUILD.bazelRepository: NVIDIA/nvcf
Length of output: 27791
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- stargate image rule context ---'
cat -n src/libraries/rust/stargate/crates/stargate/BUILD.bazel | sed -n '1,155p'
printf '%s\n' '--- shared image macro implementation ---'
cat -n src/libraries/rust/stargate/rules/oci/private/common.bzl | sed -n '1,115p'
printf '%s\n' '--- standalone OCI graph probe ---'
python3 - <<'PY'
import hashlib
import json
import os
import tempfile
import tarfile
from pathlib import Path
TARGET = "usr/local/bin/stargate-k8s-router"
def digest_bytes(data):
return hashlib.sha256(data).hexdigest()
def write_blob(root, digest, data):
path = root / "blobs" / "sha256" / digest
path.parent.mkdir(parents=True, exist_ok=True)
path.write_bytes(data)
return digest
def layer(entries):
out = tempfile.SpooledTemporaryFile()
with tarfile.open(fileobj=out, mode="w") as tf:
for name, body in entries:
info = tarfile.TarInfo(name)
info.size = len(body)
tf.addfile(info, body and __import__("io").BytesIO(body))
out.seek(0)
return out.read()
def make_layout(platform_layers):
root = Path(tempfile.mkdtemp())
manifests = []
for platform, layers in platform_layers.items():
layer_desc = []
diff_ids = []
for entries in layers:
data = layer(entries)
digest = write_blob(root, digest_bytes(data), data)
layer_desc.append({
"mediaType": "application/vnd.oci.image.layer.v1.tar",
"digest": "sha256:" + digest,
"size": len(data),
})
diff_ids.append("sha256:" + digest)
config = json.dumps({
"architecture": platform,
"os": "linux",
"rootfs": {"type": "layers", "diff_ids": diff_ids},
}).encode()
config_digest = write_blob(root, digest_bytes(config), config)
manifest = json.dumps({
"schemaVersion": 2,
"config": {
"mediaType": "application/vnd.oci.image.config.v1+json",
"digest": "sha256:" + config_digest,
"size": len(config),
},
"layers": layer_desc,
}).encode()
manifest_digest = write_blob(root, digest_bytes(manifest), manifest)
manifests.append({
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"digest": "sha256:" + manifest_digest,
"size": len(manifest),
"platform": {"os": "linux", "architecture": platform},
})
index = json.dumps({"schemaVersion": 2, "manifests": manifests}).encode()
index_digest = write_blob(root, digest_bytes(index), index)
(root / "index.json").write_text(json.dumps({
"schemaVersion": 2,
"manifests": [{
"mediaType": "application/vnd.oci.image.index.v1+json",
"digest": "sha256:" + index_digest,
"size": len(index),
}],
}))
return root
def all_blob_tar_members(root):
found = set()
for path in (root / "blobs" / "sha256").iterdir():
try:
with tarfile.open(path) as tf:
found.update(member.name.lstrip("./").lstrip("/")
for member in tf.getmembers())
except tarfile.TarError:
pass
return found
def referenced_layer_members(root):
outer = json.loads((root / "index.json").read_text())
nested_digest = outer["manifests"][0]["digest"].split(":", 1)[1]
nested = json.loads((root / "blobs" / "sha256" / nested_digest).read_text())
result = {}
for desc in nested["manifests"]:
manifest_digest = desc["digest"].split(":", 1)[1]
manifest = json.loads((root / "blobs" / "sha256" / manifest_digest).read_text())
visible = set()
for layer_desc in manifest["layers"]:
digest = layer_desc["digest"].split(":", 1)[1]
with tarfile.open(root / "blobs" / "sha256" / digest) as tf:
names = {m.name.lstrip("./").lstrip("/") for m in tf.getmembers()}
for name in names:
if name.startswith(".wh."):
visible.discard(name[4:])
else:
visible.add(name)
result[desc["platform"]["architecture"]] = visible
return result
cases = {
"unreferenced stale blob": {
"platforms": {"amd64": [[("etc/ok", b"ok")]]},
"stale": [(TARGET, b"stale")],
},
"platform-specific layer": {
"platforms": {
"amd64": [[("etc/ok", b"ok")]],
"arm64": [[(TARGET, b"router")]],
},
"stale": [],
},
"later whiteout": {
"platforms": {
"amd64": [[(TARGET, b"router")], [(".wh.stargate-k8s-router", b"")]],
},
"stale": [],
},
}
for name, case in cases.items():
root = make_layout(case["platforms"])
for entries in case["stale"]:
data = layer([entries])
write_blob(root, digest_bytes(data), data)
any_blob = TARGET in all_blob_tar_members(root)
final = referenced_layer_members(root)
print(f"{name}: current_any_blob={'PASS' if any_blob else 'FAIL'}; "
f"referenced_final={{{', '.join(f'{k}: {'PASS' if TARGET in v else 'FAIL'}' for k, v in final.items())}}}")
PYRepository: NVIDIA/nvcf
Length of output: 11739
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import hashlib
import io
import json
import tempfile
import tarfile
from pathlib import Path
TARGET = "usr/local/bin/stargate-k8s-router"
def digest_bytes(data):
return hashlib.sha256(data).hexdigest()
def write_blob(root, digest, data):
path = root / "blobs" / "sha256" / digest
path.parent.mkdir(parents=True, exist_ok=True)
path.write_bytes(data)
return digest
def layer(entries):
out = io.BytesIO()
with tarfile.open(fileobj=out, mode="w") as tf:
for name, body in entries:
info = tarfile.TarInfo(name)
info.size = len(body)
tf.addfile(info, io.BytesIO(body))
return out.getvalue()
def make_layout(platform_layers):
root = Path(tempfile.mkdtemp())
manifests = []
for platform, layers in platform_layers.items():
layer_desc = []
diff_ids = []
for entries in layers:
data = layer(entries)
digest = write_blob(root, digest_bytes(data), data)
layer_desc.append({
"mediaType": "application/vnd.oci.image.layer.v1.tar",
"digest": "sha256:" + digest,
"size": len(data),
})
diff_ids.append("sha256:" + digest)
config = json.dumps({
"architecture": platform,
"os": "linux",
"rootfs": {"type": "layers", "diff_ids": diff_ids},
}).encode()
config_digest = write_blob(root, digest_bytes(config), config)
manifest = json.dumps({
"schemaVersion": 2,
"config": {
"mediaType": "application/vnd.oci.image.config.v1+json",
"digest": "sha256:" + config_digest,
"size": len(config),
},
"layers": layer_desc,
}).encode()
manifest_digest = write_blob(root, digest_bytes(manifest), manifest)
manifests.append({
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"digest": "sha256:" + manifest_digest,
"size": len(manifest),
"platform": {"os": "linux", "architecture": platform},
})
index = json.dumps({"schemaVersion": 2, "manifests": manifests}).encode()
index_digest = write_blob(root, digest_bytes(index), index)
(root / "index.json").write_text(json.dumps({
"schemaVersion": 2,
"manifests": [{
"mediaType": "application/vnd.oci.image.index.v1+json",
"digest": "sha256:" + index_digest,
"size": len(index),
}],
}))
return root
def all_blob_tar_members(root):
found = set()
for path in (root / "blobs" / "sha256").iterdir():
try:
with tarfile.open(path) as tf:
found.update(member.name.lstrip("./").lstrip("/")
for member in tf.getmembers())
except tarfile.TarError:
pass
return found
def referenced_layer_members(root):
outer = json.loads((root / "index.json").read_text())
nested_digest = outer["manifests"][0]["digest"].split(":", 1)[1]
nested = json.loads((root / "blobs" / "sha256" / nested_digest).read_text())
result = {}
for desc in nested["manifests"]:
manifest_digest = desc["digest"].split(":", 1)[1]
manifest = json.loads((root / "blobs" / "sha256" / manifest_digest).read_text())
visible = set()
for layer_desc in manifest["layers"]:
digest = layer_desc["digest"].split(":", 1)[1]
with tarfile.open(root / "blobs" / "sha256" / digest) as tf:
names = {m.name.lstrip("./").lstrip("/") for m in tf.getmembers()}
for name in names:
if name.startswith(".wh."):
visible.discard(name[4:])
else:
visible.add(name)
result[desc["platform"]["architecture"]] = visible
return result
cases = {
"unreferenced stale blob": (
{"amd64": [[("etc/ok", b"ok")]]},
[(TARGET, b"stale")],
),
"platform-specific layer": (
{
"amd64": [[("etc/ok", b"ok")]],
"arm64": [[(TARGET, b"router")]],
},
[],
),
"later whiteout": (
{
"amd64": [[(TARGET, b"router")], [(".wh.stargate-k8s-router", b"")]],
},
[],
),
}
for name, (platforms, stale_entries) in cases.items():
root = make_layout(platforms)
if stale_entries:
data = layer([stale_entries])
write_blob(root, digest_bytes(data), data)
any_blob = TARGET in all_blob_tar_members(root)
final = referenced_layer_members(root)
final_status = ", ".join(
f"{arch}: {'PASS' if TARGET in members else 'FAIL'}"
for arch, members in final.items()
)
print(f"{name}: current_any_blob={'PASS' if any_blob else 'FAIL'}; "
f"referenced_final={{{final_status}}}")
PYRepository: NVIDIA/nvcf
Length of output: 320
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import hashlib
import io
import json
import tempfile
import tarfile
from pathlib import Path
TARGET = "usr/local/bin/stargate-k8s-router"
def digest_bytes(data):
return hashlib.sha256(data).hexdigest()
def write_blob(root, digest, data):
path = root / "blobs" / "sha256" / digest
path.parent.mkdir(parents=True, exist_ok=True)
path.write_bytes(data)
def make_layer(entries):
out = io.BytesIO()
with tarfile.open(fileobj=out, mode="w") as tf:
for name, body in entries:
info = tarfile.TarInfo(name)
info.size = len(body)
tf.addfile(info, io.BytesIO(body))
return out.getvalue()
def make_layout(platform_layers):
root = Path(tempfile.mkdtemp())
manifests = []
for platform, layers in platform_layers.items():
layer_desc = []
for entries in layers:
data = make_layer(entries)
digest = digest_bytes(data)
write_blob(root, digest, data)
layer_desc.append({
"mediaType": "application/vnd.oci.image.layer.v1.tar",
"digest": "sha256:" + digest,
"size": len(data),
})
config = json.dumps({
"architecture": platform,
"os": "linux",
"rootfs": {"type": "layers", "diff_ids": []},
}).encode()
config_digest = digest_bytes(config)
write_blob(root, config_digest, config)
manifest = json.dumps({
"schemaVersion": 2,
"config": {
"mediaType": "application/vnd.oci.image.config.v1+json",
"digest": "sha256:" + config_digest,
"size": len(config),
},
"layers": layer_desc,
}).encode()
manifest_digest = digest_bytes(manifest)
write_blob(root, manifest_digest, manifest)
manifests.append({
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"digest": "sha256:" + manifest_digest,
"size": len(manifest),
"platform": {"os": "linux", "architecture": platform},
})
nested = json.dumps({"schemaVersion": 2, "manifests": manifests}).encode()
nested_digest = digest_bytes(nested)
write_blob(root, nested_digest, nested)
(root / "index.json").parent.mkdir(parents=True, exist_ok=True)
(root / "index.json").write_text(json.dumps({
"schemaVersion": 2,
"manifests": [{
"mediaType": "application/vnd.oci.image.index.v1+json",
"digest": "sha256:" + nested_digest,
"size": len(nested),
}],
}))
return root
def current_any_blob_contains(root):
for path in (root / "blobs" / "sha256").iterdir():
try:
with tarfile.open(path) as tf:
if any(m.name.lstrip("./").lstrip("/") == TARGET for m in tf.getmembers()):
return True
except tarfile.TarError:
pass
return False
def referenced_final_paths(root):
outer = json.loads((root / "index.json").read_text())
nested_digest = outer["manifests"][0]["digest"].split(":", 1)[1]
nested = json.loads((root / "blobs" / "sha256" / nested_digest).read_text())
result = {}
for descriptor in nested["manifests"]:
manifest_digest = descriptor["digest"].split(":", 1)[1]
manifest = json.loads((root / "blobs" / "sha256" / manifest_digest).read_text())
visible = set()
for layer_descriptor in manifest["layers"]:
layer_digest = layer_descriptor["digest"].split(":", 1)[1]
with tarfile.open(root / "blobs" / "sha256" / layer_digest) as tf:
names = {m.name.lstrip("./").lstrip("/") for m in tf.getmembers()}
for name in names:
if name.startswith(".wh."):
visible.discard(name[4:])
else:
visible.add(name)
result[descriptor["platform"]["architecture"]] = TARGET in visible
return result
cases = [
("unreferenced stale blob",
{"amd64": [[("etc/ok", b"ok")]]},
[(TARGET, b"stale")]),
("platform-specific layer",
{"amd64": [[("etc/ok", b"ok")]], "arm64": [[(TARGET, b"router")]]},
None),
("later whiteout",
{"amd64": [[(TARGET, b"router")], [(".wh.stargate-k8s-router", b"")]]},
None),
]
for name, platforms, stale_entries in cases:
root = make_layout(platforms)
if stale_entries is not None:
data = make_layer(stale_entries)
write_blob(root, digest_bytes(data), data)
print(
f"{name}: current_any_blob={'PASS' if current_any_blob_contains(root) else 'FAIL'}; "
f"referenced_final={referenced_final_paths(root)}"
)
PYRepository: NVIDIA/nvcf
Length of output: 400
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import hashlib
import io
import json
import tempfile
import tarfile
from pathlib import Path
target = "usr/local/bin/stargate-k8s-router"
def digest(data):
return hashlib.sha256(data).hexdigest()
def tar_layer(entries):
out = io.BytesIO()
with tarfile.open(fileobj=out, mode="w") as tf:
for name, body in entries:
info = tarfile.TarInfo(name)
info.size = len(body)
tf.addfile(info, io.BytesIO(body))
return out.getvalue()
def build_layout():
root = Path(tempfile.mkdtemp())
blobs = root / "blobs" / "sha256"
blobs.mkdir(parents=True)
layers = [
tar_layer([(target, b"router")]),
tar_layer([("usr/local/bin/.wh.stargate-k8s-router", b"")]),
]
layer_desc = []
for data in layers:
d = digest(data)
(blobs / d).write_bytes(data)
layer_desc.append({"digest": "sha256:" + d, "size": len(data)})
config = b'{"architecture":"amd64","os":"linux"}'
config_d = digest(config)
(blobs / config_d).write_bytes(config)
manifest = json.dumps({
"schemaVersion": 2,
"config": {"digest": "sha256:" + config_d},
"layers": layer_desc,
}).encode()
manifest_d = digest(manifest)
(blobs / manifest_d).write_bytes(manifest)
nested = json.dumps({
"schemaVersion": 2,
"manifests": [{
"digest": "sha256:" + manifest_d,
"platform": {"os": "linux", "architecture": "amd64"},
}],
}).encode()
nested_d = digest(nested)
(blobs / nested_d).write_bytes(nested)
(root / "index.json").write_text(json.dumps({
"schemaVersion": 2,
"manifests": [{"digest": "sha256:" + nested_d}],
}))
return root
root = build_layout()
any_blob = False
for path in (root / "blobs" / "sha256").iterdir():
try:
with tarfile.open(path) as tf:
any_blob |= any(
member.name.lstrip("./").lstrip("/") == target
for member in tf.getmembers()
)
except tarfile.TarError:
pass
visible = set()
outer = json.loads((root / "index.json").read_text())
nested_d = outer["manifests"][0]["digest"].split(":", 1)[1]
nested = json.loads((root / "blobs" / "sha256" / nested_d).read_text())
manifest_d = nested["manifests"][0]["digest"].split(":", 1)[1]
manifest = json.loads((root / "blobs" / "sha256" / manifest_d).read_text())
for descriptor in manifest["layers"]:
layer_d = descriptor["digest"].split(":", 1)[1]
with tarfile.open(root / "blobs" / "sha256" / layer_d) as tf:
for member in tf.getmembers():
name = member.name.lstrip("./").lstrip("/")
if name.startswith("usr/local/bin/.wh."):
visible.discard("usr/local/bin/" + name.split(".wh.", 1)[1])
else:
visible.add(name)
print(f"later whiteout: current_any_blob={'PASS' if any_blob else 'FAIL'}; "
f"referenced_final={'PASS' if target in visible else 'FAIL'}")
PYRepository: NVIDIA/nvcf
Length of output: 210
Validate the referenced image graph, not every blob.
This multi-architecture image test scans every tar-readable blob and can pass for an unreferenced stale layer, a layer from another platform, or a lower-layer entry removed by a later whiteout. Resolve index.json, follow each platform manifest’s layers in order, apply OCI whiteouts, and require /usr/local/bin/stargate-k8s-router in the final filesystem for every platform. Add regression fixtures for these cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/libraries/rust/stargate/tools/ci/oci_image_contains_path_test.sh` around
lines 20 - 32, Replace the all-blob scan in the image-path test with OCI graph
traversal: parse index.json, resolve each platform manifest, follow its ordered
layers, and materialize each platform’s filesystem while applying OCI whiteouts
before checking for /usr/local/bin/stargate-k8s-router. Require the path in the
final filesystem for every platform, and add regression fixtures covering stale
unreferenced layers, cross-platform layers, and entries removed by later
whiteouts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Add production-safe backend routing for LLM workers when the request router
runs multiple Stargate replicas. The change packages Stargate's
authority/SNI-aware Kubernetes router, adds an optional router workload to the
request-router chart, and adds matching TCP and UDP Gateway API routes.
Clean-cluster validation also found and fixed the last two out-of-box gaps:
the chart now issues a wildcard SAN for pod-specific Stargate names, and NVCA
uses its configured request-router address when a worker launch environment
does not provide one.
Why
Workers currently have no chart-owned path to the request router's gRPC
registration and reverse QUIC endpoints. Sending both protocols through a
plain shared load balancer is unsafe with the default three replicas because
registration, watches, and reverse tunnels must reach the Stargate pod named
by gRPC authority or QUIC SNI.
A clean install on fresh k3d clusters on an isolated test VM exposed two additional
integration failures. The generated server certificate did not cover the
pod-specific hostname used for authority and SNI, and workload translation
rejected an LLM launch when its environment omitted the router address even
though NVCA had an operator-configured default. Both prevented the packaged
path from working without test-only shims.
What changed
/usr/local/bin/stargate-k8s-routerin the versioned Stargateruntime image, publish the three Stargate OCI targets to distinct image
repositories, and reject duplicate publish destinations.
probes, metrics discovery, TLS and ServiceAccount validation, and pylon
dial-address validation to the
helm-nvcf-llm-request-routerchart.replica while preserving its per-pod advertised identity.
Certificate when backend routing is enabled.
LLM_REQUEST_ROUTER_ADDRESS, then the legacySTARGATE_ADDRESS, then theoperator-configured workload default. Continue injecting both environment
aliases into the worker sidecar.
TCPRoute,UDPRoute, andReferenceGrantresources tonvcf-gateway-routes.present on the first Prometheus scrape.
Customer Release Notes
Self-managed NVCF can route external LLM workers to a highly available,
multi-replica request router.
Plan Summary
When enabled, the request-router chart adds one Deployment with one replica by
default, one ClusterIP Service, one Role, and one RoleBinding. The
gateway-routes chart adds one TCPRoute, one UDPRoute, and one ReferenceGrant.
All new routing resources are disabled by default. The existing chart-managed Certificate
also receives the pod-hostname wildcard SAN.
Usage
Enable
llmRequestRouter.backendRouterwith worker-reachable gRPC and QUICdial addresses. Enable
nvcfGatewayRoutes.routes.llmWorkerand point theseparate TCP and UDP Gateway references at compatible listeners.
Testing
Passed:
deploy/helm/llm-request-router/scripts/check-backend-router-render.shdeploy/helm/gateway-routes/scripts/check-llm-worker-routes.shhelm lint deploy/helm/llm-request-router/llm-request-routerhelm lint deploy/helm/gateway-routes/chartGOWORK=off GOFLAGS=-mod=vendor go test ./pkg/icms-translate/translate/function -count=1bazel test //src/libraries/go/lib/pkg/icms-translate/translate/function:function_testGOWORK=off GOFLAGS=-mod=vendor go test ./pkg/nvca -count=1with therepository's required k8s DRA driver version linker flag
cargo test -p stargate-k8s-router(61 library tests and 11 binary tests)stargate,pylon, andstargate-k8s-routerrepositoriesrendered charts
compute k3d clusters using locally sideloaded artifacts, including an LLM
workload that exposed
the certificate SAN and configured-address fallback gaps
bash -nfor both render-check scriptsgit diff --checkThe standalone NVCA Bazel target compiled successfully on macOS, but its
monolithic test failed the unrelated
TestMetricsBackwardCompatibilitybenchmark because the Bazel-resolved process collector omitted
process_virtual_memory_bytesandprocess_resident_memory_bytes. The focusedNVCA Go package test above passed.
Notes
UDPRoutesupport is required.and enable the local multi-cluster wiring after these artifacts are
released.
resources remain disabled by default, and the stack integration must not
enable them for production until that observability work is complete.
is required.
References
Relates to #502
Relates to #584
Related Pull Requests
Dependencies
None.
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation