Files
felhom-controller/controller/internal/backup/runsummary_test.go
T
admin 88897a224e
gates / gates (push) Successful in 8s
v0.194.0 — one operator email per backup run, and nothing dropped without a trace (R-182)
MEASURED, not supposed. On 2026-08-03 nine per-app recovery_unit_capture_failed
events reached the hub and TWO operator emails went out. The hub's operator
cooldown key is customerID:eventType(+tier) and that event carries `app` but no
`tier`, so the key held no app identifier: the first refused app took the hour's
slot and every other app's failure was discarded BEFORE anything was written
down, leaving no row on any channel.

The obvious fix — put `app` in the key — was ruled against: on a full disk it
produces one email per app, the volume problem wearing the correctness problem's
clothes.

internal/backup/runsummary.go: a per-run collector with exactly admissionSet's
lifetime, fed by all three write legs, emitting backup_run_failures ONCE at the
end and only when something failed. A clean run emits nothing.

The per-app event stays and becomes the RECORD — the hub routes it record-only,
stored and logged every time, never competing for an email slot. The record and
the notification are now different things.

Deliberate skips (disconnected, decommissioned) are excluded: they have their
own alert, and a nightly email about an unplugged drive is one the operator
learns to ignore.

A manual run always reports: the digest carries a unique run_id the cooldown
cannot collapse. Someone pressing the button is actively trying to get a backup.

THE PERIODIC SWEEP GETS A DIGEST TOO. With the per-app event now record-only, a
capture failure found between runs would be recorded and never notified — a new
silence introduced while closing one. That path emits a digest with NO run_id,
so the ordinary 1-hour cooldown caps it exactly as before while the mail now
lists every failing app instead of whichever was first.

A refusal is recorded ONCE, where the verdict is taken, not at the three legs
that consult it — R-181's contract is one verdict per app per run. Noting it per
leg listed one refused app three times and produced "2 of 1 apps failed". Found
by the digest's own test, not in review.

Silence is safe because the hub's deadline check raises expected_backup_missed
from report freshness, independently of any mail this box sends
(monitor/deadline.go:396,417). Confirmed, not assumed.

7 new tests, 4 red-proofs. The main.go seam walk did NOT fail on its first
attempt — the AST test walked the backup package and not main.go; the test was
fixed and the mutation re-run rather than the pass recorded.
2026-08-03 13:46:14 +02:00

271 lines
11 KiB
Go

package backup
import (
"go/ast"
"go/parser"
"go/token"
"strings"
"testing"
)
// R-182 — one digest per run, listing every failure, and nothing in it that is not a failure.
//
// The assertions here are the CONTENT of the digest, not that a function was called: the defect this
// replaces was one where the machinery ran correctly and the operator was told about one app out of
// nine.
// digestOf runs the two legs that can run without Docker inside one digest scope and returns the
// summary that would have been sent (nil when none). It mirrors what runDBDumpsInternal does; the
// DB leg's wiring is pinned structurally by TestRunSummary_IsWiredIntoTheProductionPath, because
// DiscoverDatabases shells out to `docker`.
func (h *admissionHarness) digestOf(kind string) *RunSummary {
var got *RunSummary
h.m.SetRunSummaryNotify(func(rs RunSummary) { got = &rs })
doneAdm := h.m.beginAdmissionRun()
doneSum := h.m.beginRunSummary(kind, "run-test")
h.m.runVolumeDumps()
h.m.captureAllRecoveryUnits()
h.m.emitRunSummary()
doneSum()
doneAdm()
return got
}
// ── Scenario A — several failures, ONE digest, all of them in it ─────────────────────────────────
func TestRunSummary_ListsEveryFailedApp(t *testing.T) {
apps := []string{"opengist", "privatebin", "immich", "homebox", "nextcloud"}
h := newAdmissionHarness(t, apps...)
for _, a := range apps {
h.setSpace(a, 99, 0.1, 70) // every app refused by the reserve
}
rs := h.digestOf(runKindNightly)
if rs == nil {
t.Fatal("no digest was emitted although five apps failed — this is the 2026-08-03 defect: " +
"nine failures arrived and two e-mails went out")
}
if rs.Failed != len(apps) {
t.Fatalf("digest reports %d failures, want %d — the others were silently discarded, which is "+
"exactly what the per-app path did", rs.Failed, len(apps))
}
seen := map[string]bool{}
for _, a := range rs.Apps {
seen[a.App] = true
if a.Leg == "" {
t.Errorf("%s has no leg name — the operator cannot tell a failed database dump from a "+
"failed volume dump, and they are different problems", a.App)
}
if !strings.Contains(a.Reason, "reserve") {
t.Errorf("%s reason %q does not say why", a.App, a.Reason)
}
}
for _, a := range apps {
if !seen[a] {
t.Errorf("%s is missing from the digest", a)
}
}
// The denominator: "5 of 5" and "5 of 40" are different nights.
if rs.Attempted < len(apps) {
t.Fatalf("attempted=%d, want >= %d — without the denominator a catastrophe and a bad night "+
"read identically", rs.Attempted, len(apps))
}
if rs.Usage == nil {
t.Fatal("the digest carries no filesystem figures — 'one broken app' and 'a full disk' must " +
"be distinguishable before the reasons are read")
}
if !strings.Contains(rs.Message, "of") || !strings.Contains(rs.Message, "nightly") {
t.Fatalf("summary message is not readable as a sentence: %q", rs.Message)
}
}
// ── Scenario B — a clean run is silent ───────────────────────────────────────────────────────────
func TestRunSummary_CleanRunEmitsNothing(t *testing.T) {
h := newAdmissionHarness(t, "opengist", "privatebin")
h.setSpace("opengist", 20, 55, 70)
h.setSpace("privatebin", 20, 55, 70)
if rs := h.digestOf(runKindNightly); rs != nil {
t.Fatalf("a clean run emitted a digest (%+v) — an empty nightly mail is an unread mail within "+
"a week, and it would destroy the property this design rests on: that silence means the "+
"run finished and found nothing wrong", rs)
}
}
// ── Scenario F — deliberate skips are NOT failures ───────────────────────────────────────────────
// A drive that is unplugged or decommissioned has its own alert. Putting it in the digest produces a
// nightly e-mail on a box with an unplugged drive, which trains the operator to ignore digests — and
// an ignored digest is the same outcome as no digest.
func TestRunSummary_DeliberateSkipsAreNotFailures(t *testing.T) {
h := newAdmissionHarness(t, "gone", "retired", "healthy")
for _, a := range []string{"gone", "retired", "healthy"} {
h.setSpace(a, 20, 55, 70) // ample room — nothing may be refused for headroom
}
h.markDisconnected("gone")
h.markDecommissioned("retired")
rs := h.digestOf(runKindNightly)
if rs != nil {
t.Fatalf("a run whose only 'issues' were a disconnected drive and a decommissioned one "+
"emitted a digest: %+v — those have their own alerts", rs)
}
}
// ── The collector's lifetime — the admission.go rule, restated ───────────────────────────────────
// An absent collector must mean "no run in flight", never a stale answer from last night.
func TestRunSummary_ResetsBetweenRuns(t *testing.T) {
h := newAdmissionHarness(t, "opengist")
h.setSpace("opengist", 99, 0.1, 70)
if rs := h.digestOf(runKindNightly); rs == nil || rs.Failed != 1 {
t.Fatalf("run 1: want 1 failure, got %+v", rs)
}
h.setSpace("opengist", 20, 55, 70) // space freed between runs
if rs := h.digestOf(runKindNightly); rs != nil {
t.Fatalf("run 2 reported %+v — the previous run's failures were carried over, so freeing "+
"space could never take effect", rs)
}
}
// A failure noted with NO run in flight must not panic and must not accumulate anywhere.
func TestRunSummary_NoteOutsideARunIsInert(t *testing.T) {
h := newAdmissionHarness(t, "opengist")
h.m.noteFailure("opengist", "volume dump", "boom") // no scope open
h.m.noteAttempted("opengist")
h.m.emitRunSummary() // must be a no-op, not a nil dereference
}
// ── The refresh sweep carries NO run id, deliberately ────────────────────────────────────────────
// The run digest gets a unique id so the hub's 1-hour cooldown can never collapse two real runs. The
// periodic sweep is the opposite case — it can fire on every status poll — so it must fall UNDER the
// cooldown. Getting this backwards turns a full disk into a mail flood, which is the same failure as
// the one being fixed, arriving from the other side.
func TestRunSummary_RefreshSweepHasNoRunID(t *testing.T) {
h := newAdmissionHarness(t, "opengist")
h.setSpace("opengist", 99, 0.1, 70)
var got *RunSummary
h.m.SetRunSummaryNotify(func(rs RunSummary) { got = &rs })
func() {
defer h.m.beginAdmissionRun()()
defer h.m.beginRunSummary(runKindRefresh, "")()
defer h.m.emitRunSummary()
h.m.captureAllRecoveryUnits()
}()
if got == nil {
t.Fatal("the periodic sweep emitted no digest — with the per-app event now record-only, a " +
"capture failure found between runs would be recorded and NEVER notified, which is a " +
"new silence introduced while closing one")
}
if got.RunID != "" {
t.Fatalf("the refresh sweep carries run_id=%q — it must be EMPTY so the hub's ordinary "+
"1-hour cooldown caps it, or a polled status page becomes a mail flood", got.RunID)
}
if got.RunKind != runKindRefresh {
t.Fatalf("run kind = %q, want %q", got.RunKind, runKindRefresh)
}
}
// ── The seam is WIRED — walked as an AST, not grepped ────────────────────────────────────────────
// Four mechanisms in this project have been built and left disconnected. `strings.Contains` cannot
// tell a live call from a commented-out one, so this parses.
func TestRunSummary_IsWiredIntoTheProductionPath(t *testing.T) {
calls := callsByFunc(t, "backup.go")
if !hasStr(calls["runDBDumpsInternal"], "beginRunSummary") {
t.Fatal("runDBDumpsInternal does not open a run-summary scope — every failure would be " +
"recorded per app and none of them summarised, which is the pre-R-182 behaviour")
}
if !hasStr(calls["runDBDumpsInternal"], "emitRunSummary") {
t.Fatal("runDBDumpsInternal never emits the summary — the collector fills and is discarded")
}
// Both dump legs and the capture leg must feed it, or a whole class of failure is invisible.
for fn, where := range map[string]string{
"runDBDumpsInternal": "the database leg",
"runVolumeDumps": "the volume leg",
} {
if !hasStr(calls[fn], "noteFailure") {
t.Fatalf("%s (%s) never calls noteFailure — its failures cannot reach the digest", fn, where)
}
}
capCalls := callsByFunc(t, "recovery_unit.go")
if !hasStr(capCalls["captureAllRecoveryUnits"], "noteFailure") {
t.Fatal("the capture leg never calls noteFailure")
}
// AND THE SEAM ITSELF must be wired in main.go. Without this assertion the collector fills, the
// digest is built, and `runSummaryNotify` is nil — so nothing is ever sent and every test above
// still passes. That is the built-but-never-wired shape exactly, and the first version of this
// test missed it: commenting the wiring out left the suite green.
mainCalls := callsByFunc(t, "../../cmd/controller/main.go")
wired := false
for _, calls := range mainCalls {
if hasStr(calls, "SetRunSummaryNotify") {
wired = true
}
}
if !wired {
t.Fatal("main.go never calls SetRunSummaryNotify — the digest is assembled and handed to a " +
"nil seam, so no operator mail is ever sent. Note this is an AST walk: a commented-out " +
"call still CONTAINS the string, which is why strings.Contains cannot be used here")
}
// The manual paths must tag the run, or Scenario E cannot hold.
for _, f := range []string{"../web/handler_debug.go", "../api/router.go"} {
src := parseFile(t, f)
found := false
ast.Inspect(src, func(n ast.Node) bool {
if ce, ok := n.(*ast.CallExpr); ok {
if sel, ok := ce.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "MarkManualRun" {
found = true
}
}
return true
})
if !found {
t.Fatalf("%s never calls MarkManualRun — an operator-triggered run would be labelled "+
"nightly and could be collapsed into it", f)
}
}
}
func parseFile(t *testing.T, path string) *ast.File {
t.Helper()
f, err := parser.ParseFile(token.NewFileSet(), path, nil, 0) // comments dropped
if err != nil {
t.Fatal(err)
}
return f
}
// callsByFunc maps each top-level func to the names it calls, comments excluded.
func callsByFunc(t *testing.T, path string) map[string][]string {
t.Helper()
file := parseFile(t, path)
out := map[string][]string{}
var cur string
ast.Inspect(file, func(n ast.Node) bool {
switch v := n.(type) {
case *ast.FuncDecl:
cur = v.Name.Name
case *ast.CallExpr:
name := ""
switch fn := v.Fun.(type) {
case *ast.Ident:
name = fn.Name
case *ast.SelectorExpr:
name = fn.Sel.Name
}
if name != "" && cur != "" {
out[cur] = append(out[cur], name)
}
}
return true
})
return out
}