fix(http): make /readyz reflect startup readiness, plus gitignore and coverage-ratchet fixes#10989
Merged
Conversation
/readyz was registered as a static handler returning 200 unconditionally, so it carried no information: it was green whenever it could be reached at all. Readiness could not distinguish "serving" from "still starting", and any future change that started the HTTP listener earlier would silently turn the probe into a lie. Track startup completion on the Application (atomic flag, flipped at the very end of New() on the success path only) and have the readiness handler consult it per request, returning 503 with a small JSON body while startup is in progress. A nil readiness source fails open so embedders keep the historical behaviour. /healthz is deliberately left readiness-independent. Liveness and readiness answer different questions, and failing liveness during a long preload makes an orchestrator restart the pod mid-download so the preload never finishes. This matters because since #10949 the startup preload materializes HuggingFace artifacts for managed backends: tens of GB for a large model (31 GB observed on a live cluster). Both probes stay in quietPaths and stay exempt from auth. Note the listener is still started only after New() returns, so today the not-ready state is not observable over HTTP. Moving the listener earlier is a separate, deliberate decision and is not made here. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-8 [Claude Code]
…s traversable The bare `mock-backend` pattern matched the *directory* tests/e2e/mock-backend/, not just the binary built into it. Git will not descend into an ignored directory even for tracked files, so `git add tests/e2e/mock-backend/main.go` required -f. This was hit while working on #10970. Anchor it to the artifact's full path. The built binary stays ignored (it is also covered by tests/e2e/mock-backend/.gitignore) while the source directory becomes traversable again. Verified with `git check-ignore -v`: a new source file under tests/e2e/mock-backend/ is no longer ignored, and the binary produced by `make build-mock-backend` still is. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-8 [Claude Code]
The committed baseline had drifted well below reality: it still read 48.5% while a full instrumented run measures 54.2%. A stale-low baseline makes the gate meaningless — coverage could regress by more than 5 percentage points and still pass. Raising a ratchet is a deliberate act, not something to fold into an unrelated fix, so it gets its own commit. The headroom was earned by tests landed in #10946, #10947, #10948, #10949, #10956, #10967, #10968, #10970 and #10975. Measured with `make test-coverage` on this branch (the same instrumented run `make test-coverage-baseline` uses: ginkgo over ./pkg and ./core plus the in-process tests/e2e suite, --covermode=atomic, --coverpkg over core/... and pkg/..., generated protobuf excluded). The run completed with exit 0 and zero spec failures; the total was then written with the exact command the test-coverage-baseline target uses: go tool cover -func=coverage/coverage.out \ | awk '/^total:/{gsub(/%/,"",$NF); print $NF}' > coverage-baseline.txt Verified afterwards with scripts/coverage-check.sh, which reports OK. Note the measured figure includes the readiness specs added earlier on this branch, so it is a demonstrated floor rather than an estimate. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-8 [Claude Code]
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.
Three unrelated, independently reviewable fixes.
1.
/readyzreflects readiness (3f5339e93)/readyzwas a static handler returning 200 unconditionally — green whenever it was reachable at all. It could not distinguish "serving" from "still starting", and any future change that started the listener earlier would silently make it lie.Startup completion is now tracked on the
Application(atomic.Bool, flipped at the end ofNew()on the success path only — the earlyreturn nil, errexits must never mark ready) and the handler consults it per request, since readiness changes after the router is built. Not-ready returns 503 with{"status":"starting"}. A nilreadyfunc fails open, so embedders keep the old behaviour.HealthRoutestakes afunc() boolrather than the*Application, to keepcore/http/routesfree of a dependency oncore/application./healthzis unchanged and deliberately readiness-independent: failing liveness during a long preload would make an orchestrator restart the pod mid-download, so the preload never finishes.Motivation: since #10949 the startup preload materializes HuggingFace artifacts for managed backends — 31 GB observed on a live cluster.
Scope limit, stated plainly:
core/cli/run.gocallsapplication.New()(the entire preload) and only thenappHTTP.Start(). The listener binds after preload, so the not-ready state is not yet observable over HTTP — probes still get connection-refused. This PR buys a truthful probe, a tested contract, and no silent lie if the listener ever moves. It does not by itself fix traffic reaching a preloading replica.2.
.gitignoreno longer blocks a tracked directory (369ec41d9)The bare
mock-backendpattern matched the directorytests/e2e/mock-backend/, so git would not descend into it andgit addon a tracked file there required-f(hit during #10970). Anchored to the artifact's full path; verified both directions withgit check-ignore -v.Note
tests/e2e/mock-backend/.gitignorewas always doing the real work, so the root entry is arguably redundant — it is kept anchored with a comment rather than deleted, but deleting it would be equally correct.3. Coverage ratchet raised to 54.2% (
7f33d93fd)The baseline read 48.5% while a full instrumented run measures 54.2% — coverage could have regressed more than 5pp and still passed the gate.
Measured, not estimated:
make test-coverage(the same runtest-coverage-baselinedepends on), exit 0, zero spec failures, total computed with the target's exact command:Headroom earned by #10946, #10947, #10948, #10949, #10956, #10967, #10968, #10970, #10975. Its own commit, since raising a ratchet is a deliberate act rather than a side effect.
Follow-up: should the listener start earlier?
Recommended, but deliberately not in this PR. A refused connection is indistinguishable from a crashed pod, so answering "not ready" is strictly more useful to an operator. Three blockers a naive move would hit:
LoadModelConfigsFromPathruns afterInstallModels, so/v1/modelswould return empty and chat would 404 for models that are about to exist — clients get confidently wrong 200s instead of connection-refused. Mitigation: bind the full router early behind a gate middleware that 503s every path except/healthzand/readyzuntil ready.waitForServerReadywould fire early.core/cli/run.gopolls for connectivity then callsStartAgentPool(), which needs the embeddings API. It must poll/readyzfor 200, not mere connectivity.DockerfileHEALTHCHECK (--interval=1m --retries=10, a 10-minute grace) targets/readyz. Today a preloading container is "starting"; after the move it would go unhealthy at 10 minutes, and a 31 GB preload blows well past that. Needs--start-periodfirst, or the change trades one operational bug for another. See also Docker HEALTHCHECK probes a frontend endpoint, so every worker container is permanently unhealthy #10987.Verification
go build ./core/... ./pkg/...— cleango test ./core/http/... ./core/application/...— allokmake lint— 0 issuesmake test-coverage— exit 0, zero failures, total 54.2%TDD: both readiness specs shown failing on behaviour before implementation (a compiling skeleton with
Ready()stubbedreturn truelanded first), so neither failed merely on a missing symbol. The preload-window spec parks a fake materializer mid-preload via channels, so it genuinely observes the not-ready window rather than inferring it.Repo sweep including
tests/e2e/: nothing asserts an unconditional 200 from/readyz. Notecore/services/nodes/file_transfer_server.gohas a separate worker/readyz(whatdocker-compose.distributed.yamlprobes on 50050) — untouched.🤖 Generated with Claude Code