amadan
cli, provision: the unflag and re-add guards also refuse a host with a layout journal (plan 4 final review)
A row whose paths are still flat can be mid-move, and the journal is what says so. carlos route --hibernate=false, carlos add and the reconcile flag-clear now refuse such a host too, with the approved texts unchanged. The Lstat predicate moves to provision.LayoutJournalExists so provision can use it; layout.JournalPresent delegates to it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6 files changed,
+120
−8
cli.go+7 −4cli_test.go+52 −0internal/layout/journal.go+1 −2internal/provision/provision.go+9 −0internal/provision/reconcile.go+5 −2internal/provision/reconcile_test.go+46 −0
diff --git a/cli.go b/cli.go| index c995a392f..3775df883 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -70,9 +70,11 @@ func reservedSlugs() map[string]bool { |
| // with this verb's flat defaults, which would leave the row pointing at |
| // the shared directory over files that live under the instance tree, and |
| // the next wake would restore a second database there (design §8.2, |
| -// downgrade rules). A host with no row, or a flat one, goes ahead. |
| +// downgrade rules). A host with a layout journal is mid-move, row or no |
| +// row, and a re-add would rewrite what recovery is about to check, so it |
| +// is refused the same way. A host with no row, or a flat one, goes ahead. |
| func refuseAddNested(host string, existing *registry.Route, dataDir string) error { |
| - if existing != nil && provision.NestedPaths(dataDir, *existing) { |
| + if (existing != nil && provision.NestedPaths(dataDir, *existing)) || provision.LayoutJournalExists(dataDir, host) { |
| return fmt.Errorf("add %s: its paths are nested. revert the layout first; re-adding it would point it back at the shared directory", host) |
| } |
| return nil |
| @@ -1393,9 +1395,10 @@ func portGrantConflict(routes []registry.Route, host, tok string) error { |
| // management, and the layout revert only quiesces and moves rows the |
| // activator manages, so the row's files would be stranded under the |
| // instance tree (design §8.2, downgrade rules). The test is by path shape, |
| -// whatever else the row says. |
| +// whatever else the row says, or by a layout journal, which means the row |
| +// is mid-move and recovery needs it managed. |
| func refuseUnflagNested(rt registry.Route, dataDir string) error { |
| - if provision.NestedPaths(dataDir, rt) { |
| + if provision.NestedPaths(dataDir, rt) || provision.LayoutJournalExists(dataDir, rt.Host) { |
| return fmt.Errorf("route: %s: its paths are nested. revert the layout first; a nested instance must stay managed so the revert can stop it", rt.Host) |
| } |
| return nil |
diff --git a/cli_test.go b/cli_test.go| index ebea590c4..a2a499472 100644 |
| --- a/cli_test.go |
| +++ b/cli_test.go |
| @@ -1327,6 +1327,58 @@ func TestRefuseUnflagNested(t *testing.T) { |
| } |
| } |
| +// writeLayoutJournal puts a journal for host under dataDir/layout. The |
| +// guards only ask whether one is there, so its contents do not matter. |
| +func writeLayoutJournal(t *testing.T, dataDir, host string) { |
| + t.Helper() |
| + path := provision.LayoutJournalPath(dataDir, host) |
| + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.WriteFile(path, []byte("{}"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| +} |
| + |
| +// A row whose paths are still flat can be mid-move: the journal is what |
| +// says so. Unflagging it would take it out of the management recovery |
| +// needs, so the guard refuses it with the same approved text. |
| +func TestRefuseUnflagNestedRefusesAFlatRowWithAJournal(t *testing.T) { |
| + host := "acme.example.com" |
| + dd := t.TempDir() |
| + flat := registry.Route{Host: host, Kind: "instance", Hibernate: true, |
| + DBPath: provision.DefaultDBPath(dd, host), Socket: provision.DefaultSocket(host)} |
| + writeLayoutJournal(t, dd, host) |
| + err := refuseUnflagNested(flat, dd) |
| + want := "route: acme.example.com: its paths are nested. revert the layout first; a nested instance must stay managed so the revert can stop it" |
| + if err == nil || err.Error() != want { |
| + t.Fatalf("refuseUnflagNested(flat, journal) = %v, want %q", err, want) |
| + } |
| +} |
| + |
| +// A re-add over a host with a journal would rewrite the row a recovery is |
| +// about to check and flip, row or no row, so it refuses too. |
| +func TestRefuseAddNestedRefusesAHostWithAJournal(t *testing.T) { |
| + host := "acme.example.com" |
| + dd := t.TempDir() |
| + flat := ®istry.Route{Host: host, Kind: "instance", DBPath: provision.DefaultDBPath(dd, host), Socket: provision.DefaultSocket(host)} |
| + writeLayoutJournal(t, dd, host) |
| + want := "add acme.example.com: its paths are nested. revert the layout first; re-adding it would point it back at the shared directory" |
| + for name, existing := range map[string]*registry.Route{"flat row": flat, "no row": nil} { |
| + if err := refuseAddNested(host, existing, dd); err == nil || err.Error() != want { |
| + t.Errorf("%s: refuseAddNested = %v, want %q", name, err, want) |
| + } |
| + } |
| + // A journal this process cannot even read the target of still counts. |
| + other := "other.example.com" |
| + if err := os.Symlink("/nonexistent", provision.LayoutJournalPath(dd, other)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := refuseAddNested(other, nil, dd); err == nil { |
| + t.Error("a dangling journal symlink was not taken as a journal") |
| + } |
| +} |
| + |
| // `carlos add` over an existing nested host would rewrite its paths to the |
| // flat defaults and strand the files under the instance tree, so it |
| // refuses. A new host, and a re-add of a flat one, go ahead. |
diff --git a/internal/layout/journal.go b/internal/layout/journal.go| index 2dee2e1d9..2c43980af 100644 |
| --- a/internal/layout/journal.go |
| +++ b/internal/layout/journal.go |
| @@ -270,8 +270,7 @@ func RemoveJournal(dataDir, host string) error { |
| // permission problem, say) is treated as present: silently proceeding |
| // past a journal this process cannot even stat is worse than refusing. |
| func JournalPresent(dataDir, host string) bool { |
| - _, err := os.Lstat(provision.LayoutJournalPath(dataDir, host)) |
| - return !errors.Is(err, os.ErrNotExist) |
| + return provision.LayoutJournalExists(dataDir, host) |
| } |
| // Validate is the recovery check of design §8.2: the journal's endpoints |
diff --git a/internal/provision/provision.go b/internal/provision/provision.go| index 4709ef0fa..38bd3c09e 100644 |
| --- a/internal/provision/provision.go |
| +++ b/internal/provision/provision.go |
| @@ -1320,6 +1320,15 @@ func LayoutJournalPath(dataDir, host string) string { |
| return filepath.Join(dataDir, "layout", host+".json") |
| } |
| +// LayoutJournalExists reports whether a journal is at LayoutJournalPath. |
| +// It Lstats, so a dangling symlink there counts, and any error other than |
| +// "not exist" counts too: a journal this process cannot even stat is not |
| +// proof there isn't one. |
| +func LayoutJournalExists(dataDir, host string) bool { |
| + _, err := os.Lstat(LayoutJournalPath(dataDir, host)) |
| + return !errors.Is(err, os.ErrNotExist) |
| +} |
| + |
| // IsControlEndpoint reports whether path is the control channel or |
| // anything else under its directory. The edge and the agent refuse to |
| // dial such a path as an upstream (design §5.4): a registry row pointing |
diff --git a/internal/provision/reconcile.go b/internal/provision/reconcile.go| index 4bbb3b978..549e5bbda 100644 |
| --- a/internal/provision/reconcile.go |
| +++ b/internal/provision/reconcile.go |
| @@ -968,8 +968,11 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg |
| // A nested row keeps its flag whatever the record says: only a |
| // row the activator manages is one the layout revert can stop |
| // and move back, so clearing the flag would strand its files |
| - // under the instance tree (design §8.2, downgrade rules). |
| - if rec.Hibernate != nil && existing.Hibernate != *rec.Hibernate && !*rec.Hibernate && NestedPaths(cfg.DataDir, existing) { |
| + // under the instance tree (design §8.2, downgrade rules). A row |
| + // with a layout journal is mid-move whatever its paths say, and |
| + // recovery needs it managed just the same. |
| + if rec.Hibernate != nil && existing.Hibernate != *rec.Hibernate && !*rec.Hibernate && |
| + (NestedPaths(cfg.DataDir, existing) || LayoutJournalExists(cfg.DataDir, rec.Host)) { |
| log.Printf("provision: %s: record asks to clear hibernate but the row's paths are nested; revert the layout first", rec.Host) |
| } else if rec.Hibernate != nil && existing.Hibernate != *rec.Hibernate { |
| dropped := "" |
diff --git a/internal/provision/reconcile_test.go b/internal/provision/reconcile_test.go| index afbe61aaf..4067c63ec 100644 |
| --- a/internal/provision/reconcile_test.go |
| +++ b/internal/provision/reconcile_test.go |
| @@ -784,6 +784,52 @@ func TestReconcileKeepsANestedRowFlagged(t *testing.T) { |
| } |
| } |
| +// A flat row with a layout journal is mid-move, and clearing its flag |
| +// would strand it the same way, so the record is not applied and the same |
| +// approved line is logged. |
| +func TestReconcileKeepsAFlatRowFlaggedWhileAJournalExists(t *testing.T) { |
| + s, sqid := setup(t) |
| + rec := acmeRecord(sqid) |
| + on := true |
| + rec.Hibernate = &on |
| + if err := CreateRecord(ctx, s, rec); err != nil { |
| + t.Fatal(err) |
| + } |
| + reg := regWith(t) |
| + cfg := ReconcileConfig{Region: "ie", Pools: []string{"tito"}, DataDir: t.TempDir()} |
| + if err := Reconcile(ctx, s, reg, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + journal := LayoutJournalPath(cfg.DataDir, rec.Host) |
| + if err := os.MkdirAll(filepath.Dir(journal), 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.WriteFile(journal, []byte("{}"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + var logged bytes.Buffer |
| + log.SetOutput(&logged) |
| + t.Cleanup(func() { log.SetOutput(os.Stderr) }) |
| + |
| + off := false |
| + rec.Hibernate = &off |
| + putReplace(t, s, RecordKey(sqid, "tito", rec.Host), mustJSON(t, rec)) |
| + if err := Reconcile(ctx, s, reg, cfg); err != nil { |
| + t.Fatal(err) |
| + } |
| + if NestedPaths(cfg.DataDir, *mustGet(t, reg, rec.Host)) { |
| + t.Fatal("setup: the row is nested; this test is about a flat row") |
| + } |
| + if !mustGet(t, reg, rec.Host).Hibernate { |
| + t.Fatal("the record cleared hibernate on a row with a layout journal") |
| + } |
| + want := "provision: acme.go.tito.io: record asks to clear hibernate but the row's paths are nested; revert the layout first" |
| + if !strings.Contains(logged.String(), want) { |
| + t.Errorf("no refusal logged; want %q in:\n%s", want, logged.String()) |
| + } |
| +} |
| + |
| // A nested row's paths differ from the record's flat derivation by design, |
| // so the drift log names the layout rather than a shape mismatch that |
| // points the operator at `carlos remove`. |