Skip to content

fix: nok8s preflight misses replica ports and Envoy admin, and passes silently without ss - #1755

Merged
mengmeiye merged 1 commit into
llm-d:mainfrom
UgaTheDev:fix/nok8s-preflight-port-check
Aug 11, 2026
Merged

mengmeiye merged 1 commit into
llm-d:mainfrom
UgaTheDev:fix/nok8s-preflight-port-check

Conversation

@UgaTheDev

Copy link
Copy Markdown
Contributor

The nok8s preflight port check has three gaps, all of which let a standup proceed into a conflict it could have predicted.

hostPort is checked for replica 0 only. With nok8s.vllm.replicas: N the workers bind hostPort .. hostPort+N-1 (34_nok8s-containers.yaml.j2:24), so 8001..800N were never looked at. Envoy's admin port 19000 is hard-coded in 33_nok8s-envoy.yaml.j2 and listed as a required free port in docs/nok8s.md:37, but was not in the list either.

The third one is the reason I noticed. The check runs ss -ltn inside if res.success and res.stdout:, so on any host without iproute2 it reports nothing and passes. On macOS ss does not exist, so the check has never done anything there. tests/test_nok8s_plan.py:157-170 only covered the path where ss succeeds and finds no listeners, so nothing caught it.

This expands the port list, adds lsof -nP -iTCP -sTCP:LISTEN as a fallback probe so the check works on macOS, and warns when no probe is available rather than passing silently. A warning that says it could not check is worth more than a silent pass that reads as all clear.

Four regression tests, each verified to fail before the change: every replica port checked, admin port checked, lsof fallback used when ss is absent, and the no-probe-available warning. Full suite matches the pre-existing baseline on main (same 4 unrelated failures in test_config_schema.py).

Fixes #1703

@mengmeiye

Copy link
Copy Markdown
Collaborator

Hi @UgaTheDev The PR looks good to me! Can you resolve the conflicts so we'll approve and merge it? Thanks.

@UgaTheDev
UgaTheDev force-pushed the fix/nok8s-preflight-port-check branch from 66deac4 to 4e2a6ef Compare August 11, 2026 06:10
@mengmeiye
mengmeiye merged commit 0e9f128 into llm-d:main Aug 11, 2026
29 checks passed
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.

nok8s: nok8s preflight port check misses replica ports and Envoy admin, and silently no-ops when ss is unavailable

2 participants