Files
felhom-controller/controller/internal/backup/r47_replay_order_test.go
T
admin 78ff991f1c v0.153.0 — R-47: the DB replay no longer races the app, on BOTH restore paths
Closes R-47. No new agent coupling — MinAgent stays 0.90.0.

The replay needs a running DB container, so both restore paths started the
WHOLE stack first, giving the application a window to rebuild the very schema
objects the dump was about to create. Measured live on 2026-07-19 (H4,
DIAG-immich-restore-round2): immich-server rebuilt clip_index two seconds
before the dump's CREATE INDEX, the replay aborted "already exists" under
ON_ERROR_STOP=1, and immich reported schema drift. The data survived only
because pg_dump emits COPY before CREATE INDEX.

Both paths now open a DB-ONLY window: only the stack's database service(s)
come up, the dump is replayed with the app still down, and the full start
runs only after the replay exits 0. Fail-closed: a dump with no identifiable
DB service refuses BEFORE the first mutation. Every exit from the window
still does a best-effort full start, so a failed restore never leaves a box
with a database and no application.

New: appbackup.DBServiceNames (yaml.v3 services-map parse — never a line
scan; immich's top-level volume keys are the decoy) sharing dbTypeForImage
with DiscoverDatabases; stacks.Manager.StartStackServices (refuses an empty
list — argument-less `up -d` is a full start); RedeployFromEnv split into
PersistUnitRedeployConfig + its unchanged tail. StackDataProvider's
RecreateStackFromUnit becomes RecreateStackDefinitionFromUnit — the hidden
`up -d` inside the old name is what carried the defect on the local path.

19 new tests (ordering plus state-at-replay-time, zero-mutation fail-closed
effects, replay-failure bring-up, parser decoys, empty-list refusal); three
companion red-proofs run and reverted. 23/23 packages green.

Not yet live-validated: STOP-1 supervised reconstitute, golden 0.153.0.
2026-07-20 17:01:52 +02:00

375 lines
16 KiB
Go

package backup
import (
"context"
"io"
"log"
"os"
"path/filepath"
"strings"
"testing"
)
// R-47 (v0.153.0) — the DB replay must not race the app, on BOTH restore paths.
//
// Every test here is a regression guard for a measured incident, not a description of the code.
// On 2026-07-19 (DIAG-immich-restore-round2-2026-07-19, H4) the offsite reconstitution started the
// WHOLE stack before replaying the dump. immich-server used the window to rebuild `clip_index` two
// seconds before the dump's own CREATE INDEX; the replay aborted `relation "clip_index" already
// exists` under ON_ERROR_STOP=1, and immich then reported schema drift. The data survived only by
// accident of pg_dump's ordering (COPY before CREATE INDEX) — a collision earlier in the script
// would have left a genuinely half-restored database, reported identically.
//
// The property under test is therefore an ORDERING plus a STATE-AT-REPLAY-TIME: at the moment the
// import fires, the database service must be up and the full stack must NOT be. Asserting only
// "no error" would pass on the pre-fix shape, which is exactly how this shipped.
// immichLikeCompose is the catalog's immich template reduced to what the resolver reads: the app
// services, the DB service, a redis that must never be mistaken for a database, and the top-level
// `volumes:`/`networks:` keys (including `immich_postgres_data`) that a line scan would misread.
const immichLikeCompose = `services:
immich-server:
image: ghcr.io/immich-app/immich-server:v3.0.3
immich-machine-learning:
image: ghcr.io/immich-app/immich-machine-learning:v3.0.3
immich-postgres:
image: ghcr.io/immich-app/postgres:16-vectorchord0.4.3-pgvectors0.2.0
immich-redis:
image: redis:7-alpine
volumes:
immich_ml_cache:
immich_postgres_data:
networks:
traefik-public:
external: true
`
// noDBCompose is a DB-free app: nothing here may ever trigger the DB-only phase.
const noDBCompose = `services:
app:
image: ghcr.io/x/app:1
cache:
image: redis:7-alpine
`
// --- Group A: offsite reconstitute, DB-bearing app (the H4 killer) --------------------------------
// TestReconstituteReplaysWithOnlyTheDBServiceUp is the core R-47 assertion for the offsite path.
// It does not merely check the call ORDER — it captures the provider's state AT THE MOMENT the
// import fires, because that is what H4 was: the sequence looked right, and the app was up.
//
// COMPANION RED-PROOF: replacing the DB-only bring-up in ReconstituteFromOffsite with the pre-fix
// full StartStack makes this fail on `full stack was ALREADY UP when the replay fired`.
func TestReconstituteReplaysWithOnlyTheDBServiceUp(t *testing.T) {
m, prov, imported := reconFixture(t, "20260719T060000Z", "2026-07-19T06:00:00Z", pgDump(1))
var dbUpAtReplay, fullUpAtReplay bool
m.importDBDump = func(_ context.Context, _ DiscoveredDB, p string) error {
dbUpAtReplay = len(prov.gotServices) > 0
fullUpAtReplay = prov.fullStarted
*imported = append(*imported, p)
return nil
}
res, err := m.ReconstituteFromOffsite(context.Background(), "immich")
if err != nil {
t.Fatalf("reconstitute: %v", err)
}
if res.DBsReplayed != 1 {
t.Fatalf("expected exactly one replay, got %d", res.DBsReplayed)
}
if !dbUpAtReplay {
t.Fatal("the database service was NOT started before the replay — ImportDump has no container to talk to")
}
if fullUpAtReplay {
t.Fatal("the FULL stack was already up when the replay fired — this is H4 exactly: the app races the dump's schema")
}
if got := strings.Join(prov.gotServices, ","); got != "immich-postgres" {
t.Fatalf("DB-only phase started %q, want only the database service immich-postgres", got)
}
if got := strings.Join(prov.calls, ","); got != "stop,startsvc:immich-postgres,start" {
t.Fatalf("sequence = %q, want stop → db-only start → replay → full start", got)
}
// The undo must have existed before any of it.
if res.SafetyDump == "" {
t.Fatal("no safety dump recorded")
}
if _, sErr := os.Stat(res.SafetyDump); sErr != nil {
t.Fatalf("safety dump not on disk before the mutation: %v", sErr)
}
}
// --- Group B: offsite reconstitute, no-DB app (flow unchanged) ------------------------------------
// TestReconstituteNoDBAppNeverStartsServicesOnly asserts the NEGATIVE: an app with no database must
// take exactly one full start and must never enter the DB-only phase. Without this, a bug that
// armed the phase for every app would show up first as a customer's stack half-started.
func TestReconstituteNoDBAppNeverStartsServicesOnly(t *testing.T) {
m, prov, imported := reconFixture(t, "run1", "2026-07-19T06:00:00Z", "")
prov.composePath = writeLiveCompose(t, noDBCompose)
m.discoverDBs = func(context.Context) ([]DiscoveredDB, error) { return nil, nil }
res, err := m.ReconstituteFromOffsite(context.Background(), "immich")
if err != nil {
t.Fatalf("a no-DB app must restore unchanged, got: %v", err)
}
if len(prov.gotServices) != 0 {
t.Fatalf("the DB-only phase ran for an app with no database: %v", prov.gotServices)
}
if got := strings.Join(prov.calls, ","); got != "stop,start" {
t.Fatalf("sequence = %q, want the unchanged stop → full start", got)
}
if len(*imported) != 0 || res.DBsReplayed != 0 {
t.Fatalf("a no-DB app must not replay anything: imported=%v replayed=%d", *imported, res.DBsReplayed)
}
}
// --- Group C: fail-closed, both paths -------------------------------------------------------------
// TestReconstituteRefusesWhenNoDBServiceIdentifiable is the security-adjacent gate. A dump exists and
// a live database was discovered, but the live compose names no startable database service. The only
// alternative to refusing would be to start everything and replay into the H4 race, so this must
// refuse — and it must refuse with ZERO mutations, which is what the effect assertions below prove.
// Asserting `err != nil` alone would pass even if the app had already been stopped and overwritten.
//
// COMPANION RED-PROOF: deleting the `len(dbServices) == 0` gate makes this fail on
// `the app was stopped despite the refusal`.
func TestReconstituteRefusesWhenNoDBServiceIdentifiable(t *testing.T) {
m, prov, imported := reconFixture(t, "run1", "2026-07-19T06:00:00Z", pgDump(1))
// A live compose whose services are all app/cache images — nothing to start alone.
prov.composePath = writeLiveCompose(t, noDBCompose)
var copied bool
m.SetOffboxFullPlaceCopier(func(_, _ string) (int, error) { copied = true; return 1, nil })
_, err := m.ReconstituteFromOffsite(context.Background(), "immich")
if err == nil {
t.Fatal("expected a refusal: a dump exists but no database service can be started for it")
}
if !strings.Contains(err.Error(), "nem azonosítható") {
t.Fatalf("refusal must say the database service could not be identified, got: %v", err)
}
if len(prov.calls) != 0 {
t.Fatalf("ZERO mutations required, but the provider was called: %v", prov.calls)
}
if copied {
t.Fatal("files were overwritten despite the refusal")
}
if len(*imported) != 0 {
t.Fatalf("a replay happened despite the refusal: %v", *imported)
}
}
// TestRestoreFromUnitRefusesWhenNoDBServiceIdentifiable is the local path's sibling gate, with the
// same zero-mutation requirement: no stop, no volume restore, no definition recreate.
func TestRestoreFromUnitRefusesWhenNoDBServiceIdentifiable(t *testing.T) {
m, prov, _ := r47UnitFixture(t, noDBCompose, true)
err := m.RestoreFromRecoveryUnit("app")
if err == nil {
t.Fatal("expected a refusal: the unit carries a dump but names no startable database service")
}
if !strings.Contains(err.Error(), "nem azonosítható") {
t.Fatalf("refusal must say the database service could not be identified, got: %v", err)
}
if len(prov.calls) != 0 {
t.Fatalf("ZERO mutations required, but the provider was called: %v", prov.calls)
}
if prov.stopped {
t.Fatal("the app was stopped despite the refusal")
}
if prov.gotEnv != nil {
t.Fatal("the definition was recreated despite the refusal")
}
}
// --- Group D: local restore-from-unit ordering ----------------------------------------------------
// TestRestoreFromUnitReplaysWithOnlyTheDBServiceUp is Group A's twin on the local path — the SAME
// class defect lived here, in the shape `RecreateStackFromUnit` (which ended in a full `up -d`)
// followed by the replay. Splitting the persist from the start is what makes this orderable at all.
//
// COMPANION RED-PROOF: restoring the pre-fix shape (RecreateStackDefinitionFromUnit performing a
// full start, replay after) makes this fail on `full stack was ALREADY UP when the replay fired`.
func TestRestoreFromUnitReplaysWithOnlyTheDBServiceUp(t *testing.T) {
m, prov, imported := r47UnitFixture(t, immichLikeCompose, true)
var dbUpAtReplay, fullUpAtReplay, definitionPersisted bool
m.importDBDump = func(_ context.Context, _ DiscoveredDB, p string) error {
dbUpAtReplay = len(prov.gotServices) > 0
fullUpAtReplay = prov.fullStarted
definitionPersisted = prov.gotEnv != nil
*imported = append(*imported, p)
return nil
}
if err := m.RestoreFromRecoveryUnit("app"); err != nil {
t.Fatalf("restore-from-unit: %v", err)
}
if len(*imported) != 1 {
t.Fatalf("expected exactly one replay, got %v", *imported)
}
if !definitionPersisted {
t.Fatal("the app definition was not persisted before the replay — the DB service could not have been started from it")
}
if !dbUpAtReplay {
t.Fatal("the database service was NOT started before the replay")
}
if fullUpAtReplay {
t.Fatal("the FULL stack was already up when the replay fired — the H4 race, on the local path")
}
if got := strings.Join(prov.calls, ","); got != "stop,recreate,startsvc:immich-postgres,start" {
t.Fatalf("sequence = %q, want stop → recreate(definition only) → db-only start → replay → full start", got)
}
}
// TestRestoreFromUnitNoDumpsTakesOneFullStart is the local no-DB negative: without a replayable dump
// there is no DB-only window at all, just the definition and one full start.
func TestRestoreFromUnitNoDumpsTakesOneFullStart(t *testing.T) {
m, prov, imported := r47UnitFixture(t, noDBCompose, false)
if err := m.RestoreFromRecoveryUnit("app"); err != nil {
t.Fatalf("restore-from-unit: %v", err)
}
if len(prov.gotServices) != 0 {
t.Fatalf("the DB-only phase ran with nothing to replay: %v", prov.gotServices)
}
if got := strings.Join(prov.calls, ","); got != "stop,recreate,start" {
t.Fatalf("sequence = %q, want stop → recreate → full start", got)
}
if len(*imported) != 0 {
t.Fatalf("nothing should have been replayed, got %v", *imported)
}
}
// TestRestoreFromUnitIgnoresSafetyDumpsWhenDecidingToReplay guards the one file-naming trap in the
// gate: `pre-restore-*.sql` safety dumps live in the SAME directory as the real dumps (deliberately —
// an undo the customer cannot see is not much of one) but are never a replay source. Counting them
// would arm the DB-only phase, and its refusal, for an app that has nothing to replay.
func TestRestoreFromUnitIgnoresSafetyDumpsWhenDecidingToReplay(t *testing.T) {
m, prov, _ := r47UnitFixture(t, noDBCompose, false)
// A safety dump present for a DB-less app must not arm anything — including the refusal.
mustWrite(t, filepath.Join(AppDBDumpPath(prov.hdd, "app"),
preRestoreDumpPrefix+"20260720T101010Z-app-postgres.sql"), pgDump(1))
if err := m.RestoreFromRecoveryUnit("app"); err != nil {
t.Fatalf("a lone safety dump must not turn into a refusal: %v", err)
}
if len(prov.gotServices) != 0 {
t.Fatalf("a safety dump armed the DB-only phase: %v", prov.gotServices)
}
}
// --- Group E: a failed replay never strands the box DB-only ---------------------------------------
// TestReconstituteReplayFailureStillBringsTheStackUp: the DB-only window is a deliberate half-started
// state, so EVERY exit from it must end in a full start. Otherwise a failed restore leaves the
// customer with a running database and no application — an outage caused by the recovery tool.
func TestReconstituteReplayFailureStillBringsTheStackUp(t *testing.T) {
m, prov, _ := reconFixture(t, "run1", "2026-07-19T06:00:00Z", pgDump(1))
m.importDBDump = func(context.Context, DiscoveredDB, string) error {
return context.DeadlineExceeded
}
res, err := m.ReconstituteFromOffsite(context.Background(), "immich")
if err == nil {
t.Fatal("a failed replay must be surfaced, not swallowed")
}
if !prov.fullStarted {
t.Fatal("the stack was left DB-ONLY after a failed replay — the app is down and nothing will bring it up")
}
if got := strings.Join(prov.calls, ","); got != "stop,startsvc:immich-postgres,start" {
t.Fatalf("sequence = %q, want the best-effort full start after the failure", got)
}
// The existing message shape stays: the operator needs the undo's filename.
if !strings.Contains(err.Error(), filepath.Base(res.SafetyDump)) {
t.Fatalf("the error must name the safety dump so the operator can undo, got: %v", err)
}
}
// TestReconstituteDBOnlyStartFailureStillBringsTheStackUp covers the other exit from the window: the
// DB-only start itself failing. Same requirement — the app must not be left down.
func TestReconstituteDBOnlyStartFailureStillBringsTheStackUp(t *testing.T) {
m, prov, imported := reconFixture(t, "run1", "2026-07-19T06:00:00Z", pgDump(1))
prov.startSvcErr = context.DeadlineExceeded
if _, err := m.ReconstituteFromOffsite(context.Background(), "immich"); err == nil {
t.Fatal("a failed DB-only start must be surfaced")
}
if !prov.fullStarted {
t.Fatal("the stack was left down after a failed DB-only start")
}
if len(*imported) != 0 {
t.Fatalf("nothing may be replayed when the database never came up: %v", *imported)
}
}
// TestRestoreFromUnitReplayFailureStillBringsTheStackUp is the local path's version, and it also
// pins the pre-existing semantics: a replay error becomes a dataErr and surfaces as the "completed
// with data errors" outcome, with the app back up.
func TestRestoreFromUnitReplayFailureStillBringsTheStackUp(t *testing.T) {
m, prov, _ := r47UnitFixture(t, immichLikeCompose, true)
m.importDBDump = func(context.Context, DiscoveredDB, string) error {
return context.DeadlineExceeded
}
err := m.RestoreFromRecoveryUnit("app")
if err == nil {
t.Fatal("a failed replay must be surfaced, not swallowed")
}
if !strings.Contains(err.Error(), "completed with data errors") {
t.Fatalf("the pre-existing outcome semantics must be preserved, got: %v", err)
}
if !prov.fullStarted {
t.Fatal("the stack was left DB-ONLY after a failed replay")
}
if got := strings.Join(prov.calls, ","); got != "stop,recreate,startsvc:immich-postgres,start" {
t.Fatalf("sequence = %q, want the full start to follow the failed replay", got)
}
}
// --- fixtures -------------------------------------------------------------------------------------
// writeLiveCompose drops a compose file in its own temp dir and returns the path.
func writeLiveCompose(t *testing.T, body string) string {
t.Helper()
p := filepath.Join(t.TempDir(), "docker-compose.yml")
if err := os.WriteFile(p, []byte(body), 0o644); err != nil {
t.Fatal(err)
}
return p
}
// r47UnitFixture builds a Manager whose local recovery unit for "app" carries the given compose and,
// optionally, a replayable `app-postgres.sql` dump. The provider records call order; the DB
// discovery/import seams are injected so no Docker is touched.
func r47UnitFixture(t *testing.T, compose string, withDump bool) (*Manager, *fakeRecoveryProvider, *[]string) {
t.Helper()
drive := filepath.Join(t.TempDir(), "drive")
composeDir := RecoveryUnitComposePath(drive, "app")
mustWrite(t, filepath.Join(composeDir, "app.yaml"), "deployed: true\nenv:\n SUBDOMAIN: app\n")
mustWrite(t, filepath.Join(composeDir, "docker-compose.yml"), compose)
man := &RecoveryManifest{SchemaVersion: 1, AppName: "app", ControllerVer: "v"}
if err := writeManifest(RecoveryUnitManifestPath(drive, "app"), man); err != nil {
t.Fatal(err)
}
if withDump {
mustWrite(t, filepath.Join(AppDBDumpPath(drive, "app"), "app-postgres.sql"), pgDump(1))
}
prov := &fakeRecoveryProvider{hdd: drive, running: true}
m := &Manager{
logger: log.New(io.Discard, "", 0),
systemDataPath: filepath.Join(drive, "..", "sys"),
stackProvider: prov,
}
db := DiscoveredDB{StackName: "app", ContainerName: "immich-postgres", DBType: DBTypePostgres}
m.discoverDBs = func(context.Context) ([]DiscoveredDB, error) { return []DiscoveredDB{db}, nil }
var imported []string
m.importDBDump = func(_ context.Context, _ DiscoveredDB, p string) error {
imported = append(imported, p)
return nil
}
return m, prov, &imported
}