Repository navigation
ci(docker): add engine-specific image build pipelines - #604
Merged
Merged
Conversation
gongwei-130
requested review from
CatherineSue,
XinyueZhang369,
key4ng and
slin1237
as code owners
March 3, 2026 22:55
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
github-actions
Bot
added
ci
Contributor
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 introduces a robust system for building specialized SMG Docker images, each optimized for a particular large language model inference engine (vLLM, SGLang, TensorRT-LLM, TGL). By providing a multi-stage Dockerfile and corresponding installation scripts, it streamlines the process of creating reproducible and engine-specific container environments, enhancing flexibility and deployment efficiency for SMG. Highlights
Changelog
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds three GitHub Actions workflows to build and publish SMG engine Docker images (vLLM, SGLang, TRTLLM), a multi-variant engine Dockerfile, and per-engine plus SMG installation scripts for building engine variants and pushing finalized images to GHCR with computed tags. Changes
Sequence Diagram(s)sequenceDiagram
participant GHA as "GitHub Actions"
participant Repo as "Source Repos\n(SMG / Engine)"
participant Buildx as "Docker Buildx"
participant Docker as "Docker Daemon"
participant GHCR as "GHCR Registry"
GHA->>Repo: checkout SMG & engine (if provided)
GHA->>GHA: resolve inputs & compute image_tag (version/commit/date)
GHA->>Buildx: setup buildx & prepare build context
GHA->>Docker: build target from docker/Dockerfile.engine
Docker->>Docker: run stages (sources → variant engine install → SMG install)
Buildx->>GHCR: push final image ghcr.io/<owner>/smg:<image_tag>
GHA->>GHA: output summary (final image reference)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related issues
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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Hi @gongwei-130, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a multi-stage Dockerfile (docker/Dockerfile.engine) and several installation scripts to facilitate building engine-specific Docker images for vllm, sglang, trtllm, and tgl. The approach of using a shared sources stage and then parallel target stages for each engine is clear and appropriate for this use case.
My review has identified a critical issue in the install-smg.sh script that will prevent the Docker images from building successfully due to an incorrect protoc version. I have also noted a minor inconsistency in one of the installation scripts that could be improved for robustness. Please see the detailed comments below.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 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:
- Around line 59-62: The workflow currently sets the Buildx driver to "docker"
which disables layer caching; update the "Set up Docker Buildx" step that uses
docker/setup-buildx-action@v3 to use the "docker-container" driver and add cache
configuration (e.g., cache-from and cache-to or inline type) so Docker layer
caching is enabled for faster builds; if you need load: true, handle push/load
accordingly (use --push with cache-to remote or implement explicit load
handling).
- Around line 77-89: The current SGLANG_COMMIT sanitization only replaces
slashes and can still produce characters invalid for Docker tags; update the
sanitization of SGLANG_COMMIT before composing IMAGE_TAG so it yields only
characters matching Docker's allowed set and length: replace any character not
in [A-Za-z0-9._-] with a safe separator (e.g., '-'), collapse consecutive
separators, ensure the first character is alphanumeric or underscore (prefix
with a safe character like 'v' if needed), and truncate the sanitized
SGLANG_COMMIT to keep IMAGE_TAG <= 128 characters for the tag component; apply
this sanitized value when setting IMAGE_TAG and echoing image_tag to
GITHUB_OUTPUT.
In @.github/workflows/release-trtllm-docker.yml:
- Around line 38-40: The workflow sets env GITHUB_TOKEN from
secrets.ROBOT_GITHUB_TOKEN but later the GHCR login uses secrets.GITHUB_TOKEN;
make the token source consistent and add explicit workflow permissions. Update
the GHCR login step to use the env variable (e.g., ${{ env.GITHUB_TOKEN }}) or
change the env to ${{ secrets.GITHUB_TOKEN }} so both use the same secret
(reference the env key GITHUB_TOKEN and the GHCR login step that currently uses
secrets.GITHUB_TOKEN), and add a top-level permissions block (e.g., permissions:
contents: read and packages: write) to ensure the token has rights to push to
GHCR.
- Around line 46-55: The echo blocks that append to GITHUB_STEP_SUMMARY (the
series of echo lines that print the "| Parameter | Value |" table and the input
values like inputs.base_image_ref, inputs.trtllm_repo, etc.) are causing
shellcheck SC2086/SC2129; quote the variable and consolidate redirection by
grouping the echo statements and redirecting once, e.g. wrap the block in { echo
"..."; echo "..."; ...; } >> "$GITHUB_STEP_SUMMARY" and ensure you reference
"$GITHUB_STEP_SUMMARY" (quoted) and quote any variable expansions like "${{
inputs.base_image_ref }}" where appropriate.
In @.github/workflows/release-vllm-docker.yml:
- Around line 1-122: Extract a reusable workflow for publishing engine Docker
images and replace the near-duplicate release workflows by calling it: create a
workflow (e.g., release-engine-docker-reusable.yml) that exposes inputs such as
engine_name, base_image_ref, engine_repo, engine_commit, smg_repo, smg_commit
and outputs image_tag/image_name, move shared steps (Print inputs, Set up
Buildx, Login, Resolve image tag logic including SMG_VERSION/DATE/IMAGE_TAG
generation and sanitization of VLLM_COMMIT/sglang_commit, Build image, Push
image, Summary) into that reusable workflow, and update this file to call it
with engine_name=vllm and map inputs (vllm_repo→engine_repo,
vllm_commit→engine_commit) while keeping unique build args and Dockerfile
target; also centralize tag sanitization (the VLLM_COMMIT handling that replaces
slashes/spaces) inside the reusable workflow so both callers benefit.
In `@docker/Dockerfile.engine`:
- Around line 26-35: The Dockerfile stage custom-vllm currently runs as root (no
USER set); add a non-root user and switch to it after setup: create a user
(e.g., smguser with a fixed UID/GID), chown relevant directories created/used in
the stage (/opt/smg-src, /opt/vllm-src, /tmp if needed) so the non-root user can
access them, and add a final USER smguser instruction in the custom-vllm stage
(ensure any install scripts run as root remain before the USER switch and that
ENV SMG_DEFAULT_BACKEND and subsequent runtime expectations work for the
non-root user).
- Around line 59-62: The custom-tgl build stage erroneously sets the environment
variable SMG_DEFAULT_BACKEND to "sglang" (likely a copy/paste from the sglang
variant); update the ENV declaration in the custom-tgl stage so
SMG_DEFAULT_BACKEND is set to "tgl" instead, i.e., locate the custom-tgl stage
(FROM ${BASE_IMAGE_REF} AS custom-tgl) and change the SMG_DEFAULT_BACKEND value
from "sglang" to "tgl".
- Around line 11-17: The git clone commands in the RUN block should use shallow
clones to speed builds: modify the two git clone invocations for ENGINE (using
environment vars ENGINE_REPO and ENGINE_COMMIT, located in the RUN block) and
SMG (SMG_REPO/SMG_COMMIT) to use --depth 1 and --branch when checking out
latest; when a specific commit is requested (ENGINE_COMMIT or SMG_COMMIT not
"latest"), perform a shallow clone of the branch then run a shallow fetch for
that commit (e.g., git fetch --depth 1 origin <commit>) before git checkout,
ensuring the conditional logic around [ "${ENGINE_COMMIT}" = "latest" ] and [
"${SMG_COMMIT}" = "latest" ] still applies.
In `@scripts/installation/install-smg.sh`:
- Around line 1-2: Update the script shebang from a generic POSIX sh to Bash by
replacing the existing shebang line (the interpreter declaration at the top of
the script) with "#!/bin/bash" so that non-POSIX builtins like ulimit used in
the script will be available; ensure the first line is updated and no other
changes are required to invocation or permissions.
- Around line 21-24: The script hardcodes export PATH="/root/.cargo/bin:${PATH}"
which assumes root; change it to export PATH="${HOME}/.cargo/bin:${PATH}" and
ensure the rustup install command (the curl ... | sh -s -- -y invocation) runs
in the same user context so $HOME is respected; update any profile persistence
logic (if present) to append "${HOME}/.cargo/bin" rather than
"/root/.cargo/bin".
- Around line 15-18: The install script hardcodes the x86_64 protoc binary name;
change the wget/unzip block to detect the machine architecture (e.g., use uname
-m), map common outputs (x86_64 -> linux-x86_64, aarch64/arm64 -> linux-aarch_64
or the correct release name), set a PROTOC_ARCH variable, build the filename
like "protoc-32.0-${PROTOC_ARCH}.zip", and use that variable in the
wget/unzip/rm commands so ARM runners download the correct archive; update the
block that currently references "protoc-32.0-linux-x86_64.zip" to use the
constructed filename instead.
In `@scripts/installation/install-trtllm.sh`:
- Around line 12-13: The install-trtllm.sh script uses a risky glob
"tensorrt_llm*.whl" and assumes git-lfs is present; update the script to (1)
verify git-lfs is installed before calling "git lfs pull" (use a command
existence check like checking "git-lfs" or "git lfs version" and exit with a
clear error if missing) and (2) replace the ambiguous pip install line by
locating the built wheel deterministically (e.g., use a find/ls pattern to
capture the exact filename produced by python3 ./scripts/build_wheel.py --clean,
assert exactly one matching file exists, then pass that single filename to pip
install) so pip never receives multiple matches; reference the script file
scripts/installation/install-trtllm.sh and the commands python3
./scripts/build_wheel.py --clean, pip install ./build/tensorrt_llm*.whl and the
earlier git lfs pull call when making these changes.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/release-sglang-docker.yml.github/workflows/release-trtllm-docker.yml.github/workflows/release-vllm-docker.ymldocker/Dockerfile.enginescripts/installation/install-sglang.shscripts/installation/install-smg.shscripts/installation/install-tgl.shscripts/installation/install-trtllm.shscripts/installation/install-vllm.sh
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 85-97: The SMG_VERSION extraction using SMG_VERSION=$(grep -m1
'^version = ' bindings/python/pyproject.toml | sed 's/version = "\(.*\)"/\1/')
can produce an empty value if bindings/python/pyproject.toml is missing or
malformed; add validation right after that command to check SMG_VERSION is
non-empty and abort with a clear error (or set a sensible default) to avoid
generating invalid IMAGE_TAGs; update the script that constructs IMAGE_TAG
(referencing SMG_VERSION, VLLM_COMMIT and IMAGE_TAG) to exit with a failure and
log a helpful message if SMG_VERSION is empty so the workflow fails fast instead
of producing tags like -vllm-... .
- Around line 12-20: The workflow currently triggers on pull_request but the
image push step runs unconditionally; update the push step (the job step that
publishes images to GHCR) to skip on PRs by adding a condition such as if:
github.event_name != 'pull_request' (or equivalent job-level if) so the push
only runs for non-PR events; locate the push/publish step in the workflow (the
step that pushes images to GHCR) and add the conditional to prevent publishing
during pull_request events.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
.github/workflows/release-vllm-docker.yml (2)
85-97:⚠️ Potential issue | 🟡 MinorAdd validation for
SMG_VERSIONextraction.If
bindings/python/pyproject.tomlis missing or malformed,SMG_VERSIONwill be empty, producing an invalid image tag like-vllm-v0.16.0-cu130-20260304.Proposed fix
run: | SMG_VERSION=$(grep -m1 '^version = ' bindings/python/pyproject.toml | sed 's/version = "\(.*\)"/\1/') + if [ -z "${SMG_VERSION}" ]; then + echo "::error::Failed to extract SMG version from pyproject.toml" + exit 1 + fi if [ -n "${VLLM_REPO}" ]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-vllm-docker.yml around lines 85 - 97, Validate that SMG_VERSION (from bindings/python/pyproject.toml) is non-empty after extraction and fail the job with a clear error if it's missing or empty; in the block that sets SMG_VERSION and constructs IMAGE_TAG, check the SMG_VERSION variable and call exit 1 (or use a GitHub Actions error output) with a descriptive message so you don't produce tags like "-vllm-..."; reference SMG_VERSION and IMAGE_TAG in your check and ensure the workflow prints/logs the error before exiting.
12-20:⚠️ Potential issue | 🟠 MajorPR events will publish images to GHCR.
The workflow triggers on
pull_requestevents (lines 12-20), but the push step (lines 115-124) executes unconditionally. This will publish images from untested PR branches.Add a condition to skip the push on PR events:
- name: Push image to GHCR id: push-ghcr + if: github.event_name != 'pull_request' env:Also applies to: 115-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-vllm-docker.yml around lines 12 - 20, The workflow currently triggers on pull_request but the image push (the push job/step around lines 115-124 in .github/workflows/release-vllm-docker.yml) runs unconditionally and will publish images from PRs; update the workflow to skip the push when the event is a pull_request by adding a conditional (e.g., set an if: github.event_name != 'pull_request' on the push job or the specific push step) so that push only runs on non-PR events such as push or release..github/workflows/release-sglang-docker.yml (2)
77-89:⚠️ Potential issue | 🟡 MinorTag sanitization may produce invalid Docker tags.
The current sanitization only handles slashes and spaces. Branch names with special characters (e.g.,
feature/foo@bar) could produce invalid tags. Docker tags must match[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}.Proposed robust sanitization
if [ -n "${SGLANG_REPO}" ]; then - SGLANG_COMMIT="${SGLANG_COMMIT//\//-}" - SGLANG_COMMIT="${SGLANG_COMMIT// /}" + SGLANG_COMMIT=$(echo "${SGLANG_COMMIT}" | sed 's/[^a-zA-Z0-9._-]/-/g' | sed 's/^[^a-zA-Z0-9]/0/') else🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-sglang-docker.yml around lines 77 - 89, The tag-building snippet produces SGLANG_COMMIT values that can contain characters invalid for Docker tags; update the sanitization around SGLANG_COMMIT (the logic that currently replaces "/" and spaces) to: replace any character not in [A-Za-z0-9._-] with a hyphen, ensure the first character is alphanumeric or underscore (prefix with a valid char if needed), and truncate the final IMAGE_TAG component to fit Docker's 128-character limit before composing IMAGE_TAG; apply this to both branches of the conditional that set SGLANG_COMMIT so the resulting IMAGE_TAG always conforms to Docker tag rules.
59-62: 🧹 Nitpick | 🔵 TrivialConsider enabling Docker layer caching.
Using
driver: dockerdisables layer caching, which increases build times for large engine images.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-sglang-docker.yml around lines 59 - 62, The workflow step using the docker/setup-buildx-action@v3 currently sets driver: docker which disables layer caching; change the Buildx setup to use the docker-container driver (or remove the explicit driver) and configure build cache options in the subsequent build step (e.g., add cache-from/cache-to or --cache-to=type=registry,local, or buildx cache flags) so Docker layer caching is enabled for the "Set up Docker Buildx" step and the image build steps that reference it..github/workflows/release-trtllm-docker.yml (2)
38-40:⚠️ Potential issue | 🟠 MajorUnify GHCR token source and add explicit workflow permissions.
Line 39 defines
env.GITHUB_TOKENfromsecrets.ROBOT_GITHUB_TOKEN, but Line 110 usessecrets.GITHUB_TOKENdirectly. This mismatch can cause intermittent auth failures, and missing top-levelpermissionscan block GHCR push.Proposed patch
on: workflow_dispatch: @@ +permissions: + contents: read + packages: write + env: GITHUB_TOKEN: ${{ secrets.ROBOT_GITHUB_TOKEN }} @@ - name: Login to GitHub Container Registry uses: docker/login-action@v3 with: registry: ghcr.io username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} + password: ${{ env.GITHUB_TOKEN }}Also applies to: 105-110
🤖 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 38 - 40, Unify the GHCR token source and add explicit workflow permissions: set the workflow to use a single token source by assigning env.GITHUB_TOKEN consistently from secrets.ROBOT_GITHUB_TOKEN (or swap all usages to secrets.GITHUB_TOKEN) so usages of env.GITHUB_TOKEN and direct references to secrets.GITHUB_TOKEN match, and add a top-level permissions block (e.g., permissions: packages: write, contents: read) to allow GHCR pushes; update references in the workflow where env.GITHUB_TOKEN, secrets.ROBOT_GITHUB_TOKEN, and secrets.GITHUB_TOKEN appear so they are consistent.
46-55:⚠️ Potential issue | 🟡 MinorFix shellcheck SC2086/SC2129 in summary-writing blocks.
Lines 47-55 and 189-193 repeatedly redirect to unquoted
$GITHUB_STEP_SUMMARY. Group commands and redirect once to"$GITHUB_STEP_SUMMARY".Proposed patch
- name: Print inputs to summary run: | - echo "## Inputs" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "| Parameter | Value |" >> $GITHUB_STEP_SUMMARY - echo "|-----------|-------|" >> $GITHUB_STEP_SUMMARY - echo "| base_image_ref | \`${{ inputs.base_image_ref }}\` |" >> $GITHUB_STEP_SUMMARY - echo "| trtllm_repo | \`${{ inputs.trtllm_repo }}\` |" >> $GITHUB_STEP_SUMMARY - echo "| trtllm_commit | \`${{ inputs.trtllm_commit }}\` |" >> $GITHUB_STEP_SUMMARY - echo "| smg_repo | \`${{ inputs.smg_repo }}\` |" >> $GITHUB_STEP_SUMMARY - echo "| smg_commit | \`${{ inputs.smg_commit }}\` |" >> $GITHUB_STEP_SUMMARY + { + echo "## Inputs" + echo "" + echo "| Parameter | Value |" + echo "|-----------|-------|" + echo "| base_image_ref | \`${{ inputs.base_image_ref }}\` |" + echo "| trtllm_repo | \`${{ inputs.trtllm_repo }}\` |" + echo "| trtllm_commit | \`${{ inputs.trtllm_commit }}\` |" + echo "| smg_repo | \`${{ inputs.smg_repo }}\` |" + echo "| smg_commit | \`${{ inputs.smg_commit }}\` |" + } >> "$GITHUB_STEP_SUMMARY" @@ - name: Summary run: | - echo "## Image" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "**Image name:** \`${{ steps.push-ghcr.outputs.image_name }}\`" >> $GITHUB_STEP_SUMMARY - echo "**Base image:** \`${{ steps.resolve-base.outputs.base_image_ref }}\`" >> $GITHUB_STEP_SUMMARY - echo "**Engine repo:** \`${{ steps.resolve-base.outputs.engine_repo }}\`" >> $GITHUB_STEP_SUMMARY + { + echo "## Image" + echo "" + echo "**Image name:** \`${{ steps.push-ghcr.outputs.image_name }}\`" + echo "**Base image:** \`${{ steps.resolve-base.outputs.base_image_ref }}\`" + echo "**Engine repo:** \`${{ steps.resolve-base.outputs.engine_repo }}\`" + } >> "$GITHUB_STEP_SUMMARY"Also applies to: 187-193
🤖 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 46 - 55, The repeated echo lines redirecting to $GITHUB_STEP_SUMMARY trigger ShellCheck SC2086/SC2129; fix by grouping the summary writes and quoting the variable once. Replace the multiple lines that individually append to $GITHUB_STEP_SUMMARY with a single grouped redirect (for example using a here-doc: cat <<'EOF' >> "$GITHUB_STEP_SUMMARY" ... EOF, or a brace/grouped block { echo "..."; echo "..."; } >> "$GITHUB_STEP_SUMMARY") so all echo statements append once to the quoted "$GITHUB_STEP_SUMMARY"; apply the same change to both summary-writing blocks around the existing run steps that reference GITHUB_STEP_SUMMARY.
🤖 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 42: Add the custom runner label "8-gpu-h200" to the actionlint
configuration so actionlint recognizes it; update .github/actionlint.yaml to
include the label (the same way vLLM workflow labels are listed) to silence the
unknown label warning for the runs-on entry "8-gpu-h200" in the
release-sglang-docker.yml workflow.
- Around line 77-78: The SMG_VERSION extraction using the grep/sed pipeline can
produce an empty value if the pyproject is missing or malformed; update the
workflow step that sets SMG_VERSION (the line running grep -m1 '^version = ' ...
| sed 's/version = "\(.*\)"/\1/') to validate the result immediately after
extraction by testing if SMG_VERSION is empty and failing the job with a clear
error message (exit non‑zero) if so, so the run does not proceed to create an
invalid image tag; ensure the error message mentions SMG_VERSION and the failing
extraction command for easier debugging.
In @.github/workflows/release-vllm-docker.yml:
- Line 50: Add the custom self-hosted runner label "8-gpu-h200" to the
actionlint config so actionlint stops flagging it as unknown; update the
.github/actionlint.yaml by adding a self-hosted-runner.labels entry that
includes "8-gpu-h200" (matching the runs-on value in the workflow) to document
the custom runner label repository-wide.
- Around line 67-70: Change the "Set up Docker Buildx" step that currently
specifies driver: docker to use the default docker-container driver so layer
caching can be used, and update the corresponding build step to add GitHub
Actions cache options (cache-from: type=gha and cache-to: type=gha,mode=max);
specifically, remove or replace the driver: docker line in the "Set up Docker
Buildx" step and modify the docker build action invocation (the step that runs
the image build/push) to include cache-from and cache-to so layer caching is
enabled while also accounting for the docker-container driver's different
handling of load.
---
Duplicate comments:
In @.github/workflows/release-sglang-docker.yml:
- Around line 77-89: The tag-building snippet produces SGLANG_COMMIT values that
can contain characters invalid for Docker tags; update the sanitization around
SGLANG_COMMIT (the logic that currently replaces "/" and spaces) to: replace any
character not in [A-Za-z0-9._-] with a hyphen, ensure the first character is
alphanumeric or underscore (prefix with a valid char if needed), and truncate
the final IMAGE_TAG component to fit Docker's 128-character limit before
composing IMAGE_TAG; apply this to both branches of the conditional that set
SGLANG_COMMIT so the resulting IMAGE_TAG always conforms to Docker tag rules.
- Around line 59-62: The workflow step using the docker/setup-buildx-action@v3
currently sets driver: docker which disables layer caching; change the Buildx
setup to use the docker-container driver (or remove the explicit driver) and
configure build cache options in the subsequent build step (e.g., add
cache-from/cache-to or --cache-to=type=registry,local, or buildx cache flags) so
Docker layer caching is enabled for the "Set up Docker Buildx" step and the
image build steps that reference it.
In @.github/workflows/release-trtllm-docker.yml:
- Around line 38-40: Unify the GHCR token source and add explicit workflow
permissions: set the workflow to use a single token source by assigning
env.GITHUB_TOKEN consistently from secrets.ROBOT_GITHUB_TOKEN (or swap all
usages to secrets.GITHUB_TOKEN) so usages of env.GITHUB_TOKEN and direct
references to secrets.GITHUB_TOKEN match, and add a top-level permissions block
(e.g., permissions: packages: write, contents: read) to allow GHCR pushes;
update references in the workflow where env.GITHUB_TOKEN,
secrets.ROBOT_GITHUB_TOKEN, and secrets.GITHUB_TOKEN appear so they are
consistent.
- Around line 46-55: The repeated echo lines redirecting to $GITHUB_STEP_SUMMARY
trigger ShellCheck SC2086/SC2129; fix by grouping the summary writes and quoting
the variable once. Replace the multiple lines that individually append to
$GITHUB_STEP_SUMMARY with a single grouped redirect (for example using a
here-doc: cat <<'EOF' >> "$GITHUB_STEP_SUMMARY" ... EOF, or a brace/grouped
block { echo "..."; echo "..."; } >> "$GITHUB_STEP_SUMMARY") so all echo
statements append once to the quoted "$GITHUB_STEP_SUMMARY"; apply the same
change to both summary-writing blocks around the existing run steps that
reference GITHUB_STEP_SUMMARY.
In @.github/workflows/release-vllm-docker.yml:
- Around line 85-97: Validate that SMG_VERSION (from
bindings/python/pyproject.toml) is non-empty after extraction and fail the job
with a clear error if it's missing or empty; in the block that sets SMG_VERSION
and constructs IMAGE_TAG, check the SMG_VERSION variable and call exit 1 (or use
a GitHub Actions error output) with a descriptive message so you don't produce
tags like "-vllm-..."; reference SMG_VERSION and IMAGE_TAG in your check and
ensure the workflow prints/logs the error before exiting.
- Around line 12-20: The workflow currently triggers on pull_request but the
image push (the push job/step around lines 115-124 in
.github/workflows/release-vllm-docker.yml) runs unconditionally and will publish
images from PRs; update the workflow to skip the push when the event is a
pull_request by adding a conditional (e.g., set an if: github.event_name !=
'pull_request' on the push job or the specific push step) so that push only runs
on non-PR events such as push or release.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/release-sglang-docker.yml.github/workflows/release-trtllm-docker.yml.github/workflows/release-vllm-docker.yml
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
.github/workflows/release-vllm-docker.yml (3)
85-97:⚠️ Potential issue | 🟡 MinorAdd validation for SMG_VERSION extraction.
If
bindings/python/pyproject.tomlis missing or malformed,SMG_VERSIONwill be empty, producing an invalid image tag like-vllm-v0.16.0-cu130-20260304.Proposed fix
run: | SMG_VERSION=$(grep -m1 '^version = ' bindings/python/pyproject.toml | sed 's/version = "\(.*\)"/\1/') + if [ -z "${SMG_VERSION}" ]; then + echo "::error::Failed to extract SMG version from bindings/python/pyproject.toml" + exit 1 + fi if [ -n "${VLLM_REPO}" ]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-vllm-docker.yml around lines 85 - 97, Validate the extracted SMG_VERSION after running the grep into SMG_VERSION (from bindings/python/pyproject.toml) and ensure it is non-empty before constructing IMAGE_TAG; if empty, print a clear error to stderr (including the file path and extraction command context) and exit non-zero so the workflow fails early instead of producing an invalid IMAGE_TAG, otherwise proceed to build IMAGE_TAG as before using SMG_VERSION and VLLM_COMMIT/DATE.
50-50: 🧹 Nitpick | 🔵 TrivialDocument custom runner label in actionlint config.
The
k8s-runner-cpulabel is flagged by actionlint as unknown. Add it to.github/actionlint.yamlto silence this warning:self-hosted-runner: labels: - k8s-runner-cpu🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-vllm-docker.yml at line 50, Add the custom runner label "k8s-runner-cpu" to the actionlint configuration so actionlint recognizes the label used in the workflow (runs-on: k8s-runner-cpu). Update .github/actionlint.yaml by adding a self-hosted-runner -> labels entry that includes "k8s-runner-cpu" so the label used in release-vllm-docker.yml is whitelisted.
12-20:⚠️ Potential issue | 🟠 MajorPR builds will still push images to GHCR.
The workflow triggers on
pull_requestevents, but the push step (lines 115-124) executes unconditionally. This means PR builds from potentially untrusted branches will publish images to GHCR.Add a condition to skip the push step on PR events:
- name: Push image to GHCR id: push-ghcr + if: github.event_name != 'pull_request' env:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-vllm-docker.yml around lines 12 - 20, The push step currently runs for pull_request events and will publish images from untrusted PRs; modify the push step (the step named "push" / "Build and push" in the release-vllm-docker workflow) to only run when the event is not a pull_request by adding a conditional like if: github.event_name != 'pull_request' at the step (or job) level so pushes are skipped for PR runs.
🤖 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 3-10: The run-name is interpolating github.event.inputs.* which is
only set for workflow_dispatch, causing empty values on pull_request runs;
update the run-name logic (symbol: run-name) to conditionally choose values
based on the event (github.event_name) — e.g., use the workflow_dispatch inputs
when github.event_name == 'workflow_dispatch' and fall back to
pull-request-friendly fields (like github.head_ref, github.sha, or simple static
text) when github.event_name == 'pull_request' so PR-triggered runs show
meaningful names instead of empty input placeholders.
- Around line 126-130: The Summary step currently always writes the pushed image
name from steps.push-ghcr.outputs.image_name which will be empty when the push
step is skipped; make the Summary step conditional by adding the same guard used
on the push step (if: github.event_name != 'pull_request') or alternatively
implement a conditional branch in the Summary step to check if
steps.push-ghcr.outputs.image_name is set and emit a PR-specific message (e.g.,
"Build succeeded; image push skipped for PRs") so the Summary never shows an
empty image name; update the step named "Summary" and reference outputs from the
"push-ghcr" step accordingly.
- Around line 115-124: Repository owner may have uppercase characters which GHCR
rejects; update the workflow step that builds TARGET_IMAGE to lowercase the
owner. Change the interpolation that creates TARGET_IMAGE to call lowercase on
github.repository_owner (i.e. ensure the value used when constructing
TARGET_IMAGE/IMAGE_TAG/BASE_IMAGE_TAG is converted to lowercase) so the final
TARGET_IMAGE string passed to docker tag/push is all-lowercase.
---
Duplicate comments:
In @.github/workflows/release-vllm-docker.yml:
- Around line 85-97: Validate the extracted SMG_VERSION after running the grep
into SMG_VERSION (from bindings/python/pyproject.toml) and ensure it is
non-empty before constructing IMAGE_TAG; if empty, print a clear error to stderr
(including the file path and extraction command context) and exit non-zero so
the workflow fails early instead of producing an invalid IMAGE_TAG, otherwise
proceed to build IMAGE_TAG as before using SMG_VERSION and VLLM_COMMIT/DATE.
- Line 50: Add the custom runner label "k8s-runner-cpu" to the actionlint
configuration so actionlint recognizes the label used in the workflow (runs-on:
k8s-runner-cpu). Update .github/actionlint.yaml by adding a self-hosted-runner
-> labels entry that includes "k8s-runner-cpu" so the label used in
release-vllm-docker.yml is whitelisted.
- Around line 12-20: The push step currently runs for pull_request events and
will publish images from untrusted PRs; modify the push step (the step named
"push" / "Build and push" in the release-vllm-docker workflow) to only run when
the event is not a pull_request by adding a conditional like if:
github.event_name != 'pull_request' at the step (or job) level so pushes are
skipped for PR runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dc38b13f-5f6b-4eed-9a0a-3f7333aa5863
📒 Files selected for processing (1)
.github/workflows/release-vllm-docker.yml
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 48-50: Add the custom runner label k8s-runner-cpu to the
actionlint configuration so actionlint no longer flags it as unknown; update the
actionlint config (actionlint.yaml) under the self-hosted-runner.labels list to
include k8s-runner-cpu (and other custom labels like 8-gpu-h200 if present) to
match the workflow's runs-on usage and suppress false positives.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 422beead-8753-4efc-8fd0-32ba7486527d
📒 Files selected for processing (1)
.github/workflows/release-vllm-docker.yml
Add manual workflow_dispatch pipelines for building SMG images on top of inference engine base images (SGLang, vLLM, TensorRT-LLM, TGL), along with the multi-stage Dockerfile.engine and installation scripts they depend on. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: gongwei-130 <56567052+gongwei-130@users.noreply.github.com> Signed-off-by: gongwei-130 <weigong28@gmail.com>
gongwei-130
force-pushed
the
wei/backend-image-build
branch
from
March 5, 2026 18:44
e8bddd2 to
c0110c9
Compare
gongwei-130
changed the title
slin1237
approved these changes
Mar 5, 2026
Contributor
|
Hi @gongwei-130, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
Signed-off-by: gongwei-130 <weigong28@gmail.com>
gongwei-130
force-pushed
the
wei/backend-image-build
branch
from
March 5, 2026 21:42
4b688b7 to
d158c97
Compare
5 tasks
2 tasks
4 tasks
2 of 6 tasks
8 tasks
4 tasks
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
release-sglang-docker.yml: aworkflow_dispatchpipeline to build and push a SMG+SGLang Docker image to GHCR, supporting custom base image, engine repo/commit, SMG repo/commit, and optional tag override as inputsrelease-trtllm-docker.yml: aworkflow_dispatchpipeline to build and push a SMG+TensorRT-LLM Docker image to GHCR, supporting two build paths:base_image_ref), optionally refreshing engine code viatrtllm_repotrtllm_repo+trtllm_commit)release-vllm-docker.yml: aworkflow_dispatchpipeline to build and push a SMG+vLLM Docker image to GHCR, supporting custom base image, engine repo/commit, SMG repo/commit, and optional tag override as inputs8-gpu-h200runners, resolve a deterministic image tag (<smg_version>-<engine>-<engine_commit>), and print a summary with the pushed image nameChanges
.github/workflows/release-sglang-docker.yml: New SGLang image build workflowworkflow_dispatchor PR touching related paths<smg_version>-sglang-<sglang_commit>ghcr.io/<owner>/smg:<tag>.github/workflows/release-trtllm-docker.yml: New TensorRT-LLM image build workflow<smg_version>-trtllm-<trtllm_commit>.github/workflows/release-vllm-docker.yml: New vLLM image build workflowdocker builddirectly (no Buildx) withcustom-vllmtarget<smg_version>-vllm-<vllm_commit>ghcr.io/<owner>/smg:<tag>Test Plan
release-sglang-dockerviaworkflow_dispatchwith a known base image, verify image is pushed to GHCR with correct tagrelease-trtllm-dockerwithbase_image_refset, verify build succeedsrelease-trtllm-dockerwithbase_image_refempty andtrtllm_reposet, verify base image is built from sourcerelease-vllm-dockerviaworkflow_dispatchwith a known base image, verify image is pushed to GHCR with correct tag