Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
This commit is contained in:
@@ -1,99 +1,84 @@
|
||||
# REPORT — controller v0.260.0: a box ahead of the catalog, and a pin that never moves backwards
|
||||
# REPORT — controller v0.261.0: the box stops updating itself out from under an app update
|
||||
|
||||
**R-524 (P2).** Base `19ef0329ab66` → **v0.260.0** (`8f8a64cad7a5`). MinAgent 0.131.0 unchanged.
|
||||
Architecture read first and named: `felhom.eu/documentation/architecture/09-update-architecture.md`
|
||||
(§3 the nine decisions, §5.4 the render table, §6.1 slice 4 as shipped, §8 the limitations).
|
||||
**R-608 (P2) + R-609 (P3).** Base `d0d431b42b51` (v0.260.0) → **v0.261.0** (`811f75736ec8`).
|
||||
MinAgent 0.131.0 unchanged. Architecture read first and named:
|
||||
`felhom.eu/documentation/architecture/09-update-architecture.md` §3 (the ten decisions), §3b (the
|
||||
seven open questions), §6.1 (the guarded update's phases), §6.2.
|
||||
|
||||
## Not done / changed from the brief — first, because it is the point
|
||||
|
||||
| item | state |
|
||||
|---|---|
|
||||
| Part 2 (the lock, the reason on the wire, the red-proofs) | **done in full** |
|
||||
| Part 0 (floor to 0.260.0) | **done** — see `felhom.eu/REPORT.md` |
|
||||
| Parts 1, 3, 4 | **not in this repo** — they are measurements and documents; see `felhom.eu` |
|
||||
| the caller script's ≤150-line budget | **167 lines.** Over by 17. Not trimmed: the excess is the `same_major` rule and its comment, and shortening it would have meant a terser rule, which is the one thing in that file that must be readable. Named rather than hidden. |
|
||||
|
||||
## What was wrong
|
||||
|
||||
**Measured, not imagined** — BIGNIGHT Phase 6, 2026-09-15, VM 333. privatebin was updated
|
||||
2.0.5 → 2.0.6 through the guarded Update; the catalog was then reverted to 2.0.5. At 22:13:37Z the
|
||||
box read `installed privatebin/pdo:2.0.6`, `catalog privatebin/pdo:2.0.5`, and the app page showed
|
||||
„**Frissítés elérhető — ma**" with a title inviting the household to press Frissítés. The comparison
|
||||
asked only *does the installed reference DIFFER?*, so **a catalog revert — an operator act on our
|
||||
side — presented itself to a customer as an update**, and the guarded Update behind it would have
|
||||
advanced the pin 2.0.6 → 2.0.5, onto a datadir the newer version may already have migrated, with §4's
|
||||
ruling saying that cannot be undone.
|
||||
The controller updates **itself** — daily at `self_update.auto_update_time`, **default 04:30**
|
||||
(`config/config.go` L422, scheduled `cmd/controller/main.go` ~L1365), and again from
|
||||
`MaybeAutoUpdate` after **any** hub report once a floor sits above the box, so at any hour. That swap
|
||||
restarts the controller container, which is the supervisor of a running app update. `09` §3b Q1
|
||||
proposes **02:30–05:00** for automatic app updates. **It contains 04:30.**
|
||||
|
||||
## What the brief got wrong, measured rather than assumed
|
||||
|
||||
**The gap was narrower than "the whole update", and that matters for where the fix goes.** The brief
|
||||
said the self-updater's only busy gate is `backupRunning` and left open whether that covers the
|
||||
update's `backing-up` phase. **It does:** `RunAppBackupNow` calls `acquireRunning`
|
||||
(`internal/backup/update_guard.go:333`), so `backupMgr.IsRunning()` was already true for that one
|
||||
phase. It was false for `checking`, `safety-dump`, `pinning`, `pulling`, `starting` and `verifying` —
|
||||
and the last two are exactly where the new version may already have touched the customer's data.
|
||||
Everything else the brief asserted about the three call sites and the wiring held at source.
|
||||
|
||||
## What shipped
|
||||
|
||||
- **`stacks.CatalogOrder`** (`controller/internal/stacks/updateorder.go`) — the comparison gains a
|
||||
fourth verdict (Unknown / Current / Behind / **Ahead**) and **moves out of `web`**. That move is the
|
||||
substance: two callers must reach the same verdict — the badge and `Manager.UpdatePreflight` — and a
|
||||
comparison implemented twice is a comparison that drifts. `web.compareInstalledToTemplate` is now a
|
||||
thin wrapper and keeps every property it had (absent means UNKNOWN and never „Naprakész"; it reads
|
||||
`CatalogImages` and never `TemplateImages`; it queries no registry).
|
||||
- **The badge.** Ahead reads „Naprakész" / "Up to date", `tag-ok` — the same word and class as level,
|
||||
because there is nothing for the household to do — with a title that says why
|
||||
(`badge.update.ahead.title`, born as a key in both bundles). No version number reaches the customer.
|
||||
- **The refusal.** `UpdatePreflight` returns reason `downgrade`, HTTP 409,
|
||||
„Ez a változat újabb a katalógusban lévőnél — visszalépés csak az üzemeltető kérésére.", logged with
|
||||
both image maps. **The API now renders update refusals through `errText`** — without that one line
|
||||
the new key would have been a seam built and never wired, which is a documented failure class here.
|
||||
- **Ahead is the NARROW arm.** Every differing service must be orderable AND newer; one older, one
|
||||
unorderable, and the verdict falls back to Behind — i.e. to v0.233.0..v0.259.0 behaviour. This gate
|
||||
can BLOCK an update, so it errs towards letting one run.
|
||||
- **Ordering is `util.Version.Compare` and nothing else** (house rule: one comparator). The new code
|
||||
is a tag NORMALISER in front of it.
|
||||
- **`stacks.Manager.AnyUpdating()`** — is a guarded update in flight for ANY app.
|
||||
- **`Updater.SetAppUpdatingCheck`** — a deliberate sibling of `SetBackupRunningCheck`, consulted in
|
||||
the **same three places** (the dry run, `TriggerUpdate`, `maybeAutoUpdate`). One busy-gate pattern
|
||||
in that file, not two.
|
||||
- **`Manager.SetSelfUpdatingCheck`** — the reverse. `UpdatePreflight` refuses `self_updating`.
|
||||
- **Both halves wired in `main.go`**, the only place holding both objects. **`stacks` never imports
|
||||
`selfupdate`** — the dependency is inverted with a plain callback rather than by widening
|
||||
`UpdateGuards`, which is the backup side's interface and has nothing to do with this.
|
||||
- **Two sentences, born as bundle keys**, in both bundles, registered in `i18n_go_keys.json`.
|
||||
- **`data.reason` on every update refusal (R-609)** — additive; the sentence is unchanged, so no page
|
||||
moves. `busy`/`updating`/`deploying`/`migrating`/`self_updating` are transient;
|
||||
`held`/`downgrade` terminal; `memory`/`disk`/`no_backup` need a person.
|
||||
|
||||
## What the fixture caught that the design did not
|
||||
## The two things the tests found that reading did not
|
||||
|
||||
**The first implementation called every real catalog tag unorderable.** It accepted only bare
|
||||
`X.Y`/`X.Y.Z`, and the test fixture uses `nextcloud:31.0.14-apache` — the real catalog pin. The
|
||||
refusal test failed with `got nil`, and the cause was the code being right about a rule that was
|
||||
wrong. The rule now takes the version at the FRONT of the tag and requires the trailing suffix to be
|
||||
**identical on both sides**, so `31.0.14-apache → 31.0.15-apache` orders while `26.05.2-ls310 →
|
||||
-ls311` (a build number with no rule), `postgres:16-alpine` (a major LINE, not a version),
|
||||
`kimai/kimai2:apache-2.57.0` (version at the back), a date stamp and a digest pin all stay
|
||||
unorderable. **Measured against the real catalog: 8 of 66 pins float and one puts its version last.**
|
||||
1. **The router refuses a HELD app on its own line, BEFORE `UpdatePreflight`** (`api/router.go`
|
||||
~L601). Without a second edit, `held` — the single reason an unattended caller most needs — would
|
||||
have been the one missing from the wire, and such a caller would press a terminally-refused button
|
||||
on every pass for ever. Found while writing the table, not while reading the code.
|
||||
2. **The lock must not latch.** `Stack.Updating` is cleared on done, failed **and held**, so a held
|
||||
app does not block the controller's own updates — including the release that might fix whatever
|
||||
held it. A latching gate would be a worse failure than the one prevented, and silent for weeks.
|
||||
`TestR608_LockReleasesAfterHold` is a consequence test and exists for that alone.
|
||||
|
||||
## Red-proofs — three, each SEEN to fail
|
||||
## Red-proofs — five, each SEEN to fail
|
||||
|
||||
| # | the mutation | what failed |
|
||||
|---|---|---|
|
||||
| 1 | make `CatalogOrder`'s Ahead arm unreachable | `TestR524_PreflightRefusesDowngrade/ahead` — *"this update must be REFUSED with reason \"downgrade\", got nil"*. The update is ALLOWED and the next thing it does is move the pin back. |
|
||||
| 2 | treat an UNORDERABLE pair as ahead (`cmp < 0` with the `ok` dropped) | the floating-tag, different-image and digest-pin cases all fail with Ahead — the verdict that would suppress a real „Frissítés elérhető" on the floating pins |
|
||||
| 3 | delete the ahead arm from `localeFuncs` | `TestUpdateBadgeFollowsTheLanguage` — *"an app ahead of the catalog must carry a badge"* |
|
||||
| 1 | `AnyUpdating` always false | `TestR608_AnyUpdatingSeesAnUpdateInFlight` — the gate reads false with an update in flight |
|
||||
| 2 | `AnyUpdating` true for a held app too | `TestR608_LockReleasesAfterHold` — *"a HELD app must not hold the self-update lock for ever"* |
|
||||
| 3 | delete the preflight's `self_updating` block | `TestR608_PreflightRefuses…` — *"while the controller swaps itself the app update must be REFUSED"* |
|
||||
| 4 | delete `TriggerUpdate`'s app-update block | `TestR608_TriggerUpdateRefused…` — the swap proceeds over a live app update |
|
||||
| 5 | drop `Data` from the refusal | `TestR609_EveryRefusalCarriesItsReason` — `reason = ""` on `no_backup` and `self_updating` |
|
||||
|
||||
An honest note on #1: the first attempt deleted the preflight block and failed to BUILD (an unused
|
||||
import), which proves nothing. It was redone by neutering the Ahead arm instead, and that failed for
|
||||
the right reason.
|
||||
## Green gate
|
||||
|
||||
## A claim in the brief that was wrong, named
|
||||
`go build ./... && go vet ./... && go test ./...` — **all green, full suite.**
|
||||
`controller_gates.py --fast` — **17 gates OK**; `go-parity` convicted the two new keys first and was
|
||||
satisfied properly by registering both as BORN-AS-KEYS with the test that pins each. No
|
||||
`--no-verify`; the pre-push hook ran and passed.
|
||||
|
||||
**R-589 was NOT open.** The brief said, reviewer-verified, that `updatebadge.go` builds the badge
|
||||
from four raw Hungarian literals with no key and that R-589 is therefore open. The literals are real;
|
||||
the conclusion does not follow. They are the Hungarian form and are deliberately frozen — that IS the
|
||||
parity guarantee — and the ENGLISH form has been rebuilt from the bundle in `web.localeFuncs` since
|
||||
**v0.258.0**, pinned by `TestUpdateBadgeFollowsTheLanguage` and proven live on a fresh box the same
|
||||
morning (`DRILL-first-hour-en-0258-2026-09-20.md` item 9). The register row was stale, not the code;
|
||||
it is closed with that citation. **The general lesson: a reviewer who reads one producer cannot see a
|
||||
second producer that overrides it.** `updatebadge.go` now says so in its own comment, and v0.260.0's
|
||||
new arm was written into BOTH producers with a test that fails if either is missing.
|
||||
Image `gitea.dooplex.hu/admin/felhom-controller:0.261.0` built and pushed.
|
||||
**Deployed to guest 9202 (the scratch guest) ONLY** — the demo boxes and the fleet stay on 0.260.0.
|
||||
**The floor is NOT raised to 0.261.0**; that is a separate ask for the operator, recorded in
|
||||
`felhom.eu/STATUS.md`.
|
||||
|
||||
## Green gate and delivery
|
||||
|
||||
`go build ./... && go vet ./... && go test ./...` — **all green** (full suite, not a subset).
|
||||
`controller_gates.py --fast` — **17 gates, all OK**; `go-parity` convicted first and was satisfied
|
||||
properly, by registering both new keys as BORN-AS-KEYS in `i18n_go_keys.json` with the test that pins
|
||||
each. No `--no-verify`; the pre-push hook ran and passed.
|
||||
|
||||
Image `gitea.dooplex.hu/admin/felhom-controller:0.260.0` built and pushed. **Deployed and healthy on
|
||||
three guests**: demo-felhom 9201, demo-hp 9201, and demo-hp 9202 (the scratch guest, upgraded from
|
||||
0.245.0 for the live proof).
|
||||
|
||||
**The fleet floor was NOT raised, and the number in the repo's own CONTEXT was stale.** CONTEXT.md
|
||||
said the floor stood at 0.257.0; **the hub says 0.259.0** (read live from `/configs`, not from a
|
||||
document — the operator raised it after that entry was written, which `felhom.eu/STATUS.md` records).
|
||||
v0.260.0 therefore reaches the two demo boxes by hand and **no further**.
|
||||
|
||||
Stated rather than silently skipped: the standing rule asks for the floor to be raised to deliver a
|
||||
release, and this session deliberately did not. **Why:** a floor raise reaches `peti-felhom`, a real
|
||||
customer box, and the unprompted-work fence puts anything that changes risk to customer data behind
|
||||
an operator word. The two preceding sessions recorded the raise as the operator's own act, and the
|
||||
one that happened came after the operator asked for it. Put to the operator with what happens if they
|
||||
do nothing: the fix stays on the two demo boxes and the rest of the fleet keeps offering a downgrade
|
||||
as an update — which is a badge and a button, not data at risk, so waiting costs little.
|
||||
|
||||
Live proof, drift numbers and the state of the whole arc:
|
||||
`felhom.eu/documentation/audits/UPDATE-ARC-STATE-2026-09-21.md` and `audits/update-arc-2026-09-21/`.
|
||||
The measurements this release was built for — the three power cuts and the unattended night — are in
|
||||
`felhom.eu/documentation/audits/update-arc-gaps-2026-09-21/`.
|
||||
|
||||
Reference in New Issue
Block a user