-
-
Notifications
You must be signed in to change notification settings - Fork 12.3k
[CI/Build][Docker] Add centralized version manifest for Docker builds #31492
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?
[CI/Build][Docker] Add centralized version manifest for Docker builds #31492
Conversation
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.
Code Review
This pull request is a great step towards centralizing version management by introducing docker/versions.json. This will significantly improve maintainability and reduce errors when updating dependencies. The new scripts (build.sh, get-version.sh, validate_versions.py) are well-structured and provide useful tooling. My review includes a few suggestions to enhance correctness and consistency, particularly by fixing a bug in the build.sh script's dry-run output and addressing a critical omission in the validate_versions.py script to ensure it provides complete validation.
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run You ask your reviewers to trigger select CI tests on top of Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. 🚀 |
|
Hi @mritunjaysharma394, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
cf4f8f2 to
be875f8
Compare
docker/build.sh
Outdated
| # -h, --help Show this help message | ||
| # | ||
| # Examples: | ||
| # ./docker/build.sh # Build default target |
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.
Can we standardize on bake instead of custom scripts? I started here #31477
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.
Thanks for the review and good call! I've simplified this PR to remove all the custom scripts (build.sh, get-version.sh, validate_versions.py).
Now it just adds docker/versions.json as a machine-readable version manifest that your bake configuration in #31477 can reference for the pinned versions.
The two PRs complement each other:
- versions.json = version data (what versions to use)
- docker-bake.hcl = build config (how to build)
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've also restructured versions.json to better support your bake approach. Added a docker section with:
docker.build_args - flat map that matches Dockerfile ARGs exactly:
{
"CUDA_VERSION": "12.9.1",
"PYTHON_VERSION": "3.12",
"TORCH_CUDA_ARCH_LIST": "7.0 7.5 8.0 8.9 9.0 10.0 12.0",
"FLASHINFER_VERSION": "0.5.3",
...
}docker.build_config - build settings that match your bake variables:
{
"max_jobs": 16,
"nvcc_threads": 8
}This makes it easy for bake to consume:
# Extract for bake
CUDA=$(jq -r '.docker.build_args.CUDA_VERSION' docker/versions.json)
MAX_JOBS=$(jq -r '.docker.build_config.max_jobs' docker/versions.json)Or generate HCL variables directly from the JSON. Let me know if you'd like any other fields added!
be875f8 to
acafb70
Compare
Introduces docker/versions.json as a machine-readable version manifest for all pinned dependencies used in Docker builds. This complements the docker buildx bake work in PR vllm-project#31477: - versions.json provides the version data (what versions to use) - docker-bake.hcl provides build configuration (how to build) Benefits: - Single source of truth for all pinned versions - Machine-readable format for CI and external tooling (jq-parseable) - Cleaner diffs for version bumps - Easy release comparison: git diff v0.13..v0.14 -- docker/versions.json Files: - docker/versions.json: Version manifest with CUDA, Python, FlashInfer, bitsandbytes, torch arch list, and extension commit refs - docker/Dockerfile: Added comments pointing to versions.json Signed-off-by: Mritunjay Sharma <[email protected]>
acafb70 to
1c1750e
Compare
Purpose
Introduces
docker/versions.jsonas a centralized, machine-readable version manifest for all pinned dependencies in Docker builds.Problem
Version information is currently scattered across
docker/Dockerfileand various install scripts, making it hard to:Solution
A single
docker/versions.jsonfile structured to support both human readers and tooling.Integration with Docker Bake (#31477)
This PR is designed to complement the
docker buildx bakework in #31477:versions.json= version datadocker-bake.hcl= build configStructure for Bake Compatibility
{ "docker": { "build_args": { "CUDA_VERSION": "12.9.1", "PYTHON_VERSION": "3.12", "TORCH_CUDA_ARCH_LIST": "7.0 7.5 8.0 8.9 9.0 10.0 12.0", "FLASHINFER_VERSION": "0.5.3", ... }, "build_config": { "max_jobs": 16, "nvcc_threads": 8 } } }docker.build_args: Flat map matching Dockerfile ARGs exactly - easy for bake to consumedocker.build_config: Build parallelism settings matching bake variablesUsage with Bake
Benefits
docker.build_argsmaps directly to Dockerfile ARGsjqfor CI and external toolinggit diff v0.13.0..v0.14.0 -- docker/versions.jsonFiles Changed
docker/versions.json- New version manifest with bake-compatible structuredocker/Dockerfile- Added comments pointing to versions.jsonTest Plan
docker.build_argsvalues match current Dockerfile ARG defaults