core: cluster-level environment overrides + trtllm_serve IPv6 bind support - #348
Open
v-shobhit wants to merge 6 commits into
Open
core: cluster-level environment overrides + trtllm_serve IPv6 bind support#348v-shobhit wants to merge 6 commits into
v-shobhit wants to merge 6 commits into
Conversation
A recipe shared across multiple clusters (e.g. two racks both running
the same disagg config) can only carry one value for a NIC-dependent
env var like NCCL_SOCKET_IFNAME/UCX_NET_DEVICES in its
backend.{prefill,decode}_environment blocks -- correct for one
cluster, silently wrong on another with different interface names.
Confirmed via a live crash on a second VRNVL72-type rack: a recipe
hardcoding UCX_NET_DEVICES=eth0 (correct on the original rack) caused
NIXL's UCX transfer-agent setup to abort() on worker startup, since
eth0 isn't a valid UCX device there (only InfiniBand devices are).
Add ClusterConfig.environment (srtslurm.yaml, cluster-local, not
recipe-shared) and merge it into RuntimeContext.environment between
backend env and the recipe's own top-level environment: block, so
cluster-level overrides win over the recipe's hardcoded backend env
but the recipe's explicit environment: block still wins if set --
same precedence shape as the existing default_mounts mechanism.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ClusterConfig.environment (previous commit) can override a value but not remove it. Some cluster-vs-recipe mismatches have no correct replacement value to override with -- e.g. a recipe's UCX_NET_DEVICES/NCCL_SOCKET_IFNAME pinned to a device name from another cluster's topology (a plain NIC name on one cluster, an InfiniBand HCA name like cuda0-mlx5_0:1 on another -- not even the same naming scheme). The correct fix there is to omit the variable entirely and let UCX/NCCL auto-select, which an override can't express. Add ClusterConfig.unset_environment (list of keys) and pop them from env_to_set last, after every other environment source, at both worker-env-assembly call sites. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…esses worker_stage.py already applies ClusterConfig.environment/unset_environment to every backend worker, but frontend launch (dynamo.py, sglang.py, trtllm_serve.py) only ever saw config.frontend.env -- silently missing cluster overrides like a writable HOME/TMPDIR. Confirmed live: the trtllm_serve orchestrator crashed with OSError: [Errno 30] Read-only file system: '.../.cache' (flashinfer's JIT cache trying to create a directory under an unset HOME) despite the same cluster override successfully fixing every backend worker. dynamo.py already applied runtime.environment but not unset_environment; sglang.py and trtllm_serve.py applied neither. All three now match worker_stage.py's pattern.
frontend.type: trtllm_serve's disaggregated orchestrator hardcodes ser.yaml hostname: 0.0.0.0. On an IPv6-only cluster this is unreachable from any other node in the job -- confirmed live (job 1120): the orchestrator answers GET /health with 200 on localhost, but curl from the orchestrator's own node to <node>:8000/health returns exit 7 (connection failed) from every other node, hanging srtctl's own wait_for_model() health poll indefinitely even though the full stack is healthy. Added ClusterConfig.trtllm_serve_hostname (defaults to the existing 0.0.0.0 when unset, so this is a no-op for every other cluster) so an IPv6-only cluster can set it to '::'.
v-shobhit
requested review from
alec-flowers,
csahithi,
ishandhanani and
nlevin-ui
as code owners
August 26, 2026 19:56
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #348 +/- ##
=======================================
Coverage ? 72.14%
=======================================
Files ? 96
Lines ? 13117
Branches ? 0
=======================================
Hits ? 9463
Misses ? 3654
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ClusterConfig.network_interface now accepts a comma-separated, preference-ordered list of interface names (e.g. "eth13,eth7,eth4"), tried in order per node in get_node_ip.sh's Method 1. This lets one cluster-wide setting cover a fleet where the same physical NIC has different names on different nodes, instead of silently falling through to ip-route-based auto-selection (ambiguous when both NICs share a subnet/metric) on any node where the single hardcoded name doesn't exist.
… sites RuntimeContext.head_node_ip/infra_node_ip and four frontend/backend call sites (trtllm_serve health-check URL, sglang frontend leader IPs, nginx frontend_hosts, sglang backend leader IP) called get_hostname_ip() without network_interface at all, silently bypassing the cluster's setting (including the new preference-list behavior) and always falling through to ip-route-based auto-selection. Caught live: on a rack with a preference-ordered network_interface list, one prefill worker on a node lacking the first-preference interface still resolved via the ambiguous route-based fallback instead of the node's correct second-preference NIC, because its health-check URL was built through frontends/trtllm_serve.py's unguarded call. The vLLM Dynamo-resolution call site is intentionally left unguarded per the existing comment there (scoped to avoid behavior change outside the direct-vLLM frontend); VLLM_NIXL_SIDE_CHANNEL_HOST's enclosing function had no runtime in scope, so it reads network_interface via get_srtslurm_setting() directly instead of threading a new parameter through its callers. Updates two tests whose get_hostname_ip mocks assumed the old single-argument signature.
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.
Summary
Three related additions surfaced while onboarding a new IPv6-only VRNVL72 rack (
nvidia-semianalysis-privaterunnervrnvl72-nv_0/1/2/6/7/8) to srtctl. All are cluster-scoped (srtslurm.yaml), off by default, and no-ops for existing clusters.ClusterConfig.environment/ClusterConfig.unset_environment— cluster-level env var overrides/removals, applied after the recipe's ownbackend.{prefill,decode,aggregated}_environmentbut before (environment) or after (unset_environment) the recipe's own top-levelenvironment:block. Needed when a recipe's baked-in env is correct for one cluster sharing that recipe but wrong for another (e.g. a NIC name or sitecustomize hook that only exists on one rack).worker_stage.pyalready appliedClusterConfig.environment/unset_environmentto every backend worker;frontends/{trtllm_serve,sglang,dynamo}.pynever did (dynamo appliedenvironmentbut notunset_environment; trtllm_serve and sglang applied neither). This meant a cluster env fix that worked for backend workers silently didn't reach the frontend/orchestrator process. Confirmed live: the trtllm_serve orchestrator crashed withOSError: [Errno 30] Read-only file system(flashinfer's JIT cache trying to create a directory under an unsetHOME) even after the identical cluster override fixed every backend worker.ClusterConfig.trtllm_serve_hostname—frontend.type: trtllm_serve's disaggregated orchestrator hardcodesser.yamlhostname: 0.0.0.0(IPv4-any). On an IPv6-only cluster this is unreachable from any other node in the job: the orchestrator itself answersGET /healthwith 200 onlocalhost, but every other node's request to<node>:8000/healthfails to connect, hanging srtctl's ownwait_for_model()health poll indefinitely even though the full stack is healthy. Defaults to the existing0.0.0.0when unset.ClusterConfig.network_interfacenow accepts a comma-separated, preference-ordered interface list (e.g."eth13,eth7,eth4"), tried in order per node inget_node_ip.sh's Method 1. Surfaced by a live NIC survey on the same rack: 15 of 18 nodes name their 400GbE NICeth13, but 3 don't (eth7/eth17oreth4/eth10) — a single hardcoded name silently no-ops on those nodes (ip addr show <name>returns nothing) and falls through toip route's default-source-IP guess, which is ambiguous when both of a node's NICs sit on the same subnet/metric. A preference list lets one cluster-wide setting resolve correctly everywhere, and (as a side effect) lets an operator steer a whole cluster away from a known-bad NIC by simply not listing it first.Test plan
unset_environment+environment+trtllm_serve_hostname: "::"set in that cluster'ssrtslurm.yaml.tests/test_ip_utils.py::test_get_node_ip_falls_through_comma_separated_interface_listadded for the new preference-list resolution; full suite run (uv run pytest tests/) shows no regressions from this branch (the 4 failures present are pre-existing/environment-local, reproduced identically with this branch's commits stashed out).ClusterConfig.environment/unset_environmentand the frontend env-application parity (not yet added — flagging for review sinceCLAUDE.mdcalls out that new config affecting srun should come withtest_dry_run.pycoverage)🤖 Generated with Claude Code