Skip to content

feat(sglang,vllm): run the sglang PD router on smg; move router state to /tmp - #100

Merged
remotego merged 3 commits into
mainfrom
feat/sglang-smg-router
Nov 4, 2026
Merged

remotego merged 3 commits into
mainfrom
feat/sglang-smg-router

Conversation

@remotego

@remotego remotego commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

What type of PR is this?

  • feature
  • install / chart
  • breaking: API, config key, chart value, or CRD field changed

What this PR does and why

sglang 0.9.30: the PD router now runs on smg. The router image defaults to lightseekorg/smg:1.14.0 and both router containers exec smg launch, with arguments unchanged.

Until now smg could only be run through a kubectl edit on the live Deployment, because the exec line and state dir are template literals and pd.router.imageOverride changes only the image. Image and exec have to change together: smg launch doesn't exist in the sglang image, and sglang_router doesn't exist in the smg image. A hand-edited Deployment also conflicts with every later swiss or helm apply.

Router state dir moves from /run/pd-router to /tmp/pd-router. The smg image runs as USER smg (uid 65532), so it can't create a directory under root-owned /run, and the router crashloops with a misleading error:

install: cannot change permissions of '/run/pd-router': No such file or directory

vllm 0.9.2 makes the same state dir move. This isn't a bug fix: vllm's router image sets no USER, so it runs as root and /run works today. It keeps the two charts aligned and means a non-root vllm router image won't hit the same failure. vllm's router image and exec line are unchanged.

The router image is set the way vllm sets it: a default in values.yaml plus an inline fallback in the templates. An emptied override falls back to smg, so it can't pair smg launch with the engine image.

The new tests tie the boot script's path to the liveness probe's, because a mismatch produces no error: the probe treats a missing snapshot as "still booting" and passes, so drift detection would silently stop.

Rollout impact

Merging publishes both charts, and floating variants pick them up on their next apply:

  • sglang: all 7 PD variants in the catalog float onto 0.9.30 (floors are >=0.9.21, >=0.9.26 and >=0.9.27). Each should get a heads-up and one end-to-end check at apply time.
  • vllm: the 3 PD variants in the catalog roll their routers once for the path change. With vllm + mooncake, a router can hold its listener until its bootstrap handshake with each upstream finishes, so schedule these applies rather than letting them happen incidentally.
  • Colocate: sglang's colocate pod is one Deployment, so a router change restarts the engines beside it. No catalog variant uses colocate, and vllm's colocate render is unchanged.

Rollback: pin the sglang chart below 0.9.30, or vllm below 0.9.2, in the variant's chart.version.

Known gaps

Breaking for sglang PD releases, all three also in the commit message:

  1. Sites without Docker Hub access must set the image themselves. The router used to inherit the engine's mirrored address. swiss's Registry.Mirror and modelsphere's images.yaml.gotmpl only rewrite the engine and sidecar images, and the offline bundle doesn't carry smg. vllm's router has the same gap. modelsphere pins sglang at 0.8.2, so its installs aren't affected until that pin moves, and it needs a paired change before then.
  2. An existing pd.router.imageOverride naming an sglang engine build now crashloops, with no render-time failure. No catalog variant sets one, but a site's own values could.
  3. The router now reads the model hostPath as uid 65532. A root-only model directory would break the router while the engines stay healthy, and the chart has no setting to change the router's uid.

Raised in review and not addressed here:

  • Router and engine versions no longer ship together. Only one pairing has been tested live, so the end-to-end check at apply time matters.
  • smg's tokenizer handling: its help text says a local --model-path must hold a tokenizer layout, and the chart has no router extraArgs to work around that. A model could pass on the engines and still fail on the router. This comes from help text only.
  • Colocate startup timeout: smg's worker_startup_timeout_secs defaults to 1800s, while the chart budgets 90 minutes for engine load. Colocate execs without waiting for the engines, unlike pd-ds. The outcome is unverified.
  • hack/check-chart-images.sh can't see the router image, because it renders defaults, where PD is off. vllm's pin has the same blind spot, so a future bad tag would pass CI. This deserves its own fix.
  • The router still inherits the top-level env and volumeMounts. Left as is on purpose: catalog variants only set engine tuning env (NCCL_*, MOONCAKE_*, PYTORCH_*), and the live test ran that shape under smg. A site that sets PYTHONPATH, LD_LIBRARY_PATH, HOME or HF_HOME globally could still break the router. The model mount and --model-path are required by the router and are unaffected.
  • Setting only one of imageOverride.repository and .tag keeps the default for the other. Helm merges the defaults in first, so the chart can't detect this at render time; values.yaml now says to set both.
  • /tmp can be shadowed by user volumeMounts. Neither chart's defaults mount anything there.
  • The two routers still differ in ways this PR leaves alone: vllm-router gets no --model-path although it mounts the model, and vllm's boot script doesn't have the dead-IP filtering sglang gained in sglang: router boot drops IPs that hard-refuse sidecar /engine #80. Both are existing behaviour, not changed here.
  • The drift probe's identity logic assumes smg pins bootstrap info at boot the way sglang-router did. That is unconfirmed; step 3 of the plan below exercises it.

Which issue(s) this PR fixes

No issue filed. This implements the change request "sglang chart — default the PD router to smg, fix its boot command for the smg image" (2026-11-04).

The request targeted 0.9.29, but #99 took that version, so this ships as 0.9.30.

How it was tested

Run for this PR, on both charts:

helm lint --strict                     sglang, vllm: 0 failed (CI values and PD values)
helm unittest charts/sglang            Tests: 31 passed, 31 total
helm unittest charts/vllm              Tests: 40 passed, 40 total
hack/check-chart-version.sh <base>     rc=0
  • New tests can fail: I reverted the change in five places (exec line, image fallback, boot and probe paths) and the new tests failed each time.
  • Render against main, PD values:
    • sglang pd-ds: only the router Deployment changes.
    • sglang colocate: only the router container's image and exec lines change.
    • vllm pd-ds: only the router Deployment changes.
    • vllm colocate: no change.
  • sglang 0.9.28 vs 0.9.29 render identically at these values, so a release moving from 0.9.28 picks up no other change. In the pd-ds shape the engines don't roll.
  • bash -n passes on the rendered router boot and probe scripts, checked before a final comment-only edit.
  • lightseekorg/smg:1.14.0 resolves on Docker Hub to digest sha256:a764d127…, and its image config sets User: smg.

hack/check-chart-images.sh was not run locally because crane wasn't available; CI runs it, with the blind spot noted above.

Reported in the change request, not re-run for this PR: the rendered argv was smoke-tested against the image with docker, and the same spec was applied to a live glm5.3 PD release by kubectl patch: router 1/1, drift liveness green, /v1/chat/completions round trip through the PD engines. That covers the pd-ds shape on sglang only.

Still to do, needs swiss and a cluster:

  1. swiss plan + diff on that release: only the router Deployment should change, with no engine roll.
  2. Apply it: router 1/1 in one roll, drift liveness stable for at least 30 minutes, /v1/chat/completions end to end.
  3. Restart an engine in place once and confirm the router restarts, with /tmp/pd-router/*-ips written and read by the probe.
  4. Repeat the end-to-end check once on the other sglang PD variants, and on a vllm PD variant, at their next apply.

Checks

  • Tests and linters for what changed pass locally (see the repository's README or CONTRIBUTING)
  • Docs updated where behaviour, a config key or a chart value moved (the values.yaml comments; neither chart has a README)
  • No internal hostnames, IPs, registries or credentials

Release note

action required: sglang 0.9.30 runs the PD router on lightseekorg/smg:1.14.0 instead of the engine image. A pd.router.imageOverride that names an sglang build must be removed or pointed at an smg image, and sites without Docker Hub access must mirror smg and set the override. The router now runs as uid 65532, so it needs read access to the model directory. sglang and vllm routers keep their state in /tmp/pd-router, which rolls router pods once on upgrade.

🤖 Generated with Claude Code

… to /tmp

sglang 0.9.30 defaults the PD router to lightseekorg/smg:1.14.0 and execs
`smg launch`, set the same way vllm sets its router image: a default in
values plus an inline fallback, so an emptied override can never pair smg
with the engine image. Until now smg could only be expressed as a kubectl
edit on the live Deployment, because the exec line and state dir are
template literals.

The smg image runs as non-root (USER smg, uid 65532) and cannot create
/run/pd-router, so the boot script and the drift liveness probe move to
/tmp/pd-router. vllm 0.9.2 takes the same path so the charts stay aligned
and a non-root router image will not hit the same failure. vllm's router
image is unchanged and already runs as root, so this is not a vllm bug fix.

Tests tie the boot script's path to the probe's, because a mismatch is
silent: the probe treats a missing snapshot as "still booting" and passes.

Breaking for sglang PD releases:
- A pd.router.imageOverride naming an sglang engine build now crashloops
  (no smg binary). No catalog variant sets one.
- swiss's Registry.Mirror and modelsphere's image rewrite do not reach the
  router image, the same gap vllm's router already has. A site without
  Docker Hub access must set pd.router.imageOverride itself.
- The router now reads the model hostPath as uid 65532.
Roll back by pinning the sglang chart below 0.9.30.

Verified: helm lint --strict on both charts (CI and PD values), helm unittest
(sglang 31/31, vllm 35/35), new tests fail when the change is reverted. Render
vs main changes only the router Deployment in pd-ds, and only the router
container in sglang colocate; vllm colocate is unchanged. Not yet run on a
cluster from the chart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@zhanghaohit zhanghaohit left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor, non-blocking notes from a read of the diff (not tested on a cluster).

Router container still inherits the engine env and volumeMounts (templates/pd-ds.yaml:428, templates/pd-colocate.yaml:148, outside the diff so noting it here)

The router still gets sglang.mergeEnv over .Values.env plus the user volumeMounts. That was harmless while the router ran on the engine image, but it now runs smg (its own Python venv, uid 65532). Engine-oriented env such as PYTHONPATH, LD_LIBRARY_PATH, HOME=/root or HF_HOME=/root/.cache could make the router import the wrong packages or hit EACCES on a root-owned dir and crashloop while the engines look healthy. In colocate the router shares the engine pod, so a router-only mismatch keeps the whole pod un-Ready. Consider giving the router only the env it needs (or a separate pd.router.env).

Comment thread charts/sglang/values.yaml
imageOverride:
repository: ""
tag: ""
repository: "lightseekorg/smg"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: with non-empty defaults on both fields, a values file that overrides only one of them gets mixed with the smg default for the other. An existing site that pinned the router with only imageOverride.tag: v0.5.x now renders lightseekorg/smg:v0.5.x (no such tag); one that set only repository to its engine mirror keeps tag 1.14.0 in a repo that has no smg. A short note here, or a render-time fail when only one of the two is set, would make this obvious.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good catch! I've added a short note in values.yaml in d9a36c1 saying to set both fields, because one left out keeps its default. I went with the note rather than a fail: Helm merges the defaults in before the templates run, so the chart can't tell a lone tag from a full override. vllm got the same note, since it behaves the same way.

- name: router
{{- $img := $pd.router.imageOverride | default dict }}
image: "{{ $img.repository | default $root.Values.image.repository }}:{{ $img.tag | default $root.Values.image.tag }}"
image: "{{ $img.repository | default "lightseekorg/smg" }}:{{ $img.tag | default "1.14.0" }}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: the smg repository/tag now lives in four places (values.yaml plus the literals here and at pd-ds.yaml lines 185 and 205). A bump that misses one leaves an emptied override on a different version than the default, and the init container and router could disagree. A single helper in _helpers.tpl (e.g. sglang.pdRouterImage) called from all three sites, or required instead of the literal fallbacks, would keep it to one place.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, agreed, that's worth tidying up. d9a36c1 adds sglang.pdRouterImage and calls it from all three sites, so the init container and router can't end up on different versions. The rendered output is byte-identical. The default still has to live in both values.yaml and the helper, but changing either one alone now fails tests/pd_router_smg_test.yaml. I made the same change in vllm (vllm.pdRouterImage) so the two charts stay alike; vllm has no matching test yet.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick update to my note above: vllm now has a matching test as well. 8089d18 adds charts/vllm/tests/pd_router_test.yaml, which mirrors the sglang suite, so a change to only one of its defaults fails there too.

@zhanghaohit zhanghaohit left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The notes above are minor and non-blocking.

The fallback image was written at all three template sites, so a bump that
missed one could leave the init container and the router on different
versions. sglang.pdRouterImage and vllm.pdRouterImage now hold it. Also note
in values that repository and tag should be set together, because a field
left out keeps its default.

No render change: helm template output is byte-identical for both charts in
the pd-ds and colocate shapes. In sglang, changing the helper's fallback or
the values default alone now fails tests/pd_router_smg_test.yaml.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@remotego

remotego commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for the review, @zhanghaohit! On the router inheriting the engine env and volumeMounts: it's a fair concern, and we've decided to leave it as is for now. Catalog variants only set engine tuning env (NCCL_*, MOONCAKE_*, PYTORCH_*), not the PYTHONPATH/LD_LIBRARY_PATH/HOME/HF_HOME you mentioned. The change request's live test also ran the glm L3 variant's env and /root/.cache mount under smg without trouble, while /dev/hugepages wasn't part of that run. The router also needs the model mount and --model-path, and those stay put. A site that sets those variables globally could still get bitten, so I've listed that under known gaps in the description. If it shows up, a pd.router.env would be the natural follow-up. Your other two notes are addressed in d9a36c1 and answered on their threads.

Mirror charts/sglang/tests/pd_router_smg_test.yaml: default image on the
router and init container, exec line, override, the emptied-override
fallback, and the state dir in both shapes. This replaces the narrower
pd_router_state_dir_test.yaml, and means a change to only one of the helper's
fallback and the values default now fails in vllm too.

helm unittest charts/vllm: 40 passed. Seven deliberate breakages each failed
the new suite.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@remotego
remotego merged commit 4b02580 into main Nov 4, 2026
6 checks passed
@remotego
remotego deleted the feat/sglang-smg-router branch November 4, 2026 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants