[CI] Push multiarch manifests as nightly builds #25764
Conversation
Signed-off-by: Sahithi Chigurupati <[email protected]>
Signed-off-by: Sahithi Chigurupati <[email protected]>
![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 updates the CI pipeline to push multi-arch Docker manifests for nightly builds and fixes an issue with the nightly build cleanup script. The changes to create and push multi-arch manifests in .buildkite/release-pipeline.yaml are correct. The fix in .buildkite/scripts/cleanup-nightly-builds.sh properly authenticates with the Docker Hub API. However, I've identified a critical security vulnerability in the cleanup script where a secret token is exposed in the build logs and have provided a suggestion to resolve it.
| BEARER_TOKEN=$(curl -s -X POST \ | ||
| -H "Content-Type: application/json" \ | ||
| -d "{\"username\": \"$DOCKERHUB_USERNAME\", \"password\": \"$DOCKERHUB_TOKEN\"}" \ | ||
| "https://hub.docker.com/v2/users/login" | jq -r '.token') |
There was a problem hiding this comment.
Due to set -ex at the top of the script, the curl command will be printed to the build logs, exposing the DOCKERHUB_TOKEN. This is a critical security vulnerability. You should temporarily disable command tracing around this sensitive command to prevent the token from being logged.
| BEARER_TOKEN=$(curl -s -X POST \ | |
| -H "Content-Type: application/json" \ | |
| -d "{\"username\": \"$DOCKERHUB_USERNAME\", \"password\": \"$DOCKERHUB_TOKEN\"}" \ | |
| "https://hub.docker.com/v2/users/login" | jq -r '.token') | |
| set +x | |
| BEARER_TOKEN=$(curl -s -X POST \ | |
| -H "Content-Type: application/json" \ | |
| -d "{\"username\": \"$DOCKERHUB_USERNAME\", \"password\": \"$DOCKERHUB_TOKEN\"}" \ | |
| "https://hub.docker.com/v2/users/login" | jq -r '.token') | |
| set -x |
There was a problem hiding this comment.
Please address this comment
|
@codex review |
![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.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting
| - "aws ecr-public get-login-password --region us-east-1 | docker login --username AWS --password-stdin public.ecr.aws/q9t5s3a7" | ||
| - "docker pull public.ecr.aws/q9t5s3a7/vllm-release-repo:$BUILDKITE_COMMIT" | ||
| - "docker tag public.ecr.aws/q9t5s3a7/vllm-release-repo:$BUILDKITE_COMMIT vllm/vllm-openai:nightly" | ||
| - "docker tag public.ecr.aws/q9t5s3a7/vllm-release-repo:$BUILDKITE_COMMIT vllm/vllm-openai:nightly-$BUILDKITE_COMMIT" | ||
| - "docker push vllm/vllm-openai:nightly" | ||
| - "docker push vllm/vllm-openai:nightly-$BUILDKITE_COMMIT" | ||
| - "docker manifest create vllm/vllm-openai:nightly public.ecr.aws/q9t5s3a7/vllm-release-repo:$BUILDKITE_COMMIT-x86_64 public.ecr.aws/q9t5s3a7/vllm-release-repo:$BUILDKITE_COMMIT-aarch64 --amend" | ||
| - "docker manifest create vllm/vllm-openai:nightly-$BUILDKITE_COMMIT public.ecr.aws/q9t5s3a7/vllm-release-repo:$BUILDKITE_COMMIT-x86_64 public.ecr.aws/q9t5s3a7/vllm-release-repo:$BUILDKITE_COMMIT-aarch64 --amend" | ||
| - "docker manifest push vllm/vllm-openai:nightly" | ||
| - "docker manifest push vllm/vllm-openai:nightly-$BUILDKITE_COMMIT" |
There was a problem hiding this comment.
[P1] Manifest pushes lack per-arch images in DockerHub
The nightly step now creates and pushes a multi-arch manifest that references the images stored in ECR but no longer pulls and retags those per‑architecture images into the vllm/vllm-openai repository before pushing. docker manifest push cannot upload the underlying manifests/layers from another registry, so the command will either fail with manifest unknown or publish a manifest whose digests are missing in DockerHub. In either case, pulling vllm/vllm-openai:nightly will fail for all users. The pipeline still needs to copy each architecture image into DockerHub before creating the manifest.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
+1 you are no longer pushing the image here?
There was a problem hiding this comment.
fixed the issue by pushing the images first
Signed-off-by: Sahithi Chigurupati <[email protected]>
Purpose
Currently only x86 images are being pushed as nightly builds made available through #24102.
This PR helps push multiarch manifests instead as nightly builds.
The nightly build cleanup script is also throwing access token related errors - https://buildkite.com/vllm/release/builds/8648#01997f3f-1fee-409b-87a2-b9cab746378e
This PR fixes these issues with the cleanup script.
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.