Skip to content

override healthcheck in dockerfile#946

Open
areebahmeddd wants to merge 1 commit into
docker:mainfrom
areebahmeddd:fix/docker-healthcheck
Open

override healthcheck in dockerfile#946
areebahmeddd wants to merge 1 commit into
docker:mainfrom
areebahmeddd:fix/docker-healthcheck

Conversation

@areebahmeddd
Copy link
Copy Markdown
Contributor

inherit the correct port and /engines/status endpoint for all image variants

fixes #942

Signed-off-by: Areeb Ahmed <areebahmed0709@gmail.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a 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 exposes the model runner port and adds a healthcheck command to the Dockerfile. However, a critical issue was identified where the healthcheck will fail in the final-llamacpp image variant because curl is not installed, which would mark the container as permanently unhealthy. To resolve this, curl must be installed in the llamacpp stage.

Comment thread Dockerfile
Comment on lines +86 to +87
HEALTHCHECK --interval=30s --timeout=10s --start-period=10s --retries=3 \
CMD curl -f "http://localhost:${MODEL_RUNNER_PORT}/engines/status" || exit 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

The final-llamacpp image variant does not have curl installed, which causes this healthcheck to fail with a command-not-found error, marking the container as permanently unhealthy. Since the vllm and sglang stages explicitly install curl, it is not present in the base llamacpp image. To resolve this, curl must be installed in the llamacpp stage (for example, by adding it to scripts/apt-install.sh).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not valid. curl is present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docker Healthcheck failing

1 participant