Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Core] Enable use docker image w/ non-default entrypoint as runtime env #3867

Merged
merged 5 commits into from
Aug 27, 2024

Conversation

cblmemo
Copy link
Collaborator

@cblmemo cblmemo commented Aug 23, 2024

Previously, if the docker image had an entry point that was not default (/bin/bash), it would fail when running as runtime env on real clouds (in k8s it works since we have special handling for this), e.g. the vllm official docker uses python3 -m vllm.entrypoints.openai.api_server as entry point, as ref here: https://github.com/vllm-project/vllm/blob/9db93de20ca282feb4dfaabbc56032c9312bde7b/Dockerfile#L222

This PR fixed it by overriding the entry point to /bin/bash.

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below)
    • The following YAML successfully launch a vllm endpoint
resources:
  cloud: aws
  image_id: docker:vllm/vllm-openai:latest
  accelerators: T4
run: |
  conda deactivate
  python3 -c "import huggingface_hub; huggingface_hub.login('$HF_TOKEN')"
  python3 -m vllm.entrypoints.openai.api_server --model meta-llama/Llama-2-7b-chat-hf --tokenizer hf-internal-testing/llama-tokenizer --max-model-len 64
  • All smoke tests: pytest tests/test_smoke.py
  • Relevant individual smoke tests:
    • pytest tests/test_smoke.py::test_docker_storage_mounts
    • pytest tests/test_smoke.py::test_job_queue_with_docker
    • pytest tests/test_smoke.py::test_docker_preinstalled_package
  • Backward compatibility tests: conda deactivate; bash -i tests/backward_compatibility_tests.sh

@cblmemo
Copy link
Collaborator Author

cblmemo commented Aug 26, 2024

The smoke test passed w/ all image id (including axolotl). PTAL @Michaelvll 🫡

Copy link
Collaborator

@Michaelvll Michaelvll left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for fixing this @cblmemo! This is very helpful for many of our users.

Could we add a note/hint in our doc as well to say that we will ignore entrypoint when a docker image is specified as a runtime image_id?

@cblmemo
Copy link
Collaborator Author

cblmemo commented Aug 26, 2024

Awesome! Thanks for fixing this @cblmemo! This is very helpful for many of our users.

Could we add a note/hint in our doc as well to say that we will ignore entrypoint when a docker image is specified as a runtime image_id?

Good point! Added. Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants