Found by v0.220.0's own live walk on its first real run. writeSafetyDump
captures its DiscoveredDB before the stop; the DB-only start then re-creates the
container with a new id, so the rollback's docker exec hit a dead container and
sat in waitDBReady for 30s. The app was held for an infrastructure reason while
its data was recoverable.
Re-discover and match by {stack, engine} - what reimportDBDumpsFrom already did.
Fail closed when the container cannot be found.
No unit test caught it because they all inject the import seam and never look at
container identity. The new test asserts the identity handed to the import.
This commit is contained in:
@@ -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))
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user