From abaf086fd55aeff8c1da2f88cbdbcbef04cf3d28 Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Mon, 5 Oct 2026 06:34:57 +0200 Subject: [PATCH] A standby's portal is the leader's; the changelog as the release notes read it A proxy left pointing at a standby showed an empty cluster: the portal and the admin pages read the standby's own memory. A standby now passes its client port to the leader's, all but /health, /ready and /leader, which must describe the machine a load balancer is choosing; and its admin port passes the dashboard as well as the API. The leader trusts the control planes' X-Forwarded-For, so clients keep their own address. The changelog's candidate section is "Unreleased" again: release notes for an rc read that section, and v0.11.3-rc.1 got none. The e2e wait for two endpoints is 30s: under the race detector on a busy runner, 15s was missed once. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 25 +++-- docs/install.md | 8 +- internal/cli/leaderforward.go | 93 +++++++++++++++++- internal/cli/leaderforward_test.go | 62 +++++++++++- internal/cli/server.go | 3 + .../__pycache__/release_notes.cpython-314.pyc | Bin 0 -> 7571 bytes test/e2e/phase3_test.go | 4 +- 7 files changed, 172 insertions(+), 23 deletions(-) create mode 100644 scripts/gitea/__pycache__/release_notes.cpython-314.pyc diff --git a/CHANGELOG.md b/CHANGELOG.md index a08a344..b80abf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,20 +23,6 @@ APIs change between minors without a deprecation period. ## Unreleased -### Added - -- **Guardrails in the console**: a view of the policies and what each checks, - whether their judge models are serving, where they are attached (attach - and detach from it), and what they did in a window. `GET - /api/v1/guard/events` answers the same, and `GET /api/v1/guard` now says - what each policy does. - -### Fixed - -- The `CONTROL` line named no floating address. - -## v0.11.3-rc.1 — 2026-10-05 - Two control planes, run in anger on the reference cluster. ### Added @@ -56,6 +42,16 @@ Two control planes, run in anger on the reference cluster. come back stops it. The other control plane is upgraded first, over ssh. - `server add` says when the mirror's `--api-allow` would refuse the new control plane. +- **Guardrails in the console**: a view of the policies and what each checks, + whether their judge models are serving, where they are attached (attach + and detach from it), and what they did in a window. `GET + /api/v1/guard/events` answers the same, and `GET /api/v1/guard` now says + what each policy does. + +- **A standby's portal is the leader's.** Its client port passes everything + but `/health`, `/ready` and `/leader` to the leader, and its admin port the + dashboard as well as the API; a proxy left pointing at a standby showed an + empty cluster. The leader trusts the control planes' `X-Forwarded-For`. ### Fixed @@ -67,6 +63,7 @@ Two control planes, run in anger on the reference cluster. - An agent's reconnect backoff carried over from one outage to the next, so each failover made the next one slower; it starts again after a session that worked. +- The `CONTROL` line named no floating address. ## v0.11.2 — 2026-10-05 diff --git a/docs/install.md b/docs/install.md index 3916b4e..27f1e30 100644 --- a/docs/install.md +++ b/docs/install.md @@ -1940,8 +1940,12 @@ Without it, put a load balancer in front that checks `/ready` — not `CONTROL` line: which leads, which stand by, one that does not answer, and whether the floating address is on the leader. A standby with nothing to take over from is the thing to see before it is needed. -- **Either machine's console.** A standby's admin API passes everything under - `/api/` to the leader, over the port control planes already reach each +- **Either machine, the leader's cluster.** A standby passes its client port + — the portal, its admin pages, the API — to the leader's, keeping only + `/health`, `/ready` and `/leader` to describe itself, so a proxy pointed at + the wrong machine still shows the cluster; the leader trusts a control + plane's `X-Forwarded-For`, so clients keep their own address. Its admin + API passes everything but `/debug/` and `/metrics` to the leader, over the port control planes already reach each other on and with the cluster's certificates both ways, so `sudo llmgw` and `llmgw ctl` show the same cluster on both — the leader's — and a change made on the standby is made by the leader. A node's certificate is refused diff --git a/internal/cli/leaderforward.go b/internal/cli/leaderforward.go index 46065cb..b4d4136 100644 --- a/internal/cli/leaderforward.go +++ b/internal/cli/leaderforward.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "log/slog" + "net" "net/http" "net/http/httputil" "net/url" @@ -16,6 +17,7 @@ import ( "golang.org/x/net/http2" + "gitea.mynodes.xyz/ilker/llmgw/internal/clientip" "gitea.mynodes.xyz/ilker/llmgw/internal/control" "gitea.mynodes.xyz/ilker/llmgw/internal/store" "gitea.mynodes.xyz/ilker/llmgw/internal/tlsconf" @@ -53,7 +55,7 @@ type leaderForward struct { // process is not it. func (f *leaderForward) standby(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if !strings.HasPrefix(r.URL.Path, "/api/") || f.lead.IsLeader() { + if adminStaysHere(r.URL.Path) || f.lead.IsLeader() { next.ServeHTTP(w, r) return } @@ -90,6 +92,89 @@ func (f *leaderForward) standby(next http.Handler) http.Handler { }) } +// adminStaysHere are the admin paths about this process rather than the +// cluster: its profile, its own metrics, its own health. Never forwarded, and +// never answered for another — pprof dumps the heap, keys and all. +func adminStaysHere(path string) bool { + return strings.HasPrefix(path, "/debug/") || path == "/metrics" || path == "/health" +} + +// publicStaysHere are the client port's paths that must describe this +// process: a load balancer and keepalived ask them which machine to send +// traffic to, and a standby saying the leader is ready would draw it. +func publicStaysHere(path string) bool { + return path == "/health" || path == "/ready" || path == "/leader" || path == "/metrics" +} + +// public wraps the client-facing handler: on a standby, everything but its +// own health goes to the leader's client port — the portal, its admin pages +// and the API, which on a standby would show an empty cluster and serve +// nothing. Plain HTTP, as the port is; the leader trusts a control plane's +// X-Forwarded-For, so a client is still known by its own address. +func (f *leaderForward) public(next http.Handler, port string) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Once only: a request another control plane forwarded is answered + // here, leading or not, rather than sent on while the lease moves. + if publicStaysHere(r.URL.Path) || f.lead.IsLeader() || r.Header.Get(hdrForwardedBy) != "" { + next.ServeHTTP(w, r) + return + } + target, _, err := f.leader(r.Context()) + if err != nil || target.Hostname() == "" { + next.ServeHTTP(w, r) + return + } + host := net.JoinHostPort(target.Hostname(), port) + p := &httputil.ReverseProxy{ + Rewrite: func(pr *httputil.ProxyRequest) { + pr.Out.URL.Scheme, pr.Out.URL.Host = "http", host + // The name the client used, for the portal's links and cookies. + pr.Out.Host = pr.In.Host + peer, _, _ := net.SplitHostPort(pr.In.RemoteAddr) + xff := pr.In.Header.Get("X-Forwarded-For") + if xff != "" { + xff += ", " + } + pr.Out.Header.Set("X-Forwarded-For", xff+peer) + for _, h := range []string{"X-Forwarded-Proto", "X-Forwarded-Host"} { + if v := pr.In.Header.Get(h); v != "" { + pr.Out.Header.Set(h, v) + } + } + pr.Out.Header.Set(hdrForwardedBy, f.self) + }, + FlushInterval: -1, + ErrorHandler: func(w http.ResponseWriter, _ *http.Request, err error) { + writeForwardError(w, http.StatusBadGateway, "this control plane is standing by, and the leader did not answer: "+err.Error()) + }, + } + p.ServeHTTP(w, r) + }) +} + +// trustControlPlanes keeps the client port trusting the other control +// planes' X-Forwarded-For, besides router.trusted_proxies: a standby passes +// requests on, and the client is the address it was given, not the standby. +// Read again every half minute, for one added since. +func (f *leaderForward) trustControlPlanes(ctx context.Context, configured []string) { + for { + list := slices.Clone(configured) + if cps, err := f.st.ControlPlanes(ctx); err == nil { + for _, cp := range cps { + if u, err := url.Parse(cp.URL); err == nil && u.Hostname() != "" { + list = append(list, u.Hostname()) + } + } + } + clientip.SetTrusted(list) + select { + case <-ctx.Done(): + return + case <-time.After(30 * time.Second): + } + } +} + // leader is where the lease's holder is reached, and its name. func (f *leaderForward) leader(ctx context.Context) (*url.URL, string, error) { ctx, cancel := context.WithTimeout(ctx, 3*time.Second) @@ -99,6 +184,10 @@ func (f *leaderForward) leader(ctx context.Context) (*url.URL, string, error) { return nil, "", errors.New("no control plane holds the lease right now") } name := control.HolderName(lease.Owner) + if name == f.self { + // This machine's previous process, whose lease has not run out. + return nil, name, errors.New("the lease is still held by this machine's previous run; it runs out within seconds") + } cps, err := f.st.ControlPlanes(ctx) if err != nil { return nil, name, err @@ -117,7 +206,7 @@ func (f *leaderForward) leader(ctx context.Context) (*url.URL, string, error) { func (f *leaderForward) leaderSide(admin http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { path := strings.TrimPrefix(r.URL.Path, forwardPrefix) - if !strings.HasPrefix(path, "/api/") { + if adminStaysHere(path) { http.NotFound(w, r) return } diff --git a/internal/cli/leaderforward_test.go b/internal/cli/leaderforward_test.go index 1ccef1f..5f2fdc4 100644 --- a/internal/cli/leaderforward_test.go +++ b/internal/cli/leaderforward_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "io" "log/slog" + "net" "net/http" "net/http/httptest" "path/filepath" @@ -140,9 +141,15 @@ func TestAStandbysAdminAPIIsTheLeaders(t *testing.T) { if got["actor"] != "root@ctl-b" { t.Errorf("who asked was lost: %q", got["actor"]) } - // What is not the API stays on the machine: the dashboard, pprof. - if _, got := c.ask(t, "/debug/pprof/"); got["path"] != "local" { - t.Errorf("a non-API path was forwarded: %v", got) + // The dashboard's pages are the leader's too. + if _, got := c.ask(t, "/nodes/cuda01"); got["path"] != "/nodes/cuda01" { + t.Errorf("a dashboard page was answered here: %v", got) + } + // What is about this process stays on it: pprof, its metrics. + for _, p := range []string{"/debug/pprof/", "/metrics"} { + if _, got := c.ask(t, p); got["path"] != "local" { + t.Errorf("%s was forwarded: %v", p, got) + } } } @@ -186,3 +193,52 @@ func TestAStandbyWithNoLeaderSaysSo(t *testing.T) { t.Errorf("%d %v", resp.StatusCode, got) } } + +func TestAStandbysPortalIsTheLeaders(t *testing.T) { + c := newForwardCluster(t) + // The leader's client port, on the address the store lists for it. + var got http.Header + var gotHost string + leader := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got, gotHost = r.Header.Clone(), r.Host + _, _ = w.Write([]byte("leader " + r.URL.Path)) + })) + defer leader.Close() + _, port, _ := net.SplitHostPort(strings.TrimPrefix(leader.URL, "http://")) + + local := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("standby " + r.URL.Path)) }) + srv := httptest.NewServer(c.standby.public(local, port)) + defer srv.Close() + get := func(path string, hdr map[string]string) string { + req, _ := http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL+path, nil) + req.Host = "llmgw.example" + for k, v := range hdr { + req.Header.Set(k, v) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + b, _ := io.ReadAll(resp.Body) + return string(b) + } + + if body := get("/admin/replicas", map[string]string{"X-Forwarded-For": "203.0.113.7", "X-Forwarded-Proto": "https"}); body != "leader /admin/replicas" { + t.Fatalf("the portal was answered by %q", body) + } + if gotHost != "llmgw.example" || got.Get("X-Forwarded-Proto") != "https" || + !strings.HasPrefix(got.Get("X-Forwarded-For"), "203.0.113.7, ") || got.Get(hdrForwardedBy) != "ctl-b" { + t.Errorf("host %q, headers %v", gotHost, got) + } + // What a load balancer asks is about this machine. + for _, p := range []string{"/ready", "/health", "/leader"} { + if body := get(p, nil); body != "standby "+p { + t.Errorf("%s answered by %q", p, body) + } + } + // Forwarded once only. + if body := get("/v1/models", map[string]string{hdrForwardedBy: "ctl-c"}); body != "standby /v1/models" { + t.Errorf("a forwarded request was forwarded again: %q", body) + } +} diff --git a/internal/cli/server.go b/internal/cli/server.go index b1fd48d..a247a30 100644 --- a/internal/cli/server.go +++ b/internal/cli/server.go @@ -513,6 +513,9 @@ func newServerCmd() *cobra.Command { served := adminHandler if forward != nil { served = forward.standby(adminHandler) + _, port, _ := net.SplitHostPort(cfg.Listen) + apiSrv.Handler = forward.public(apiSrv.Handler, port) + go forward.trustControlPlanes(ctx, cfg.Router.TrustedProxies) } adminSrv := withLimits(&http.Server{ Handler: router.AdminGuard(router.AdminAuth(adminToken, cfg.DebugPprof, served)), diff --git a/scripts/gitea/__pycache__/release_notes.cpython-314.pyc b/scripts/gitea/__pycache__/release_notes.cpython-314.pyc new file mode 100644 index 0000000000000000000000000000000000000000..c48c9156da81f96874dc73456e13944fc4a41b27 GIT binary patch literal 7571 zcmbVReM}okmhZOPU+oXfM*?Az&_EI}0Xqvrybpm?(fWIq>*<2 zxmRtMA%vOSZN+p|SG|7q>eYL%Uj1^?<)jHH&%XEJ?QjJ_{5N{Rg(^0l{|_|M1WTyI z1i_M*NR`xUgKE%gqiWP^N~QGLq?+{FtXlNiTC8oVg*B)&YgFyyRGLiOaWvscHP~9xeTB?aCsg|==c&`|*WNqhe5d;q_K1_^P)DxXW z_I0qTM*YuYPfo+K14)$;xX281SCADk5pPnM{^7pSA^+8DL*5wAC_+SqZU%m$sxU*M zDsYS}LKDU7DPoNhm@=S6oyUIM5mB925acR6w0mD#Jx# zau5k`q`p)-o+xP(2$egcBnf;$aI6I+7_d}9sjy!>Y%MfFlD^WH5!e`!2MS%;cTaiS zPkB4rzbM!dKQd;>O~R#34RnW15(f+7pLRhVfGzLJ?_Tot1_ zgQ5|l_8c=Qz>=mJHG$TMnHH6<&@oQF10w5tEXhI}wn=^SrocL)Oi@xWJ0u{ow!~~4 z3k60M5$J~9;?4lGNkNFiWN|Kr>=mX~K@~R?u%|AXfiiSy%-(>oThJrJtn{fb&}Si z6=N-kaa}39Es;^?Kiu?xXV#N(a`!ECUSe#b@8|1+%1)xUG0i(h_@HSA~w4kC7MHv}5 zXn`J09hy;9N$K(xwB(J=#S=V?+?z|K>|*qeAp4@x*z|1b*aR3+^W6}CGUUaNAMfmv zCp778IhLxKc##6EQ71nZDTYQp-kITPBnawx2zt|mkB~^zDwI$$&_WC};l7#oBfV8>tX z)HsBX(7X7Sq?AlIsep5b8n^IQK@&*?%5d*ajmOy+B)?e|DBq#MtqBsZ`UG@c`7463 z5l4w294H<`s(*x&UN}jR_vkh#+*85Dd2n>(f-?}k!2Vj9q=YHrhytRQS0ibUO`}lA z)-1#RzJW`lLz)$wnFr63HJcDm#snDxk!Fkvam@sQS5`Hn5a&@FoKQ1O%Za4qF>AJx z*Jws?FwKPGg`$z7M#f~62q?%nP9T?Nv@^aN3LJmFf*mSyq+pA_?{;j>T09&Txe|Dx zomc(_9%bixAP~?%?lawAN+q8-E=V-ajUlR9sk?@e;!)t&(@yWtUbF? zdp1{lZmsjP+TMBN;?Tm-Qs0}G-z@veM3mP*GZT)o^yTLu2oZq;zEsen!5N6>UxKlw ziJ(z9j?6GrA_$ZmqSQsy10CM<{oDkb&j`s95*0!^)l0yNTRZiBMt!p?2-(Q%NG>Rn zDQMcyh*<+`3>q9nhXH3tb;P$&Qmko*<~8CK67Ay3wxnU)3Luq`nV13p`^Jn z>kMcONhXqFQ5PQPsK=mDkhpM~IVQkC%WKZQB-FB);!swbGV3QelXCPY;wa~88=I4a zl$(>Js2I^ne7BWE+(T*>Rg4J`NW@1Vzh)`sY%pLJWH})#U87^W9Auk( z7vm|@D8xWB@yVE^0C7A5$DU?R%F(DeiTL~|DWGV_78^9j^I;StA>LVlDj`9jqfpAH znW71fS2UA8p=KK%8w>c++!|Hj$qRZ-U!Q|BIOe}6svcSf3nVlnFw#u=S~ZKr%^^lI zvINZt0wDH^q){jp*GLeK7gUg6K>_0g6g@^VGO}H{9zV`PL#c)pr-`kT9iN`O_|wkC zt_NL9(>eRmP5bc;`|-TJX?-9)uuZ!cy$jw=n%SV4<;pd4mS%EvZ@PbrvfLm2$>>t= z^3=+Wm7(mh&OCJncV5gkp2<^ZpBb#C5pvsExj3;fvFU8wa5gUYuU2H8jhoKSyt8wS z+N!L6eE#A2pLNd<{J!brqt3@&f8Uki^OcRORo|Eh*BPLLU(VV2r>`L}RxI``^laMe zH|+Ia6BMxCvO5;fE}UJu{?iNpSy{bRUh(+g!-GGod0h9fZpF4#mn-jB?fk6#+`M_) zRg)RaoLg>RzP{{VIkejT{@M4=t~RgLuMMu9%N`oYx%|Iv&swkk>6s1j{iAXL6!cN2 zr@xi>*CVI;YfK+IX=s03WAFFaK5nE?+hat});hn>_=&yT-(vivq0b7{zqOd4D*_Bk zr4+nxfg7e@8MH&Cz|Ruc;UpqR1mNqvg(X9!U>!j7>?!*P(pQHL{5=H+KFBfu|N1Or-B^#3A!sOt zdJhV^K{BvUC@;|z4w+J5-!Fs%+)$1WnK~j=3eCXz@SbJBD_g-lqj|e{o6UvVBUs3AHgeL5&zd{)uCgUB7g^Q2h{s>z%$|xcsN226YPNz z`~LJO;kZSehcL#U(dT@H7WdZ|97hND=MN**K~t9j>Y5HSmJTdv-}T@0?>-WJq(qHW z+@b&N4X$}==e|Z$|t()+Z zgQLZp&pqJu9Tv(%Y7F~%8gpY02Q4tmkr3#-P=eOL>$tsBfbknK@+1uJmS5U|= zcVVh2laBVJ4k9e4fNkpFNfGt|#?*`ul~T46P^8g{3K&aRm6HN|hA197Z+ z)(V&QQH|a~Vp3Ir86id`L=EA+DJRn$O#m*&>+r8edNk6Pa``w(^xbU-a7;=lzLX84 zWJ-!IECR`rGBC|43m~>}LGftDA^%v)>;ni3KCmL_5+$N37sE8~pqd^T#VCyip^K;# zkOFpxNHtTSZ>)bXl2Spx`&7$OevCBT1S zK-x5;GN))1U}maDp*&wk_>GPw$Z$1>`|6=l;+i%}~C;oPPHS|eO?nD3$e;*HztXxFzUHE+{+t7tNx^c&q6-T!F zI_etRvO5!f58gN`sEp{NC=)RK0cht;3na+gAIc^?@~8 zK9INizqC2$`NjA`e0g@$=KIX%TW#NV*JR4yI{5R0|5%fAH_V&9F%tBtFD&-^_wL_I z-`j3D`Ht^x-|DgVo8N2RZ0P>9q5EO$dev`gKB}3wY}qR^a9gT_3yRadIQ(EZL%rqt zxhq@Ulx^x>8~F9`2g7S!>*Lw$Kipu)v*Q!l1L3^mFSg1LJg#|IlR5KG7qjk`1=BMt zVRJ3o7Hmtc%T(Ta{0mpbVr(I{>1x<;H7uXbxtg-h<`vhf`u^N|bE|jO4`uCF(gRxu zY9GJx@Qugg3pT)+>}88Ri#_u_+m7<3%NZpjEO-1OvU2RH?};zh(w#felRbW6?Pj*Z zpLY!Y&QZ1PI%}!C*V>;wIgmZ#Ur**-BZ#rDuWdT&vyS?VylHRVus7%I zEi3%f+fQ!)Dw?-*?At(n)U^=NkNX7=@4dFv0htu3`i#5)^>}c?3vg$g==*h$LbXzzP~UBAZa} z4fLo)k9zb#?uBGQ|8)@#KOtn4krn*U+7nCg$*6EqR-g}^(#l`LaRO;}w|c literal 0 HcmV?d00001 diff --git a/test/e2e/phase3_test.go b/test/e2e/phase3_test.go index 63d6466..0745041 100644 --- a/test/e2e/phase3_test.go +++ b/test/e2e/phase3_test.go @@ -261,7 +261,7 @@ func TestControlPlaneFeedsRouter(t *testing.T) { }() defer close(stop) - h.waitForEndpoints(t, 2, 15*time.Second) + h.waitForEndpoints(t, 2, 30*time.Second) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -304,7 +304,7 @@ func TestRouterKeepsServingWhileNodeIsLost(t *testing.T) { }() defer close(stop) - h.waitForEndpoints(t, 2, 15*time.Second) + h.waitForEndpoints(t, 2, 30*time.Second) // Continuous traffic for the whole experiment. ctx, cancel := context.WithCancel(context.Background())