Skip to content

fix various vllm/sglan bugs#877

Merged
podkidyshev merged 2 commits into
mainfrom
ipod/llm-serve-fixes
Apr 21, 2026
Merged

fix various vllm/sglan bugs#877
podkidyshev merged 2 commits into
mainfrom
ipod/llm-serve-fixes

Conversation

@podkidyshev
Copy link
Copy Markdown
Contributor

Summary

  • Bench instance should connect to router instance not via loopback but by hostname
  • [vLLM] Router instance should start on 0.0.0.0 not on 127.0.0.1
  • [vLLM] bench instance should start on 0.0.0.0 instead of 127.0.0.1
  • Default serve_port used for router instance should be not 8000 as it's blocked on some NV internal clusters

Test Plan

  • Automated CI

Additional Notes

  • Internal tickets: 1, 2, 3, and 4

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 21, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 10389f0f-4796-4b74-8003-ef828fdc7226

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ipod/llm-serve-fixes

Comment @coderabbitai help to get the list of available commands and usage tips.

@podkidyshev podkidyshev self-assigned this Apr 21, 2026
@podkidyshev podkidyshev added the bug Something isn't working label Apr 21, 2026
@podkidyshev podkidyshev marked this pull request as ready for review April 21, 2026 13:13
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/workloads/sglang/test_command_gen_strategy_slurm.py`:
- Line 154: The assertion is hardcoding the router port (":8000"); update the
test to reference the dynamic port from the strategy fixture instead: change the
assertion that checks for "--base-url http://${PREFILL_NODE}:8000" in
srun_command to build the expected substring using strategy.serve_port (e.g.
"--base-url http://${PREFILL_NODE}:" + str(strategy.serve_port) or equivalent)
so both lines (the current checks around srun_command) use strategy.serve_port
instead of the literal 8000.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: 77bd1644-0b8f-4b3e-ab21-2d6323613d73

📥 Commits

Reviewing files that changed from the base of the PR and between 8da4f99 and 6f3cc19.

📒 Files selected for processing (13)
  • src/cloudai/workloads/common/llm_serving.py
  • src/cloudai/workloads/sglang/slurm_command_gen_strategy.py
  • src/cloudai/workloads/vllm/slurm_command_gen_strategy.py
  • tests/ref_data/sglang-disagg-2nodes.sbatch
  • tests/ref_data/sglang-disagg.sbatch
  • tests/ref_data/sglang.sbatch
  • tests/ref_data/vllm-disagg-2nodes.sbatch
  • tests/ref_data/vllm-disagg.sbatch
  • tests/ref_data/vllm.sbatch
  • tests/test_acceptance.py
  • tests/workloads/sglang/test_command_gen_strategy_slurm.py
  • tests/workloads/test_llm_serving.py
  • tests/workloads/vllm/test_command_gen_strategy_slurm.py
💤 Files with no reviewable changes (1)
  • tests/test_acceptance.py

Comment thread tests/workloads/sglang/test_command_gen_strategy_slurm.py
@podkidyshev podkidyshev merged commit 0064155 into main Apr 21, 2026
5 checks passed
@podkidyshev podkidyshev deleted the ipod/llm-serve-fixes branch April 21, 2026 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants