-
Notifications
You must be signed in to change notification settings - Fork 81
Various fixes to container image and CI #254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
3604c63
to
f57f9df
Compare
f57f9df
to
250df65
Compare
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
c62588f
to
ef02649
Compare
Signed-off-by: Samuel Monson <[email protected]>
ef02649
to
e360711
Compare
Signed-off-by: Samuel Monson <[email protected]>
437ca73
to
4bb780f
Compare
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
b9dc4ad
to
cb05e48
Compare
Signed-off-by: Samuel Monson <[email protected]>
cb05e48
to
ca3f6e8
Compare
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the container image build process and CI workflows to improve caching, fix version detection, and automate container maintenance. The changes move the Containerfile to the root directory, improve image layer ordering for better build caching, and add automation for tagging latest/stable versions and cleaning up development images.
- Relocated Containerfile from deploy/ to root directory with improved layer caching strategy
- Enhanced CI workflows to properly tag container images with versions and build types
- Added automated container maintenance workflow for cleaning up old PR images and updating latest/stable tags
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
deploy/Containerfile | Removed old Containerfile from deploy directory |
Containerfile | New Containerfile at root with improved caching and PDM integration |
README.md | Added documentation for available container image tags |
.github/workflows/release.yml | Updated to use new Containerfile path and tag with version |
.github/workflows/release-candidate.yml | Updated to use new Containerfile path and tag RC versions |
.github/workflows/nightly.yml | Updated to use new Containerfile path and set nightly build type |
.github/workflows/development.yml | Updated to use new Containerfile path and set dev build type |
.github/workflows/container-maintenance.yml | New workflow for automated container cleanup and tag management |
.containerignore | Added container ignore file referencing .gitignore |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
image-names: ${{ github.event.repository.name }} | ||
image-tags: "pr-*" | ||
cut-off: 2w | ||
dry-run: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set this to dry-run
for now until we can confirm it deletes the correct set of images.
RUN apt-get update \ | ||
&& apt-get install -y --no-install-recommends git \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had hoped to keep this image distro-agnostic to enable downstream RHEL builds but we need git
to have the correct package version tag after install. There are currently two alternatives I am considering:
- Write a helper script that implements
apt
anddnf
based installs based on/etc/os-release
. - Change base_image to a community
dnf
based image.
I prefer (2) but currently there are not any minimal 3.13 images based on Fedora/CentOS/RHEL. The closest is quay.io/fedora/python-312-minimal
. I've filled sclorg/s2i-python-container#753 with the project to hopeful enable python-313-minimal
builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to use a 3.12 base image instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I left one comment.
RUN apt-get update \ | ||
&& apt-get install -y --no-install-recommends git \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to use a 3.12 base image instead?
Summary
Various fixes to containers images and CI. Containers images should now reuse most layers on build, need less hacks when running in OCP, and properly detect the guidellm version. Release and RC git tags/branches tag the container image with the version. The
latest
andstable
tags now smartly update based on highest version tag (prev most recent build).Details
.containerignore
that mirrors.gitignore
Test Plan
1. Workflows
2. Container Changes
podman run --rm ghcr.io/vllm-project/guidellm:pr-254 --version
to verify version is setpodman run --rm --entrypoint /opt/app-root/guidellm/bin/pip ghcr.io/vllm-project/guidellm:pr-254 freeze
and verify that versions match thepylock.toml
./home/guidellm
. (For tokenizer/dataset caching)Related Issues
Use of AI
## WRITTEN BY AI ##
)