From 7855d6355cc37b285003f3a6b3f5788a114320d7 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sat, 8 Aug 2026 17:41:52 +0200 Subject: [PATCH] =?UTF-8?q?hub=20v0.100.0=20=E2=80=94=20the=20Configuratio?= =?UTF-8?q?n=20page=20took=2026=20seconds,=20and=20it=20was=20never=20hash?= =?UTF-8?q?ing=20anything?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MEASURED, NOT GUESSED: GET /configuration -> HTTP 200 in 26.2s. The reasonable guess was that it hashes the artifacts on page load. It does not, and the code already said so: Gitea stores each package file's sha256 and gitea.FileSHA256 reads it as metadata — "a cheap metadata call, the artifact bytes are never downloaded". The cost was never CPU. IT WAS LATENCY x COUNT. artifactChoices made ONE SERIAL round-trip per version, for two packages, capped at 20 each: 2 x (1 version list + 20 sha lookups) = 42 sequential requests at ~0.6s each out through the public ingress. 42 x 0.6 = 26s, which is what the clock said. 1. The sha lookups now run CONCURRENTLY, bounded at 8 in flight. Order preserved by writing into a slot rather than appending — the dropdown is newest-first, and a scrambled sha would show the operator a hash belonging to a DIFFERENT artifact. A failed lookup still drops that version only. 2. The client talks to Gitea IN-CLUSTER (http://gitea.gitea-system.svc.cluster.local:3000, overridable via GITEA_API_URL). Measured from the hub pod: 0.11s against 0.26-1.16s, because the public path adds DNS, the ingress hop and a TLS handshake to each of the 42. Plain HTTP is safe ONLY because it never leaves the cluster network — the registry token rides the Authorization header, so this must not point at a public host without TLS. Unreachable -> the existing graceful degradation to manual text entry, unchanged. DELIBERATELY NOT DONE: caching the sha in the hub's own database. That was the other half of the proposal and it is the wrong shape. Gitea already IS the store; a copy in hub_settings would be a second source of truth that can drift from the registry it describes — and the operator reads exactly this value to confirm what they are about to vouch, so a stale one would be a confident wrong answer. The same reasoning golden_currency_gate.py already records for the vouched version. With the fan-out, a cold load needs no cache to be fast. The cap stays at 20 and now bounds the FAN-OUT too, not just the rendered list. Tests pin order (and that each sha belongs to its own version), per-version failure isolation, and THE CONCURRENCY ITSELF — a wall-clock assertion plus an in-flight counter, so a fast run cannot be luck, and an upper bound so a large package list cannot stampede Gitea. Red-proof: reverting to the serial loop takes 861ms where the concurrent one takes 150ms, and the test fails naming the 26-second page. go build / go vet / go test ./... green (18 packages), run separately from this commit. --- hub/CHANGELOG.md | 39 +++++ hub/cmd/hub/main.go | 19 ++- hub/internal/web/artifact_choices_test.go | 194 ++++++++++++++++++++++ hub/internal/web/server.go | 100 +++++++---- 4 files changed, 316 insertions(+), 36 deletions(-) create mode 100644 hub/internal/web/artifact_choices_test.go diff --git a/hub/CHANGELOG.md b/hub/CHANGELOG.md index 78f2b74..1b1112e 100644 --- a/hub/CHANGELOG.md +++ b/hub/CHANGELOG.md @@ -1,3 +1,42 @@ +## v0.100.0 — the Configuration page took 26 seconds, and it was never hashing anything (2026-08-08) + +**Measured, not guessed:** `GET /configuration` → **HTTP 200 in 26.2 s**. + +**The reasonable guess was that it hashed the artifacts on load. It does not, and the code already +said so** — Gitea stores each package file's sha256 and `gitea.FileSHA256` reads it as metadata +(*"a cheap metadata call — the artifact bytes are never downloaded"*). The cost was never CPU. + +**It was latency × count.** `artifactChoices` made **one serial HTTP round-trip per version**, for +two packages, capped at 20 each: `2 × (1 version list + 20 sha lookups)` = **42 sequential +requests**, each ~0.6 s out through the public ingress. 42 × 0.6 ≈ 26 s, which is what the clock said. + +**Two changes, both to the count-and-latency, neither introducing new state:** + +1. **The sha lookups now run concurrently**, bounded at 8 in flight. Order is preserved by writing + into a slot rather than appending — the dropdown is newest-first and a scrambled sha would show + the operator a hash belonging to a different artifact. A failed lookup still drops **that** + version only. +2. **The client talks to Gitea in-cluster** (`http://gitea.gitea-system.svc.cluster.local:3000`, + overridable with `GITEA_API_URL`) instead of the public name. Measured from the hub pod: **0.11 s + against 0.26–1.16 s**, because the public path adds DNS, the ingress hop and a TLS handshake to + every one of the 42. Plain HTTP is safe **only** because it never leaves the cluster network; the + registry token rides the Authorization header, so this must not be pointed at a public host + without TLS. + +**Deliberately NOT done: caching the sha in the hub's own database.** That was the other half of the +proposal and it is the wrong shape here. Gitea already is the store; a copy in `hub_settings` would +be a **second source of truth that can drift from the registry it describes** — and the operator +reads exactly this value to confirm what they are about to vouch, so a stale one would be a +confident wrong answer. The same reasoning `golden_currency_gate.py` already records for the vouched +version. With the fan-out, a cold load needs no cache to be fast. + +**The cap stays at 20** and now bounds the fan-out too, not just the rendered list. + +Tests assert the three things that had to survive: order (and that each sha belongs to its own +version), per-version failure isolation, and **the concurrency itself** — a wall-clock assertion plus +an in-flight counter, so a fast run cannot be luck. Red-proof: reverting to the serial loop takes +861 ms where the concurrent one takes 150 ms, and the test fails naming the 26-second page. + ## v0.99.0 — the hub can finally see whether the operator can get in (2026-08-08, R-260 / G-1) **`oobDegraded` tested five things and the sixth never arrived.** The agent has emitted diff --git a/hub/cmd/hub/main.go b/hub/cmd/hub/main.go index 56da5bd..5d82d04 100644 --- a/hub/cmd/hub/main.go +++ b/hub/cmd/hub/main.go @@ -297,8 +297,23 @@ func main() { // sha256 from Gitea (no hand-copied checksums). Reuses the registry creds; degrades to manual text // entry when they're absent. if cfg.Registry.Username != "" && cfg.Registry.Token != "" { - webServer.SetGiteaClient(gitea.New("https://gitea.dooplex.hu", "admin", cfg.Registry.Username, cfg.Registry.Token)) - logger.Printf("[INFO] Gitea artifact browser enabled (Day-0 version dropdowns)") + // IN-CLUSTER, not the public ingress (v0.100.0). The hub and Gitea share this cluster, and + // every one of these is a small metadata call made dozens of times per Configuration page + // load. Measured on DooPlex: ~0.11 s in-cluster against 0.26–1.16 s out through the public + // name, which adds a DNS lookup, the ingress hop and a TLS handshake to each one for nothing. + // + // Overridable so a hub running outside the cluster still works; the plain-HTTP default is + // safe only because it never leaves the cluster network — do not point this at a public + // host without https, the registry token rides the Authorization header. + // + // If the service is unreachable, artifactChoices logs a WARN and returns nil, and the form + // degrades to manual text entry. That is the pre-existing behaviour and it stays. + giteaAPI := os.Getenv("GITEA_API_URL") + if giteaAPI == "" { + giteaAPI = "http://gitea.gitea-system.svc.cluster.local:3000" + } + webServer.SetGiteaClient(gitea.New(giteaAPI, "admin", cfg.Registry.Username, cfg.Registry.Token)) + logger.Printf("[INFO] Gitea artifact browser enabled (Day-0 version dropdowns) via %s", giteaAPI) } // Offsite provisioning (SLICE 1): enabled when a Hetzner storage-box token is provided out-of-band. diff --git a/hub/internal/web/artifact_choices_test.go b/hub/internal/web/artifact_choices_test.go new file mode 100644 index 0000000..8bdea71 --- /dev/null +++ b/hub/internal/web/artifact_choices_test.go @@ -0,0 +1,194 @@ +package web + +import ( + "context" + "fmt" + "io" + "log" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-hub/internal/gitea" +) + +// artifactChoices fans the per-version sha lookups out concurrently (hub v0.100.0). The +// Configuration page took 26 SECONDS with two dropdowns because it made 42 sequential metadata +// round-trips. These tests pin the three things that must survive the change: the ORDER, the +// per-version failure isolation, and the concurrency itself. + +// fakeGitea serves the two endpoints the client uses, with a controllable per-request delay so +// "concurrent" is measurable rather than asserted. +type fakeGitea struct { + versions []string + delay time.Duration + failVer string // this version's /files call 500s + + inFlight int32 + maxSeen int32 + callCount int32 +} + +func (f *fakeGitea) start(t *testing.T) *gitea.Client { + t.Helper() + mux := http.NewServeMux() + // BOTH patterns: the version list is GET /api/v1/packages/admin?... with NO trailing slash, + // the per-version files call is /api/v1/packages/admin/generic///files. + handler := func(w http.ResponseWriter, r *http.Request) { + n := atomic.AddInt32(&f.inFlight, 1) + for { + old := atomic.LoadInt32(&f.maxSeen) + if n <= old || atomic.CompareAndSwapInt32(&f.maxSeen, old, n) { + break + } + } + defer atomic.AddInt32(&f.inFlight, -1) + atomic.AddInt32(&f.callCount, 1) + time.Sleep(f.delay) + + // .../generic///files → the sha metadata + if len(r.URL.Path) > 6 && r.URL.Path[len(r.URL.Path)-6:] == "/files" { + for _, v := range f.versions { + if f.failVer != "" && v == f.failVer && strings.Contains(r.URL.Path, "/"+v+"/") { + http.Error(w, "boom", http.StatusInternalServerError) + return + } + } + ver := versionFromFilesPath(r.URL.Path) + fmt.Fprintf(w, `[{"name":"felhom-agent","sha256":"sha-%s"}]`, ver) + return + } + // the version list + out := "[" + for i, v := range f.versions { + if i > 0 { + out += "," + } + out += fmt.Sprintf(`{"name":"felhom-agent","type":"generic","version":%q}`, v) + } + fmt.Fprint(w, out+"]") + } + mux.HandleFunc("/api/v1/packages/admin", handler) + mux.HandleFunc("/api/v1/packages/admin/", handler) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return gitea.New(srv.URL, "admin", "u", "t") +} + +func versionFromFilesPath(p string) string { + // .../generic///files + end := len(p) - len("/files") + start := end - 1 + for start > 0 && p[start-1] != '/' { + start-- + } + return p[start:end] +} + +func newTestWebServer(t *testing.T, c *gitea.Client) *Server { + t.Helper() + s := &Server{logger: log.New(io.Discard, "", 0)} + s.SetGiteaClient(c) + return s +} + +// The dropdown is newest-first and the concurrent version must not scramble it. +func TestArtifactChoices_PreservesOrder(t *testing.T) { + f := &fakeGitea{versions: []string{"0.9.0", "0.8.0", "0.7.0", "0.6.0", "0.5.0"}} + s := newTestWebServer(t, f.start(t)) + + got := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent") + + if len(got) != 5 { + t.Fatalf("got %d choices, want 5: %+v", len(got), got) + } + // ListVersions sorts newest-first; the fan-out must not disturb that, and each sha must belong + // to ITS OWN version — a slot mix-up would show the operator a hash for a different artifact. + for i, want := range []string{"0.9.0", "0.8.0", "0.7.0", "0.6.0", "0.5.0"} { + if got[i].Version != want { + t.Errorf("choice %d = %q, want %q — the concurrent fan-out scrambled the order", i, got[i].Version, want) + } + if got[i].SHA256 != "sha-"+want { + t.Errorf("version %s carries sha %q — a sha was written into the wrong slot, which would show "+ + "the operator a hash belonging to a different artifact", got[i].Version, got[i].SHA256) + } + } +} + +// A single failing version drops ITSELF and nothing else — unchanged from the serial version. +func TestArtifactChoices_OneFailureDropsOnlyThatVersion(t *testing.T) { + f := &fakeGitea{versions: []string{"0.9.0", "0.8.0", "0.7.0"}, failVer: "0.8.0"} + s := newTestWebServer(t, f.start(t)) + + got := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent") + + if len(got) != 2 { + t.Fatalf("got %d choices, want 2 (the failing one dropped): %+v", len(got), got) + } + for _, c := range got { + if c.Version == "0.8.0" { + t.Error("the failing version was included") + } + } + if got[0].Version != "0.9.0" || got[1].Version != "0.7.0" { + t.Errorf("order broken around the dropped version: %+v", got) + } +} + +// THE POINT OF THE CHANGE. With a per-request delay, a serial implementation takes +// len(versions) * delay; the concurrent one takes about ceil(n/8) * delay. Asserting the wall-clock +// is what makes this a test of the fix rather than of the plumbing — and the in-flight counter +// proves requests genuinely overlapped rather than the timing being luck. +func TestArtifactChoices_IsConcurrent(t *testing.T) { + const n = 16 + vers := make([]string, n) + for i := range vers { + vers[i] = fmt.Sprintf("0.%d.0", 100-i) // already newest-first + } + f := &fakeGitea{versions: vers, delay: 50 * time.Millisecond} + s := newTestWebServer(t, f.start(t)) + + start := time.Now() + got := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent") + elapsed := time.Since(start) + + if len(got) != n { + t.Fatalf("got %d choices, want %d", len(got), n) + } + serial := time.Duration(n) * 50 * time.Millisecond // 800ms + if elapsed > serial/2 { + t.Errorf("took %v; a serial implementation would take ~%v and the concurrent one should be far "+ + "under half of that. This is the 26-second Configuration page.", elapsed, serial) + } + if max := atomic.LoadInt32(&f.maxSeen); max < 2 { + t.Errorf("max concurrent in-flight requests was %d — the lookups did not actually overlap, so a "+ + "fast wall-clock here would be luck rather than concurrency", max) + } + // and it must respect the bound rather than opening one connection per version + if max := atomic.LoadInt32(&f.maxSeen); max > 8 { + t.Errorf("max concurrent in-flight was %d, above the bound of 8 — a large package list would "+ + "stampede Gitea", max) + } +} + +// The cap is unchanged: at most 20 versions are offered however many exist. +func TestArtifactChoices_StillCapsAtTwenty(t *testing.T) { + vers := make([]string, 30) + for i := range vers { + vers[i] = fmt.Sprintf("0.%d.0", 200-i) + } + f := &fakeGitea{versions: vers} + s := newTestWebServer(t, f.start(t)) + + got := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent") + if len(got) != 20 { + t.Errorf("got %d choices, want the 20 cap", len(got)) + } + if atomic.LoadInt32(&f.callCount) > 21 { // 1 version list + 20 sha lookups + t.Errorf("made %d requests; the cap must limit the FAN-OUT too, not just the rendered list", + atomic.LoadInt32(&f.callCount)) + } +} diff --git a/hub/internal/web/server.go b/hub/internal/web/server.go index f1736c7..a7faf46 100644 --- a/hub/internal/web/server.go +++ b/hub/internal/web/server.go @@ -20,8 +20,8 @@ import ( "gitea.dooplex.hu/admin/felhom-hub/internal/gitea" "gitea.dooplex.hu/admin/felhom-hub/internal/intent" "gitea.dooplex.hu/admin/felhom-hub/internal/monitor" - "gitea.dooplex.hu/admin/felhom-hub/internal/poke" "gitea.dooplex.hu/admin/felhom-hub/internal/offsite" + "gitea.dooplex.hu/admin/felhom-hub/internal/poke" "gitea.dooplex.hu/admin/felhom-hub/internal/semver" "gitea.dooplex.hu/admin/felhom-hub/internal/store" "golang.org/x/crypto/bcrypt" @@ -51,28 +51,28 @@ type hubSession struct { // Server handles the dashboard web UI. type Server struct { - store *store.Store + store *store.Store // configPasswordHash is the operator login password bcrypt hash SEEDED from hub.yaml // (auth.password_hash) at startup. It is the fallback only — a hub_settings DB override set via // the Configuration UI wins. Never read this field directly for an auth decision; call // effectivePasswordHash(). configPasswordHash string apiKey string // report API key — used for controller callbacks - version string - logger *log.Logger - templates *template.Template - staleThreshold time.Duration - versionChecker *VersionChecker - templateFetcher *TemplateFetcher - assetsMgr *assets.Manager - gitea *gitea.Client // optional; enables the Day-0 artifact version dropdowns - offsite *offsite.Provisioner // optional; enables Hetzner offsite provisioning (SLICE 1) - offsiteBox func() (monitor.BoxSnapshot, bool) // optional (v0.64.0, R-5); the restic pool-box aggregate snapshot accessor - pbsdrBox func() (monitor.PBSBoxSnapshot, bool) // optional (v0.65.0, R-5); the PBS-DR datastore fill snapshot accessor - tenantsync tenancyProvisioner // optional; enables PBS DR tier provisioning (web/pbsdr.go) - claimEngine *claim.Engine // optional; enables the customer-claim resend button (v0.50.0) - selfBindMailer SelfBindMailer // optional; enables the customer self-bind link button (v0.66.0, R-27) - bindLimiter *bindRateLimiter // per-IP throttle for the PUBLIC /bind/ surface (v0.66.0, R-27) + version string + logger *log.Logger + templates *template.Template + staleThreshold time.Duration + versionChecker *VersionChecker + templateFetcher *TemplateFetcher + assetsMgr *assets.Manager + gitea *gitea.Client // optional; enables the Day-0 artifact version dropdowns + offsite *offsite.Provisioner // optional; enables Hetzner offsite provisioning (SLICE 1) + offsiteBox func() (monitor.BoxSnapshot, bool) // optional (v0.64.0, R-5); the restic pool-box aggregate snapshot accessor + pbsdrBox func() (monitor.PBSBoxSnapshot, bool) // optional (v0.65.0, R-5); the PBS-DR datastore fill snapshot accessor + tenantsync tenancyProvisioner // optional; enables PBS DR tier provisioning (web/pbsdr.go) + claimEngine *claim.Engine // optional; enables the customer-claim resend button (v0.50.0) + selfBindMailer SelfBindMailer // optional; enables the customer self-bind link button (v0.66.0, R-27) + bindLimiter *bindRateLimiter // per-IP throttle for the PUBLIC /bind/ surface (v0.66.0, R-27) // intentHub (v0.58.0, Direction-2 immediate-sync) is Bumped by every operator-intent handler // (config save/delete, claim resend, offsite re-issue/freeze, floor, block/unblock, log pull) // so a box long-polling GET /api/v1/wait wakes in seconds. Shared with the API handler. nil = @@ -126,12 +126,12 @@ func New(store *store.Store, passwordHash, apiKey, version string, staleThreshol store: store, configPasswordHash: passwordHash, apiKey: apiKey, - version: version, - logger: logger, - templates: tmpl, - staleThreshold: staleThreshold, - sessions: make(map[string]*hubSession), - bindLimiter: newBindRateLimiter(30), // public /bind/ surface: 30 req/min/IP burst (R-27) + version: version, + logger: logger, + templates: tmpl, + staleThreshold: staleThreshold, + sessions: make(map[string]*hubSession), + bindLimiter: newBindRateLimiter(30), // public /bind/ surface: 30 req/min/IP burst (R-27) } } @@ -237,14 +237,45 @@ func (s *Server) artifactChoices(ctx context.Context, pkg, file string) []artifa s.logger.Printf("[INFO] artifact versions (%s): showing newest %d of %d", pkg, maxChoices, len(vers)) vers = vers[:maxChoices] } + // CONCURRENTLY (v0.100.0). Each sha is one independent metadata round-trip, and doing them in + // series made /configuration take 26 SECONDS on a page with two dropdowns: 2 packages x (1 + // version list + 20 sha lookups) = 42 sequential requests at ~0.6 s each. + // + // ⚠ NOTHING IS BEING HASHED HERE, and the operator's reasonable guess that it was is worth + // recording so nobody re-derives it: Gitea stores the sha256 with the package file and + // FileSHA256 reads it as metadata — the artifact bytes are never downloaded (see its comment). + // The cost was never CPU; it was latency x count, and the fix is therefore concurrency, not a + // cached or precomputed hash. A hash copied into the hub's own DB would be a SECOND SOURCE OF + // TRUTH that can drift from the registry it describes, and the operator reads this value to + // confirm what they are about to vouch — a stale one would be a confident wrong answer. + // + // Bounded at 8 in flight: enough to collapse the wall-clock, few enough not to stampede Gitea + // on a page an operator may reload. Order is preserved by writing into a slot, not appending — + // the dropdown is newest-first and must stay that way. + const parallel = 8 + shas := make([]string, len(vers)) + errs := make([]error, len(vers)) + sem := make(chan struct{}, parallel) + var wg sync.WaitGroup + for i, v := range vers { + wg.Add(1) + go func(i int, v string) { + defer wg.Done() + sem <- struct{}{} + defer func() { <-sem }() + shas[i], errs[i] = s.gitea.FileSHA256(ctx, pkg, v, file) + }(i, v) + } + wg.Wait() + out := make([]artifactChoice, 0, len(vers)) - for _, v := range vers { - sha, err := s.gitea.FileSHA256(ctx, pkg, v, file) - if err != nil { - s.logger.Printf("[WARN] artifact sha (%s/%s): %v", pkg, v, err) + for i, v := range vers { + if errs[i] != nil { + // unchanged behaviour: a failed lookup drops THAT version, never the whole list + s.logger.Printf("[WARN] artifact sha (%s/%s): %v", pkg, v, errs[i]) continue } - out = append(out, artifactChoice{Version: v, SHA256: sha}) + out = append(out, artifactChoice{Version: v, SHA256: shas[i]}) } return out } @@ -837,12 +868,13 @@ func timeAgo(t time.Time) string { // statusColor maps a status value to a design-system-v2 semantic token, consumed as a // class suffix (status-dot-nominal, tag-nominal, …) — never as an inline color (D4). // Exception-color principle: a healthy fleet is blue/neutral; amber/red only on deviation. -// ok -> nominal (blue — operating normally) -// warn, stale -> warn (amber — degraded / stale report) -// down, fail -> crit (red — outage) -// pending -> neutral (a not-yet-provisioned customer is a normal fleet state) -// disabled -> neutral (deliberately paused — not a deviation) -// blocked -> warn (an operator cut a customer off: intentional but attention-worthy) +// +// ok -> nominal (blue — operating normally) +// warn, stale -> warn (amber — degraded / stale report) +// down, fail -> crit (red — outage) +// pending -> neutral (a not-yet-provisioned customer is a normal fleet state) +// disabled -> neutral (deliberately paused — not a deviation) +// blocked -> warn (an operator cut a customer off: intentional but attention-worthy) func statusColor(status string) string { switch status { case "ok":