Skip to content
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

[CI] Adding CPU docker pipeline #11261

Merged
merged 4 commits into from
Dec 19, 2024

Conversation

zhouyuan
Copy link
Contributor

@zhouyuan zhouyuan commented Dec 17, 2024

This patch adds the pipeline for CPU docker image. On each release(tag) it will build push the image to vllm/vllm-cpu
Fixes: #4771

Copy link

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

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 do one of these:

  • Add ready label to the PR
  • Enable auto-merge.

🚀

@mergify mergify bot added the ci/build label Dec 17, 2024
@mgoin mgoin requested a review from simon-mo December 17, 2024 16:13
This patch adds the pipeline for CPU docker image

Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@zhouyuan zhouyuan force-pushed the wip_cpu_docker_pipeline branch from 8744af2 to 2fad51d Compare December 18, 2024 00:51
@zhouyuan zhouyuan marked this pull request as ready for review December 18, 2024 00:53
@simon-mo simon-mo requested a review from khluu December 18, 2024 01:00
depends_on: ~
if: build.tag != null
agents:
queue: cpu_queue_postmerge
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@khluu not sure if this cpu-queue-postmerge queue is right to build the publish docker image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use this .. but it should push to an ECR instead of Dockerhub. If the cadence is per release version, we can pull the release image manually from ECR then retag and push to Dockerhub cause it's safer that way (putting Dockerhub token on the CI machine has some risk)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhouyuan Can you add this command:
aws ecr-public get-login-password --region us-east-1 | docker login --username AWS --password-stdin public.ecr.aws/q9t5s3a7
then we can tag it like public.ecr.aws/q9t5s3a7/vllm-cpu-release-repo:$BUILDKITE_TAG then push

depends_on: ~
if: build.tag != null
agents:
queue: cpu_queue_postmerge
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use this .. but it should push to an ECR instead of Dockerhub. If the cadence is per release version, we can pull the release image manually from ECR then retag and push to Dockerhub cause it's safer that way (putting Dockerhub token on the CI machine has some risk)

depends_on: ~
if: build.tag != null
agents:
queue: cpu_queue_postmerge
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhouyuan Can you add this command:
aws ecr-public get-login-password --region us-east-1 | docker login --username AWS --password-stdin public.ecr.aws/q9t5s3a7
then we can tag it like public.ecr.aws/q9t5s3a7/vllm-cpu-release-repo:$BUILDKITE_TAG then push

@@ -55,3 +55,18 @@ steps:
password-env: DOCKERHUB_TOKEN
env:
DOCKER_BUILDKIT: "1"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a block step here, since it only needs to run on-demand (every release)? The syntax is similar to the Build release image step.

@khluu
Copy link
Collaborator

khluu commented Dec 18, 2024

I think the PR is ready but the thing is I'm not sure if BUILDKITE_TAG works here. For example, @simon-mo tagged a commit earlier today to release and this build was triggered: https://buildkite.com/vllm/release/builds/2257 but it didn't register any tag (you can see in the Environment tab for each step).
Maybe we can just remove the if: build.tag ... line and have it use $RELEASE_VERSION as tag instead of $BUILDKITE_TAG. We can input $RELEASE_VERSION manually when triggering a new build on Release pipeline.

Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@zhouyuan
Copy link
Contributor Author

@khluu
Hi Kevin, I have updated the code to use $RELEASE_VERSION. Could you please help to take a look? The CI failure seems not related with this patch(?)

I think the PR is ready but the thing is I'm not sure if BUILDKITE_TAG works here. For example, @simon-mo tagged a commit earlier today to release and this build was triggered: https://buildkite.com/vllm/release/builds/2257 but it didn't register any tag (you can see in the Environment tab for each step). Maybe we can just remove the if: build.tag ... line and have it use $RELEASE_VERSION as tag instead of $BUILDKITE_TAG. We can input $RELEASE_VERSION manually when triggering a new build on Release pipeline.

@khluu
Copy link
Collaborator

khluu commented Dec 19, 2024

@khluu Hi Kevin, I have updated the code to use $RELEASE_VERSION. Could you please help to take a look? The CI failure seems not related with this patch(?)

I think the PR is ready but the thing is I'm not sure if BUILDKITE_TAG works here. For example, @simon-mo tagged a commit earlier today to release and this build was triggered: https://buildkite.com/vllm/release/builds/2257 but it didn't register any tag (you can see in the Environment tab for each step). Maybe we can just remove the if: build.tag ... line and have it use $RELEASE_VERSION as tag instead of $BUILDKITE_TAG. We can input $RELEASE_VERSION manually when triggering a new build on Release pipeline.

Yes let me test it on the Release pipeline then will merge!

@khluu khluu added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 19, 2024
@khluu khluu enabled auto-merge (squash) December 19, 2024 11:00
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) December 19, 2024 14:52
@DarkLight1337
Copy link
Member

@khluu did you forget to approve this?

@khluu
Copy link
Collaborator

khluu commented Dec 19, 2024

@khluu did you forget to approve this?

oops yes

@DarkLight1337 DarkLight1337 merged commit a985f7a into vllm-project:main Dec 19, 2024
44 of 46 checks passed
@zhouyuan
Copy link
Contributor Author

Hi @khluu @DarkLight1337 Thank you for the review and quick merge!

Thanks,
-yuan

lucas-tucker pushed a commit to lucas-tucker/vllm-lucas-tucker that referenced this pull request Dec 21, 2024
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
Co-authored-by: Kevin H. Luu <kevin@anyscale.com>
Signed-off-by: lucast2021 <lucast2021@headroyce.org>
BKitor pushed a commit to BKitor/vllm that referenced this pull request Dec 30, 2024
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
Co-authored-by: Kevin H. Luu <kevin@anyscale.com>
joennlae pushed a commit to 44ai-labs/vllm that referenced this pull request Jan 19, 2025
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
Co-authored-by: Kevin H. Luu <kevin@anyscale.com>
joennlae pushed a commit to 44ai-labs/vllm that referenced this pull request Jan 19, 2025
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
Co-authored-by: Kevin H. Luu <kevin@anyscale.com>
abmfy pushed a commit to abmfy/vllm-flashinfer that referenced this pull request Jan 24, 2025
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
Co-authored-by: Kevin H. Luu <kevin@anyscale.com>
Signed-off-by: Bowen Wang <abmfy@icloud.com>
abmfy pushed a commit to abmfy/vllm-flashinfer that referenced this pull request Jan 24, 2025
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
Co-authored-by: Kevin H. Luu <kevin@anyscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/build ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Host CPU Docker image on Docker Hub
3 participants