amadan
provision, registry, domains: missing-row purge and a compare-and-delete for trash (Plan 7 Task 5 fix round 1)
I1: when the import's row is gone, the rollback still removes its local data under the fence, at the host's default nested database path, when step 6's test says it is the import's (record gone, or naming this seed abandoned, and no other record holding the host). Another record holding the host skips the removal (logged); a test that cannot decide lifts the fence and defers. I2: registry.DeleteIfUnchanged deletes a route only while it still has the identity it was listed with (kind, account, app, provisioned, import gate, seed column, creation time). The domains trash loop uses it, so a rollback that restores another owner's original row mid-pass keeps it and its grants. Minors: lift this rollback's fence on the pre-check hold and on a failed fence hook; one dry-run phrasing; the fault table's trigger "crash" rows documented.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
7 files changed,
+379
−15
internal/activator/seedpurge_linux_test.go+34 −0internal/domains/importjournal_test.go+66 −0internal/domains/reconcile.go+13 −1internal/provision/seedrollback.go+45 −5internal/provision/seedrollback_test.go+148 −9internal/registry/importjournal_test.go+47 −0internal/registry/routecas.go+26 −0
diff --git a/internal/activator/seedpurge_linux_test.go b/internal/activator/seedpurge_linux_test.go| index de228fe04..23d2f8d72 100644 |
| --- a/internal/activator/seedpurge_linux_test.go |
| +++ b/internal/activator/seedpurge_linux_test.go |
| @@ -3,6 +3,7 @@ |
| package activator |
| import ( |
| + "context" |
| "os" |
| "path/filepath" |
| "testing" |
| @@ -84,3 +85,36 @@ func TestPurgeSeedLocalNeverFollowsASymlink(t *testing.T) { |
| t.Fatalf("the link's target was touched: %q %v", b, err) |
| } |
| } |
| + |
| +// Fix round 1, I1, the activator half: an aborted import's database left at |
| +// a host's nested path is what a later plain instance at the host wakes |
| +// on. Left there, the plain wake takes it as a warm cache (not fresh); |
| +// after PurgeSeedLocal, the wake finds no file and comes up fresh. |
| +func TestAPlainWakeAfterPurgeSeedLocalFindsNoDatabase(t *testing.T) { |
| + for _, purge := range []bool{false, true} { |
| + b, rt := nestedBackend(t, &fakeControl{}, &fakeUnit{}) |
| + if err := os.MkdirAll(filepath.Dir(rt.DBPath), 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, f := range []string{rt.DBPath, rt.DBPath + "-wal", rt.DBPath + SeedMarkerSuffix} { |
| + if err := os.WriteFile(f, []byte("an aborted import's data"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if purge { |
| + if err := PurgeSeedLocal(rt.DBPath); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + oldRestore := restoreReplica |
| + restoreReplica = func(context.Context, string, string, string, string) (bool, error) { return false, nil } |
| + fresh, err := b.Restore(context.Background(), rt) // a plain row: no seed column |
| + restoreReplica = oldRestore |
| + if err != nil || fresh != purge { |
| + t.Fatalf("purge %v: fresh %v err %v", purge, fresh, err) |
| + } |
| + if _, err := os.Lstat(rt.DBPath); purge && !os.IsNotExist(err) { |
| + t.Fatalf("the wake found a database at %s", rt.DBPath) |
| + } |
| + } |
| +} |
diff --git a/internal/domains/importjournal_test.go b/internal/domains/importjournal_test.go| index cec4ec1eb..0ce34e405 100644 |
| --- a/internal/domains/importjournal_test.go |
| +++ b/internal/domains/importjournal_test.go |
| @@ -1,6 +1,7 @@ |
| package domains_test |
| import ( |
| + "context" |
| "database/sql" |
| "path/filepath" |
| "reflect" |
| @@ -104,3 +105,68 @@ func TestReconcileTrashRemovesNothingWhenTheJournalCannotBeRead(t *testing.T) { |
| mustGet(t, reg, gated.Host) |
| mustGet(t, reg, "b.shop.example.com") |
| } |
| + |
| +// hookStore runs once, on the first read of one key, a change another |
| +// process makes in the middle of a domains pass. |
| +type hookStore struct { |
| + release.Store |
| + key string |
| + once func() |
| +} |
| + |
| +func (h *hookStore) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + if key == h.key && h.once != nil { |
| + f := h.once |
| + h.once = nil |
| + f() |
| + } |
| + return h.Store.Get(ctx, key) |
| +} |
| + |
| +// Fix round 1, I2: the domains pass lists the routes and sees the trashed |
| +// app's import row. Before its trash loop runs, the provision pass's |
| +// rollback restores the original row (the operator's, carrying a grant) |
| +// and retires the journal row. The trash delete must not remove that row |
| +// on the strength of the import row it listed. |
| +func TestReconcileTrashLeavesARowARollbackRestoredMidPass(t *testing.T) { |
| + s := dirStore(t) |
| + sqid := writeAccountManifest(t, s, 793481920157, "Shop") |
| + putKey(t, s, "apps/"+sqid+"/shop/channels/stable.json") |
| + reg := regWith(t, registry.Route{Host: "a.shop.example.com", Kind: "service", Addr: "127.0.0.1:8401", |
| + App: "shop", Account: "bab", Status: "ready", Channel: "stable", Caps: "port:25", CreatedAt: 1700000000}) |
| + original := *mustGet(t, reg, "a.shop.example.com") |
| + const seedID = "sd_aaaaaaaaaaaaaaaaaaaaaaaaaa" |
| + next := registry.Route{Host: original.Host, Kind: "instance", Socket: "/run/carlos/instances/a/app.sock", |
| + DBPath: "/data/instances/a/a.shop.example.com.db", App: "shop", Account: sqid, Status: "ready", Channel: "stable", |
| + Hibernate: true, Provisioned: true, ImportGate: "abc123", |
| + SeedSource: `{"id":"` + seedID + `","kind":"import","state":"swapped"}`} |
| + if err := reg.SwapForImport(registry.ImportJournal{Host: original.Host, SeedID: seedID, Original: original, |
| + RecordKey: "apps/" + sqid + "/shop/instances/a.shop.example.com.json", At: 1}, next); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := release.TrashApp(ctx, s, sqid, "shop", "owner@example.com", time.Unix(1753900000, 0)); err != nil { |
| + t.Fatal(err) |
| + } |
| + ran := false |
| + store := &hookStore{Store: s, key: release.AppTombstoneKey(sqid, "shop"), once: func() { |
| + ran = true |
| + for _, step := range []func() error{ |
| + func() error { return reg.CloseImportGate(original.Host, seedID, 1) }, |
| + func() error { return reg.RestoreImportRoute(original.Host, seedID) }, |
| + func() error { return reg.DeleteImportJournal(original.Host, seedID) }, |
| + } { |
| + if err := step(); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + }} |
| + if err := domains.Reconcile(ctx, store, reg, cfg(t)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if !ran { |
| + t.Fatal("the rollback never ran between the list and the trash loop") |
| + } |
| + if got := mustGet(t, reg, original.Host); !reflect.DeepEqual(*got, original) { |
| + t.Fatalf("the restored row changed: %+v, want %+v", got, original) |
| + } |
| +} |
diff --git a/internal/domains/reconcile.go b/internal/domains/reconcile.go| index 9e0488db3..cd69ca76a 100644 |
| --- a/internal/domains/reconcile.go |
| +++ b/internal/domains/reconcile.go |
| @@ -210,6 +210,12 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg |
| // same reason. The row goes on a later pass, once the journal lets go. |
| // A journal that cannot be read could hide any host, so then no trashed |
| // route is removed this pass. |
| + // |
| + // The list is from the top of the pass, and this loop runs alongside |
| + // the provision pass. A rollback can put another owner's original row |
| + // back at a listed host and retire its journal row in between, so the |
| + // delete is conditional on the row still being the one listed |
| + // (DeleteIfUnchanged); a row that changed is left for the next pass. |
| if len(trashed) > 0 { |
| journaled, jerr := importJournalHosts(reg) |
| if jerr != nil { |
| @@ -225,11 +231,17 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg |
| keptLive = append(keptLive, rt) |
| continue |
| } |
| - if err := reg.Delete(rt.Host); err != nil { |
| + deleted, err := reg.DeleteIfUnchanged(rt) |
| + if err != nil { |
| log.Printf("domains: remove route %s of trashed app %s: %v", rt.Host, rt.App, err) |
| keptLive = append(keptLive, rt) // still there; try next pass |
| continue |
| } |
| + if !deleted { |
| + log.Printf("domains: not removing route %s of trashed app %s: the row changed since this pass listed it", rt.Host, rt.App) |
| + keptLive = append(keptLive, rt) // judged again next pass, from a fresh list |
| + continue |
| + } |
| log.Printf("domains: removed route %s — app %s is in the trash", rt.Host, rt.App) |
| continue |
| } |
diff --git a/internal/provision/seedrollback.go b/internal/provision/seedrollback.go| index 68d8df5b5..99741b531 100644 |
| --- a/internal/provision/seedrollback.go |
| +++ b/internal/provision/seedrollback.go |
| @@ -30,14 +30,15 @@ import ( |
| // journal row came back from a snapshot, and the rollback holds there, |
| // touching nothing, until an operator looks. A host with no row at all is |
| // rolled back without a restore: whatever removed the import's row decided |
| -// the host should not be served. |
| +// the host should not be served. Its local data at the host's default path |
| +// is still removed when step 6's test says it is the import's. |
| // |
| // rec is the pass's view of the journal's record (nil when gone). It chose |
| // the rollback; the destructive cleanup reads the record again (step 6). |
| func rollbackImport(ctx context.Context, s release.Store, reg *registry.Registry, cfg ReconcileConfig, j registry.ImportJournal, rec *Record) { |
| h := cfg.Seeds |
| if cfg.DryRun { |
| - log.Printf("provision: dry-run: would roll back import %s of %s (%s)", j.SeedID, j.Host, j.State) |
| + log.Printf("provision: dry-run: would roll back %s's import %s", j.Host, j.SeedID) |
| return |
| } |
| if h == nil || h.FenceForRollback == nil || h.LiftFence == nil || h.RemoveLocal == nil || h.DeleteReplica == nil || h.NextFenceGen == nil { |
| @@ -47,7 +48,7 @@ func rollbackImport(ctx context.Context, s release.Store, reg *registry.Registry |
| switch j.State { |
| case registry.JournalSwapped, registry.JournalRollingBack: |
| var ok bool |
| - if j, ok = rollBackRoute(ctx, reg, cfg, j); !ok { |
| + if j, ok = rollBackRoute(ctx, s, reg, cfg, j); !ok { |
| return |
| } |
| case registry.JournalCleanupPending: |
| @@ -68,7 +69,7 @@ func importRowOf(cfg ReconcileConfig, rt registry.Route, j registry.ImportJourna |
| // rollBackRoute is steps 1 to 5. It returns the journal as it now stands |
| // and whether the route steps are done (the journal is cleanup-pending). |
| -func rollBackRoute(ctx context.Context, reg *registry.Registry, cfg ReconcileConfig, j registry.ImportJournal) (registry.ImportJournal, bool) { |
| +func rollBackRoute(ctx context.Context, s release.Store, reg *registry.Registry, cfg ReconcileConfig, j registry.ImportJournal) (registry.ImportJournal, bool) { |
| h := cfg.Seeds |
| // Nothing is closed, fenced or removed for a row that is not the |
| // import's. The registry refuses the gate close and the restore for |
| @@ -79,6 +80,12 @@ func rollBackRoute(ctx context.Context, reg *registry.Registry, cfg ReconcileCon |
| return j, false |
| } |
| if row != nil && !importRowOf(cfg, *row, j) { |
| + // A fence an earlier pass kept for this rollback must not hold the |
| + // host for whoever owns it now. LiftFence lifts only this |
| + // generation. |
| + if j.State == registry.JournalRollingBack { |
| + h.LiftFence(j.Host, j.FenceGen) |
| + } |
| log.Printf("provision: %s: import %s is not rolled back: the route at the host is not the import's, so nothing there is touched", j.Host, j.SeedID) |
| return j, false |
| } |
| @@ -103,6 +110,9 @@ func rollBackRoute(ctx context.Context, reg *registry.Registry, cfg ReconcileCon |
| // hook lifts any fence it installs when it does not proceed. |
| proceed, err := h.FenceForRollback(ctx, j.Host, j.FenceGen) |
| if err != nil || !proceed { |
| + if err != nil { |
| + h.LiftFence(j.Host, j.FenceGen) // a hook that failed may have fenced first |
| + } |
| log.Printf("provision: %s: import %s rollback deferred: the host is not fenced and stopped yet (%v)", j.Host, j.SeedID, err) |
| return j, false |
| } |
| @@ -116,7 +126,26 @@ func rollBackRoute(ctx context.Context, reg *registry.Registry, cfg ReconcileCon |
| } |
| switch { |
| case row == nil: |
| - log.Printf("provision: %s: import %s has no route on this box: nothing local is removed and no route is put back", j.Host, j.SeedID) |
| + // Something removed the import's row, but not its local data: a |
| + // later plain instance at the host would open that database as a |
| + // warm cache and replicate it as its own. Whether the data at the |
| + // host's default path is the import's is step 6's question. |
| + owned, why, err := importOwnsHost(ctx, s, j) |
| + switch { |
| + case err != nil: |
| + h.LiftFence(j.Host, j.FenceGen) |
| + log.Printf("provision: %s: import %s rollback deferred: whose local data is at the host cannot be told: %v", j.Host, j.SeedID, err) |
| + return j, false |
| + case !owned: |
| + log.Printf("provision: %s: import %s has no route on this box, and its local data is left alone: %s", j.Host, j.SeedID, why) |
| + default: |
| + local := registry.Route{Host: j.Host, Kind: "instance", DBPath: NestedDBPath(cfg.DataDir, j.Host)} |
| + if err := h.RemoveLocal(local); err != nil { |
| + log.Printf("provision: %s: import %s: remove its local data: %v", j.Host, j.SeedID, err) |
| + return j, false |
| + } |
| + log.Printf("provision: %s: import %s has no route on this box: its local data is removed and no route is put back", j.Host, j.SeedID) |
| + } |
| case !importRowOf(cfg, *row, j) || row.ImportGate != registry.ImportGateClosed: |
| h.LiftFence(j.Host, j.FenceGen) |
| log.Printf("provision: %s: import %s is not rolled back: the route changed under the fence, so nothing there is touched", j.Host, j.SeedID) |
| @@ -196,6 +225,17 @@ func cleanUpImport(ctx context.Context, s release.Store, reg *registry.Registry, |
| } |
| } |
| +// importOwnsHost is replicaIsTheImports on a fresh read of the journal's |
| +// record: whether what the box holds for the host by host name alone (its |
| +// replica, and its local data at the default path) is still the import's. |
| +func importOwnsHost(ctx context.Context, s release.Store, j registry.ImportJournal) (owned bool, why string, err error) { |
| + rec, err := journalRecordByKey(ctx, s, j) |
| + if err != nil { |
| + return false, "", fmt.Errorf("its record cannot be read: %w", err) |
| + } |
| + return replicaIsTheImports(ctx, s, j, rec) |
| +} |
| + |
| // replicaIsTheImports decides step 6's replica delete. The host's replica |
| // is the import's to delete only while the journal's record, if there is |
| // one, still names this seed in an abandoned state, and no other record |
diff --git a/internal/provision/seedrollback_test.go b/internal/provision/seedrollback_test.go| index 5cc2ac9b3..41b41294d 100644 |
| --- a/internal/provision/seedrollback_test.go |
| +++ b/internal/provision/seedrollback_test.go |
| @@ -149,7 +149,8 @@ func TestRollbackDefersWhenTheFenceIsNotOurs(t *testing.T) { |
| } |
| } |
| -// A fence hook that answers with an error defers, whatever proceed says. |
| +// A fence hook that answers with an error defers, whatever proceed says, |
| +// and this rollback's generation is lifted (review Minor 1). |
| func TestRollbackDefersOnAFenceError(t *testing.T) { |
| x := abortedImport(t) |
| rr := &rollbackRecorder{proceed: true, fenceErr: errors.New("stop unproven")} |
| @@ -158,7 +159,7 @@ func TestRollbackDefersOnAFenceError(t *testing.T) { |
| if j, _ := x.reg.ImportJournal(x.rec.Host); j == nil || j.State != registry.JournalRollingBack { |
| t.Fatalf("journal = %+v", j) |
| } |
| - if !reflect.DeepEqual(rr.steps, []string{"fence"}) { |
| + if !reflect.DeepEqual(rr.steps, []string{"fence", "lift"}) { |
| t.Fatalf("steps = %q", rr.steps) |
| } |
| } |
| @@ -362,7 +363,9 @@ func (b *rollbackBox) hooks(h *SeedHooks) { |
| if j := b.journal(); j == nil || j.State != registry.JournalRollingBack { |
| b.t.Errorf("removed local data with the journal at %+v", j) |
| } |
| - if rt.ImportGate != registry.ImportGateClosed || rt.DBPath != NestedDBPath("/data", rt.Host) { |
| + // The closed import row, or (its row gone) the host's default |
| + // nested database path. |
| + if rt.DBPath != NestedDBPath("/data", rt.Host) || (rt.ImportGate != registry.ImportGateClosed && !b.noRoute) { |
| b.t.Errorf("removed local data for a row that is not the closed import row: %+v", rt) |
| } |
| if err := b.trip("remove-local"); err != nil { |
| @@ -534,6 +537,10 @@ func count(steps []string, step string) int { |
| // with nothing lost and nothing done twice that must not be: one fence |
| // generation per close, no local removal or fence once the route is back, |
| // and no bucket delete before it is. |
| +// |
| +// The three registry-trigger rows "crash" as a failed transaction followed |
| +// by a restart, not as a panic inside the transaction: SQLite rolls the |
| +// transaction back either way, so the state left is the same. |
| func TestRollbackResumesAfterAFailureBetweenAnyTwoSteps(t *testing.T) { |
| const raise = ` BEGIN SELECT RAISE(ABORT, 'injected'); END` |
| for _, f := range []struct { |
| @@ -747,7 +754,9 @@ func TestRollbackOfAStaleJournalTouchesNothingTheNewInstanceOwns(t *testing.T) { |
| t.Fatalf("the new instance's record changed: %+v", got) |
| } |
| for _, s := range x.box.steps { |
| - if s != "delete-snapshot" { |
| + // A lift names only the stale row's own generation, |
| + // which no fence on the host holds. |
| + if s != "delete-snapshot" && s != "lift" { |
| t.Fatalf("the stale journal reached the box: %q\n%s", x.box.steps, logged) |
| } |
| } |
| @@ -772,8 +781,9 @@ func TestRollbackOfAStaleJournalTouchesNothingTheNewInstanceOwns(t *testing.T) { |
| } |
| // The rollback tolerates a missing import row (the Task 4 review's carry): |
| -// something removed it, so no route is put back, nothing local is removed |
| -// for a row that is not there, and the cleanup carries on. |
| +// something removed it, so no route is put back, and the cleanup carries |
| +// on. Its local data is still removed under the fence (fix round 1, I1): |
| +// the record names this seed, aborted, and no other record holds the host. |
| func TestRollbackOfAMissingRowSkipsTheRestore(t *testing.T) { |
| x := abortedImport(t) |
| if err := x.reg.Delete(x.rec.Host); err != nil { |
| @@ -784,12 +794,12 @@ func TestRollbackOfAMissingRowSkipsTheRestore(t *testing.T) { |
| if rt, _ := x.reg.Get(x.rec.Host); rt != nil { |
| t.Fatalf("a route came back: %+v", rt) |
| } |
| - want := []string{"gen", "fence", "lift", "delete-snapshot", "delete-replica"} |
| + want := []string{"gen", "fence", "remove-local", "lift", "delete-snapshot", "delete-replica"} |
| if !reflect.DeepEqual(x.box.steps, want) { |
| t.Fatalf("steps = %q, want %q", x.box.steps, want) |
| } |
| - if x.journalState(t) != "" || x.box.replica { |
| - t.Fatalf("journal %q replica %v", x.journalState(t), x.box.replica) |
| + if x.journalState(t) != "" || x.box.replica || x.box.local { |
| + t.Fatalf("journal %q replica %v local %v", x.journalState(t), x.box.replica, x.box.local) |
| } |
| if st := seedState(t, x.raw, x.rec); st.State != seeds.StateRolledBack { |
| t.Fatalf("record = %s", st.State) |
| @@ -929,3 +939,132 @@ func TestRollbackCleanupOfAnInvalidSeedIDDeletesNothing(t *testing.T) { |
| t.Fatalf("row = %+v", got) |
| } |
| } |
| + |
| +// Fix round 1, I1: a missing row whose host another record now holds. The |
| +// data at the host's default path may be that record's, so it is left, |
| +// and logged; the replica is left for the same reason. |
| +func TestRollbackOfAMissingRowLeavesAnotherRecordsLocalData(t *testing.T) { |
| + x := abortedImport(t) |
| + if err := x.reg.Delete(x.rec.Host); err != nil { |
| + t.Fatal(err) |
| + } |
| + x.box.noRoute = true |
| + other := shopRecord(x.rec.Account) |
| + other.App = "other" |
| + if err := CreateRecord(ctx, x.raw, other); err != nil { |
| + t.Fatal(err) |
| + } |
| + logged := captureLog(t) |
| + x.pass(t) |
| + if count(x.box.steps, "remove-local") != 0 || !x.box.local || !x.box.replica { |
| + t.Fatalf("steps %q local %v replica %v", x.box.steps, x.box.local, x.box.replica) |
| + } |
| + if !strings.Contains(logged.String(), "its local data is left alone: the record "+RecordKey(other.Account, other.App, other.Host)+" holds the host") { |
| + t.Fatalf("not logged:\n%s", logged) |
| + } |
| + if x.journalState(t) != "" || len(x.box.fenced) != 0 { |
| + t.Fatalf("journal %q fenced %v", x.journalState(t), x.box.fenced) |
| + } |
| +} |
| + |
| +// Fix round 1, I1: a missing row whose record cannot be read under the |
| +// fence. Nothing is removed, the fence is lifted, and the rollback waits |
| +// at rolling-back. |
| +func TestRollbackOfAMissingRowDefersWhenItCannotTell(t *testing.T) { |
| + x := abortedImport(t) |
| + if err := x.reg.Delete(x.rec.Host); err != nil { |
| + t.Fatal(err) |
| + } |
| + x.box.noRoute = true |
| + // The pass's own read works; the one under the fence fails. |
| + x.store = &failNthGet{Store: x.store, key: RecordKey(x.rec.Account, x.rec.App, x.rec.Host), n: 2} |
| + x.pass(t) |
| + want := []string{"gen", "fence", "lift"} |
| + if !reflect.DeepEqual(x.box.steps, want) || !x.box.local || len(x.box.fenced) != 0 { |
| + t.Fatalf("steps %q (want %q) local %v fenced %v", x.box.steps, want, x.box.local, x.box.fenced) |
| + } |
| + if st := x.journalState(t); st != registry.JournalRollingBack { |
| + t.Fatalf("journal = %q", st) |
| + } |
| +} |
| + |
| +// Review Minor 1: a fence hook that fails is followed by a lift of this |
| +// rollback's generation, in case it fenced before it failed. |
| +func TestRollbackLiftsTheFenceWhenTheFenceHookFails(t *testing.T) { |
| + x := abortedImport(t) |
| + fence := x.cfg.Seeds.FenceForRollback |
| + x.cfg.Seeds.FenceForRollback = func(ctx context.Context, host string, gen uint64) (bool, error) { |
| + _, _ = fence(ctx, host, gen) // installs it |
| + return true, errors.New("stop evidence unreadable") |
| + } |
| + x.pass(t) |
| + if len(x.box.fenced) != 0 || count(x.box.steps, "remove-local") != 0 { |
| + t.Fatalf("fenced %v steps %q", x.box.fenced, x.box.steps) |
| + } |
| +} |
| + |
| +// Fix round 1, I1, the provision half of the end-to-end check: with the |
| +// import's row gone, the local removal is aimed at the host's default |
| +// nested database path, which is exactly where a plain instance created at |
| +// the host afterwards is minted. (The activator half, seedpurge_linux_test.go, |
| +// shows a plain wake at that path finds no file once PurgeSeedLocal ran.) |
| +func TestRollbackOfAMissingRowPurgesWhereTheNextInstanceIsMinted(t *testing.T) { |
| + s, sqid := shopSetup(t) |
| + rec := importRecord(t, s, sqid) |
| + reg := regWith(t, addrRow(rec, sqid)) |
| + cfg := importCfg(t, nil) |
| + rr := &rollbackRecorder{proceed: true} |
| + rr.hooks(cfg.Seeds) |
| + var purged []string |
| + cfg.Seeds.RemoveLocal = func(rt registry.Route) error { purged = append(purged, rt.DBPath); return nil } |
| + if err := Reconcile(ctx, s, reg, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := SetRecordSeed(ctx, s, rec.Account, rec.App, rec.Host, SeedAbort(rec.Seed.ID)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := reg.Delete(rec.Host); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := Reconcile(ctx, s, reg, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + if want := []string{NestedDBPath(cfg.DataDir, rec.Host)}; !reflect.DeepEqual(purged, want) { |
| + t.Fatalf("purged %q, want %q", purged, want) |
| + } |
| + if err := DeleteRecord(ctx, s, rec.Account, rec.App, rec.Host); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := CreateRecord(ctx, s, shopRecord(sqid)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := Reconcile(ctx, s, reg, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + if next := mustGet(t, reg, rec.Host); next.DBPath != purged[0] || next.SeedSource != "" { |
| + t.Fatalf("the plain instance's row = %+v, want it at %s", next, purged[0]) |
| + } |
| +} |
| + |
| +// Review Minor 1: a fence this rollback kept after a failed step is lifted |
| +// when, by the next pass, the row at the host is no longer the import's. |
| +func TestRollbackLiftsAKeptFenceWhenTheRowIsNoLongerTheImports(t *testing.T) { |
| + x := abortedImport(t) |
| + x.box.fault = "remove-local" // fails once; the fence is kept for the resume |
| + x.pass(t) |
| + if len(x.box.fenced) != 1 { |
| + t.Fatalf("fenced %v, want this rollback's fence kept", x.box.fenced) |
| + } |
| + other := addrRow(x.rec, x.rec.Account) |
| + other.Addr = "127.0.0.1:9999" |
| + if err := x.reg.Delete(x.rec.Host); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := x.reg.Upsert(other); err != nil { |
| + t.Fatal(err) |
| + } |
| + x.pass(t) |
| + if len(x.box.fenced) != 0 || x.journalState(t) != registry.JournalRollingBack { |
| + t.Fatalf("fenced %v journal %q", x.box.fenced, x.journalState(t)) |
| + } |
| +} |
diff --git a/internal/registry/importjournal_test.go b/internal/registry/importjournal_test.go| index dd9d228ea..4bd90293e 100644 |
| --- a/internal/registry/importjournal_test.go |
| +++ b/internal/registry/importjournal_test.go |
| @@ -680,3 +680,50 @@ func TestIsImportRow(t *testing.T) { |
| t.Error("a journal with no seed id recognised a row") |
| } |
| } |
| + |
| +// Fix round 1, I2: a delete decided from an earlier list removes the row |
| +// only while it is still the row listed. A rollback that put the original |
| +// (another owner's, with its grants) back in between keeps it. |
| +func TestDeleteIfUnchanged(t *testing.T) { |
| + reg, stored := swappedRegistry(t) |
| + listed := mustRoute(t, reg, importHost) |
| + if err := reg.CloseImportGate(importHost, importSeedID, 7); err != nil { |
| + t.Fatal(err) |
| + } |
| + closed := mustRoute(t, reg, importHost) |
| + if err := reg.RestoreImportRoute(importHost, importSeedID); err != nil { |
| + t.Fatal(err) |
| + } |
| + for name, stale := range map[string]Route{"the gated import row": listed, "the closed import row": closed} { |
| + if ok, err := reg.DeleteIfUnchanged(stale); err != nil || ok { |
| + t.Fatalf("%s: deleted %v err %v", name, ok, err) |
| + } |
| + } |
| + if got := mustRoute(t, reg, importHost); got != stored { |
| + t.Fatalf("the restored row changed: %+v", got) |
| + } |
| + for name, mutate := range map[string]func(*Route){ |
| + "another account": func(r *Route) { r.Account = "bbw" }, |
| + "another app": func(r *Route) { r.App = "other" }, |
| + "another kind": func(r *Route) { r.Kind = "instance" }, |
| + "provisioned": func(r *Route) { r.Provisioned = true }, |
| + "a gate": func(r *Route) { r.ImportGate = ImportGateClosed }, |
| + "a seed column": func(r *Route) { r.SeedSource = `{"id":"` + importSeedID + `"}` }, |
| + "another creation": func(r *Route) { r.CreatedAt++ }, |
| + } { |
| + stale := stored |
| + mutate(&stale) |
| + if ok, err := reg.DeleteIfUnchanged(stale); err != nil || ok { |
| + t.Fatalf("%s: deleted %v err %v", name, ok, err) |
| + } |
| + } |
| + if ok, err := reg.DeleteIfUnchanged(stored); err != nil || !ok { |
| + t.Fatalf("the row as listed: deleted %v err %v", ok, err) |
| + } |
| + if rt, _ := reg.Get(importHost); rt != nil { |
| + t.Fatalf("still there: %+v", rt) |
| + } |
| + if ok, err := reg.DeleteIfUnchanged(stored); err != nil || ok { |
| + t.Fatalf("a second delete: %v %v", ok, err) |
| + } |
| +} |
diff --git a/internal/registry/routecas.go b/internal/registry/routecas.go| new file mode 100644 |
| index 000000000..5891ca154 |
| --- /dev/null |
| +++ b/internal/registry/routecas.go |
| @@ -0,0 +1,26 @@ |
| +package registry |
| + |
| +// DeleteIfUnchanged deletes listed.Host's route only while the stored row |
| +// still has the identity the caller listed it with: its kind, account and |
| +// app (the owner), whether a record provisioned it, its import gate and |
| +// seed column, and its creation time. It reports whether a row was |
| +// deleted. |
| +// |
| +// It is for a pass that decides from a route list read earlier and |
| +// deletes by host. Between the list and the delete, another writer can |
| +// put a different row at the host: an import's rollback restoring the |
| +// original route, owned by someone else, is the case it was written for. |
| +// A plain Delete would remove that row, and the operator grants it |
| +// carries. Here the check and the delete are one statement, so a row that |
| +// changed identity is never removed on the strength of the old one. |
| +func (r *Registry) DeleteIfUnchanged(listed Route) (bool, error) { |
| + res, err := r.sql.Exec(`DELETE FROM routes WHERE host = ? AND kind = ? AND account = ? AND app = ? |
| + AND provisioned = ? AND import_gate = ? AND seed_source = ? AND created_at = ?`, |
| + listed.Host, listed.Kind, listed.Account, listed.App, b2iDB(listed.Provisioned), |
| + listed.ImportGate, listed.SeedSource, listed.CreatedAt) |
| + if err != nil { |
| + return false, err |
| + } |
| + n, err := res.RowsAffected() |
| + return n > 0, err |
| +} |