Add UBI 9 container with vLLM and GuideLLM (OCTOET-1123)#48
Closed
bennyturns wants to merge 11 commits intomainfrom
Closed
Add UBI 9 container with vLLM and GuideLLM (OCTOET-1123)#48bennyturns wants to merge 11 commits intomainfrom
bennyturns wants to merge 11 commits intomainfrom
Conversation
- Add Containerfile based on UBI 9 Python 3.11 - Add multi-arch GitHub Actions workflow (amd64/arm64) - Add Cosign image signing - Add local test script - Add container README Fixes for UBI 9 compatibility: - Removed numactl-devel (not available in UBI 9) - Fixed directory permissions for non-root user - Removed invalid OMP_NUM_THREADS=auto Container includes: - vLLM 0.16.0 with CPU support - GuideLLM 0.5.3 for benchmarking - NumPy, Pandas, psutil Tested locally - imports work correctly. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
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 a vLLM CPU performance evaluation container (Containerfile, README, tests) and a GitHub Actions workflow to build multi‑arch images (amd64, arm64), run PR tests, sign images with Cosign, upload per‑arch digests, and create/publish a multi‑arch manifest. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions Runner
participant Registry as Container Registry (Quay/RedHat)
participant Artifacts as GitHub Artifacts
participant Cosign as Cosign
Dev->>GH: push or open PR (or workflow_dispatch)
GH->>GH: checkout, setup QEMU & buildx
GH->>GH: compute tags (branch/PR/semver/sha) and arch suffixes
GH->>GH: build per-arch images (amd64, arm64)
alt PR event
GH->>GH: export image tar per-arch
GH->>Artifacts: upload tar + digest artifacts
GH->>GH: load tar and run in-container tests (vLLM, GuideLLM)
else Non-PR (push)
GH->>Registry: login (RedHat base, Quay for push)
GH->>Registry: buildx build & push per-arch images
GH->>Cosign: sign pushed images (using digest refs)
GH->>Artifacts: upload digest artifacts
GH->>GH: create-manifest job downloads digests
GH->>Registry: buildx imagetools create/push multi-arch manifest
GH->>Cosign: sign manifest
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/build-and-push.yml:
- Around line 97-103: Assign the image tag to a shell variable before using it
in the docker run commands instead of using unquoted command substitution;
capture the first tag from ${{ steps.meta.outputs.tags }} (the same expression
used with head -1) into a variable (e.g., image_tag) and then use docker run
--rm "$image_tag" when invoking the tests (the three places that call docker run
--rm $(echo "${{ steps.meta.outputs.tags }}" | head -1) and the echo/test lines
should reference the variable).
In `@container/README.md`:
- Around line 3-35: Update the README to match the actual container: change the
UBI reference from "UBI 10" to "UBI 9" and correct the OMP_NUM_THREADS
description to reflect that the container does not set a default (remove
"default: auto" or explicitly document it as "not set by container"); either
update the text under "Environment Variables" for OMP_NUM_THREADS to "not set by
container (user must set)" or, if you prefer to enforce the default, set
OMP_NUM_THREADS to "auto" in the container startup/Dockerfile and document that
change; ensure the variable names OMP_NUM_THREADS and VLLM_CPU_KVCACHE_SPACE
remain present and accurate.
In `@container/tests/test-container.sh`:
- Line 4: Replace the fragile "set -e" with "set -euo pipefail" so the script
fails on unset vars and on any failed command in a pipeline, and adjust the CLI
checks that use "vllm --help | head -20" and "guidellm --help | head -20" to
avoid masking failures (e.g., invoke the help commands directly or
capture/redirect their output and test exit statuses rather than piping through
head); also ensure any shell variables used in the script are safely quoted (use
"$VAR" wherever variables appear) so the checks and other commands fail
correctly and safely on error.
- Around line 6-10: The podman build invocation uses a relative Containerfile
path and fails if the script isn't run from the repo root; change the script to
resolve the script directory and use that absolute path for the build context
and Containerfile. Specifically, compute the script's directory (so IMAGE_NAME
and IMAGE_TAG stay unchanged) and pass "-f ${SCRIPT_DIR}/Containerfile
${SCRIPT_DIR}" (or equivalent) to the podman build command so the build is
location-agnostic when invoking podman build -t ${IMAGE_NAME}:${IMAGE_TAG} -f
... .
In `@Containerfile`:
- Around line 42-54: Pin the pip installs in the Containerfile to fixed versions
to ensure reproducible builds: update the RUN pip install lines that install
vllm and guidellm (and optionally numpy, pandas, psutil) to specify explicit
versions (e.g., guidellm>=0.5.0,<0.6.0 or whatever your target), replacing the
floating package names in the vllm and guidellm install commands so the image
build always pulls the intended releases; keep the --no-cache-dir flag and
ensure constraints match your deployment requirements.
- Line 5: Pin the base image by digest instead of using :latest; run skopeo
inspect docker://registry.redhat.io/ubi9/python-311:latest --format
'{{.Digest}}' to obtain the image digest and replace the FROM line (FROM
registry.redhat.io/ubi9/python-311:latest) with the digest form (e.g., FROM
registry.redhat.io/ubi9/python-311@sha256:<digest>), commit the updated
Containerfile, and optionally verify by rebuilding the image to ensure
determinism.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
.github/workflows/build-and-push.ymlContainerfilecontainer/README.mdcontainer/tests/test-container.sh
- Fix GitHub Actions: use shell variable for image tag instead of command substitution
- Update container/README.md: change UBI 10 to UBI 9, fix OMP_NUM_THREADS docs
- Improve test-container.sh: use set -euo pipefail, make path-agnostic, fix CLI tests
- Pin all dependencies for reproducible builds:
- Base image pinned by digest (sha256:56193de...)
- vllm>=0.16.0,<0.17.0
- guidellm>=0.5.0,<0.6.0
- numpy, pandas, psutil with version constraints
All changes improve build reproducibility, error handling, and documentation accuracy.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/build-and-push.yml (1)
174-179: Quote command substitutions to prevent word splitting.Shellcheck SC2046 flags the unquoted
$(jq ...)and$(printf ...)expansions. While image tags typically don't contain spaces, quoting is safer and silences the linter.Suggested fix
- name: Create manifest list and push working-directory: /tmp/digests run: | - docker buildx imagetools create \ - $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.REGISTRY }}/${{ env.QUAY_ORG }}/${{ env.IMAGE_NAME }}@sha256:%s ' *) + tags=$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") + digests=$(printf '${{ env.REGISTRY }}/${{ env.QUAY_ORG }}/${{ env.IMAGE_NAME }}@sha256:%s ' *) + docker buildx imagetools create $tags $digests🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push.yml around lines 174 - 179, The unquoted command substitutions in the docker buildx imagetools create command (the $(jq -cr ...) and $(printf ...) expansions) can cause word-splitting; update the GitHub Actions step that runs docker buildx imagetools create to wrap those substitutions in quotes so they are passed as single arguments (i.e., quote the output of the jq expression and the printf expression used to build image@sha256 strings) while keeping the overall command and working-directory the same.
🤖 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/build-and-push.yml:
- Around line 3-21: The YAML fails yamllint: change the top-level key on to a
quoted key ("on") and ensure all string scalars use double quotes; specifically
update the "on" key, the branch names under push and pull_request (e.g., main),
the paths entries ('Containerfile' and '.github/workflows/build-and-push.yml'),
the workflow_dispatch key names if unquoted, and the default tag value (default:
"latest") so that "on", branch names, path strings and the tag default use
double quotes throughout the workflow.
---
Nitpick comments:
In @.github/workflows/build-and-push.yml:
- Around line 174-179: The unquoted command substitutions in the docker buildx
imagetools create command (the $(jq -cr ...) and $(printf ...) expansions) can
cause word-splitting; update the GitHub Actions step that runs docker buildx
imagetools create to wrap those substitutions in quotes so they are passed as
single arguments (i.e., quote the output of the jq expression and the printf
expression used to build image@sha256 strings) while keeping the overall command
and working-directory the same.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
.github/workflows/build-and-push.ymlContainerfilecontainer/README.mdcontainer/tests/test-container.sh
added 2 commits
March 3, 2026 16:13
- Quote 'on' key as per yamllint requirements - Use double quotes for all string scalars (branches, paths, defaults) - Ensures workflow passes yamllint validation Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Remove 'prefix={{branch}}-' from type=sha tag generation
- The {{branch}} variable is empty for PRs, causing invalid tags like '-40be98d'
- Now generates valid tags: 'pr-48-amd64' and 'sha-40be98d-amd64'
Fixes: ERROR: invalid tag 'quay.io/redhat-et/vllm-cpu-perf-eval:-40be98d-amd64'
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/build-and-push.yml (1)
195-200: Step name/behavior mismatch in “Comment on PR with image info”.This block runs only on push (Line 196) and only prints logs; it never comments on a PR. Consider renaming it to reflect behavior, or implement an actual PR comment action.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push.yml around lines 195 - 200, The step named "Comment on PR with image info" is misleading because it only runs on push (if: github.event_name == 'push') and just echoes messages; either rename the step to something like "Announce image published" to match behavior, or change its triggering and implementation to actually comment on a PR by running during pull_request (or both push and pull_request) and replacing the echo block with a PR-comment action (e.g., use actions/github-script or peter-evans/create-or-update-comment) that posts the message containing ${{ env.REGISTRY }}/${{ env.QUAY_ORG }}/${{ env.IMAGE_NAME }}:latest and cosign info; update the step name string "Comment on PR with image info" and the if condition accordingly and ensure the chosen action has access to GITHUB_TOKEN.
🤖 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/build-and-push.yml:
- Around line 16-21: The workflow defines a manual input tag under
workflow_dispatch (inputs.tag) but never uses github.event.inputs.tag when
building/pushing images, so manual runs ignore the provided tag; update the
relevant steps that set the image tag (the build/push job steps that currently
compute or default the tag) to read and prefer github.event.inputs.tag when
present (fallback to existing defaults like env.GITHUB_REF or "latest"), e.g.,
ensure the image tag variable used in the build and push actions references
github.event.inputs.tag, and apply the same change for the other occurrences
mentioned so all three manual-input blocks consume the provided tag.
- Around line 184-188: The GitHub Actions "Sign the manifest" step currently
calls `cosign sign` with mutable tags (the `cosign sign` invocation using `${{
steps.meta.outputs.tags }}`), which is unsafe; update this step to first resolve
each tag to its immutable digest (e.g., use `crane digest` or equivalent to get
DIGEST for each tag) and then call `cosign sign` against the image@"$DIGEST"
reference; for multi-arch images ensure you resolve and sign the OCI index
digest (not just individual architecture manifests) so the signature covers all
variants, and preserve the `COSIGN_EXPERIMENTAL: "true"` env if needed.
---
Nitpick comments:
In @.github/workflows/build-and-push.yml:
- Around line 195-200: The step named "Comment on PR with image info" is
misleading because it only runs on push (if: github.event_name == 'push') and
just echoes messages; either rename the step to something like "Announce image
published" to match behavior, or change its triggering and implementation to
actually comment on a PR by running during pull_request (or both push and
pull_request) and replacing the echo block with a PR-comment action (e.g., use
actions/github-script or peter-evans/create-or-update-comment) that posts the
message containing ${{ env.REGISTRY }}/${{ env.QUAY_ORG }}/${{ env.IMAGE_NAME
}}:latest and cosign info; update the step name string "Comment on PR with image
info" and the if condition accordingly and ensure the chosen action has access
to GITHUB_TOKEN.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
.github/workflows/build-and-push.yml
added 2 commits
March 3, 2026 16:28
- Add login step for registry.redhat.io before building - Allows pulling UBI base image with digest - Uses REDHAT_REGISTRY_USERNAME and REDHAT_REGISTRY_PASSWORD secrets Fixes: 401 Unauthorized when pulling UBI9 base image Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Security improvements: - Individual arch signing: Sign digest directly without tag reference - Manifest signing: Resolve and sign multi-arch manifest digest - Use docker buildx imagetools inspect to get OCI index digest - Preserve COSIGN_EXPERIMENTAL for keyless signing Before: cosign sign --yes tag:latest (mutable, unsafe) After: cosign sign --yes image@sha256:... (immutable, secure) Ensures signatures are cryptographically bound to image content. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/build-and-push.yml (2)
221-226: Step name is misleading for actual behavior.Line [221] says “Comment on PR,” but the step runs on push and only prints logs. Rename the step to avoid operator confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push.yml around lines 221 - 226, The step named "Comment on PR with image info" is misleading because it only runs on push (if: github.event_name == 'push') and echoes logs instead of commenting on a PR; rename the step name to accurately reflect behavior (e.g., "Log published image info" or "Publish image info (push)") by updating the step's name string so operators aren't confused about its effect while leaving the existing if and run block unchanged.
18-21: Remove the non-default branch premise; the suggested UX improvement remains valid.
workflow_dispatchcan only be triggered from the default branch (main per GitHub Actions documentation). However, defaulting thetaginput to"latest"(line 21) means manual runs will publishlatesttags unless the operator explicitly provides a different value.Changing the default to
""(empty string) prevents unintendedlatestpublications and makes the input truly optional:Suggested UX improvement
workflow_dispatch: inputs: tag: - description: "Image tag (default: latest)" + description: "Optional image tag override" required: false - default: "latest" + default: ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push.yml around lines 18 - 21, The workflow input "tag" currently has default: "latest" which causes manual workflow_dispatch runs to publish a latest tag unintentionally; update the workflow input definition for "tag" (the tag input under workflow_dispatch) to use default: "" (empty string) and ensure downstream steps that consume the tag input (e.g., any uses of inputs.tag or env.TAG) treat an empty string as "no tag" before deciding to publish or default to latest programmatically—this prevents implicit latest publications while keeping the input optional.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/build-and-push.yml:
- Around line 221-226: The step named "Comment on PR with image info" is
misleading because it only runs on push (if: github.event_name == 'push') and
echoes logs instead of commenting on a PR; rename the step name to accurately
reflect behavior (e.g., "Log published image info" or "Publish image info
(push)") by updating the step's name string so operators aren't confused about
its effect while leaving the existing if and run block unchanged.
- Around line 18-21: The workflow input "tag" currently has default: "latest"
which causes manual workflow_dispatch runs to publish a latest tag
unintentionally; update the workflow input definition for "tag" (the tag input
under workflow_dispatch) to use default: "" (empty string) and ensure downstream
steps that consume the tag input (e.g., any uses of inputs.tag or env.TAG) treat
an empty string as "no tag" before deciding to publish or default to latest
programmatically—this prevents implicit latest publications while keeping the
input optional.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
.github/workflows/build-and-push.yml
Change the container registry organization to match the actual Quay.io repository location. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/build-and-push.yml (1)
221-226: Step name is misleading (Comment on PR) but only logs on push.Line 222 restricts to push events, and Lines 223-226 only echo output. Consider renaming the step to match behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push.yml around lines 221 - 226, The step named "Comment on PR with image info" is misleading because it only runs on push and only echoes output; rename the step to something accurate like "Log published image info" or "Publish image log" (update the name string for that step) and keep the existing if: github.event_name == 'push' and the run block as-is; alternatively, if you actually want to comment on PRs instead, change the condition to run on pull_request and replace the echo lines with a PR-comment action—refer to the step name "Comment on PR with image info" and the if condition to locate the change.
🤖 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/build-and-push.yml:
- Around line 16-21: The workflow currently defaults the manual input "tag" to
"latest" under workflow_dispatch which lets non-main manual runs accidentally
publish :latest; add a validation step early in the build-and-test job (e.g., a
step named "Validate manual tag policy") that runs only when github.event_name
== 'workflow_dispatch' and checks github.event.inputs.tag == 'latest' and
github.ref_name != 'main', and if so prints an error and exits non‑zero to abort
the run; keep the existing tag usage elsewhere but ensure this guard prevents
applying the raw manual tag when invoked from non-main branches.
- Around line 203-207: The manifest digest resolution uses docker buildx
imagetools inspect with the template '{{json .}}' and then jq
'.manifest.digest', which can return null; change the inspect template to
'{{json .Manifest}}' and extract '.digest' from that JSON instead (update the
variable manifest_digest assignment that calls docker buildx imagetools inspect
and the subsequent jq extraction to use .Manifest → .digest while leaving
first_tag and the surrounding logic unchanged).
---
Nitpick comments:
In @.github/workflows/build-and-push.yml:
- Around line 221-226: The step named "Comment on PR with image info" is
misleading because it only runs on push and only echoes output; rename the step
to something accurate like "Log published image info" or "Publish image log"
(update the name string for that step) and keep the existing if:
github.event_name == 'push' and the run block as-is; alternatively, if you
actually want to comment on PRs instead, change the condition to run on
pull_request and replace the echo lines with a PR-comment action—refer to the
step name "Comment on PR with image info" and the if condition to locate the
change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
.github/workflows/build-and-push.ymlcontainer/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- container/README.md
Include automation scripts, test suites, model configurations, and documentation in the container so all test execution tools are available at runtime. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Change the imagetools inspect template from '{{json .}}' to
'{{json .Manifest}}' and extract .digest directly instead of
.manifest.digest to ensure reliable digest resolution.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Install Ansible to enable running the playbooks in automation/test-execution/ansible/. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update Ansible version constraint from >=2.16.0,<3.0.0 to >=9.0.0,<13.0.0 since version 13+ requires Python 3.12 and the container uses Python 3.11. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Collaborator
|
I think guidellm has a dependency on python 3.13 and I don't think htere's a UBI image that supports this yet. https://github.com/vllm-project/guidellm/blob/main/Containerfile#L1C31-L1C41 https://catalog.redhat.com/en/software/containers/ubi9/python-312/657b08d023df896ebfacf402 <-- is the latest version of python I can find supported |
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.
Fixes for UBI 9 compatibility:
Container includes:
Tested locally - imports work correctly.
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
Summary by CodeRabbit
New Features
Documentation
Tests