Repository navigation
ci(docker): build 3 engine versions on release, test all engines on PR - #858
Conversation
Previously only 1 engine version was built per release (sglang v0.5.9, vllm v0.18.0, trtllm 1.3.0rc8), and only sglang was tested on PRs. Workflow changes: - release-sglang-docker.yml: add version matrix (v0.5.9, v0.5.8, v0.5.7) on push; PR and workflow_dispatch still build 1 version - release-vllm-docker.yml: add pull_request trigger with dry_run, add version matrix (v0.18.0, v0.17.2, v0.16.0) on push - release-trtllm-docker.yml: add pull_request trigger with dry_run, add version matrix (1.3.0rc8, 1.2.0, 1.1.0) on push Trigger behavior: - push (release): 3 engines × 3 versions = 9 images pushed to GHCR - pull_request: 3 engines × 1 version each, dry_run (build only) - workflow_dispatch: 1 image with user-specified version Documentation: - docs/getting-started/index.md: document engine images (GHCR pull commands for sglang, vllm, trtllm) in Docker install tab and add "All-in-one with engine images" deploy section - docs/concepts/architecture/high-availability.md: add tip about engine image tags for Kubernetes StatefulSet deployments - docs/concepts/architecture/service-discovery.md: add tip about engine image tags for Kubernetes Deployment manifests Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
github-actions
Bot
added
documentation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Docker image build and testing infrastructure by expanding the number of engine versions built on release and introducing comprehensive PR-triggered dry-run builds for all supported engines. This change addresses the user need for access to multiple engine versions for compatibility and gradual upgrades, which previously required manual intervention. Furthermore, it improves user experience by thoroughly documenting the new engine-bundled Docker images, making them easier to discover and utilize for all-in-one deployments. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughWorkflows for engine releases (sglang, trtllm, vllm) were changed to use a conditional base_image matrix (3-version on push, single-value otherwise), pass Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the CI to build multiple engine versions for Docker images and adds comprehensive documentation for these new images. The changes are well-structured and the documentation additions are clear. My main feedback is to replace the hardcoded version numbers in the documentation examples with placeholders to improve maintainability and prevent the docs from becoming outdated with future releases.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 841336d3af
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| pull_request: | ||
| branches: [main] | ||
| paths: |
There was a problem hiding this comment.
This new pull_request trigger lets fork-originated PRs run the reusable build job, which executes checkout and Docker build steps on the self-hosted 8-gpu-h200 runner (.github/workflows/_build-engine-image.yml, runs-on). dry_run only skips pushing images; it does not prevent arbitrary PR code from running on that host. If public forks can open PRs (default for public repos), this materially increases runner-compromise risk; gate these jobs to trusted actors/branches or move PR validation to isolated GitHub-hosted runners.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release-sglang-docker.yml:
- Line 5: The workflow uses the label "multi-version" in the conditional
expression (the string literal "multi-version" in the line with
github.event_name) which is inconsistent with the terminology used in
release-trtllm-docker.yml ("3-version matrix"); update the label to a consistent
term across workflows (e.g., replace "multi-version" with "3-version matrix" or
the chosen canonical phrase) so both workflows use the same string; ensure you
update any other usages of the label in this file and in
release-trtllm-docker.yml to match exactly.
In @.github/workflows/release-trtllm-docker.yml:
- Around line 60-75: The reusable workflow called by this file expects a dry_run
input but does not declare its type; update
.github/workflows/_build-engine-image.yml to add an explicit input definition
for dry_run (e.g., under inputs: add dry_run with type: boolean and a default of
false) so the caller's expression dry_run: ${{ github.event_name ==
'pull_request' }} is correctly coerced as a boolean; reference the dry_run input
name and the _build-engine-image.yml workflow when making the change.
In @.github/workflows/release-vllm-docker.yml:
- Around line 5-7: The workflow uses the matrix variables (base, engine, smg)
but the run name is currently just "matrix" and should match the TRT-LLM naming
for consistency; update the workflow's run-name/name field to the same label
used in TRT-LLM (e.g., "3-version matrix") so runs are consistently labeled
across engine workflows and easier to read in the Actions UI—locate the
workflow's top-level name or job run-name that references these matrix variables
and replace "matrix" with the agreed label.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: db8d18f3-3e2b-4fe7-bfb6-10be9ef739ae
📒 Files selected for processing (6)
.github/workflows/release-sglang-docker.yml.github/workflows/release-trtllm-docker.yml.github/workflows/release-vllm-docker.ymldocs/concepts/architecture/high-availability.mddocs/concepts/architecture/service-discovery.mddocs/getting-started/index.md
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| base_image: ${{ github.event_name == 'push' && fromJSON('["nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc8","nvcr.io/nvidia/tensorrt-llm/release:1.2.0","nvcr.io/nvidia/tensorrt-llm/release:1.1.0"]') || fromJSON(format('["{0}"]', inputs.base_image_ref || 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc8')) }} | ||
| uses: ./.github/workflows/_build-engine-image.yml | ||
| with: | ||
| engine: trtllm | ||
| base_image_ref: ${{ inputs.base_image_ref || 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc8' }} | ||
| base_image_ref: ${{ matrix.base_image }} | ||
| engine_repo: ${{ inputs.trtllm_repo }} | ||
| engine_commit: ${{ inputs.trtllm_commit || 'latest' }} | ||
| smg_repo: ${{ inputs.smg_repo || 'https://github.com/lightseekorg/smg' }} | ||
| smg_commit: ${{ github.event_name == 'pull_request' && 'main' || inputs.smg_commit || 'v1.3.3' }} | ||
| tag: ${{ inputs.tag }} | ||
| source_build_repo: ${{ inputs.trtllm_repo }} | ||
| source_build_ref: ${{ inputs.trtllm_commit || 'latest' }} | ||
| dry_run: ${{ github.event_name == 'pull_request' }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check that _build-engine-image.yml declares dry_run as boolean input and uses it to skip push
rg -n 'dry_run' .github/workflows/_build-engine-image.yml -A 3 -B 1Repository: lightseekorg/smg
Length of output: 582
Declare dry_run as type: boolean in the reusable workflow input.
The dry_run: ${{ github.event_name == 'pull_request' }} expression passes a boolean value, but the _build-engine-image.yml workflow does not explicitly declare dry_run as a boolean type input. While the default is correctly set to false, the input type should be explicitly declared as boolean to ensure proper type coercion and maintain consistency with the calling expression.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release-trtllm-docker.yml around lines 60 - 75, The
reusable workflow called by this file expects a dry_run input but does not
declare its type; update .github/workflows/_build-engine-image.yml to add an
explicit input definition for dry_run (e.g., under inputs: add dry_run with
type: boolean and a default of false) so the caller's expression dry_run: ${{
github.event_name == 'pull_request' }} is correctly coerced as a boolean;
reference the dry_run input name and the _build-engine-image.yml workflow when
making the change.
- Fix image tag format in docs: use `1.3.3-sglang-v0.5.9` instead of
`v1.3.3-sglang-v0.5.9` to match actual tags generated by the workflow
(SMG_VERSION from pyproject.toml has no `v` prefix)
- Standardize run-name labels across all 3 engine workflows to use
"3-version matrix" on push events for consistent Actions UI display
- Use placeholder format `{smg_version}-{engine}-{engine_version}` in
k8s doc tips (high-availability.md, service-discovery.md) to prevent
staleness on version bumps
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
There was a problem hiding this comment.
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 @.github/workflows/release-vllm-docker.yml:
- Line 62: The base_image array in the workflow includes a non-existent tag
"vllm/vllm-openai:v0.17.2"; update the expression that sets base_image (the line
assigning base_image using fromJSON and format) to remove
"vllm/vllm-openai:v0.17.2" so the list contains only the valid tags
"vllm/vllm-openai:v0.18.0" and "vllm/vllm-openai:v0.16.0", ensuring the
conditional branch for github.event_name == 'push' still returns the corrected
JSON array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 97966ab6-d4d1-4afa-9f20-7e913698559c
📒 Files selected for processing (5)
.github/workflows/release-sglang-docker.yml.github/workflows/release-vllm-docker.ymldocs/concepts/architecture/high-availability.mddocs/concepts/architecture/service-discovery.mddocs/getting-started/index.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82e8847faa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| pull_request: | ||
| branches: [main] | ||
| paths: | ||
| - 'docker/engine.Dockerfile' | ||
| - '.github/workflows/_build-engine-image.yml' |
There was a problem hiding this comment.
This newly added pull_request trigger allows fork-originated PRs that touch these paths to run the reusable build job, and that job executes on the self-hosted 8-gpu-h200 runner (.github/workflows/_build-engine-image.yml). Because dry_run only skips pushing images, attacker-controlled PR content can still execute arbitrary build steps on that host; for public/forked PRs this is a runner-compromise risk and should be gated to trusted actors/branches or moved to isolated GitHub-hosted runners.
Useful? React with 👍 / 👎.
vllm/vllm-openai:v0.17.2 does not exist on Docker Hub. Available versions are v0.17.0 and v0.17.1; use v0.17.1. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
There was a problem hiding this comment.
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 @.github/workflows/release-vllm-docker.yml:
- Around line 5-7: The run-name can show a hardcoded SMG default even when the
workflow forces smg_commit to "main" for pull_request builds; update the
run-name SMG expression so it uses the resolved smg commit value instead of the
literal default. Replace the current "smg=${{ inputs.smg_commit || 'v1.3.3' }}"
usage in the run-name construction with a conditional that reflects PR behavior
(e.g., use github.event_name == 'pull_request' ? 'main' : inputs.smg_commit ||
'v1.3.3') or otherwise reference the same resolved smg_commit variable so
run-name always displays the actual SMG ref (symbols: run-name, smg,
inputs.smg_commit, smg_commit).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 25e2fb88-e9a2-416b-b617-bee5bc7772ac
📒 Files selected for processing (1)
.github/workflows/release-vllm-docker.yml
All 3 engine workflows showed smg=v1.3.3 in run-name even on PR builds where smg_commit is forced to 'main'. Now the run-name correctly reflects the actual smg ref used. Also added consistent 'dry-run' label to sglang and vllm run-names on PR builds (trtllm already had it). Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c504095c88
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| serve \ | ||
| --backend sglang \ | ||
| --model-path /models/meta-llama/Llama-3.1-8B-Instruct \ | ||
| --port 30000 |
There was a problem hiding this comment.
This command omits --host 0.0.0.0, so users following it can end up with the router bound to loopback inside the container (smg serve defaults --host to 127.0.0.1 in bindings/python/src/smg/serve.py), which makes the published -p 30000:30000 port unreachable from the host and breaks the immediately following curl http://localhost:30000/... verification steps.
Useful? React with 👍 / 👎.
1 participant
Summary
What changed
Workflows
release-sglang-docker.ymlpush, builds 3 SGLang versions (v0.5.9, v0.5.8, v0.5.7)workflow_dispatchstill build 1 version (unchanged behavior)release-vllm-docker.ymlpull_requesttrigger withdry_run: true(build-only, no push)push, builds 3 vLLM versions (v0.18.0, v0.17.2, v0.16.0)release-trtllm-docker.ymlpull_requesttrigger withdry_run: true(build-only, no push)push, builds 3 TRT-LLM versions (1.3.0rc8, 1.2.0, 1.1.0)Trigger behavior
pull_request(docker paths)push(version bump)workflow_dispatchDocumentation
docs/getting-started/index.md: Added engine image pull commands (ghcr.io/lightseekorg/smg:{version}-{engine}-{engine_version}) to Docker install tab. Added "All-in-one with engine images" section under Deploy with Docker showingdocker run --gpus allwithservecommand.docs/concepts/architecture/high-availability.md: Added tip about using engine image tags in Kubernetes StatefulSet manifests.docs/concepts/architecture/service-discovery.md: Added tip about using engine image tags in Kubernetes Deployment manifests.Why
Users need access to multiple engine versions for compatibility testing and gradual upgrades. Previously each release only published 1 version per engine, requiring manual
workflow_dispatchfor older versions. Engine images were also completely undocumented.Test Plan
docker/engine.Dockerfile— all 3 engine workflows should trigger withdry_runworkflow_dispatchstill builds exactly 1 image with the user-specified baseOpen Questions
Summary by CodeRabbit
Documentation
Chores