diff --git a/CHANGELOG.md b/CHANGELOG.md index cce9b25..f316474 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,75 @@ +## v0.218.0 — the database nobody backed up, and the restore that returned most apps nothing (2026-08-22, R-354/R-355) +**MinAgent: 0.129.0** (unchanged — no new agent coupling) + +Both fixes were found by watching a real machine on the night of 2026-08-21, and both are confirmed +the same way. Neither is a refactor: each removes a case where the product told a customer something +that was not true about their own data. + +**The database that was dumped and then abandoned (R-355).** `paperless-ngx` runs its PostgreSQL in a +container called `paperless-postgres`. `deriveStackName` (`internal/appbackup/dbdump.go:770`) strips the +role suffix to `paperless`, finds that is not a deployed stack, finds no known stack is a prefix of the +container name — **and then returns the unresolved candidate anyway**. The nightly dump therefore landed +in `backups/primary/paperless/db-dumps/` — a directory for an app that does not exist, on the SYSTEM +drive — while the app's own recovery unit, on the data drive, recorded `db_dumps: null`. Nothing +collected it, nothing off-sited it, nothing restored it. Measured live: **284 617 bytes, 72 tables, +valid, unreachable**, and reproduced unattended by the box's own 02:30 cycle. + +**And it compounded, which is why it went first.** `writeSafetyDump` filters the discovered databases +with the same value, so a destructive restore of that app found no database, **took no undo copy**, and +the fail-closed refusal that protects every other app could not fire — it was never reached. A customer +could press restore, lose the live database and have no copy anywhere. Verified live on 2026-08-21: +`find /mnt -name "pre-restore-*"` was empty both before and after a full restore over a live 72-table +PostgreSQL. + +**The fix is to stop guessing.** Every container the controller starts carries +`com.docker.compose.project`, and that label IS the stack name by construction: compose is run with +`cmd.Dir` set to `/opt/docker/stacks/` and no `-p` (`internal/stacks/manager.go:1218`). The label +is now read and preferred whenever it names a deployed stack; the old derivation stays as the fallback +for containers not started by compose, and an attribution that resolves to no known stack is now **loud** +instead of silent. **A catalogue-wide sweep — proven able to convict by planting a second mismatch, +watching it caught, removing it and watching it clear — reports exactly one affected app of 53.** The +fix is in the controller, not the catalogue: renaming the container would have fixed this one app and +left the guessing in place for the next. + +**The restore that returned most apps nothing (R-354).** `ReconstituteFromOffsite` skipped every +placement flagged as the unit, and the named-volume archives live INSIDE the unit — so the off-site +restore had a files leg and a database leg and **no volume leg at all**. Proven live with planted, +hash-recorded files: calibre-web's `calibre_web_config.tar` was in the unit, in the off-site snapshot +and in the verification folder, and the restore returned the five declared files, reported success, and +did not replay it. **For the 40 of 53 catalogue apps that declare no data drive, that archive is +everything the customer owns.** + +**The comment beside the skip was half false and is corrected rather than left.** It justified the skip +by saying the snapshot's dump is replayed from the scratch unit so nothing is lost — true of the +database, false of the volumes, and the reason a reader would not look. **The half that still holds is +named:** the live unit is the LOCAL restore path's own source and must never be clobbered. Scenario D +now fingerprints the whole live unit across the operation and compares. + +`restoreDockerVolumesFrom` is the local path's own replay with an explicit directory — the same shape +`reimportDBDumpsFrom` already had, and deliberately ONE implementation with two callers, because a +second copy of that loop is what produced the divergence. Volumes replay **before** the database, so a +logical dump still wins over a volume-tar copy of the same database, and inside the stopped window, +because Docker will not replace a volume a container holds. + +**And it reaches the sentence.** `VolumesReplayed` is on the result and in the message: „5 fájl **és 1 +adatkötet** visszaállítva". A restore that replayed an app's entire dataset and mentioned only its file +count is how a silent loss reads as a success. A snapshot with no volumes produces the byte-identical +sentence it produced before. + +**„Ennek az alkalmazásnak nincs adatbázisa" is no longer inferred from a counter.** `DBsReplayed == 0` +has two causes — the app has none, or it has one and the snapshot carried no dump — and both printed the +same confident sentence over a live 72-table database. The undo copy is the honest discriminator, and +the second case now says so and names the undo. + +**Seven red-proofs, each mutation asserted applied and reverted.** The name fix reverted returned the +empty database record with both divergent paths printed; the message predicate reverted returned the +false „nincs adatbázisa" sentence verbatim; the refusal removed was seen letting a restore proceed with +no undo; the volume leg removed returned the silent loss; the volume count dropped returned the +true-but-incomplete sentence verbatim. **Two of them found defects in the tests rather than the code:** +scenario D PASSED with the unit guard removed, because the fingerprint had been narrowed to the volume +directory and was blind to a placement writing into the unit root — the R-181 class, in my own test, and +the reason the red-proof is mandatory. + ## v0.217.0 — the restore knows where the data lived (2026-08-21, R-351/R-352/R-353) **MinAgent: 0.129.0** (unchanged — no new agent coupling) diff --git a/controller/internal/appbackup/dbdump.go b/controller/internal/appbackup/dbdump.go index a22d88d..aa96d07 100644 --- a/controller/internal/appbackup/dbdump.go +++ b/controller/internal/appbackup/dbdump.go @@ -91,7 +91,17 @@ func DiscoverDatabases(ctx context.Context, logger *log.Logger, debug bool, know if debug { logger.Printf("[DEBUG] DiscoverDatabases: running docker ps to find database containers") } - cmd := exec.CommandContext(ctx, "docker", "ps", "--format", "{{.ID}}\t{{.Names}}\t{{.Image}}", "--filter", "status=running") + // R-355: the compose PROJECT label is asked for, because it is the stack name BY CONSTRUCTION and + // the container name is only a guess at it. The controller runs compose with `cmd.Dir` set to + // `/opt/docker/stacks/` and never passes `-p` (stacks/manager.go:1218-1226), so compose + // derives the project from that directory — i.e. from the stack name itself. + // + // The label sits in the MIDDLE of the format on purpose: `strings.TrimSpace` is applied to the + // whole `docker ps` output, so a trailing empty field on the LAST line would be eaten and that + // row would arrive one column short. Image is never empty, so ending on it keeps every row the + // same width. + cmd := exec.CommandContext(ctx, "docker", "ps", "--format", + "{{.ID}}\t{{.Names}}\t{{.Label \"com.docker.compose.project\"}}\t{{.Image}}", "--filter", "status=running") out, err := cmd.Output() if err != nil { return nil, fmt.Errorf("docker ps failed: %w", err) @@ -108,12 +118,12 @@ func DiscoverDatabases(ctx context.Context, logger *log.Logger, debug bool, know if line == "" { continue } - parts := strings.SplitN(line, "\t", 3) - if len(parts) < 3 { + parts := strings.SplitN(line, "\t", 4) + if len(parts) < 4 { continue } - id, name, image := parts[0], parts[1], strings.ToLower(parts[2]) + id, name, project, image := parts[0], parts[1], strings.TrimSpace(parts[2]), strings.ToLower(parts[3]) // R-47: the same predicate that DBServiceNames applies to compose `image:` values, so a dump // that exists is always attributable to a startable service (see dbservices.go). @@ -134,7 +144,7 @@ func DiscoverDatabases(ctx context.Context, logger *log.Logger, debug bool, know ContainerID: id, ContainerName: name, DBType: dbType, - StackName: deriveStackName(name, known), + StackName: resolveStackName(name, project, known, logger, debug), } // Get env vars from container @@ -767,6 +777,44 @@ func getMariaDBPassword(ctx context.Context, containerID string) string { // the stack list is empty/unavailable, so nothing regresses). // // A nil/empty `known` map = the legacy fast path (pure suffix-strip). +// resolveStackName attributes a running DB container to the stack that owns it. +// +// R-355, and the reason this exists rather than more string-munging: the container name is a GUESS at +// the stack name and the compose project label is the ANSWER. `paperless-ngx` runs a container called +// `paperless-postgres`; no amount of suffix-stripping or prefix-matching can bridge that, and +// deriveStackName's last line returns its unresolved candidate as though it had resolved it. The dump +// then went to `backups/primary/paperless/db-dumps/` — a directory for a stack that does not exist, on +// the system drive — while the app's own recovery unit recorded `db_dumps: null`. Nothing collected it, +// nothing off-sited it, nothing restored it, and because writeSafetyDump filters on this same value the +// destructive restore took NO undo copy and told the customer the app had no database. Measured live +// 2026-08-21: 284 617 bytes, 72 tables, valid, and unreachable. +// +// The label is authoritative BY CONSTRUCTION — see the note on the `docker ps` format above — so it is +// preferred whenever it names a stack we know. It is not trusted blindly: a project label naming an +// unknown stack would write into an unknown app's directory, which is the very fault being fixed. +// +// The fallback is unchanged, so every container whose name already resolves keeps its exact previous +// attribution (pinned by TestResolveStackName_UnaffectedAppsAreUnchanged). +// +// When NEITHER route resolves to a known stack we still return the old candidate — refusing here would +// change behaviour for any container legitimately relying on the legacy path — but we say so loudly, +// because the silent version of this line is what hid R-355 for the life of the feature. +func resolveStackName(containerName, composeProject string, known map[string]bool, logger *log.Logger, debug bool) string { + if composeProject != "" && (len(known) == 0 || known[composeProject]) { + if debug && composeProject != deriveStackName(containerName, known) { + logger.Printf("[DEBUG] DiscoverDatabases: %s → stack %q from the compose project label (the container name would have given %q)", + containerName, composeProject, deriveStackName(containerName, known)) + } + return composeProject + } + fallback := deriveStackName(containerName, known) + if len(known) > 0 && !known[fallback] { + logger.Printf("[WARN] [backup] DB container %q could not be attributed to any deployed stack (compose project %q, name gives %q) — its dump will be written under %q, which no recovery unit reads. This is the R-355 shape; investigate before trusting that app's backup.", + containerName, composeProject, fallback, fallback) + } + return fallback +} + func deriveStackName(containerName string, known map[string]bool) string { candidate := suffixStripStackName(containerName) diff --git a/controller/internal/appbackup/resolvestackname_test.go b/controller/internal/appbackup/resolvestackname_test.go new file mode 100644 index 0000000..e9d6b77 --- /dev/null +++ b/controller/internal/appbackup/resolvestackname_test.go @@ -0,0 +1,175 @@ +package appbackup + +import ( + "bytes" + "log" + "path/filepath" + "strings" + "testing" +) + +// The deployed set on demo-hp on 2026-08-22, used by every case below so the tests argue about the +// real fleet rather than a convenient fiction. +func fleetKnown() map[string]bool { + return map[string]bool{ + "calibre-web": true, "kimai": true, "opengist": true, + "paperless-ngx": true, "privatebin": true, "romm": true, + } +} + +func quietLogger() (*log.Logger, *bytes.Buffer) { + var buf bytes.Buffer + return log.New(&buf, "", 0), &buf +} + +// TestResolveStackName_R355_ComposeProjectResolvesWhatTheNameCannot is the headline case. +// +// `paperless-ngx` runs its database in a container called `paperless-postgres`. The container name +// carries no route to the stack name: suffix-stripping gives `paperless`, which is not a stack, and +// no known stack is a prefix of it. deriveStackName then returns that unresolved candidate anyway. +// The compose project label is the answer the name cannot give. +func TestResolveStackName_R355_ComposeProjectResolvesWhatTheNameCannot(t *testing.T) { + known := fleetKnown() + lg, _ := quietLogger() + + // First, pin the shape of the defect, so this test still means something if the fallback changes. + if got := deriveStackName("paperless-postgres", known); got == "paperless-ngx" { + t.Fatalf("precondition lost: deriveStackName now resolves paperless-postgres correctly (%q) — "+ + "this test exists because it cannot", got) + } else if got != "paperless" { + t.Fatalf("precondition changed: deriveStackName(paperless-postgres) = %q, expected the "+ + "unresolved candidate %q", got, "paperless") + } + + got := resolveStackName("paperless-postgres", "paperless-ngx", known, lg, false) + if got != "paperless-ngx" { + t.Errorf("resolveStackName(paperless-postgres, project=paperless-ngx) = %q, want %q", got, "paperless-ngx") + } +} + +// TestR355_DumpLandsWhereTheRecoveryUnitReads asserts the CONSEQUENCE rather than the mechanism. +// +// It is not enough that the name resolves: the dump must land in the directory the unit assembler +// enumerates. Before the fix those were two different directories on two different drives — +// `…/backups/primary/paperless/db-dumps/` was written and `…/backups/primary/paperless-ngx/db-dumps/` +// was read — which is exactly why the manifest recorded `db_dumps: null` while a valid 72-table dump +// existed on disk. +func TestR355_DumpLandsWhereTheRecoveryUnitReads(t *testing.T) { + const nsRoot = "/mnt/felhom-drives/hdd_1/felhom-data" + const stack = "paperless-ngx" + known := fleetKnown() + lg, _ := quietLogger() + + // Where the unit assembler looks (CaptureRecoveryUnit enumerates exactly this). + readBy := AppDBDumpPath(nsRoot, stack) + + // Where the dump writer would put it, for the container this app actually runs. + writtenTo := AppDBDumpPath(nsRoot, resolveStackName("paperless-postgres", "paperless-ngx", known, lg, false)) + + if writtenTo != readBy { + t.Errorf("the dump would be written to %q but the recovery unit reads %q — the unit will record no database", writtenTo, readBy) + } + + // And demonstrate the pre-fix divergence explicitly, so the failure mode is documented by the suite + // rather than only by the report. + preFix := AppDBDumpPath(nsRoot, deriveStackName("paperless-postgres", known)) + if preFix == readBy { + t.Fatalf("precondition lost: the pre-fix path %q no longer diverges from %q", preFix, readBy) + } + if !strings.Contains(preFix, filepath.Join("primary", "paperless")+string(filepath.Separator)) { + t.Errorf("expected the pre-fix path to point at the phantom `paperless` stack, got %q", preFix) + } +} + +// TestResolveStackName_UnaffectedAppsAreUnchanged is scenario D: every container whose name already +// resolved must keep its exact previous attribution. A naming change that moved another app's dumps +// would be a far worse defect than the one being fixed. +func TestResolveStackName_UnaffectedAppsAreUnchanged(t *testing.T) { + known := fleetKnown() + lg, _ := quietLogger() + + // container name → compose project, as read from `docker ps` on demo-hp 2026-08-22. + fleet := map[string]string{ + "kimai-db": "kimai", + "romm-db": "romm", + "romm-redis": "romm", + "calibre-web": "calibre-web", + "privatebin": "privatebin", + "opengist": "opengist", + "paperless-redis": "paperless-ngx", + } + for container, project := range fleet { + old := deriveStackName(container, known) + got := resolveStackName(container, project, known, lg, false) + if container == "paperless-redis" { + // The one app the fix moves — asserted by the headline test, not here. + continue + } + if got != old { + t.Errorf("resolveStackName(%q, project=%q) = %q but deriveStackName gave %q — the fix moved an app it should not have", + container, project, got, old) + } + } +} + +// TestResolveStackName_NoLabelFallsBack covers a container not started by compose: the label is empty +// and the legacy path is all there is. +func TestResolveStackName_NoLabelFallsBack(t *testing.T) { + known := fleetKnown() + lg, _ := quietLogger() + if got := resolveStackName("romm-db", "", known, lg, false); got != "romm" { + t.Errorf("with no compose label, resolveStackName(romm-db) = %q, want %q", got, "romm") + } +} + +// TestResolveStackName_UnknownProjectIsNotTrusted covers the fail-safe direction. A project label +// naming a stack we do not know must NOT be used to choose a directory — writing into an unknown +// app's tree is the very fault being fixed, and doing it on the strength of an unverified label would +// simply move the bug. +func TestResolveStackName_UnknownProjectIsNotTrusted(t *testing.T) { + known := fleetKnown() + lg, buf := quietLogger() + + got := resolveStackName("romm-db", "some-other-project", known, lg, false) + if got != "romm" { + t.Errorf("an unknown compose project must not be trusted: got %q, want the derived %q", got, "romm") + } + if buf.Len() != 0 { + t.Errorf("a container that still resolves via the fallback must not warn; logged: %q", buf.String()) + } +} + +// TestResolveStackName_UnresolvableIsLoud pins the residual behaviour. When neither route reaches a +// known stack we keep the legacy answer — refusing would change behaviour for containers legitimately +// on the old path — but the line must be LOUD, because the silent version of it is what hid R-355 for +// the life of the feature. +func TestResolveStackName_UnresolvableIsLoud(t *testing.T) { + known := fleetKnown() + lg, buf := quietLogger() + + got := resolveStackName("mystery-postgres", "", known, lg, false) + if got != "mystery" { + t.Errorf("unresolvable container: got %q, want the legacy candidate %q", got, "mystery") + } + out := buf.String() + if !strings.Contains(out, "[WARN]") { + t.Errorf("an unattributable DB container must WARN; logged: %q", out) + } + for _, want := range []string{"mystery-postgres", "R-355"} { + if !strings.Contains(out, want) { + t.Errorf("the warning must name %q so it is greppable; logged: %q", want, out) + } + } +} + +// TestResolveStackName_LegacyCallersUnaffected: appexport passes no known set. The label must still be +// honoured there (len(known)==0 → trust it), because that caller has no other route to the truth. +func TestResolveStackName_LegacyCallersUnaffected(t *testing.T) { + lg, _ := quietLogger() + if got := resolveStackName("paperless-postgres", "paperless-ngx", nil, lg, false); got != "paperless-ngx" { + t.Errorf("with no known set the compose label is the only truth available: got %q, want %q", got, "paperless-ngx") + } + if got := resolveStackName("romm-postgres", "", nil, lg, false); got != "romm" { + t.Errorf("no label and no known set → legacy strip: got %q, want %q", got, "romm") + } +} diff --git a/controller/internal/backup/backup.go b/controller/internal/backup/backup.go index 671bd00..21944ca 100644 --- a/controller/internal/backup/backup.go +++ b/controller/internal/backup/backup.go @@ -141,6 +141,10 @@ type Manager struct { // disconnected) can be unit-tested without Docker. Nil → the real DumpAppVolumesSafe. dumpVolumesSafe func(stackName string) error + // R-354 volume-REPLAY seam — the mirror of the F17 DB seams above, so the off-site path's new + // volume leg is unit-testable without Docker. Nil → the real restoreDockerVolumesFrom. + volumeReplayFrom func(stackName, dumpDir string) (int, error) + // F7 tar seam — the ONE docker exec inside DumpAppVolumes, overridable so the atomic-write // behaviour (tmp+fsync+rename; the last good `.tar` survives a mid-write failure) is unit-testable // without Docker. It must write the tar to `/.tar.tmp` and return the combined diff --git a/controller/internal/backup/offbox_reconstitute.go b/controller/internal/backup/offbox_reconstitute.go index 6023906..685434e 100644 --- a/controller/internal/backup/offbox_reconstitute.go +++ b/controller/internal/backup/offbox_reconstitute.go @@ -62,14 +62,20 @@ const preRestoreDumpPrefix = "pre-restore-" // OffsiteReconstituteResult reports what a reconstitution actually did, so the flash can state an // OUTCOME instead of a mechanism. Every field here exists because the v0.147 flash could not say it. type OffsiteReconstituteResult struct { - SnapshotID string - FilesPlaced int - DBsReplayed int - SafetyDump string // path of the pre-restore dump (the undo), "" when the app has no DB - DumpsAt time.Time // when the snapshot's DB half was taken (zero = unknown/legacy unit) - OffsiteRunID string // "" for a pre-v0.148 snapshot — an unverified pair - Skewed bool // the snapshot carries no coherence stamp: files and DB may differ in age - LooksEmpty bool // R-44 sniff on the dump about to be replayed + SnapshotID string + FilesPlaced int + DBsReplayed int + // VolumesReplayed (R-354) is how many named-volume archives came back from the snapshot. It is on + // the result for the same reason every other field here is: so the OUTCOME can state what + // happened rather than a mechanism. Without it the message said "5 fájl visszaállítva" over a + // restore that had silently dropped a 1.4 MB volume archive — a true sentence leaving a false + // impression, which is the shape this surface keeps having removed from it. + VolumesReplayed int + SafetyDump string // path of the pre-restore dump (the undo), "" when the app has no DB + DumpsAt time.Time // when the snapshot's DB half was taken (zero = unknown/legacy unit) + OffsiteRunID string // "" for a pre-v0.148 snapshot — an unverified pair + Skewed bool // the snapshot carries no coherence stamp: files and DB may differ in age + LooksEmpty bool // R-44 sniff on the dump about to be replayed // Placement (R-351) is what the backup recorded about where this app's data lived, compared // against where this restore actually wrote. Carried on the RESULT and not only on the refusal, // so a restore that proceeded into a different destination says so in its own outcome rather @@ -344,8 +350,19 @@ func (m *Manager) ReconstituteFromOffsite(ctx context.Context, stack string, ack for _, pl := range placements { if pl.isUnit { // The live recovery unit is still never overwritten — it is the LOCAL restore path's - // source and clobbering it would trade one recovery route for another. The snapshot's - // dump is replayed from the scratch unit instead, so nothing is lost by skipping it. + // source and clobbering it would trade one recovery route for another. THAT reason is + // sound and still holds; it is why this skip stays. + // + // R-354 — THE SECOND HALF OF THIS COMMENT USED TO BE FALSE AND IS CORRECTED HERE. It said + // "the snapshot's dump is replayed from the scratch unit instead, so nothing is lost by + // skipping it". That was true of the DATABASE dump and false of the VOLUME archives, which + // live in the same unit and were replayed by nothing at all. Skipping the placement is + // correct; treating the skip as harmless was not. Measured live 2026-08-21: calibre-web's + // 1 422 848-byte `calibre_web_config.tar` was in the unit, in the snapshot and in the + // verification folder, and the restore reported "5 fájl visszaállítva" without it. + // + // Both legs are now replayed FROM THE SCRATCH UNIT below — volumes first, then the DB, so + // the logical dump still wins over any volume-tar copy of the same database. continue } n, cErr := copier(pl.src, pl.dst) @@ -360,6 +377,30 @@ func (m *Manager) ReconstituteFromOffsite(ctx context.Context, stack string, ack res.FilesPlaced += n } + // --- NAMED VOLUMES (R-354) ------------------------------------------------------------------ + // Replayed from the SCRATCH unit, exactly as the database dump is, and for the same reason: the + // live unit is never overwritten by a placement, so the snapshot's copy exists only under the + // scratch. Same helper as the local restore path — one implementation, two callers. + // + // ORDER IS LOAD-BEARING and mirrors RestoreFromRecoveryUnit: volumes FIRST, database after, so a + // logical .sql dump still wins over whatever copy of the same database a volume tar happens to + // contain. It also has to happen inside the stopped window, because replacing a named volume means + // removing it, and Docker refuses that while a container holds it. + volReplay := m.volumeReplayFrom + if volReplay == nil { + volReplay = m.restoreDockerVolumesFrom + } + nVols, vErr := volReplay(stack, filepath.Join(scratchUnit, "volume-dumps")) + res.VolumesReplayed = nVols + if vErr != nil { + // A partial replay must never read as a completion. Bring the app back up rather than leaving + // an outage, then surface it — the same shape the file leg above uses. + if sErr := restartStack(); sErr != nil { + m.logger.Printf("[WARN] [offbox] %s: restart after failed volume replay also failed: %v", stack, sErr) + } + return res, fmt.Errorf("a(z) %s adatkötetének visszaállítása sikertelen: %w", stack, vErr) + } + // --- DATABASE ------------------------------------------------------------------------------- // The DB container must be UP for the replay (ImportDump talks to it with its own discovered // credentials), but NOTHING ELSE may be — R-47. Until v0.153.0 this was a full StartStack, which diff --git a/controller/internal/backup/r354_volume_replay_test.go b/controller/internal/backup/r354_volume_replay_test.go new file mode 100644 index 0000000..6a27bc0 --- /dev/null +++ b/controller/internal/backup/r354_volume_replay_test.go @@ -0,0 +1,216 @@ +package backup + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "testing" +) + +// R-354. The off-site reconstitution replayed the snapshot's DATABASE and never its named VOLUMES, +// because the volume archives live inside the recovery unit and the unit placement is (correctly) +// skipped. Measured live 2026-08-21: calibre-web's 1 422 848-byte `calibre_web_config.tar` was in the +// unit, in the off-site snapshot and in the verification folder, and the restore returned five files, +// reported success, and did not replay it. For the 40 of 53 catalogue apps that declare no data drive, +// that archive is the entire dataset. + +// seedScratchVolumes writes volume tars into the scratch unit the reconstitution will read from, and +// returns that directory. +func seedScratchVolumes(t *testing.T, m *Manager, stack string, names ...string) string { + t.Helper() + scratch, _, err := m.offboxRestoreScratchDir(stack) + if err != nil { + t.Fatal(err) + } + unit := findScratchUnitDir(scratch, stack) + if unit == "" { + t.Fatalf("no scratch unit dir for %s under %s", stack, scratch) + } + dir := filepath.Join(unit, "volume-dumps") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + for _, n := range names { + if err := os.WriteFile(filepath.Join(dir, n), []byte("tar:"+n), 0o644); err != nil { + t.Fatal(err) + } + } + return dir +} + +func fingerprintTree(t *testing.T, root string) string { + t.Helper() + var lines []string + _ = filepath.Walk(root, func(p string, fi os.FileInfo, err error) error { + if err != nil || fi.IsDir() { + return nil + } + b, rErr := os.ReadFile(p) + if rErr != nil { + return nil + } + // The pre-restore undo copies are the ONE documented write into the live unit; everything + // else in the tree must be byte-identical across the operation. + if strings.HasPrefix(filepath.Base(p), preRestoreDumpPrefix) { + return nil + } + sum := sha256.Sum256(b) + rel, _ := filepath.Rel(root, p) + lines = append(lines, rel+":"+hex.EncodeToString(sum[:])) + return nil + }) + sort.Strings(lines) + return strings.Join(lines, "\n") +} + +// TestR354_ScenarioA_VolumeOnlyAppGetsItsVolumeBack is the case that matters: an app whose data is +// entirely in a named volume. Before the fix this returned nothing and said it had succeeded. +func TestR354_ScenarioA_VolumeOnlyAppGetsItsVolumeBack(t *testing.T) { + m, _, _ := reconFixture(t, "20260719T060000Z", "2026-07-19T06:00:00Z", "") + // A volume-only app places no files at all — the shape that reported "0 fájl" and success. + m.SetOffboxFullPlaceCopier(func(_, _ string) (int, error) { return 0, nil }) + m.discoverDBs = func(context.Context) ([]DiscoveredDB, error) { return nil, nil } + wantDir := seedScratchVolumes(t, m, "immich", "immich_immich_data.tar") + + var gotDir string + var gotStack string + m.volumeReplayFrom = func(stack, dumpDir string) (int, error) { + gotStack, gotDir = stack, dumpDir + return 1, nil + } + + res, err := m.ReconstituteFromOffsite(context.Background(), "immich", false) + if err != nil { + t.Fatalf("ReconstituteFromOffsite: %v", err) + } + if res.VolumesReplayed != 1 { + t.Errorf("VolumesReplayed = %d, want 1 — the snapshot's volume did not come back", res.VolumesReplayed) + } + if gotStack != "immich" { + t.Errorf("replayed for stack %q, want %q", gotStack, "immich") + } + // It must read the SCRATCH unit, never the live one. + if gotDir != wantDir { + t.Errorf("volume replay read %q, want the scratch unit's %q", gotDir, wantDir) + } +} + +// TestR354_ScenarioB_BothLegsReturnAndAreCounted — declared files AND a volume. +func TestR354_ScenarioB_BothLegsReturn(t *testing.T) { + m, _, _ := reconFixture(t, "20260719T060000Z", "2026-07-19T06:00:00Z", pgDump(1)) + seedScratchVolumes(t, m, "immich", "immich_a.tar", "immich_b.tar") + m.volumeReplayFrom = func(_, _ string) (int, error) { return 2, nil } + + res, err := m.ReconstituteFromOffsite(context.Background(), "immich", false) + if err != nil { + t.Fatalf("ReconstituteFromOffsite: %v", err) + } + if res.FilesPlaced != 3 { + t.Errorf("FilesPlaced = %d, want 3", res.FilesPlaced) + } + if res.VolumesReplayed != 2 { + t.Errorf("VolumesReplayed = %d, want 2", res.VolumesReplayed) + } + if res.DBsReplayed != 1 { + t.Errorf("DBsReplayed = %d, want 1", res.DBsReplayed) + } +} + +// TestR354_ScenarioC_NoVolumesIsUnchanged — a snapshot with no volume archives must behave exactly as +// before. The real helper runs here (no seam), so the absent-directory path is the one under test. +func TestR354_ScenarioC_NoVolumeArchivesIsANoOp(t *testing.T) { + m, _, _ := reconFixture(t, "20260719T060000Z", "2026-07-19T06:00:00Z", pgDump(1)) + // deliberately NO seedScratchVolumes and NO seam + + res, err := m.ReconstituteFromOffsite(context.Background(), "immich", false) + if err != nil { + t.Fatalf("a snapshot without volume archives must not fail the restore: %v", err) + } + if res.VolumesReplayed != 0 { + t.Errorf("VolumesReplayed = %d, want 0", res.VolumesReplayed) + } +} + +// TestR354_ScenarioD_LiveRecoveryUnitIsNeverWritten. The skip that caused R-354 also protects the +// local restore path's own source, and that reason still holds. Fingerprint the live unit across the +// whole operation and compare — the doctrine's own answer to the R-181 class, where every test +// asserted a mechanism inside one function and the tree still moved. +func TestR354_ScenarioD_LiveRecoveryUnitIsNeverWritten(t *testing.T) { + m, _, _ := reconFixture(t, "20260719T060000Z", "2026-07-19T06:00:00Z", pgDump(1)) + _, liveNs, err := m.offboxRestoreScratchDir("immich") + if err != nil { + t.Fatal(err) + } + liveUnit := RecoveryUnitPath(liveNs, "immich") + liveVols := filepath.Join(liveUnit, "volume-dumps") + if err := os.MkdirAll(liveVols, 0o755); err != nil { + t.Fatal(err) + } + // The live unit's own copy — the local restore path's source. It must survive untouched. + if err := os.WriteFile(filepath.Join(liveVols, "immich_immich_data.tar"), []byte("THE LIVE UNIT COPY"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(liveUnit, "manifest.json"), []byte(`{"app_name":"immich"}`), 0o644); err != nil { + t.Fatal(err) + } + // Fingerprint the WHOLE live unit. Narrowing this to the volume archives made the test blind to a + // placement writing into the unit ROOT — caught by red-proof 6, which passed against the narrowed + // version. The excluded set is exactly the pre-restore undo copies, and those are asserted below. + before := fingerprintTree(t, liveUnit) + + seedScratchVolumes(t, m, "immich", "immich_immich_data.tar") + m.volumeReplayFrom = func(_, _ string) (int, error) { return 1, nil } + // A copier that really writes, so "the unit is never written to" is observable rather than assumed. + m.SetOffboxFullPlaceCopier(func(src, dst string) (int, error) { + _ = os.MkdirAll(dst, 0o755) + return 1, os.WriteFile(filepath.Join(dst, "PLACED"), []byte("from the copier"), 0o644) + }) + + if _, err := m.ReconstituteFromOffsite(context.Background(), "immich", false); err != nil { + t.Fatalf("ReconstituteFromOffsite: %v", err) + } + + if after := fingerprintTree(t, liveUnit); after != before { + t.Errorf("the LIVE recovery unit's backup content changed across the restore — the local path's source was clobbered\nbefore:\n%s\nafter:\n%s", before, after) + } + + // The ONE write into the live unit that IS expected: the pre-restore safety dump. It lives in the + // app's own db-dumps dir deliberately (see preRestoreDumpPrefix) — it is the undo, and an undo the + // customer cannot see is not much of one. Asserted here rather than merely excluded, so "the unit + // is untouched" cannot quietly come to mean "the undo stopped being written". + undo, _ := filepath.Glob(filepath.Join(liveUnit, "db-dumps", "pre-restore-*.sql")) + if len(undo) != 1 { + t.Errorf("expected exactly one pre-restore undo copy in the live unit, found %d", len(undo)) + } +} + +// TestR354_ScenarioE_PartialReplayIsAFailure — a volume replay that fails must never read as a +// completion, and must name what failed. +func TestR354_ScenarioE_PartialReplayIsReportedAsFailure(t *testing.T) { + m, prov, _ := reconFixture(t, "20260719T060000Z", "2026-07-19T06:00:00Z", pgDump(1)) + seedScratchVolumes(t, m, "immich", "immich_a.tar", "immich_b.tar") + m.volumeReplayFrom = func(_, _ string) (int, error) { + return 1, fmt.Errorf("failed to restore 1 volume(s): [immich_b]") + } + + res, err := m.ReconstituteFromOffsite(context.Background(), "immich", false) + if err == nil { + t.Fatal("a partial volume replay must be reported as a failure, not a completion") + } + if !strings.Contains(err.Error(), "immich_b") { + t.Errorf("the failure must name the volume that did not come back; got %q", err.Error()) + } + // Best-effort bring-up: a failed restore must not also be an outage. + if !prov.fullStarted { + t.Error("the app was left stopped after a failed volume replay") + } + // The count of what DID come back is still carried, so the report can say "1 of 2". + if res.VolumesReplayed != 1 { + t.Errorf("VolumesReplayed = %d, want the partial count 1", res.VolumesReplayed) + } +} diff --git a/controller/internal/backup/r355_safetydump_test.go b/controller/internal/backup/r355_safetydump_test.go new file mode 100644 index 0000000..b8e23d3 --- /dev/null +++ b/controller/internal/backup/r355_safetydump_test.go @@ -0,0 +1,125 @@ +package backup + +import ( + "context" + "io" + "log" + "os" + "path/filepath" + "strings" + "testing" +) + +// R-355's second half. The naming defect does not stop at the backup: writeSafetyDump filters the +// discovered databases with `db.StackName == stackName`, so an app whose database is attributed to the +// wrong stack has NO database as far as the destructive restore is concerned. It therefore takes no +// undo copy, and the fail-closed refusal that protects every other app cannot fire — the guard is not +// bypassed, it is never reached. +// +// Measured live on 2026-08-21: a destructive restore of `paperless-ngx` ran to completion over a live +// 72-table PostgreSQL with `find /mnt -name "pre-restore-*"` empty both before and after. + +func newSafetyTestManager() *Manager { + return &Manager{logger: log.New(io.Discard, "", 0)} +} + +// TestR355_SafetyDumpIsTakenForTheCorrectlyAttributedApp is scenario B: the undo copy exists. +func TestR355_SafetyDumpIsTakenForTheCorrectlyAttributedApp(t *testing.T) { + nsRoot := t.TempDir() + m := newSafetyTestManager() + + // Post-fix attribution: the compose project label resolves this container to `paperless-ngx`. + m.discoverDBs = func(ctx context.Context) ([]DiscoveredDB, error) { + return []DiscoveredDB{ + {StackName: "paperless-ngx", DBType: DBTypePostgres, ContainerName: "paperless-postgres", ContainerID: "cid"}, + }, nil + } + m.safetyDumpFn = func(ctx context.Context, db DiscoveredDB, dumpDir string) DumpResult { + p := filepath.Join(dumpDir, string(db.StackName)+"-"+string(db.DBType)+".sql") + if err := os.MkdirAll(dumpDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte("-- 72 tables\n"), 0o644); err != nil { + t.Fatal(err) + } + return DumpResult{DB: db, FilePath: p, Size: 13} + } + + safety, err := m.writeSafetyDump(context.Background(), "paperless-ngx", nsRoot) + if err != nil { + t.Fatalf("writeSafetyDump: %v", err) + } + if safety == "" { + t.Fatal("no safety dump was taken for an app that HAS a database — the restore would proceed with no undo") + } + if _, err := os.Stat(safety); err != nil { + t.Fatalf("the safety dump path %q is not on disk: %v", safety, err) + } + if !strings.Contains(filepath.Base(safety), "pre-restore-") { + t.Errorf("the undo copy must carry the pre-restore prefix so it can never be replayed as a source; got %q", filepath.Base(safety)) + } + // The consequence that matters: it lives inside THIS app's unit, not a phantom's. + if got, want := filepath.Dir(safety), AppDBDumpPath(nsRoot, "paperless-ngx"); got != want { + t.Errorf("undo copy written to %q, want %q", got, want) + } +} + +// TestR355_MisattributedAppGetsNoUndoCopy demonstrates the WRONG OUTCOME — the state the fix removes. +// It models the pre-fix attribution (`paperless`) against a restore of `paperless-ngx` and asserts the +// undo silently does not happen. This is the shape that made a destructive restore unrecoverable. +func TestR355_MisattributedAppGetsNoUndoCopy(t *testing.T) { + nsRoot := t.TempDir() + m := newSafetyTestManager() + + // PRE-FIX attribution: deriveStackName gave `paperless` for container `paperless-postgres`. + m.discoverDBs = func(ctx context.Context) ([]DiscoveredDB, error) { + return []DiscoveredDB{ + {StackName: "paperless", DBType: DBTypePostgres, ContainerName: "paperless-postgres", ContainerID: "cid"}, + }, nil + } + called := false + m.safetyDumpFn = func(ctx context.Context, db DiscoveredDB, dumpDir string) DumpResult { + called = true + return DumpResult{DB: db} + } + + safety, err := m.writeSafetyDump(context.Background(), "paperless-ngx", nsRoot) + if err != nil { + t.Fatalf("writeSafetyDump: %v", err) + } + if safety != "" || called { + t.Fatalf("precondition lost: the misattributed shape now takes an undo copy (safety=%q called=%v) — "+ + "this test documents the defect and must keep failing to find one", safety, called) + } + // And this is precisely why it was invisible: no error, no dump, and the caller reads + // `hasDB == false` — indistinguishable from an app that genuinely has no database. +} + +// TestR355_RestoreRefusesWhenTheUndoCannotBeTaken is scenario C, the fail-closed direction. The +// invariant already existed and was proven working live on 2026-08-21 for `romm`; this pins it for the +// app that could not reach it before, so the two cannot drift apart. +func TestR355_RestoreRefusesWhenTheUndoCannotBeTaken(t *testing.T) { + nsRoot := t.TempDir() + m := newSafetyTestManager() + + m.discoverDBs = func(ctx context.Context) ([]DiscoveredDB, error) { + return []DiscoveredDB{ + {StackName: "paperless-ngx", DBType: DBTypePostgres, ContainerName: "paperless-postgres", ContainerID: "cid"}, + }, nil + } + m.safetyDumpFn = func(ctx context.Context, db DiscoveredDB, dumpDir string) DumpResult { + return DumpResult{DB: db, Error: os.ErrPermission} + } + + safety, err := m.writeSafetyDump(context.Background(), "paperless-ngx", nsRoot) + if err == nil { + t.Fatal("a database that cannot be dumped must be a hard error — the undo would not exist") + } + if safety != "" { + t.Errorf("a failed undo must return no path, got %q", safety) + } + // The message must say the restore did not start, because that is the customer's only signal. + if !strings.Contains(err.Error(), "nem indult el") { + t.Errorf("the refusal must state that the restore did not start; got %q", err.Error()) + } +} diff --git a/controller/internal/backup/restore.go b/controller/internal/backup/restore.go index eec105d..d0b2566 100644 --- a/controller/internal/backup/restore.go +++ b/controller/internal/backup/restore.go @@ -98,15 +98,35 @@ func (m *Manager) RestoreApp(stackName, snapshotID string) error { return nil } -// restoreDockerVolumes populates Docker volumes from tar files in the volume dump directory. +// restoreDockerVolumes populates Docker volumes from the tars in the app's LIVE recovery unit. func (m *Manager) restoreDockerVolumes(stackName, drivePath string) error { - dumpDir := AppVolumeDumpPath(m.namespaceRoot(drivePath), stackName) + _, err := m.restoreDockerVolumesFrom(stackName, AppVolumeDumpPath(m.namespaceRoot(drivePath), stackName)) + return err +} + +// restoreDockerVolumesFrom is restoreDockerVolumes with an EXPLICIT dump directory, and it returns how +// many volumes it replayed. +// +// R-354. The off-site reconstitution needs exactly this, for the same reason reimportDBDumpsFrom +// exists beside reimportDBDumps: the snapshot's archives live under the restored SCRATCH unit, because +// the live unit is deliberately never overwritten by a placement. Until now no such variant existed, +// so the off-site path had no way to replay a volume and simply did not — the tar sat in the unit, in +// the snapshot and in the verification folder, and the restore reported success without it. For an app +// whose data is entirely in a named volume — 40 of the 53 in the catalogue — that is everything the +// customer owns. +// +// ONE implementation, two callers. A second copy of this loop is what produced the divergence in the +// first place: the local path replayed volumes and the off-site path did not, and nothing compared the +// two. +// +// It only ever READS dumpDir; the recovery unit is never written to here, on either path. +func (m *Manager) restoreDockerVolumesFrom(stackName, dumpDir string) (int, error) { entries, err := os.ReadDir(dumpDir) if err != nil { if os.IsNotExist(err) { - return nil // No volume dumps to restore + return 0, nil // No volume dumps to restore } - return fmt.Errorf("reading volume dump dir: %w", err) + return 0, fmt.Errorf("reading volume dump dir: %w", err) } var restored int @@ -154,11 +174,12 @@ func (m *Manager) restoreDockerVolumes(stackName, drivePath string) error { m.logger.Printf("[INFO] [backup] Restored %d Docker volume(s) for %s", restored, stackName) } // F17: a per-volume failure used to be a swallowed WARN; surface it so the restore is reported as - // failed rather than silently partial. + // failed rather than silently partial. The count is returned ALONGSIDE the error, not instead of + // it: a caller that replayed three of four volumes needs both numbers to say what happened. if len(failed) > 0 { - return fmt.Errorf("failed to restore %d volume(s): %v", len(failed), failed) + return restored, fmt.Errorf("failed to restore %d volume(s): %v", len(failed), failed) } - return nil + return restored, nil } // waitForHealthy waits for a stack to reach running state after restore. diff --git a/controller/internal/web/offbox_handlers.go b/controller/internal/web/offbox_handlers.go index 1cdf510..2627574 100644 --- a/controller/internal/web/offbox_handlers.go +++ b/controller/internal/web/offbox_handlers.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "net/url" + "path/filepath" "strconv" "strings" "time" @@ -467,12 +468,38 @@ func reconstituteOutcomeMsg(app string, res backup.OffsiteReconstituteResult) st if !res.DumpsAt.IsZero() { when = " (mentés: " + res.DumpsAt.In(getTimezone()).Format("2006-01-02 15:04") + ")" } + // R-354 — WHAT ACTUALLY CAME BACK, NAMED. The volume leg is stated whenever it returned anything, + // because a restore that replayed an app's entire dataset and mentioned only its file count is + // precisely how a silent loss reads as a success: on 2026-08-21 calibre-web was told + // „5 fájl visszaállítva" over a run that had dropped a 1 422 848-byte volume archive. Every clause + // here is conditional on having done the thing, so a snapshot with no volumes produces the exact + // sentence it produced before (pinned by TestReconstituteOutcome_NoVolumesWordingUnchanged). + what := fmt.Sprintf("%d fájl", res.FilesPlaced) + if res.VolumesReplayed > 0 { + what += fmt.Sprintf(" és %d adatkötet", res.VolumesReplayed) + } + if res.DBsReplayed > 0 { + what += " és az adatbázis" + } + msg := fmt.Sprintf("A(z) %s: %s visszaállítva%s — az alkalmazás újraindult.", app, what, when) + if res.DBsReplayed == 0 { // A no-database app: saying "és az adatbázis" here would be a lie, and this is precisely the // class of sentence the DIAG found being printed over a no-op. - return fmt.Sprintf("A(z) %s: %d fájl visszaállítva%s — az alkalmazás újraindult. Ennek az alkalmazásnak nincs adatbázisa.", app, res.FilesPlaced, when) + // + // R-355: „nincs adatbázisa" is a claim ABOUT THE APP and must not be inferred from a counter. + // `DBsReplayed == 0` has two causes — the app has no database, or it has one and the snapshot + // carried no dump for it — and until now both printed the same confident sentence. On + // 2026-08-21 that sentence was shown over a live 72-table PostgreSQL the controller had dumped + // five minutes earlier. `SafetyDump` is the honest discriminator: writeSafetyDump returns a + // path only when a live database for this app was found AND successfully dumped, so a non-empty + // value proves the app HAS one. Same counter, two different facts, and now two sentences. + if res.SafetyDump != "" { + return msg + fmt.Sprintf(" FIGYELEM: ennek az alkalmazásnak VAN adatbázisa, de a mentés nem tartalmazott adatbázis-mentést, ezért az adatbázis NEM állt vissza. A visszaállítás előtti állapot mentése megvan: %s", filepath.Base(res.SafetyDump)) + } + return msg + " Ennek az alkalmazásnak nincs adatbázisa." } - return fmt.Sprintf("A(z) %s: %d fájl és az adatbázis visszaállítva%s — az alkalmazás újraindult.", app, res.FilesPlaced, when) + return msg } // offboxVerifyCopyDeleteHandler removes ONE verification copy (v0.147.0, 4a). diff --git a/controller/internal/web/r355_outcome_msg_test.go b/controller/internal/web/r355_outcome_msg_test.go new file mode 100644 index 0000000..f051685 --- /dev/null +++ b/controller/internal/web/r355_outcome_msg_test.go @@ -0,0 +1,118 @@ +package web + +import ( + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/backup" +) + +// R-355, the sentence. „Ennek az alkalmazásnak nincs adatbázisa" is a claim ABOUT THE APP, and it was +// being inferred from a counter that has two different causes. On 2026-08-21 it was printed over a live +// 72-table PostgreSQL that the controller had dumped five minutes earlier. +// +// `SafetyDump` is the honest discriminator: writeSafetyDump returns a path only when a live database +// for this app was found AND successfully dumped. + +func TestReconstituteOutcome_NoDatabaseOnlyWhenThereIsNone(t *testing.T) { + msg := reconstituteOutcomeMsg("opengist", backup.OffsiteReconstituteResult{ + FilesPlaced: 3, DBsReplayed: 0, SafetyDump: "", + }) + if !strings.Contains(msg, "nincs adatbázisa") { + t.Errorf("an app with genuinely no database should still say so; got %q", msg) + } +} + +// The one that matters: a database exists, none was replayed. Saying "this app has no database" here +// is false, and saying nothing at all would leave a silent loss under a success. +func TestReconstituteOutcome_DatabaseExistsButWasNotRestored(t *testing.T) { + msg := reconstituteOutcomeMsg("paperless-ngx", backup.OffsiteReconstituteResult{ + FilesPlaced: 0, DBsReplayed: 0, + SafetyDump: "/mnt/x/db-dumps/pre-restore-20260822T060000Z-paperless-ngx-postgres.sql", + }) + if strings.Contains(msg, "nincs adatbázisa") { + t.Fatalf("FALSE CLAIM: told the customer the app has no database while its undo copy proves it does; got %q", msg) + } + for _, want := range []string{"VAN adatbázisa", "NEM állt vissza"} { + if !strings.Contains(msg, want) { + t.Errorf("the message must state that a database exists and did not come back; missing %q in %q", want, msg) + } + } + // The undo copy is the customer's way back, so it has to be named. + if !strings.Contains(msg, "pre-restore-20260822T060000Z-paperless-ngx-postgres.sql") { + t.Errorf("the message must name the undo copy; got %q", msg) + } + // It must never leak the directory — only the file name. + if strings.Contains(msg, "/mnt/x/") { + t.Errorf("the message leaked a filesystem path; got %q", msg) + } +} + +func TestReconstituteOutcome_DatabaseRestoredIsUnchanged(t *testing.T) { + msg := reconstituteOutcomeMsg("romm", backup.OffsiteReconstituteResult{ + FilesPlaced: 4, DBsReplayed: 1, SafetyDump: "/x/pre-restore-romm-mariadb.sql", + }) + if !strings.Contains(msg, "és az adatbázis visszaállítva") { + t.Errorf("the full case must keep its wording; got %q", msg) + } + if strings.Contains(msg, "FIGYELEM") { + t.Errorf("a complete restore must not carry a warning; got %q", msg) + } +} + +// ── R-354: the volume leg has to reach the sentence ───────────────────────────────────────────── + +// The 2026-08-21 case, as the customer saw it and as they must see it now. +func TestReconstituteOutcome_VolumesAreNamed(t *testing.T) { + msg := reconstituteOutcomeMsg("calibre-web", backup.OffsiteReconstituteResult{ + FilesPlaced: 5, VolumesReplayed: 1, DBsReplayed: 0, SafetyDump: "", + }) + if !strings.Contains(msg, "5 fájl és 1 adatkötet visszaállítva") { + t.Errorf("the message must name the volume that came back; got %q", msg) + } + // The old sentence — five files and nothing else — must be gone. + if strings.Contains(msg, "5 fájl visszaállítva") { + t.Errorf("the pre-fix wording is still being produced; got %q", msg) + } +} + +// A volume-only app: the whole dataset is the volume, and "0 fájl" alone said nothing about it. +func TestReconstituteOutcome_VolumeOnlyAppSaysWhatCameBack(t *testing.T) { + msg := reconstituteOutcomeMsg("privatebin", backup.OffsiteReconstituteResult{ + FilesPlaced: 0, VolumesReplayed: 1, DBsReplayed: 0, SafetyDump: "", + }) + if !strings.Contains(msg, "0 fájl és 1 adatkötet visszaállítva") { + t.Errorf("a volume-only restore must state the volume; got %q", msg) + } +} + +// Scenario C: a snapshot with no volume archives must produce the EXACT sentence it produced before, +// so the change cannot be read as "a volume was expected and did not arrive". +func TestReconstituteOutcome_NoVolumesWordingUnchanged(t *testing.T) { + noDB := reconstituteOutcomeMsg("opengist", backup.OffsiteReconstituteResult{ + FilesPlaced: 3, VolumesReplayed: 0, DBsReplayed: 0, SafetyDump: "", + }) + if noDB != "A(z) opengist: 3 fájl visszaállítva — az alkalmazás újraindult. Ennek az alkalmazásnak nincs adatbázisa." { + t.Errorf("the no-volume, no-database wording changed; got %q", noDB) + } + withDB := reconstituteOutcomeMsg("romm", backup.OffsiteReconstituteResult{ + FilesPlaced: 4, VolumesReplayed: 0, DBsReplayed: 1, SafetyDump: "/x/pre-restore-romm-mariadb.sql", + }) + if withDB != "A(z) romm: 4 fájl és az adatbázis visszaállítva — az alkalmazás újraindult." { + t.Errorf("the no-volume, with-database wording changed; got %q", withDB) + } + if strings.Contains(noDB, "adatkötet") || strings.Contains(withDB, "adatkötet") { + t.Error("a restore that replayed no volume must not mention volumes at all") + } +} + +// All three legs at once. +func TestReconstituteOutcome_AllThreeLegs(t *testing.T) { + msg := reconstituteOutcomeMsg("paperless-ngx", backup.OffsiteReconstituteResult{ + FilesPlaced: 12, VolumesReplayed: 3, DBsReplayed: 1, SafetyDump: "/x/pre-restore-p.sql", + }) + want := "A(z) paperless-ngx: 12 fájl és 3 adatkötet és az adatbázis visszaállítva — az alkalmazás újraindult." + if msg != want { + t.Errorf("got %q\nwant %q", msg, want) + } +}