Skip to content

Commit 65bdbc4

Browse files
localai-botmudler
andauthored
fix(http): make /readyz reflect startup readiness, plus gitignore and coverage-ratchet fixes (#10989)
* fix(http): make /readyz reflect startup readiness instead of always 200 /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] * chore(gitignore): anchor the mock-backend pattern so its source dir is 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] * chore(coverage): raise the coverage ratchet from 48.5% to 54.2% 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] --------- Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
1 parent 0d9d07d commit 65bdbc4

8 files changed

Lines changed: 250 additions & 10 deletions

File tree

.gitignore

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,12 @@ models/*
4141
test-models/
4242
test-dir/
4343
tests/e2e-aio/backends
44-
mock-backend
44+
# The mock backend binary built by `make build-mock-backend`. Anchored to its
45+
# full path: a bare `mock-backend` also matched the *directory* holding the
46+
# source, so git would not descend into it and adding a file there needed -f.
47+
# tests/e2e/mock-backend/.gitignore covers the same binary; kept here too so
48+
# the artifact stays ignored if that scoped file is ever removed.
49+
/tests/e2e/mock-backend/mock-backend
4550

4651
release/
4752

core/application/application.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,30 @@ type Application struct {
9494
// is set; otherwise initialised in start() after galleryService.
9595
localAIAssistant *mcpTools.LocalAIAssistantHolder
9696

97+
// startupComplete flips to true once New() has finished its whole startup
98+
// sequence. It backs the /readyz probe.
99+
//
100+
// The expensive step is the model preload: since #10949 it materializes
101+
// HuggingFace artifacts for managed backends, which is tens of GB for a
102+
// large model (31 GB observed on a live cluster). Tracking it explicitly
103+
// means readiness reports lifecycle state instead of "the handler was
104+
// reachable", so a replica that is still starting can be kept out of a
105+
// load balancer's rotation.
106+
startupComplete atomic.Bool
107+
97108
shutdownOnce sync.Once
98109
}
99110

111+
// Ready reports whether the application has finished starting up and can serve
112+
// traffic. It backs the /readyz probe and is safe to call from any goroutine,
113+
// including while startup is still running.
114+
func (a *Application) Ready() bool { return a.startupComplete.Load() }
115+
116+
// markStartupComplete flips the application to ready. Called once, at the very
117+
// end of New(), so every startup step — model preload included — has finished
118+
// before the process advertises itself as able to serve.
119+
func (a *Application) markStartupComplete() { a.startupComplete.Store(true) }
120+
100121
func newApplication(appConfig *config.ApplicationConfig) *Application {
101122
ml := model.NewModelLoader(appConfig.SystemState)
102123

core/application/readiness_test.go

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package application
2+
3+
import (
4+
"context"
5+
"os"
6+
"path/filepath"
7+
8+
. "github.com/onsi/ginkgo/v2"
9+
. "github.com/onsi/gomega"
10+
11+
"github.com/mudler/LocalAI/core/config"
12+
"github.com/mudler/LocalAI/pkg/modelartifacts"
13+
"github.com/mudler/LocalAI/pkg/system"
14+
)
15+
16+
// blockingMaterializer parks inside Ensure until release is closed, so a spec
17+
// can hold the startup preload open and observe readiness while it is still
18+
// running. This mirrors how a real managed-artifact preload behaves: since
19+
// #10949 it downloads a HuggingFace snapshot, which for a large model is tens
20+
// of GB and can take a long time.
21+
type blockingMaterializer struct {
22+
seen chan struct{}
23+
release chan struct{}
24+
}
25+
26+
func (b *blockingMaterializer) Ensure(ctx context.Context, _ string, spec modelartifacts.Spec) (modelartifacts.Result, error) {
27+
select {
28+
case b.seen <- struct{}{}:
29+
default:
30+
}
31+
select {
32+
case <-b.release:
33+
case <-ctx.Done():
34+
return modelartifacts.Result{}, ctx.Err()
35+
}
36+
spec.Resolved = &modelartifacts.Resolved{
37+
Endpoint: "https://huggingface.co",
38+
Revision: "0123456789abcdef0123456789abcdef01234567",
39+
CacheKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
40+
}
41+
return modelartifacts.Result{Spec: spec}, nil
42+
}
43+
44+
var _ = Describe("application readiness", func() {
45+
var modelsPath string
46+
var state *system.SystemState
47+
48+
BeforeEach(func() {
49+
modelsPath = GinkgoT().TempDir()
50+
var err error
51+
state, err = system.GetSystemState(
52+
system.WithModelPath(modelsPath),
53+
system.WithBackendPath(GinkgoT().TempDir()),
54+
)
55+
Expect(err).NotTo(HaveOccurred())
56+
})
57+
58+
It("is not ready before the startup sequence has run", func() {
59+
app := newApplication(&config.ApplicationConfig{
60+
Context: context.Background(),
61+
SystemState: state,
62+
})
63+
64+
// A constructed-but-unstarted application must never advertise itself
65+
// as able to serve traffic: /readyz reads this, and a load balancer
66+
// that believes it will route requests to a process that cannot answer.
67+
Expect(app.Ready()).To(BeFalse())
68+
})
69+
70+
It("stays not-ready while the startup model preload is still running", func() {
71+
blocker := &blockingMaterializer{
72+
seen: make(chan struct{}, 1),
73+
release: make(chan struct{}),
74+
}
75+
app := newApplication(&config.ApplicationConfig{
76+
Context: context.Background(),
77+
SystemState: state,
78+
ModelArtifactMaterializer: blocker,
79+
})
80+
81+
Expect(os.WriteFile(filepath.Join(modelsPath, "managed.yaml"), []byte(`
82+
name: managed
83+
backend: transformers
84+
artifacts:
85+
- source: {type: huggingface, repo: owner/repo}
86+
parameters: {model: owner/repo}
87+
`), 0644)).To(Succeed())
88+
Expect(app.ModelConfigLoader().LoadModelConfigsFromPath(modelsPath)).To(Succeed())
89+
90+
preloadDone := make(chan error, 1)
91+
go func() {
92+
preloadDone <- app.ModelConfigLoader().PreloadWithContext(context.Background(), modelsPath)
93+
}()
94+
95+
// Preload is now parked inside artifact materialization — exactly the
96+
// window in which a Kubernetes Service would otherwise send this
97+
// replica half the cluster's traffic.
98+
Eventually(blocker.seen).Should(Receive())
99+
Consistently(app.Ready).Should(BeFalse())
100+
101+
close(blocker.release)
102+
Expect(<-preloadDone).NotTo(HaveOccurred())
103+
104+
// Preload finished, but the rest of the startup sequence has not, so
105+
// the application is still not ready.
106+
Expect(app.Ready()).To(BeFalse())
107+
})
108+
109+
It("becomes ready once the startup sequence completes", func() {
110+
ctx, cancel := context.WithCancel(context.Background())
111+
DeferCleanup(cancel)
112+
113+
app, err := New(
114+
config.WithContext(ctx),
115+
config.WithSystemState(state),
116+
)
117+
Expect(err).NotTo(HaveOccurred())
118+
DeferCleanup(func() { _ = app.Shutdown() })
119+
120+
Expect(app.Ready()).To(BeTrue())
121+
})
122+
})

core/application/startup.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,12 @@ func New(opts ...config.AppOption) (*Application, error) {
496496
// Watch the configuration directory
497497
startWatcher(options)
498498

499+
// Everything that must happen before this process can serve a request has
500+
// happened. Flip readiness last, and only on the success path — the early
501+
// `return nil, err` exits above abort startup, and an application that
502+
// never finished starting must never report itself ready.
503+
application.markStartupComplete()
504+
499505
xlog.Info("core/startup process completed!")
500506
return application, nil
501507
}

core/http/app.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ func API(application *application.Application) (*echo.Echo, error) {
263263
}
264264

265265
// Health Checks should always be exempt from auth, so register these first
266-
routes.HealthRoutes(e)
266+
routes.HealthRoutes(e, application.Ready)
267267

268268
// Build auth middleware: use the new auth.Middleware when auth is enabled or
269269
// as a unified replacement for the legacy key-auth middleware.

core/http/routes/health.go

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,40 @@
11
package routes
22

33
import (
4+
"net/http"
5+
46
"github.com/labstack/echo/v4"
57
)
68

7-
func HealthRoutes(app *echo.Echo) {
8-
// Service health checks
9-
ok := func(c echo.Context) error {
10-
return c.NoContent(200)
11-
}
9+
// HealthRoutes registers the liveness (/healthz) and readiness (/readyz)
10+
// probes.
11+
//
12+
// ready reports whether the process has finished starting up. It is consulted
13+
// per request rather than sampled once at registration time, because readiness
14+
// is a lifecycle property that changes after the router has been built. A nil
15+
// ready fails open: an embedder that does not wire readiness keeps the
16+
// historical always-200 behaviour instead of being stuck out of rotation
17+
// forever.
18+
func HealthRoutes(app *echo.Echo, ready func() bool) {
19+
// Liveness: "is this process alive?". Deliberately independent of
20+
// readiness — a long startup preload must not read as a hung process, or
21+
// an orchestrator restarts the pod mid-download and the preload can never
22+
// finish.
23+
app.GET("/healthz", func(c echo.Context) error {
24+
return c.NoContent(http.StatusOK)
25+
})
1226

13-
app.GET("/healthz", ok)
14-
app.GET("/readyz", ok)
27+
// Readiness: "should this replica receive traffic?". Answering 200 while
28+
// the process is still starting up makes a Kubernetes Service add the pod
29+
// to its endpoints early, and traffic round-robins onto a replica that
30+
// cannot serve it.
31+
app.GET("/readyz", func(c echo.Context) error {
32+
if ready != nil && !ready() {
33+
return c.JSON(http.StatusServiceUnavailable, map[string]string{
34+
"status": "starting",
35+
"reason": "startup preload in progress",
36+
})
37+
}
38+
return c.NoContent(http.StatusOK)
39+
})
1540
}

core/http/routes/health_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package routes_test
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"sync/atomic"
7+
8+
"github.com/labstack/echo/v4"
9+
. "github.com/onsi/ginkgo/v2"
10+
. "github.com/onsi/gomega"
11+
12+
"github.com/mudler/LocalAI/core/http/routes"
13+
)
14+
15+
var _ = Describe("Health and readiness probes", func() {
16+
var e *echo.Echo
17+
var ready atomic.Bool
18+
19+
get := func(path string) *httptest.ResponseRecorder {
20+
rec := httptest.NewRecorder()
21+
e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
22+
return rec
23+
}
24+
25+
BeforeEach(func() {
26+
e = echo.New()
27+
ready.Store(false)
28+
routes.HealthRoutes(e, ready.Load)
29+
})
30+
31+
It("reports /readyz as unavailable while startup is in progress", func() {
32+
Expect(get("/readyz").Code).To(Equal(http.StatusServiceUnavailable))
33+
})
34+
35+
It("reports /readyz as available once startup completes", func() {
36+
ready.Store(true)
37+
Expect(get("/readyz").Code).To(Equal(http.StatusOK))
38+
})
39+
40+
It("keeps /healthz green during startup", func() {
41+
// Liveness and readiness answer different questions. Failing liveness
42+
// during a long preload would make Kubernetes restart the pod and the
43+
// preload would never finish.
44+
Expect(get("/healthz").Code).To(Equal(http.StatusOK))
45+
46+
ready.Store(true)
47+
Expect(get("/healthz").Code).To(Equal(http.StatusOK))
48+
})
49+
50+
It("treats a nil readiness source as ready", func() {
51+
// Fail open: an embedder that does not wire readiness keeps the
52+
// historical always-200 behaviour rather than being permanently
53+
// out of rotation.
54+
plain := echo.New()
55+
routes.HealthRoutes(plain, nil)
56+
57+
rec := httptest.NewRecorder()
58+
plain.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/readyz", nil))
59+
Expect(rec.Code).To(Equal(http.StatusOK))
60+
})
61+
})

coverage-baseline.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
48.5
1+
54.2

0 commit comments

Comments
 (0)