diff --git a/CHANGELOG.md b/CHANGELOG.md index cb96f6c..c5e7fd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,25 @@ +## v0.220.1 — the rollback poured the undo into a container that no longer existed (2026-08-22, R-379) +**MinAgent: 0.129.0** (unchanged) + +**Found by v0.220.0's own live walk, on its first real run, an hour after it shipped.** The undo FILE +is stable; the container it must be poured into is not. `writeSafetyDump` captures its +`DiscoveredDB` **before** the stop, and by the time the rollback runs the stack has been stopped and +the DB service re-created with a new id. + +Measured on `demo-hp`: `docmost-postgres` was captured as `9adbc14f9af6` at 16:05:44, re-created as +`309795897b82` at 16:05:47 by the DB-only start, and the rollback's `docker exec` against the dead id +sat in `waitDBReady` until it timed out 30 s later. **So the app was HELD for an infrastructure reason +while its data was perfectly recoverable** — the hold worked exactly as designed, on a case that +should never have reached it. + +The rollback now re-discovers the containers and matches each undo file to a live one by +`{stack, engine}`, which is what `reimportDBDumpsFrom` already did for the replay. A database whose +container cannot be found fails **closed** rather than pouring an undo into something unidentified. + +**Why no unit test caught it:** every rollback test injects the import seam and never looks at +container identity. The new one asserts the identity handed to the import, and its red-proof — using +the captured id again — convicts. + ## v0.220.0 — when a database restore fails, the customer's own copy goes back (2026-08-22, R-379/R-380/R-381/R-382) **MinAgent: 0.129.0** (unchanged — no new agent coupling) diff --git a/controller/internal/backup/offbox_reconstitute.go b/controller/internal/backup/offbox_reconstitute.go index c924141..9dabb7e 100644 --- a/controller/internal/backup/offbox_reconstitute.go +++ b/controller/internal/backup/offbox_reconstitute.go @@ -202,6 +202,14 @@ func (m *Manager) writeSafetyDump(ctx context.Context, stackName, nsRoot string) return set, nil } +// shortID trims a docker id for logs. Never used for identity — only for reading. +func shortID(id string) string { + if len(id) > 12 { + return id[:12] + } + return id +} + // maxUndoCopiesPerApp is how many `pre-restore-` copies an app keeps. // // THREE, and the reasoning rather than a number pulled from the air. One is not enough: the case @@ -342,13 +350,53 @@ func (m *Manager) rollbackSafetyDump(ctx context.Context, stack string, set safe return ImportDump(ctx, db, path, m.logger, m.isDebug()) } } + // RE-DISCOVER THE CONTAINERS. The undo FILE is stable; the container it must be poured into is + // NOT. `writeSafetyDump` captured its DiscoveredDB before the stop, and by the time the rollback + // runs the stack has been stopped and the DB service re-created — a NEW container id. + // + // MEASURED LIVE ON demo-hp 2026-08-22, which is the only reason this is here: the first live run + // of this code captured `docmost-postgres id=9adbc14f9af6` at 16:05:44, the DB-only start + // re-created it as `309795897b82` at 16:05:47, and the rollback's `docker exec` against the dead + // id sat in `waitDBReady` until it timed out 30 s later — so the app was HELD for an + // infrastructure reason when its data was recoverable. The unit tests could not see it: they + // inject the import seam and never touch container identity. `reimportDBDumpsFrom` already + // re-discovers for exactly this reason. + discover := m.discoverDBs + if discover == nil { + discover = func(ctx context.Context) ([]DiscoveredDB, error) { + return DiscoverDatabases(ctx, m.logger, m.isDebug(), m.knownStackNames()) + } + } + live, dErr := discover(ctx) + if dErr != nil { + return fmt.Errorf("a visszavonás előtt nem sikerült felderíteni az adatbázisokat: %w", dErr) + } + liveFor := func(want DiscoveredDB) (DiscoveredDB, bool) { + for _, db := range live { + if db.StackName == want.StackName && db.DBType == want.DBType { + return db, true + } + } + return DiscoveredDB{}, false + } + for _, f := range set.Files { if _, sErr := os.Stat(f.Path); sErr != nil { return fmt.Errorf("a visszavonáshoz szükséges mentés nem található (%s): %w", filepath.Base(f.Path), sErr) } + target, ok := liveFor(f.DB) + if !ok { + // Fail closed: pouring an undo into a container we cannot identify is worse than saying + // we could not do it. + return fmt.Errorf("a(z) %s adatbázis-tárolója nem található a visszavonáshoz", f.DB.ContainerName) + } + if target.ContainerID != f.DB.ContainerID { + m.logger.Printf("[DEBUG] [offbox] %s: %s was re-created during the restore (%s → %s) — rolling back into the live container", + stack, f.DB.ContainerName, shortID(f.DB.ContainerID), shortID(target.ContainerID)) + } m.logger.Printf("[INFO] [offbox] %s: rolling back to the pre-restore state from %s", stack, filepath.Base(f.Path)) - if err := imp(ctx, f.DB, f.Path); err != nil { - return fmt.Errorf("a korábbi állapot visszaállítása sikertelen (%s): %w", f.DB.ContainerName, err) + if err := imp(ctx, target, f.Path); err != nil { + return fmt.Errorf("a korábbi állapot visszaállítása sikertelen (%s): %w", target.ContainerName, err) } } m.logger.Printf("[INFO] [offbox] %s: rollback complete — %d database(s) returned to the pre-restore state", stack, len(set.Files)) diff --git a/controller/internal/backup/r379_rollback_test.go b/controller/internal/backup/r379_rollback_test.go index 6c07193..580ccb1 100644 --- a/controller/internal/backup/r379_rollback_test.go +++ b/controller/internal/backup/r379_rollback_test.go @@ -328,3 +328,85 @@ func TestR381_CustomerMessageCarriesNoEngineOutput(t *testing.T) { t.Fatal("fixture: the planted noise does not contain the marker this test greps for") } } + +// THE DEFECT THE LIVE WALK FOUND, and the unit tests could not. +// +// The undo FILE is stable; the container it must be poured into is NOT. writeSafetyDump captures its +// DiscoveredDB BEFORE the stop, and by rollback time the stack has been stopped and the DB service +// re-created with a new container id. Measured on demo-hp 2026-08-22: captured `9adbc14f9af6` at +// 16:05:44, re-created as `309795897b82` at 16:05:47, and the rollback's docker exec against the dead +// id sat in waitDBReady until it timed out 30 s later — so the app was HELD for an infrastructure +// reason while its data was perfectly recoverable. +// +// Every other test here injects the import seam and never looks at container identity, which is +// exactly why none of them saw it. This one asserts the identity handed to the import. +func TestR379_RollbackUsesTheLiveContainerNotTheCapturedOne(t *testing.T) { + m, _, _ := reconFixture(t, "run1", "2026-07-19T06:00:00Z", pgDump(1)) + + const capturedID, liveID = "9adbc14f9af6", "309795897b82" + calls := 0 + // Discovery returns the CAPTURED id first (safety-dump time), then the LIVE one — the real + // sequence, because the DB service is re-created in between. + m.discoverDBs = func(context.Context) ([]DiscoveredDB, error) { + calls++ + id := liveID + if calls == 1 { + id = capturedID + } + return []DiscoveredDB{{StackName: "immich", DBType: DBTypePostgres, ContainerName: "immich-postgres", ContainerID: id}}, nil + } + m.importDBDump = func(context.Context, DiscoveredDB, string) error { return errors.New("replay failed") } + + var rolledInto []string + m.SetRollbackImportFn(func(_ context.Context, db DiscoveredDB, _ string) error { + rolledInto = append(rolledInto, db.ContainerID) + return nil + }) + + if _, err := m.ReconstituteFromOffsite(context.Background(), "immich", false); err == nil { + t.Fatal("the replay failure must surface") + } + if len(rolledInto) != 1 { + t.Fatalf("the rollback must have run once, got %v", rolledInto) + } + if rolledInto[0] == capturedID { + t.Fatalf("the rollback used the CAPTURED container id %s — that container no longer exists by rollback time; "+ + "it must re-discover and use the live one", capturedID) + } + if rolledInto[0] != liveID { + t.Fatalf("the rollback must target the LIVE container %s, got %s", liveID, rolledInto[0]) + } +} + +// Fail closed: if the database's container cannot be found at rollback time, say so rather than pour +// the undo into something unidentified. +func TestR379_RollbackFailsClosedWhenTheContainerIsGone(t *testing.T) { + m, prov, _ := reconFixture(t, "run1", "2026-07-19T06:00:00Z", pgDump(1)) + // Three discovery calls happen, in this order: writeSafetyDump, reimportDBDumpsFrom (the + // replay), then rollbackSafetyDump. The container vanishes only for the THIRD. + calls := 0 + m.discoverDBs = func(context.Context) ([]DiscoveredDB, error) { + calls++ + if calls >= 3 { + return nil, nil // vanished by rollback time + } + return []DiscoveredDB{{StackName: "immich", DBType: DBTypePostgres, ContainerName: "immich-postgres", ContainerID: "a"}}, nil + } + m.importDBDump = func(context.Context, DiscoveredDB, string) error { return errors.New("replay failed") } + rolled := 0 + m.SetRollbackImportFn(func(context.Context, DiscoveredDB, string) error { rolled++; return nil }) + + _, err := m.ReconstituteFromOffsite(context.Background(), "immich", false) + if err == nil { + t.Fatal("a rollback with no container must surface") + } + if rolled != 0 { + t.Errorf("nothing may be imported when the target cannot be identified, got %d", rolled) + } + if prov.fullStarted { + t.Error("a failed rollback must leave the app held, not started") + } + if held, _ := m.RestoreHoldFor("immich"); !held { + t.Error("the app must be held when the rollback could not run") + } +}