fix: Supervise both children instead of gating startup on the model - #15
Merged
Conversation
entrypoint.sh polled llama-server's health for 120 seconds before starting the proxy, and on timeout called a cleanup that ran `jobs -p | xargs kill` followed by a bare `wait`. `jobs` lists nothing in a non-interactive shell, so nothing was killed and the wait blocked forever on a live child. The script never reached its exit, leaving a container that stayed up with a healthy engine on 127.0.0.1:8080 and nothing listening on 8090. The same function was on the INT/TERM trap, so docker stop hung too. The gate is gone. Both children start in the background with recorded PIDs and the script exits when either dies, propagating its status. The proxy already probes upstream on /health and answers 503 when it is unreachable, so readiness is reported rather than gated, and llama-server dying after startup is now noticed instead of silently breaking every request. HEALTHCHECK start-period rises to 180s, matching the readiness bounds the Makefile, CI and integration helpers already use, since the healthcheck is now the signal for a model that never finishes loading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The proxy section said existing wait loops work unchanged, which holds only for loops polling /health for a 200. The port opens before the upstream is reachable there too, so a TCP liveness check has the same gap the bundled tags now document. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
harshaneel
force-pushed
the
hg/entrypoint-no-startup-gate
branch
from
August 5, 2026 00:57
6a7547c to
15dbe44
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #13.
Summary
entrypoint.shno longer gates startup on the model. It polledllama-server's health for 120 seconds before starting the proxy, and on timeout called a cleanup that ranjobs -p | xargs killfollowed by a barewait.jobslists nothing in a non-interactive shell, so nothing was killed and the wait blocked forever on a live child. The script never reached itsexit, leaving a container that stayed up with a healthy engine on127.0.0.1:8080and nothing listening on 8090. The same function was on theINT/TERMtrap, sodocker stophung too.exec localaikhanded off and stopped watching, sollama-servercrashing after startup silently broke every request instead of stopping the container./healthalready probes upstream and answers 503 when it is unreachable, so the proxy answers immediately and tells you the truth while the model loads.HEALTHCHECK --start-periodgoes 60s to 180s, matching the readiness bounds theMakefile, CI and the integration helpers already use. With the shell-level bound gone, the healthcheck is now the signal for a model that never finishes loading, so it should not be shorter than the waits everything else uses.Behaviour changes worth knowing about
These images have 1,367 pulls, so the changes are called out rather than buried.
nc -z, or Composedepends_onwithoutcondition: service_healthywill now let requests through early, and those get a 502 from the proxy. Wait forGET /healthto return 200 instead. This is documented in Quick start, not only in Limitations.Test Plan
internal/entrypoint/drive the real script with stub children: the proxy starts without waiting for the model, a crashing engine propagates its status, a child exiting 0 still escalates to non-zero, a crashing proxy stops the engine, andSIGTERMstops both and exits 0./bin/dashwhen present, since that is what the container runs, rather than silently testing only bash locally.internal/somake test-unitruns them. Inintegration/they would only run aftermake docker-upsucceeded, which is the step a startup regression breaks first.make lintclean, all unit tests pass./healthreturns 503{"status":"unhealthy"}immediately instead of refusing connections, then 200 once loaded, Docker reportshealthy, and a real chat completion succeeds.docker inspectconfirms Docker flips to healthy on the first passing check, so the longer start-period does not delay readiness.docker stopreturns in 0s with exit code 0.LLAMA_SERVER_BIN=/bin/false) exits the container with status 1 and logslocalaik: llama-server exited with status 1, stopping the container.Review notes
Three review passes ran. Two findings changed the design:
set -u,kill "${A:-}" "${B:-}"errors on the empty argument and|| trueswallows it, so the second child would never be signaled. It iterates per PID instead.Also addressed: the
SIGTERMtest raced the trap-installation window, the tests discarded the script's output on failure, and the surviving child is now waited for so its final log lines reachdocker logs.Not done, worth a follow-up: adding
shellchecktomake lint, which would have caught the originaljobs -pbug. It is not installed in this environment, so it could not be verified here.🤖 Generated with Claude Code