From 2bf7de1a430b0e3ee2d2fa54d02fec93563e962b Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Sun, 27 Sep 2026 07:52:54 +0200 Subject: [PATCH] Give two control planes a shared address, and a probe that means something MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last piece of HA, and it is a deployment pattern rather than code: llmgw picks one leader and agents follow it, but client traffic arrives in front of both and llmgw is not in front of anything. Writing the pattern found a bug in the pattern. The first draft said to health-check `/health` and claimed a standby still routes to replicas the leader placed, so sending it traffic was fine. It does not. A standby's reconciler places nothing and it holds no agent streams to adopt from, so it publishes an empty endpoint list — a load balancer following that advice would have sent half the traffic to something that answers every request with 503. So there is now a `/ready` that means "this process has somewhere to send a request": 503 on a standby, 503 on a leader whose replicas have all failed, 200 when there is something to serve. `/health` keeps meaning the process is up, which is a different and less useful question. Both are open, because a load balancer has no key. There is a gap during failover where neither passes — three seconds when the leader stops cleanly, up to the lease TTL when it does not — and the document says so rather than implying otherwise. Traffic cannot go to a control plane that has nothing to route to. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 + deploy/ha/README.md | 87 +++++++++++++++++++++++++++++++ deploy/ha/keepalived.conf.example | 65 +++++++++++++++++++++++ docs/install.md | 12 +++++ internal/router/handlers.go | 24 ++++++++- test/e2e/health_test.go | 68 ++++++++++++++++++++++++ 6 files changed, 257 insertions(+), 1 deletion(-) create mode 100644 deploy/ha/README.md create mode 100644 deploy/ha/keepalived.conf.example create mode 100644 test/e2e/health_test.go diff --git a/README.md b/README.md index 26c1e9b..19c137d 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,8 @@ on a machine that has them. - [docs/reference.md](docs/reference.md) — every configuration key, generated from the structs and checked by a test, so it cannot describe a setting that does not exist or miss one that does +- [deploy/ha/](deploy/ha/) — running two control planes, and the one health + check that has to be right - [docs/node-onboarding.md](docs/node-onboarding.md), [docs/onboarding.md](docs/onboarding.md) — adding machines, and adding people diff --git a/deploy/ha/README.md b/deploy/ha/README.md new file mode 100644 index 0000000..d7c7f5f --- /dev/null +++ b/deploy/ha/README.md @@ -0,0 +1,87 @@ +# Two control planes + +llmgw handles the hard half of this itself: only one control plane places work, +and agents find whichever one is leading. What it does not do is move client +traffic, because that happens in front of both and llmgw is not in front of +anything. + +This directory is the missing half. It is a pattern rather than a product: +substitute your own load balancer if you have one. + +## What llmgw does on its own + +**One leader.** Leadership is a lease in the shared store. A second control +plane starts, sees the lease, and stands by — it places nothing, evicts +nothing and drains nothing. Start one by accident and your cluster is +unharmed. + +**Agents follow.** A standby refuses agent streams, so agents move to the +leader by themselves. Give each agent both addresses: + +```yaml +servers: + - "https://ctl-1.internal:7443" + - "https://ctl-2.internal:7443" +``` + +Measured on a cluster: leadership moved three seconds after the leader was +stopped, every agent had arrived six seconds later, and every replica was +adopted rather than restarted — the engines never stopped serving. + +## What it does not do + +Client traffic. During that same failover the standby served inference +correctly on its own port while the public address returned 502, because +nothing in front of the two had moved. + +So something has to. The options, in the order most people should consider +them: + +**A load balancer you already run.** Health-check `/ready` on each control +plane and send traffic to whichever answers. This is the right answer if you +have one, and it needs nothing else from this directory. + +**A floating address.** `keepalived.conf.example` here moves one IP between +the two machines. Fewer moving parts than a load balancer and no extra hop, +at the cost of one machine carrying all the traffic. + +**DNS.** Works, eventually. A record with a short TTL is a failover measured +in whatever your resolvers actually honour rather than what you set, which on +a bad day is minutes. Fine for a cluster people use interactively, wrong for +one an agent is driving. + +## Check `/ready`, not `/health` + +This is the one thing to get right, and the obvious choice is the wrong one. + +`/health` says the process is up. It says nothing about whether it can serve, +and a standby is exactly that case: alive, answering, and holding no endpoints +at all — its reconciler placed nothing and it has no agent streams to adopt +from. A load balancer checking `/health` would send it half the traffic, and +half the traffic would be a 503. + +`/ready` says this process has somewhere to send a request. It is 503 on a +standby, 503 on a leader whose replicas have all failed, and 200 when there is +something to serve — which is the question a load balancer is actually asking. +Both are open, because a load balancer has no key. + +During a failover neither passes for about the length of the lease handover: +three seconds when the leader is stopped cleanly, up to the lease TTL when it +is not. That gap is real and there is no way to have it both ways — traffic +cannot go to a control plane that has nothing to route to. + +Not the admin port, for a different reason: it serves pprof, and pprof can +dump the process heap, where API keys sit in plaintext. + +## Both control planes need the same store + +Leadership lives in it, and so do keys, budgets and conversations. Two control +planes on two SQLite files are two clusters that happen to share a name. + +```yaml +auth: + store: "@/etc/llmgw/secrets/store-dsn" +``` + +See [docs/install.md](../../docs/install.md) for moving an existing +installation onto PostgreSQL without reissuing every key. diff --git a/deploy/ha/keepalived.conf.example b/deploy/ha/keepalived.conf.example new file mode 100644 index 0000000..c728d40 --- /dev/null +++ b/deploy/ha/keepalived.conf.example @@ -0,0 +1,65 @@ +# A floating address for two control planes. +# +# One IP that lives on whichever machine is up. Clients and the reverse proxy +# in front of llmgw point at the address, not at either host, so a failover is +# invisible to them. +# +# This is not a load balancer: all traffic goes to the holder. That is usually +# the right trade for a control plane, which serves far less than the nodes +# it schedules. +# +# Copy to /etc/keepalived/keepalived.conf on both machines, changing state, +# priority and interface. + +global_defs { + enable_script_security + script_user nobody +} + +# The address moves only while llmgw can actually serve, not merely while the +# machine is powered on. +# +# /ready, not /health. /health is 200 on a standby, which is up, answering and +# holding no endpoints at all — so the address would sit on a host that refuses +# every request. /ready is 200 only when there is somewhere to send one. +# +# Not the admin port either: it serves pprof, which can dump the heap that +# holds every API key. +vrrp_script check_llmgw { + script "/usr/bin/curl -fsS --max-time 2 -o /dev/null http://127.0.0.1:8080/ready" + interval 2 + timeout 3 + # Two failures before giving up the address, so one slow moment does not + # move it. Two successes before taking it back, so a flapping gateway does + # not drag the address with it. + fall 2 + rise 2 + weight -40 +} + +vrrp_instance llmgw { + # MASTER on the machine you would rather serve; BACKUP on the other. + state MASTER + interface eth0 + # Any number both machines agree on and nothing else on the segment uses. + virtual_router_id 51 + # The higher one wins while both are healthy. The weight above is enough + # to invert this when the preferred machine's gateway is not answering. + priority 100 + advert_int 1 + + authentication { + auth_type PASS + # Change this. It stops another host on the segment claiming the + # address, which is otherwise a single unicast packet away. + auth_pass CHANGEME + } + + virtual_ipaddress { + 10.0.0.150/24 + } + + track_script { + check_llmgw + } +} diff --git a/docs/install.md b/docs/install.md index 8ed32b2..da4400c 100644 --- a/docs/install.md +++ b/docs/install.md @@ -718,6 +718,18 @@ Without authentication, `threads.store` and `threads.retain_hours` in `server.yaml` keep both conversations and notes in one shared pool. That is for one trusted machine only; see [quickstart.md](quickstart.md). +## Two control planes + +llmgw handles the hard half itself: only one places work, and agents find +whichever is leading. What it does not do is move client traffic, which +happens in front of both. + +[deploy/ha/](../deploy/ha/) has the pattern and a keepalived example. The one +thing to get right is the health check: point it at `/ready`, not `/health`. +`/health` is 200 on a standby, which is alive and holds no endpoints at all, +so a load balancer using it would send half the traffic to something that +refuses every request. + ## Reclaiming disk `llmgw ctl nodes` has a DISK column: free space against the size it is free diff --git a/internal/router/handlers.go b/internal/router/handlers.go index 4974d74..84c46df 100644 --- a/internal/router/handlers.go +++ b/internal/router/handlers.go @@ -82,10 +82,32 @@ func (r *Router) Handler() http.Handler { mux.HandleFunc("POST /v1/completions", r.handleInference("/v1/completions")) mux.HandleFunc("POST /v1/embeddings", r.handleInference("/v1/embeddings")) mux.HandleFunc("GET /v1/models", r.handleModels) + // Liveness: this process is up. It says nothing about whether it can + // serve, which is why it is not the thing to put a load balancer on. mux.HandleFunc("GET /health", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) + // Readiness: this process has somewhere to send a request. + // + // The distinction is not academic. A standby control plane is up, answers + // /health, and has no endpoints at all — its reconciler placed nothing and + // it holds no agent streams to adopt from — so a load balancer checking + // /health would send it half the traffic and half the traffic would be a + // 503. A leader whose replicas have all failed is in the same position for + // a different reason, and a load balancer should treat them the same. + // + // Open, like /health: a load balancer does not have a key, and this leaks + // one bit — whether anything is servable — which the next request would + // reveal anyway. + mux.HandleFunc("GET /ready", func(w http.ResponseWriter, _ *http.Request) { + if len(r.reg.IDs()) == 0 { + http.Error(w, "no endpoints are available to serve", http.StatusServiceUnavailable) + return + } + w.WriteHeader(http.StatusOK) + }) + // The landing page says how to connect and nothing about what is here, so // it is safe to serve without a key — and it has to be, because the person // reading it is the one who does not have one yet. @@ -143,7 +165,7 @@ func (r *Router) Handler() http.Handler { // route must not be able to add an unauthenticated one by omission. Health // stays open because a load balancer has no key. return auth.Middleware(r.auth, r.log, - "/health", "/", "/_static/", "/claim/", + "/health", "/ready", "/", "/_static/", "/claim/", "/signup", "/login", "/logout", "/profile", "/profile/", "/admin", "/admin/", "/chat", "/chat/")(mux) } diff --git a/test/e2e/health_test.go b/test/e2e/health_test.go new file mode 100644 index 0000000..793765b --- /dev/null +++ b/test/e2e/health_test.go @@ -0,0 +1,68 @@ +package e2e + +import ( + "context" + "net/http" + "testing" + + "gitea.mynodes.xyz/ilker/llmgw/internal/config" +) + +// probe fetches a path with no key, as a load balancer would. +func probe(t *testing.T, url string) int { + t.Helper() + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil) + if err != nil { + t.Fatal(err) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + return resp.StatusCode +} + +func TestReadinessMeansSomethingHealthDoesNot(t *testing.T) { + // A standby control plane is up, answers /health, and has no endpoints — + // its reconciler placed nothing and it holds no agent streams to adopt + // from. A load balancer checking /health would send it half the traffic, + // and half the traffic would be a 503. + h := start(t, 2) + + if code := probe(t, h.router.URL+"/health"); code != http.StatusOK { + t.Errorf("/health with endpoints = %d", code) + } + if code := probe(t, h.router.URL+"/ready"); code != http.StatusOK { + t.Errorf("/ready with endpoints = %d", code) + } +} + +func TestWithNothingToServeReadyRefusesAndHealthDoesNot(t *testing.T) { + // The case the endpoint exists for. Both are true statements: the process + // is alive, and it cannot serve. + h := start(t, 0, func(c *config.Server) { + c.Endpoints = nil + }) + + if code := probe(t, h.router.URL+"/health"); code != http.StatusOK { + t.Errorf("/health on a live process with no endpoints = %d, want 200", code) + } + if code := probe(t, h.router.URL+"/ready"); code != http.StatusServiceUnavailable { + t.Errorf("/ready with nothing to serve = %d, want 503", code) + } +} + +func TestBothProbesWorkWithoutAKey(t *testing.T) { + // A load balancer does not have one, and an endpoint it cannot reach is + // an endpoint that reports every gateway as down. + path, mint := provision(t) + _ = mint(t, "somebody") + h := start(t, 2, paidConfig(path)) + + for _, p := range []string{"/health", "/ready"} { + if code := probe(t, h.router.URL+p); code != http.StatusOK { + t.Errorf("%s on an authenticated gateway = %d", p, code) + } + } +}