fix(self-managed): configure LLM router worker address - #678
Merged
Conversation
Signed-off-by: Mike Camp <mcamp@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe self-managed stack now configures and validates the LLM request-router worker address, renders it into API remote configuration when LLM is enabled, adds end-to-end coverage, and documents colocated and split-cluster deployment settings. ChangesLLM router worker endpoint
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant HelmfileEnvironment
participant GlobalTemplate
participant APIRemoteConfig
participant LLMWorker
Operator->>HelmfileEnvironment: configure worker endpoint
HelmfileEnvironment->>GlobalTemplate: render endpoint
GlobalTemplate->>GlobalTemplate: validate host and port
GlobalTemplate->>APIRemoteConfig: write worker-address when LLM is enabled
GlobalTemplate->>LLMWorker: set request-router gRPC port
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
This was referenced Aug 5, 2026
mikeyrcamp
marked this pull request as ready for review
August 5, 2026 14:29
![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
🤖 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/stacks/self-managed/global.yaml.gotmpl`:
- Around line 262-266: Update the validation around
llmRequestRouterWorkerAddress to reject malformed worker addresses, requiring a
valid host and port while accepting bracketed IPv6 addresses. Preserve the
existing required-field failure when LLM is enabled and the address is empty,
and add rendering tests covering valid addresses plus router, :50071, and
router:not-a-port invalid cases.
In `@deploy/stacks/self-managed/tests/llm-router-worker-address.sh`:
- Line 10: Update the cleanup trap in the test script so it retains $work_dir
and its generated artifacts by default. Add an explicit opt-in cleanup option,
and only execute the rm -rf cleanup when that option is enabled, preserving the
existing cleanup behavior otherwise.
- Around line 92-96: Update the worker-address extraction in the in_router
parsing block to decode the quoted YAML scalar before returning it, removing the
surrounding YAML quotes so assert_remote_config_address compares host:port
values consistently.
In `@tests/bdd/fixtures/self-managed-local-bdd-multi.yaml`:
- Line 36: Replace the cluster-local llmRequestRouterAddress in the
multi-cluster BDD fixture with the worker-reachable external endpoint, and make
the same update in the raw multi-cluster seed in tests/bdd/godog_test.go; keep
both sources aligned so the multi-cluster topology uses the same externally
reachable router address everywhere.
🪄 Autofix
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: 82732107-f15a-4c7a-8608-601c6743e47b
📒 Files selected for processing (12)
deploy/stacks/self-managed/Makefiledeploy/stacks/self-managed/environments/base.yamldeploy/stacks/self-managed/global.yaml.gotmpldeploy/stacks/self-managed/tests/llm-router-worker-address.shdocs/user/csp-end-to-end-example-installation.mddocs/user/helmfile-installation.mddocs/user/llm-function-enablement.mddocs/user/local-development/single-cluster-helmfile.mddocs/user/samples/configs/local-dev-env.yamltests/bdd/fixtures/self-managed-local-bdd-multi.yamltests/bdd/fixtures/self-managed-local-bdd.yamltests/bdd/godog_test.go
Signed-off-by: Mike Camp <mcamp@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/stacks/self-managed/tests/llm-router-worker-address.sh`:
- Around line 217-224: Extend the invalid_address_cases matrix with malformed
bracketed IPv6 inputs, including [::::]:50071, then update the
address-validation pattern in the global template to validate bracketed IPv6
syntax rather than allowing arbitrary additional colons. Ensure the new
malformed cases are rejected while valid bracketed IPv6 addresses continue to
render successfully.
🪄 Autofix
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: d8f39478-5251-4196-90e0-fb2ad2ade702
📒 Files selected for processing (2)
deploy/stacks/self-managed/global.yaml.gotmpldeploy/stacks/self-managed/tests/llm-router-worker-address.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- deploy/stacks/self-managed/global.yaml.gotmpl
Signed-off-by: Mike Camp <mcamp@nvidia.com>
Signed-off-by: Mike Camp <mcamp@nvidia.com>
kristinapathak
approved these changes
Aug 5, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
deploy/stacks/self-managed/global.yaml.gotmpl (1)
262-263: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse one source for the router gRPC port.
llmRequestRouter.service.grpcPortis configurable, but the self-managed defaults independently hard-code50071. Derive the chart port and worker-address default from one stack value, then add a non-default-port rendering test.🤖 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/stacks/self-managed/global.yaml.gotmpl` around lines 262 - 263, Update the llm-request-router address defaults near llmRequestRouterWorkerAddress to derive the gRPC port from the configurable llmRequestRouter.service.grpcPort stack value instead of hard-coding 50071. Use that same value when constructing the chart service address, and add a rendering test that verifies a non-default port is propagated to the worker endpoint default.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@deploy/stacks/self-managed/global.yaml.gotmpl`:
- Around line 262-263: Update the llm-request-router address defaults near
llmRequestRouterWorkerAddress to derive the gRPC port from the configurable
llmRequestRouter.service.grpcPort stack value instead of hard-coding 50071. Use
that same value when constructing the chart service address, and add a rendering
test that verifies a non-default port is propagated to the worker endpoint
default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2780d767-2900-4aab-bd2e-35de660687a7
📒 Files selected for processing (6)
deploy/stacks/self-managed/environments/base.yamldeploy/stacks/self-managed/global.yaml.gotmpldeploy/stacks/self-managed/tests/llm-router-worker-address.shdocs/user/csp-end-to-end-example-installation.mddocs/user/helmfile-installation.mddocs/user/llm-function-enablement.md
🚧 Files skipped from review as they are similar to previous changes (4)
- deploy/stacks/self-managed/environments/base.yaml
- docs/user/llm-function-enablement.md
- docs/user/csp-end-to-end-example-installation.md
- docs/user/helmfile-installation.md
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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
3 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
Configure the self-managed LLM request-router worker address in NVCF API remote configuration. Colocated deployments use the cluster-local service by default, while split deployments can provide a worker-reachable override.
Additional Details
Why
The self-managed stack did not populate the LLM request-router worker address consumed by generated LLM workers. Compute agents reject LLM launches without that address, so translation can fail before a worker pod is created. The stack has a stable cluster-local request-router service for colocated workers and needs a topology-owned override for split deployments.
What changed
global.workerEndpoints.llmRequestRouterAddressas an optional topology override.llm-request-router.nvcf.svc.cluster.local:50071.host:portand bracketed IPv6host:portoverrides, including port bounds.api.remoteConfig.configData.nvcf.llm-request-router.worker-address.Customer Release Notes
Self-managed LLM deployments with colocated workers now use
llm-request-router.nvcf.svc.cluster.local:50071automatically. Split deployments must overrideglobal.workerEndpoints.llmRequestRouterAddresswith a request-router host and port reachable from worker pods.Plan Summary
Configuration-only change. No Kubernetes resources, chart versions, or replica counts are added or changed. The stack supplies one existing API remote-config value when the LLM add-on is enabled.
Usage
Colocated deployments require no additional worker-address configuration. The cluster-local address follows
addons.llm.requestRouter.service.grpcPortwhen a non-default service port is configured. For a split deployment, configure an override before enabling the LLM add-on:Testing
make -C deploy/stacks/self-managed testgo test -short ./...fromtests/bdddeploy/stacks/self-managed/tests/observability-autoscaler.sh./tools/ci/check-docsgit diff --checkThe local
golangci-lintv2.11.4 invocation stopped before analysis withno go files to analyze;go list ./...and the BDD test suite both succeeded.Notes
Split-cluster router network exposure is tracked separately in #689.
The worker address is not injected through
api.env. No API chart bump is required because the pinned chart already supports the nested remote-config path.References
Related Pull Requests
Dependencies
None. No third-party dependency, license, or NOTICE changes.
For the Reviewer
Please focus on the default and explicit-override behavior in
deploy/stacks/self-managed/global.yaml.gotmpland the corresponding regression coverage indeploy/stacks/self-managed/tests/llm-router-worker-address.sh.For QA
QA is recommended for one colocated and one split-cluster LLM function launch. The focused render tests and non-live BDD suite are green.
Issues
Fixes #676
Checklist
Summary by CodeRabbit
New Features
Documentation
Tests