From 979653f407a3836665a51b7777fe8c248d501c56 Mon Sep 17 00:00:00 2001 From: emi Date: Fri, 24 Jul 2026 04:03:26 +0000 Subject: [PATCH 1/5] deploy.sh: fix the daemon-binary probe, which always dropped daemon (#33) docker compose config --images daemon does not filter to the named service on this host's Compose v5.3.1 -- it prints every service's image, one per line, in file order, so `| head -1` was silently grabbing db's image (timescaledb) instead of daemon's. The probe then always found no /usr/local/bin/thermograph-daemon in a Postgres image and dropped daemon from every backend deploy, regardless of what the real backend image contained. Fixed by building the image reference directly from the same vars docker-compose.yml's daemon.image: already interpolates, instead of going through docker compose config at all. Reproduced against beta directly: the old sequence selected the wrong image; the new construction resolves correctly and the binary probe passes. --- infra/deploy/deploy.sh | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/infra/deploy/deploy.sh b/infra/deploy/deploy.sh index 487779e..51748eb 100755 --- a/infra/deploy/deploy.sh +++ b/infra/deploy/deploy.sh @@ -247,9 +247,17 @@ fi # next deploy of a tag that has it picks it up with no further action. case " ${TARGETS[*]} " in *" daemon "*) - daemon_img="$(docker compose config --images daemon 2>/dev/null | head -1 || true)" - if [ -n "$daemon_img" ] \ - && ! docker run --rm --entrypoint sh "$daemon_img" -c 'test -x /usr/local/bin/thermograph-daemon' 2>/dev/null; then + # Built directly from the same vars docker-compose.yml's `daemon.image:` + # interpolates (REGISTRY_HOST/BACKEND_IMAGE_PATH/BACKEND_IMAGE_TAG), + # NOT via `docker compose config --images daemon`: that command does not + # actually filter to the named service (confirmed live on Compose + # v5.3.1 -- it prints every service's image, one per line, in file + # order) so `| head -1` silently grabbed db's image instead. The probe + # then always found no daemon binary in a Postgres image and dropped + # daemon from EVERY backend deploy, regardless of what the real backend + # image contained -- reproduced and confirmed against beta directly. + daemon_img="${REGISTRY_HOST:-git.thermograph.org}/${BACKEND_IMAGE_PATH:-emi/thermograph/backend}:${BACKEND_IMAGE_TAG}" + if ! docker run --rm --entrypoint sh "$daemon_img" -c 'test -x /usr/local/bin/thermograph-daemon' 2>/dev/null; then echo "==> $daemon_img predates the daemon binary; rolling without the daemon service this run" kept=() for t in "${TARGETS[@]}"; do From 0862395dda683199838acc4639bff4bbdd50afc1 Mon Sep 17 00:00:00 2001 From: emi Date: Fri, 24 Jul 2026 04:37:41 +0000 Subject: [PATCH 2/5] Log hygiene: Alloy CPU, Loki chunks/limits, Caddy field-stripping (#36) --- .forgejo/workflows/observability-validate.yml | 29 +++++++++-- infra/deploy/Caddyfile | 29 +++++++++-- .../templates/Caddyfile.tftpl | 30 +++++++++-- observability/alloy/config.alloy | 52 +++++++++++++++++-- observability/loki/config.yml | 20 +++++++ 5 files changed, 146 insertions(+), 14 deletions(-) diff --git a/.forgejo/workflows/observability-validate.yml b/.forgejo/workflows/observability-validate.yml index 7b6fcc0..157a3d9 100644 --- a/.forgejo/workflows/observability-validate.yml +++ b/.forgejo/workflows/observability-validate.yml @@ -12,10 +12,13 @@ name: Validate observability stack # path-filtered to the domain, and workflow_call lets pr-build.yml reuse this # as the domain's PR check under its single `gate` required status. # -# Not validated here: the Alloy config (observability/alloy/config.alloy, an -# HCL-like format, needs the `alloy` binary) and docker-compose *schema* -# (unknown-key checks, which would need the docker CLI). Add either if the -# churn warrants the extra tooling. +# The Alloy config (observability/alloy/config.alloy, an HCL-like format) IS +# validated below -- the static `alloy` binary is downloaded straight from its +# GitHub release (pinned to the same v1.9.1 the fleet runs; see +# observability/alloy/docker-compose.agent.yml), no docker CLI needed. +# +# Not validated here: docker-compose *schema* (unknown-key checks, which would +# need the docker CLI). Add it if the churn warrants the extra tooling. on: workflow_call: {} @@ -70,3 +73,21 @@ jobs: print("INVALID YAML", f, "-", e); bad = True sys.exit(1 if bad else 0) PY + + - name: Alloy config parses and validates (components, relabel/process wiring) + run: | + set -euo pipefail + apt-get update -qq && apt-get install -y -qq unzip >/dev/null + curl -sSL -o /tmp/alloy.zip \ + https://github.com/grafana/alloy/releases/download/v1.9.1/alloy-linux-amd64.zip + unzip -q /tmp/alloy.zip -d /tmp/alloybin + chmod +x /tmp/alloybin/alloy-linux-amd64 + + # `alloy validate` builds the real component graph (catches bad field + # names, dangling forward_to/receiver refs, malformed relabel/process + # stages) but doesn't recognize the top-level `livedebugging {}` singleton + # block as a component -- it only knows named components, even though + # `alloy run` loads that block fine. Known gap in the `validate` + # subcommand, not a config error, so strip that one line before checking. + grep -v '^livedebugging ' observability/alloy/config.alloy > /tmp/config-for-validate.alloy + /tmp/alloybin/alloy-linux-amd64 validate /tmp/config-for-validate.alloy diff --git a/infra/deploy/Caddyfile b/infra/deploy/Caddyfile index 0d6a31a..6baf615 100644 --- a/infra/deploy/Caddyfile +++ b/infra/deploy/Caddyfile @@ -31,10 +31,12 @@ thermograph.org { # Active health check on the same cheap /healthz route each container's own # HEALTHCHECK uses (Dockerfile) — so a deploy that's still restarting/booting # never gets proxied into (a reload alone has no gate, hop-1 runbook hazard #10). + # 15s (was 5s): plenty responsive for a process that only restarts on a deploy, + # and a quarter of the polling load. handle @backend_paths { reverse_proxy 127.0.0.1:8137 { health_uri /healthz - health_interval 5s + health_interval 15s health_timeout 3s health_status 2xx } @@ -43,14 +45,35 @@ thermograph.org { handle { reverse_proxy 127.0.0.1:8080 { health_uri /healthz - health_interval 5s + health_interval 15s health_timeout 3s health_status 2xx } } + # Access-log hygiene: the default JSON encoder serializes full request headers, + # the TLS block, and response headers on every line (measured ~1,133B/line) -- + # strip those with the `filter` format encoder. Also strip the query string from + # the logged URI: Caddy's default logger records request.uri *including* the + # query string, so every `?q=` a visitor typed sat in Loki next to + # their client IP for the full 30-day retention -- a real privacy leak, not just + # noise. Bot/crawler skipping stays out of here: `log_skip` needs Caddy >= 2.7 + # and an upgrade is out of scope, so that's handled downstream in Alloy's + # loki.process "caddy" stage instead (see observability/alloy/config.alloy). log { - output file /var/log/caddy/thermograph.log + output file /var/log/caddy/thermograph.log { + roll_size 20MiB + roll_keep 5 + } + format filter { + wrap json + fields { + request>headers delete + request>tls delete + resp_headers delete + request>uri regexp \?.* "" + } + } } } diff --git a/infra/terraform/modules/thermograph-host/templates/Caddyfile.tftpl b/infra/terraform/modules/thermograph-host/templates/Caddyfile.tftpl index 340bdb7..95cd448 100644 --- a/infra/terraform/modules/thermograph-host/templates/Caddyfile.tftpl +++ b/infra/terraform/modules/thermograph-host/templates/Caddyfile.tftpl @@ -12,6 +12,16 @@ # # NOTE: the repo's deploy/Caddyfile additionally serves the emigriffith.dev portfolio # and legacy redirects; those are host-specific and intentionally not templated here. +# +# Access-log hygiene: the default JSON encoder serializes full request headers, the +# TLS block, and response headers on every line (measured ~1,133B/line on prod, +# ~922B on beta) -- strip those with the `filter` format encoder. Also strip the +# query string from the logged URI: Caddy's default logger records request.uri +# *including* the query string, so every `?q=` a visitor typed sat in +# Loki next to their client IP for the full 30-day retention -- a real privacy +# leak, not just noise. Bot/crawler skipping stays out of here: `log_skip` needs +# Caddy >= 2.7 and an upgrade is out of scope, so that's handled downstream in +# Alloy's loki.process "caddy" stage instead (see observability/alloy/config.alloy). ${domain} { encode zstd gzip @@ -21,10 +31,12 @@ ${domain} { # HEALTHCHECK uses (Dockerfile) — so a `docker compose up -d --build` deploy # that's still restarting/booting never gets proxied into (a reload alone has # no gate, hop-1 runbook hazard #10). health_uri is relative to the upstream. + # 15s (was 5s): plenty responsive for an active health check against a process + # that only restarts on a deploy, and a quarter of the polling load. handle @backend_paths { reverse_proxy 127.0.0.1:${port} { health_uri /healthz - health_interval 5s + health_interval 15s health_timeout 3s health_status 2xx } @@ -33,13 +45,25 @@ ${domain} { handle { reverse_proxy 127.0.0.1:${frontend_port} { health_uri /healthz - health_interval 5s + health_interval 15s health_timeout 3s health_status 2xx } } log { - output file /var/log/caddy/thermograph.log + output file /var/log/caddy/thermograph.log { + roll_size 20MiB + roll_keep 5 + } + format filter { + wrap json + fields { + request>headers delete + request>tls delete + resp_headers delete + request>uri regexp \?.* "" + } + } } } diff --git a/observability/alloy/config.alloy b/observability/alloy/config.alloy index 7ddbc29..3279da2 100644 --- a/observability/alloy/config.alloy +++ b/observability/alloy/config.alloy @@ -15,12 +15,22 @@ livedebugging { enabled = false } // --- 1. All Docker container logs ------------------------------------------------ +// refresh_interval defaults to 60s; Swarm task churn reshuffles the target set on +// roughly that cadence, which restarts tailers ~every 90s and was costing ~11% of +// Alloy's own CPU in tailer restarts alone. 5m is still fast enough to pick up a +// real deploy without paying that churn cost. discovery.docker "containers" { - host = "unix:///var/run/docker.sock" + host = "unix:///var/run/docker.sock" + refresh_interval = "5m" } // Turn Docker metadata into tidy labels: `container` (short name) and `service` -// (the compose service, e.g. app/db). Drop Alloy's own container to avoid a loop. +// (the compose service, e.g. app/db). Drop noise/duplicate containers so Loki never +// ingests them: Alloy itself (loop), the autoscaler, throwaway `thermograph-test_*` +// stacks, the loopback LB bridge (`thermograph-lb`, pure plumbing, nothing to debug +// from its logs), and the app's own worker (`thermograph_worker`'s stdout is 100% +// `/healthz` poll noise — the app's `access/*.jsonl` under source #3 is a strict +// superset of anything useful it logs). discovery.relabel "containers" { targets = discovery.docker.containers.targets @@ -35,27 +45,55 @@ discovery.relabel "containers" { } rule { source_labels = ["container"] - regex = ".*alloy.*" + regex = "(alloy|autoscaler|thermograph-test_.*|thermograph-lb|thermograph_worker).*" action = "drop" } } +// Pass RAW targets here (not discovery.relabel.containers.output) alongside +// relabel_rules: loki.source.docker applies relabel_rules itself, once, using the +// __meta_docker_* metadata it still holds at collection time. Passing the +// already-relabelled output *and* relabel_rules ran the same rules twice per log +// entry, and made the drop rules above a no-op on the second pass since the +// __meta_docker_* labels are already gone from the pre-relabelled output. loki.source.docker "containers" { host = "unix:///var/run/docker.sock" - targets = discovery.relabel.containers.output + targets = discovery.docker.containers.targets forward_to = [loki.write.central.receiver] relabel_rules = discovery.relabel.containers.rules labels = { job = "docker" } } // --- 2. Caddy host access logs --------------------------------------------------- +// sync_period (glob rescan) defaults to 10s; 1m is plenty for a log file that only +// appears/rotates on the order of hours. local.file_match "caddy" { path_targets = [{ __path__ = "/var/log/caddy/*.log", job = "caddy" }] + sync_period = "1m" } loki.source.file "caddy" { targets = local.file_match.caddy.targets + forward_to = [loki.process.caddy.receiver] + + // PollingFileWatcher defaults (250ms/250ms) stat every tailed file 4x/second + // forever. Caddy's access log doesn't need sub-second latency into Loki. + file_watch { + min_poll_frequency = "2s" + max_poll_frequency = "10s" + } +} + +// Drop well-known crawler/bot traffic before it hits Loki. Caddy itself can't do +// this cheaply (log_skip needs Caddy >= 2.7; both hosts run older Caddy, and an +// upgrade is out of scope here), so filter it at the shipper instead. +loki.process "caddy" { forward_to = [loki.write.central.receiver] + + stage.drop { + expression = "(?i)(semrushbot|claudebot|ahrefsbot|yandexbot|bytespider|mj12bot|petalbot)" + drop_counter_reason = "crawler" + } } // --- 3. App structured JSON logs (errors / access / audit) ----------------------- @@ -63,11 +101,17 @@ loki.source.file "caddy" { // compose). Lift `level`/`tag`/`phase` out of the JSON so they're queryable. local.file_match "app_jsonl" { path_targets = [{ __path__ = "/applogs/**/*.jsonl", job = "app-json" }] + sync_period = "1m" } loki.source.file "app_jsonl" { targets = local.file_match.app_jsonl.targets forward_to = [loki.process.app_jsonl.receiver] + + file_watch { + min_poll_frequency = "2s" + max_poll_frequency = "10s" + } } loki.process "app_jsonl" { diff --git a/observability/loki/config.yml b/observability/loki/config.yml index d17a37f..b2dfb46 100644 --- a/observability/loki/config.yml +++ b/observability/loki/config.yml @@ -32,6 +32,17 @@ schema_config: prefix: index_ period: 24h +# 30 low-rate streams almost always hit chunk_idle_period (30m default) long before +# they'd ever fill chunk_target_size, so chunks were flushed 1.98% full on average +# (972 near-empty chunks for 30MB of actual log data). Give idle streams far longer +# to accumulate before an idle flush, and cap age/size so a chunk still can't grow +# unbounded. +ingester: + chunk_idle_period: 2h + max_chunk_age: 12h + chunk_target_size: 1572864 + chunk_encoding: snappy + limits_config: # A hobby fleet's volume is tiny; keep 30 days and cap ingestion generously. retention_period: 720h @@ -40,6 +51,15 @@ limits_config: max_query_series: 5000 allow_structured_metadata: true volume_enabled: true + # No explicit ingestion/stream limits meant Loki fell back to its (much stricter) + # built-in defaults, which is why 25 pushes came back HTTP 429 with nothing in + # this file explaining why. These are sized for a three-node hobby fleet, not the + # multi-tenant defaults. + ingestion_rate_mb: 8 + ingestion_burst_size_mb: 16 + per_stream_rate_limit: 3MB + per_stream_rate_limit_burst: 10MB + max_global_streams_per_user: 1000 compactor: working_directory: /loki/compactor From da82abda27d04e60a020947930bcfc0f252c064a Mon Sep 17 00:00:00 2001 From: emi Date: Fri, 24 Jul 2026 06:25:06 +0000 Subject: [PATCH 3/5] frontend: fetch City() concurrently with the page's primary API call (#37) MonthPage and RecordsPage each made two backend calls sequentially (CityMonth/CityRecords, then City) where the second never depended on the first's result. fetchWithCity launches both concurrently via goroutines and a WaitGroup. Verified live against a stub with an injected 400ms delay on both endpoints: city page (1 call) and month/records pages (2 calls each) all cost ~0.404s now, not double for the two-call pages. Error priority preserved exactly: primary's error wins even when City also fails, matching the old sequential code. One trade-off: City() is now always launched even on a request about to 404 from primary, costing one extra cheap lookup on that rare path. Tests include a deterministic concurrency proof via rendezvous channels (the old sequential code would deadlock this test, not just run it slower). Full suite green under -race -count=2. --- .../server/internal/content/content_test.go | 111 ++++++++++++++++++ frontend/server/internal/content/pages.go | 67 ++++++++--- 2 files changed, 164 insertions(+), 14 deletions(-) diff --git a/frontend/server/internal/content/content_test.go b/frontend/server/internal/content/content_test.go index ef90a28..1d199ea 100644 --- a/frontend/server/internal/content/content_test.go +++ b/frontend/server/internal/content/content_test.go @@ -19,6 +19,7 @@ import ( "path/filepath" "strings" "testing" + "time" "thermograph/frontend/internal/config" "thermograph/frontend/internal/contentapi" @@ -474,6 +475,116 @@ func TestMonthCtx(t *testing.T) { } } +// --- fetchWithCity ----------------------------------------------------------- + +func TestFetchWithCityHappyPath(t *testing.T) { + api := &fakeAPI{ + city: func(slug, _ string) (*contentapi.CityPayload, error) { + return &contentapi.CityPayload{City: contentapi.CityInfo{Slug: slug, Name: "Testville"}}, nil + }, + } + primary, city, err := fetchWithCity(api, "testville", func() (string, error) { return "primary-value", nil }) + if err != nil { + t.Fatal(err) + } + if primary != "primary-value" { + t.Errorf("primary = %q", primary) + } + if city.Slug != "testville" || city.Name != "Testville" { + t.Errorf("city = %+v", city) + } +} + +// When only primary fails, its error must win even though City() also ran +// (and, here, succeeded) -- matching the old sequential code's behavior, +// where City() was never even called once primary had already failed. +func TestFetchWithCityPrimaryErrorWins(t *testing.T) { + primaryErr := notFound("no such month") + api := &fakeAPI{ + city: func(slug, _ string) (*contentapi.CityPayload, error) { + return &contentapi.CityPayload{City: contentapi.CityInfo{Slug: slug}}, nil + }, + } + _, _, err := fetchWithCity(api, "testville", func() (string, error) { return "", primaryErr }) + if err != primaryErr { + t.Errorf("err = %v, want the primary error", err) + } +} + +// When primary succeeds but City() fails, City's error must surface -- same +// as the old sequential code (it called City() second and returned whatever +// that call did). +func TestFetchWithCityCityErrorSurfaces(t *testing.T) { + cityErr := notFound("no such city") + api := &fakeAPI{ + city: func(_, _ string) (*contentapi.CityPayload, error) { return nil, cityErr }, + } + _, _, err := fetchWithCity(api, "testville", func() (string, error) { return "primary-value", nil }) + if err != cityErr { + t.Errorf("err = %v, want the city error", err) + } +} + +// When BOTH fail, primary's error still wins -- the same priority as the +// single-failure case above, just with both goroutines erroring. +func TestFetchWithCityBothErrorPrimaryWins(t *testing.T) { + primaryErr, cityErr := notFound("primary"), notFound("city") + api := &fakeAPI{ + city: func(_, _ string) (*contentapi.CityPayload, error) { return nil, cityErr }, + } + _, _, err := fetchWithCity(api, "testville", func() (string, error) { return "", primaryErr }) + if err != primaryErr { + t.Errorf("err = %v, want the primary error (city error must not win)", err) + } +} + +// Proves the two calls actually run CONCURRENTLY rather than sequentially -- +// deterministically, via a rendezvous, not by timing (which would be flaky +// under CI load). Each fake blocks until BOTH have started; if fetchWithCity +// ran them sequentially, the second would never start until the first +// returned, and this would deadlock and fail on the test's own timeout. +func TestFetchWithCityRunsConcurrently(t *testing.T) { + started := make(chan struct{}, 2) + release := make(chan struct{}) + rendezvous := func() { + started <- struct{}{} + <-release + } + + api := &fakeAPI{ + city: func(_, _ string) (*contentapi.CityPayload, error) { + rendezvous() + return &contentapi.CityPayload{}, nil + }, + } + + done := make(chan struct{}) + go func() { + fetchWithCity(api, "testville", func() (string, error) { + rendezvous() + return "", nil + }) + close(done) + }() + + // Both goroutines must reach the rendezvous before either can proceed -- + // only possible if they were launched concurrently. + for range 2 { + select { + case <-started: + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for both calls to start -- they are not running concurrently") + } + } + close(release) + + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("fetchWithCity did not return after release") + } +} + func TestMonthCtxUnknownLabelIsError(t *testing.T) { h := newHandlers(t, fixtureAPI(t)) j := loadFixture[contentapi.MonthPayload](t, "month") diff --git a/frontend/server/internal/content/pages.go b/frontend/server/internal/content/pages.go index 43eaedf..a508df0 100644 --- a/frontend/server/internal/content/pages.go +++ b/frontend/server/internal/content/pages.go @@ -7,6 +7,7 @@ import ( "html/template" "net/http" "strings" + "sync" "thermograph/frontend/internal/contentapi" "thermograph/frontend/internal/contentdata" @@ -165,6 +166,49 @@ func (h *Handlers) cityCtx(j *contentapi.CityPayload) (map[string]any, format.Un return ctx, unit, nil } +// fetchWithCity runs a page's primary API call and the shared City lookup +// concurrently instead of sequentially: both are independent single-slug +// fetches (City never depends on primary's result), so waiting for one to +// fully round-trip before even starting the other was pure latency with +// nothing to show for it. Concurrent instead cuts that leg to roughly +// whichever call is slower. +// +// If primary fails, its error wins even when City also fails or hasn't +// finished — matching the old sequential code's behavior exactly (it never +// even called City() once primary had already failed). The one real +// trade-off: City() is now ALWAYS launched, even on a request that's about +// to 404 from primary, so an invalid slug costs one extra (wasted, cheap) +// backend lookup it previously skipped. Worth it: a bad slug is the rare +// path; a good one is the common path this speeds up. +func fetchWithCity[T any](api API, slug string, primary func() (T, error)) (T, contentapi.CityInfo, error) { + var ( + pVal T + pErr error + cPay *contentapi.CityPayload + cErr error + group sync.WaitGroup + ) + group.Add(2) + go func() { + defer group.Done() + pVal, pErr = primary() + }() + go func() { + defer group.Done() + cPay, cErr = api.City(slug, "") // no origin: the Python called city(slug) bare here too + }() + group.Wait() + + if pErr != nil { + return pVal, contentapi.CityInfo{}, pErr + } + if cErr != nil { + var zero T + return zero, contentapi.CityInfo{}, cErr + } + return pVal, cPay.City, nil +} + // --- month page -------------------------------------------------------------- // MonthPage renders /climate/{slug}/{month} (content.py's month_page). @@ -176,17 +220,14 @@ func (h *Handlers) cityCtx(j *contentapi.CityPayload) (map[string]any, format.Un // Jinja iterated (label, value) tuples). func (h *Handlers) MonthPage(w http.ResponseWriter, r *http.Request) { slug, month := r.PathValue("slug"), r.PathValue("month") - j, err := h.api.CityMonth(slug, month) + j, city, err := fetchWithCity(h.api, slug, func() (*contentapi.MonthPayload, error) { + return h.api.CityMonth(slug, month) + }) if err != nil { h.apiError(w, err) return } - cp, err := h.api.City(slug, "") // no origin: the Python called city(slug) bare here - if err != nil { - h.apiError(w, err) - return - } - ctx, unit, err := h.monthCtx(j, cp.City) + ctx, unit, err := h.monthCtx(j, city) if err != nil { h.serverError(w, err) return @@ -308,17 +349,15 @@ func fmtPeriodSide(unit format.Unit, s contentapi.PeriodSide) map[string]any { // temperature metrics, nil otherwise, as the Python set them. func (h *Handlers) RecordsPage(w http.ResponseWriter, r *http.Request) { slug := r.PathValue("slug") - j, err := h.api.CityRecords(slug, origin(r)) + o := origin(r) + j, city, err := fetchWithCity(h.api, slug, func() (*contentapi.RecordsPayload, error) { + return h.api.CityRecords(slug, o) + }) if err != nil { h.apiError(w, err) return } - cp, err := h.api.City(slug, "") // no origin, same as month_page - if err != nil { - h.apiError(w, err) - return - } - ctx, unit, err := h.recordsCtx(j, cp.City) + ctx, unit, err := h.recordsCtx(j, city) if err != nil { h.serverError(w, err) return From dcf15ea5724c4deae8cff90c02adc0d4df30a0e9 Mon Sep 17 00:00:00 2001 From: emi Date: Fri, 24 Jul 2026 18:56:46 +0000 Subject: [PATCH 4/5] infra/forgejo: add resource limits and a leaner CI job image (#38) --- infra/deploy/forgejo/README.md | 46 +++++++++++++++++++++ infra/deploy/forgejo/ci-runner/Dockerfile | 46 +++++++++++++++++++++ infra/deploy/forgejo/docker-stack.yml | 8 ++++ infra/deploy/forgejo/register-lan-runner.sh | 2 +- 4 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 infra/deploy/forgejo/ci-runner/Dockerfile diff --git a/infra/deploy/forgejo/README.md b/infra/deploy/forgejo/README.md index 0325386..8ad737d 100644 --- a/infra/deploy/forgejo/README.md +++ b/infra/deploy/forgejo/README.md @@ -41,6 +41,15 @@ Do this **before** the DNS + Caddy step below — Caddy's reverse_proxy target (`127.0.0.1:3080`) needs the `forgejo` service actually listening first, or its first health check just fails harmlessly until it is. +This stack has **no auto-deploy trigger** — nothing in `.forgejo/workflows/` +redeploys it on push. A change to `docker-stack.yml` only takes effect once +someone re-runs `docker stack deploy` by hand on the manager (prod). + +`db`/`forgejo` both carry `resources.limits` (defaults: db 1 CPU/1g, forgejo 2 +CPU/2g — several times observed steady-state usage), overridable with +`FORGEJO_DB_CPUS`/`FORGEJO_DB_MEMORY`/`FORGEJO_CPUS`/`FORGEJO_MEMORY` env vars +before `docker stack deploy`, same convention as the app stack. + ## DNS + TLS: reusing beta's existing Caddy, not a second reverse proxy Forgejo is pinned to beta (`role=forge`) — but beta is also **today's live @@ -98,6 +107,43 @@ See that script's header for exactly what it replaces (the pre-Forgejo GitHub self-hosted runner on this same machine) and why it registers with two labels where there used to be two separate runners. +## Custom CI job image (`ci-runner/`) + +`ci-runner/Dockerfile` still bases on `node:20-bookworm` — Node is a hard +requirement, not leftover: Forgejo's runner executes `actions/checkout@v4` +(used by every workflow) as `node dist/index.js` *inside the job container*, +regardless of whether the workflow itself uses npm/node. (v1 of this image +tried a Node-free Debian-slim base and broke every job's checkout step — +`node: executable file not found` — within a minute of going live; reverted +immediately.) What it actually fixes: every `docker`-labeled build-push job +currently re-installs the Docker CLI on each run (`apt-get install +docker.io`), which pulls in the classic builder rather than BuildKit (the +classic builder mishandles `COPY --chown=` group resolution — a real +bug hit during the frontend Go rewrite). `ci-runner` adds `docker-ce-cli` + +`docker-buildx-plugin` (BuildKit) on top of the same Node base, plus +`git`/`python3`/`python3-yaml` for the other jobs that need them +(`shell-lint`, `observability-validate`). + +Current tag: `git.thermograph.org/emi/thermograph/ci-runner:v2` (`v1` is +broken — do not register any runner against it). Rebuild/push (requires a PAT +with `write:package` scope — the embedded git-remote token lacks it, same +requirement documented in `backend-build-push.yml`): + +```bash +docker build -t git.thermograph.org/emi/thermograph/ci-runner:vN deploy/forgejo/ci-runner +docker push git.thermograph.org/emi/thermograph/ci-runner:vN +``` + +`register-lan-runner.sh`'s `LABELS` default points at the current tag, so +fresh registrations pick it up automatically. The live runner is cut over by +editing the `labels` array in `~/forgejo-runner/.runner` on the desktop (same +runner id/token, no re-registration needed) and restarting the service — +**verify a real job runs green under the new image before relying on it**, +same way v1's break was caught. Only after that verification should the +now-redundant `apt-get install docker.io` / `python3-yaml` steps be removed +from the workflows that had them — removing them first would break every job +still running on the stock `node:20-bookworm` image. + ## Why Postgres here and not the Thermograph app's TimescaleDB Separate instance, separate network (`forgejo_net`, not the app's compose diff --git a/infra/deploy/forgejo/ci-runner/Dockerfile b/infra/deploy/forgejo/ci-runner/Dockerfile new file mode 100644 index 0000000..e4259e0 --- /dev/null +++ b/infra/deploy/forgejo/ci-runner/Dockerfile @@ -0,0 +1,46 @@ +# Job-container image for the LAN Forgejo Actions runner's `docker`-labeled +# jobs (see ../register-lan-runner.sh) — used by every backend/frontend +# build-push, deploy, and validation workflow that needs `docker build`. +# +# Base stays node:20-bookworm, NOT a Node-free slim image (v1 of this file +# tried that and broke every job: `actions/checkout@v4` is a JS action that +# Forgejo's runner execs as `node dist/index.js` *inside the job container*, +# so a Node runtime is a hard requirement regardless of whether the workflow +# itself uses npm/node — this repo's own workflows don't, but the checkout +# step every one of them starts with does). +# +# What this image actually fixes: every workflow using the `docker` label +# currently re-provisions the Docker CLI on each run via `apt-get install +# docker.io` — that step costs ~15-20s per job and, more importantly, +# installs the CLASSIC Docker builder rather than BuildKit, which mishandles +# `COPY --chown=` group resolution on images without a matching system +# group (a real bug hit during the frontend Go rewrite). Installing +# docker-buildx-plugin here makes BuildKit the default, closing that bug +# class, and skips the per-job install entirely. +# +# Build/push (manual — see ../README.md for when to rebuild): +# docker build -t git.thermograph.org/emi/thermograph/ci-runner:vN \ +# infra/deploy/forgejo/ci-runner +# docker push git.thermograph.org/emi/thermograph/ci-runner:vN +# +# Cutting over the live runner to a new tag means editing the `labels` array +# in ~/forgejo-runner/.runner on the runner host directly (same runner +# id/token, no re-registration needed) and updating register-lan-runner.sh's +# LABELS default so future re-provisioning picks it up too. +FROM node:20-bookworm + +RUN apt-get update -qq \ + && apt-get install -y -qq --no-install-recommends \ + ca-certificates curl gnupg git python3 python3-yaml \ + && install -m 0755 -d /etc/apt/keyrings \ + && curl -fsSL https://download.docker.com/linux/debian/gpg -o /etc/apt/keyrings/docker.asc \ + && chmod a+r /etc/apt/keyrings/docker.asc \ + && echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.asc] https://download.docker.com/linux/debian bookworm stable" \ + > /etc/apt/sources.list.d/docker.list \ + && apt-get update -qq \ + && apt-get install -y -qq --no-install-recommends docker-ce-cli docker-buildx-plugin \ + && rm -rf /var/lib/apt/lists/* /etc/apt/sources.list.d/docker.list + +RUN docker --version && docker buildx version && git --version \ + && node --version \ + && python3 -c "import yaml; print('PyYAML', yaml.__version__)" diff --git a/infra/deploy/forgejo/docker-stack.yml b/infra/deploy/forgejo/docker-stack.yml index 9d94cbd..9891c3b 100644 --- a/infra/deploy/forgejo/docker-stack.yml +++ b/infra/deploy/forgejo/docker-stack.yml @@ -48,6 +48,10 @@ services: deploy: placement: constraints: [node.labels.role == forge] + resources: + limits: + cpus: "${FORGEJO_DB_CPUS:-1}" + memory: ${FORGEJO_DB_MEMORY:-1g} restart_policy: condition: on-failure @@ -131,6 +135,10 @@ services: deploy: placement: constraints: [node.labels.role == forge] + resources: + limits: + cpus: "${FORGEJO_CPUS:-2}" + memory: ${FORGEJO_MEMORY:-2g} restart_policy: condition: on-failure diff --git a/infra/deploy/forgejo/register-lan-runner.sh b/infra/deploy/forgejo/register-lan-runner.sh index 71f1c90..c35660d 100755 --- a/infra/deploy/forgejo/register-lan-runner.sh +++ b/infra/deploy/forgejo/register-lan-runner.sh @@ -26,7 +26,7 @@ set -euo pipefail FORGEJO_URL="${1:?usage: $0 }" TOKEN="${2:?}" RUNNER_DIR="${RUNNER_DIR:-$HOME/forgejo-runner}" -LABELS="${LABELS:-docker:docker://node:20-bookworm,thermograph-lan}" +LABELS="${LABELS:-docker:docker://git.thermograph.org/emi/thermograph/ci-runner:v2,thermograph-lan}" echo "==> Stopping and disabling the old GitHub Actions runner service, if present" systemctl --user stop github-actions-runner 2>/dev/null || true From 578fb563a4ab44de892cf9d47c47f5c33f325dac Mon Sep 17 00:00:00 2001 From: emi Date: Fri, 24 Jul 2026 18:59:46 +0000 Subject: [PATCH 5/5] Roll the daemon with backend deploys in the Swarm stack (#40) --- infra/deploy/stack/deploy-stack.sh | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/infra/deploy/stack/deploy-stack.sh b/infra/deploy/stack/deploy-stack.sh index 15d8a99..c3badc2 100755 --- a/infra/deploy/stack/deploy-stack.sh +++ b/infra/deploy/stack/deploy-stack.sh @@ -166,13 +166,19 @@ else echo "==> Rolling web + worker + lake to $BACKEND_IMAGE" docker service update --with-registry-auth --detach=false --image "$BACKEND_IMAGE" "${STACK_NAME}_web" docker service update --with-registry-auth --detach=false --image "$BACKEND_IMAGE" "${STACK_NAME}_worker" - # lake ships in the same image; a stack file predating it has no service - # yet — the next SERVICE=all stack deploy creates it, so don't fail here. - if docker service inspect "${STACK_NAME}_lake" >/dev/null 2>&1; then - docker service update --with-registry-auth --detach=false --image "$BACKEND_IMAGE" "${STACK_NAME}_lake" - else - echo " (no ${STACK_NAME}_lake service yet; created on the next full stack deploy)" - fi + # lake and daemon ship in the same image; a stack file predating either + # has no service yet — the next SERVICE=all stack deploy creates it, so + # don't fail here. The daemon especially must roll with web: they share + # the /internal/* contract, and a version skew between them is exactly + # what pinning one BACKEND_IMAGE_TAG exists to prevent (seen live: the + # first post-creation backend roll left the daemon a release behind). + for extra in lake daemon; do + if docker service inspect "${STACK_NAME}_${extra}" >/dev/null 2>&1; then + docker service update --with-registry-auth --detach=false --image "$BACKEND_IMAGE" "${STACK_NAME}_${extra}" + else + echo " (no ${STACK_NAME}_${extra} service yet; created on the next full stack deploy)" + fi + done ;; frontend) echo "==> Rolling frontend to $FRONTEND_IMAGE"