[release] Minor fixes to release annotation and wheel upload#33129
[release] Minor fixes to release annotation and wheel upload#33129
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces several fixes and improvements to the release process scripts. It updates the release annotation script to include commands for CUDA 13.0 and CPU wheels, and corrects the Docker repository for ROCm images. The wheel upload script is streamlined to focus solely on PyPI, removing the GitHub release logic and correcting the twine command usage. While these changes are beneficial, I've identified two critical copy-paste errors in the annotate-release.sh script that could result in publishing incorrect Docker images during a release. Please address these issues.
| docker tag public.ecr.aws/q9t5s3a7/vllm-release-repo:${BUILDKITE_COMMIT}-x86_64-cu130 vllm/vllm-openai:x86_64-cu130 | ||
| docker tag vllm/vllm-openai:x86_64-cu130 vllm/vllm-openai:latest-x86_64-cu130 | ||
| docker tag vllm/vllm-openai:x86_64-cu130 vllm/vllm-openai:v${RELEASE_VERSION}-x86_64-cu130 | ||
| docker push vllm/vllm-openai:latest-x86_64 |
There was a problem hiding this comment.
There seems to be a copy-paste error here. You are pushing vllm/vllm-openai:latest-x86_64, which corresponds to the non-cu130 image. In this block for cu130 images, you should be pushing the latest-x86_64-cu130 tag.
| docker push vllm/vllm-openai:latest-x86_64 | |
| docker push vllm/vllm-openai:latest-x86_64-cu130 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
| docker tag public.ecr.aws/q9t5s3a7/vllm-release-repo:${BUILDKITE_COMMIT}-x86_64-cu130 vllm/vllm-openai:x86_64-cu130 | ||
| docker tag vllm/vllm-openai:x86_64-cu130 vllm/vllm-openai:latest-x86_64-cu130 | ||
| docker tag vllm/vllm-openai:x86_64-cu130 vllm/vllm-openai:v${RELEASE_VERSION}-x86_64-cu130 | ||
| docker push vllm/vllm-openai:latest-x86_64 |
There was a problem hiding this comment.
Wrong Docker tag pushed for CUDA 13.0 x86_64 image
High Severity
The docker push command on line 49 pushes vllm/vllm-openai:latest-x86_64 instead of vllm/vllm-openai:latest-x86_64-cu130. This is in the CUDA 13.0 x86_64 section where the preceding lines correctly tag and prepare latest-x86_64-cu130, but then the push command incorrectly references the non-cu130 tag. Following these release instructions would fail to push the correct cu130 latest image.
| docker push vllm/vllm-openai:latest-rocm | ||
| docker push vllm/vllm-openai:v${RELEASE_VERSION}-rocm | ||
| docker tag public.ecr.aws/q9t5s3a7/vllm-release-repo:${BUILDKITE_COMMIT}-aarch64-cu130 vllm/vllm-openai:aarch64-cu130 | ||
| docker tag vllm/vllm-openai:aarch64 vllm/vllm-openai:latest-aarch64 |
There was a problem hiding this comment.
Wrong Docker tag command for CUDA 13.0 aarch64 image
High Severity
The docker tag command on line 59 uses vllm/vllm-openai:aarch64 as source and vllm/vllm-openai:latest-aarch64 as destination, but this is in the CUDA 13.0 aarch64 section. Both should include the -cu130 suffix: source should be aarch64-cu130 and destination should be latest-aarch64-cu130. This would result in tagging the wrong image and the subsequent push of latest-aarch64-cu130 would push the wrong content.
| docker tag public.ecr.aws/q9t5s3a7/vllm-release-repo:${BUILDKITE_COMMIT}-rocm vllm/vllm-openai-rocm:${BUILDKITE_COMMIT}-rocm | ||
| docker tag vllm/vllm-openai-rocm:${BUILDKITE_COMMIT}-rocm vllm/vllm-openai-rocm:latest-rocm | ||
| docker tag vllm/vllm-openai-rocm:${BUILDKITE_COMMIT}-rocm vllm/vllm-openai-rocm:v${RELEASE_VERSION}-rocm | ||
| docker push vllm/vllm-openai-rocm:latest-rocm |
There was a problem hiding this comment.
Thanks for updating this. I will move this to another annotate-rocm-release in my upcoming PR, so that all ROCm docker and whls info are annotated together.
| docker tag public.ecr.aws/q9t5s3a7/vllm-release-repo:${BUILDKITE_COMMIT}-rocm vllm/vllm-openai-rocm:${BUILDKITE_COMMIT}-rocm | ||
| docker tag vllm/vllm-openai-rocm:${BUILDKITE_COMMIT}-rocm vllm/vllm-openai-rocm:latest-rocm | ||
| docker tag vllm/vllm-openai-rocm:${BUILDKITE_COMMIT}-rocm vllm/vllm-openai-rocm:v${RELEASE_VERSION}-rocm | ||
| docker push vllm/vllm-openai-rocm:latest-rocm |
There was a problem hiding this comment.
@khluu since we are using a separate repo, the tag should be latest instead of latest-rocm
docker tag public.ecr.aws/q9t5s3a7/vllm-release-repo:${BUILDKITE_COMMIT}-rocm vllm/vllm-openai-rocm:${BUILDKITE_COMMIT}
docker tag vllm/vllm-openai-rocm:${BUILDKITE_COMMIT} vllm/vllm-openai-rocm:latest
docker tag vllm/vllm-openai-rocm:${BUILDKITE_COMMIT} vllm/vllm-openai-rocm:v${RELEASE_VERSION}
docker push vllm/vllm-openai-rocm:latest
docker push vllm/vllm-openai-rocm:v${RELEASE_VERSION}
There was a problem hiding this comment.
fixed! thanks for the review
|
Goal is to automate all of the docker push commands... We probably need a different tier of machine only for release though. I'm still paranoid about putting too much permission on postmerge machines. |
There was a problem hiding this comment.
Maybe just remove this file?
There was a problem hiding this comment.
oh I thought it would be shown here as a file name change, but I guess it's this way because there are diff changes
| @@ -1,108 +0,0 @@ | |||
| #!/usr/bin/env bash | |||
|
|
|||
| set -ex | |||
There was a problem hiding this comment.
@khluu Is the logic for uploading nightly wheels implemented in some other files?
There was a problem hiding this comment.
resolved! I realized I deleted the wrong file :( they are named very similar
|
This pull request has merge conflicts that must be resolved before it can be |
…oject#33129) Signed-off-by: khluu <khluu000@gmail.com>
Signed-off-by: khluu <khluu000@gmail.com> (cherry picked from commit 2284461)
…oject#33129) Signed-off-by: khluu <khluu000@gmail.com> Signed-off-by: Pai <416932041@qq.com>
…oject#33129) Signed-off-by: khluu <khluu000@gmail.com>
twine upload <args>instead oftwine <args> upload. The latter caused errors.vllm-openai-rocmrepo instead ofvllm-openai:rocmtag