amadan

carlos / platform Public

ops: FleetDBs discovers unit-tenant databases via DBPath, cli requires -db for hibernating unit routes — replication was asserted but not implemented

FleetDBs only globbed dataDir/*.db, but a unit tenant's data lives at
its systemd StateDirectory (/var/lib/carlos-app/<host>), never under
dataDir — so the box-wide litestream unit silently never replicated
any unit-backed tenant despite comments claiming otherwise. No single
filename convention can be assumed inside a unit tenant's state dir
(the app picks it), so FleetDBs now includes a BackingUnit route's
DBPath only when the operator set one via `carlos add -db`, and
`-backing unit -hibernate` now requires -db (the case where going
unreplicated is a real data-loss risk, not a missed nicety).
Non-hibernating unit routes stay unenforced by design. Comments in
litestream.go, cli.go, and backend_unit.go now state exactly what is
covered and what the operator must hand-configure.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell c0654d500ea4a10ef6e253add7803665ad7d2668 parent 4ad3377
5 files changed, +238 −22
  • cli.go +18 −2
  • e2e_test.go +74 −0
  • internal/activator/backend_unit.go +15 −4
  • internal/ops/litestream.go +57 −11
  • internal/ops/ops_test.go +74 −5
diff --git a/cli.go b/cli.go
index 2a43023ef..02511f882 100644
--- a/cli.go
+++ b/cli.go
@@ -80,11 +80,27 @@ func runAdd(args []string) {
}
}
// No DB default for unit tenants: their data lives in the unit's
- // StateDirectory (/var/lib/carlos-app/<host>), replicated by the box
- // litestream unit — -db stays an explicit operator choice there.
+ // StateDirectory (/var/lib/carlos-app/<host>), under a filename the APP
+ // chooses (no platform-wide convention the way dataDir/<host>.db is for
+ // exec children) — so there is no honest path to default to. -db stays
+ // an explicit operator choice there, and ops.FleetDBs (the box-wide
+ // litestream unit's config generator) only learns a unit tenant's
+ // database path from this flag; leave it unset and that tenant's data
+ // is replicated by nobody.
if *dbPath == "" && *kind == "instance" && *backing != registry.BackingUnit {
*dbPath = filepath.Join(dataDir(), host+".db")
}
+ // A hibernating unit tenant stops and starts with systemd, not the
+ // agent — box-wide litestream (root) is the ONLY thing that can
+ // replicate it, so skipping -db here is a genuine data-loss risk
+ // (not just a missed nicety) the moment the unit is ever stopped for
+ // idleness. Require it. A non-hibernating unit tenant is always running
+ // and so has no equivalent forced moment of data being unreachable if
+ // unreplicated — -db is still strongly recommended there but stays the
+ // operator's call, unenforced.
+ if *backing == registry.BackingUnit && *hibernate && *dbPath == "" {
+ log.Fatalf("add %s: -backing unit -hibernate requires -db <path> — the box-wide litestream unit can't find a unit tenant's database on its own (see ops.FleetDBs)", host)
+ }
r, err := registry.Open(*reg)
if err != nil {
log.Fatal(err)
diff --git a/e2e_test.go b/e2e_test.go
index 98e014010..ae3fe3ace 100644
--- a/e2e_test.go
+++ b/e2e_test.go
@@ -451,3 +451,77 @@ func TestStatusCLI(t *testing.T) {
t.Error("carlos status with no -dry-run flag: want a non-zero exit, got 0")
}
}
+
+// TestAddUnitBackingRequiresDBForHibernate pins the I1 fix's operator-facing
+// half: `carlos add -backing unit -hibernate` with no -db must refuse,
+// because ops.FleetDBs (the box-wide litestream unit) has no way to
+// discover a unit tenant's database path other than that flag, and a
+// hibernating unit tenant left unreplicated is a genuine data-loss risk the
+// moment systemd stops it for idleness. A non-hibernating unit route stays
+// unenforced (still the operator's call), and -db always persists onto the
+// route when given.
+func TestAddUnitBackingRequiresDBForHibernate(t *testing.T) {
+ goBin, err := exec.LookPath("go")
+ if err != nil {
+ t.Skip("go toolchain not found")
+ }
+ tmp := t.TempDir()
+ carlosBin := filepath.Join(tmp, "carlos")
+ build := exec.Command(goBin, "build", "-o", carlosBin, ".")
+ build.Env = os.Environ()
+ if out, err := build.CombinedOutput(); err != nil {
+ t.Fatalf("build carlos: %v\n%s", out, err)
+ }
+
+ regPath := filepath.Join(tmp, "registry.db")
+
+ // No -db: refused, non-zero exit, no row written.
+ out, err := exec.Command(carlosBin, "add", "-registry", regPath, "-app", "kass",
+ "-backing", "unit", "-hibernate", "no-db.example.com").CombinedOutput()
+ if err == nil {
+ t.Fatalf("add -backing unit -hibernate with no -db: want non-zero exit, got 0\n%s", out)
+ }
+ if !strings.Contains(string(out), "-db") {
+ t.Errorf("add refusal output = %q, want it to mention -db", out)
+ }
+
+ // With -db: succeeds, and the route's DBPath is exactly what was given
+ // (ops.FleetDBs reads this verbatim — no default synthesized here).
+ dbPath := filepath.Join(tmp, "unit-state", "app.db")
+ out, err = exec.Command(carlosBin, "add", "-registry", regPath, "-app", "kass",
+ "-backing", "unit", "-hibernate", "-db", dbPath, "with-db.example.com").CombinedOutput()
+ if err != nil {
+ t.Fatalf("add -backing unit -hibernate -db %s: %v\n%s", dbPath, err, out)
+ }
+
+ reg, err := registry.Open(regPath)
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ routes, err := reg.List()
+ if err != nil {
+ t.Fatalf("list routes: %v", err)
+ }
+ var found bool
+ for _, rt := range routes {
+ if rt.Host != "with-db.example.com" {
+ continue
+ }
+ found = true
+ if rt.DBPath != dbPath {
+ t.Errorf("with-db.example.com DBPath = %q, want %q", rt.DBPath, dbPath)
+ }
+ if rt.Backing != registry.BackingUnit {
+ t.Errorf("with-db.example.com Backing = %q, want %q", rt.Backing, registry.BackingUnit)
+ }
+ }
+ if !found {
+ t.Fatal("with-db.example.com route missing after a successful add")
+ }
+ // The refused add must not have left a partial row behind.
+ for _, rt := range routes {
+ if rt.Host == "no-db.example.com" {
+ t.Fatal("refused add left a route behind")
+ }
+ }
+}
diff --git a/internal/activator/backend_unit.go b/internal/activator/backend_unit.go
index 79fc48490..daede904d 100644
--- a/internal/activator/backend_unit.go
+++ b/internal/activator/backend_unit.go
@@ -16,10 +16,21 @@
// unit (root), which streams changes continuously whether the tenant is
// awake or asleep. That IS the snapshot-on-sleep story here: a stop only
// happens after a full idle window, so streaming replication shipped the
-// final writes long before the unit goes down. ops.FleetDBs keeps
-// unit-backed hibernating routes IN the generated config for the same
-// reason — only exec-backed hibernating hosts, whose replication the
-// activator's embedded litestream owns under the S3 lease, are excluded.
+// final writes long before the unit goes down.
+//
+// WHAT THIS ACTUALLY REQUIRES from the operator: box-wide litestream
+// (ops.FleetDBs) has no platform-wide convention for a unit tenant's
+// database filename inside its state dir — that filename is the APP's
+// own choice, unlike the dataDir/<host>.db convention exec children
+// get for free — so it can only replicate a unit tenant whose route
+// was added with an explicit `carlos add -backing unit -db <path>`.
+// `-hibernate` unit routes REQUIRE -db (cli.go's runAdd enforces it:
+// going unreplicated there is a genuine data-loss risk, the moment the
+// unit is ever stopped for idleness, not just a missed nicety).
+// Non-hibernating unit routes do NOT enforce it — -db is strongly
+// recommended but stays the operator's unchecked call, and a route
+// added without it is silently replicated by nobody. See
+// ops.FleetDBs's own doc comment for the mechanics.
// - There is no lease to trade: the unit exists on exactly one box (its
// state dir is that box's disk) and the box replicator never hands
// ownership off, so wake/sleep is not a writer handoff. When unit
diff --git a/internal/ops/litestream.go b/internal/ops/litestream.go
index b87830b08..fc9275121 100644
--- a/internal/ops/litestream.go
+++ b/internal/ops/litestream.go
@@ -81,23 +81,57 @@ func ParseLitestreamDBs(cfg string) []string {
return dbs
}
-// FleetDBs lists the non-empty *.db files under dataDir that this replicator
-// owns. Hibernating EXEC-backed routes' databases (matched by DBPath) are
-// excluded: the activator's embedded Litestream owns their replication (to
-// the instances bucket, under the lease) — a second replicator on the same
-// WAL would fight it for checkpointing and corrupt the replica. (Eleven's
-// fleet-backup.sh lesson, carried verbatim.) UNIT-backed hibernating routes
-// (carlos-app@ tenants) stay in: their wake/sleep is systemctl, the agent
-// never replicates them (it can't even read a DynamicUser's state dir), so
-// dropping them here would leave their data replicated by nobody.
+// FleetDBs lists the non-empty *.db files this box-wide replicator owns:
+// everything found by globbing dataDir (the agent's own exec-child
+// databases, plus the registry itself), PLUS — separately, not via the glob
+// — every BackingUnit route's DBPath when the operator set one.
+//
+// WHAT IS COVERED: a unit tenant (carlos-app@<host>: DynamicUser,
+// StateDirectory=/var/lib/carlos-app/<host>) whose route was added with an
+// explicit `-db <path>`, hibernating or not — its wake/sleep is systemctl
+// and the agent can't even read a DynamicUser's 0700 state dir, so this
+// box-wide replicator (running as root) is the only thing that can stream
+// its writes off-box.
+//
+// WHAT IS NOT COVERED, and why this function does not try to guess it: a
+// unit tenant's database filename inside its state dir is the APP's own
+// choice — the platform enforces no naming convention there the way it does
+// the dataDir/<host>.db convention for exec children — so there is no
+// honest default path to assume. A unit route added without -db is
+// silently invisible here. cli.go's runAdd requires -db for
+// `-backing unit -hibernate` routes for exactly this reason (going
+// unreplicated there is a genuine data-loss risk, not just a missed
+// nicety); a non-hibernating unit tenant without -db is still the
+// operator's to hand-configure. See internal/activator/backend_unit.go's
+// package comment for the fuller operator-facing story.
+//
+// Hibernating EXEC-backed routes' databases (matched by DBPath, found via
+// the dataDir glob) are excluded from the results below: the activator's
+// embedded Litestream owns their replication (to the instances bucket,
+// under the lease) — a second replicator on the same WAL would fight it for
+// checkpointing and corrupt the replica. (Eleven's fleet-backup.sh lesson,
+// carried verbatim.) That exclusion never applies to BackingUnit routes —
+// their replication has no other owner at all, so they are added here
+// rather than filtered.
func FleetDBs(reg *registry.Registry, dataDir string) ([]string, error) {
routes, err := reg.List()
if err != nil {
return nil, err
}
hibernating := map[string]bool{}
+ unitPaths := map[string]bool{}
for _, r := range routes {
- if r.Hibernate && r.Backing != registry.BackingUnit && r.DBPath != "" {
+ if r.Backing == registry.BackingUnit {
+ // Unit tenants' data lives outside dataDir (their systemd
+ // StateDirectory), so the glob below can never find them on its
+ // own — DBPath, when the operator set it, is the only way this
+ // replicator learns where they are.
+ if r.DBPath != "" {
+ unitPaths[filepath.Clean(r.DBPath)] = true
+ }
+ continue
+ }
+ if r.Hibernate && r.DBPath != "" {
hibernating[filepath.Clean(r.DBPath)] = true
}
}
@@ -106,15 +140,27 @@ func FleetDBs(reg *registry.Registry, dataDir string) ([]string, error) {
if err != nil {
return nil, err
}
+ // Unit-tenant DBPaths are added on top of the glob: they typically live
+ // entirely outside dataDir, at their StateDirectory path.
+ for p := range unitPaths {
+ matches = append(matches, p)
+ }
+
+ seen := map[string]bool{}
var dbs []string
for _, p := range matches {
+ p = filepath.Clean(p)
+ if seen[p] {
+ continue // a unit DBPath that happens to also sit under dataDir
+ }
fi, err := os.Stat(p)
if err != nil || !fi.Mode().IsRegular() || fi.Size() == 0 {
continue // gone, odd, or an empty placeholder — nothing to replicate
}
- if hibernating[filepath.Clean(p)] {
+ if hibernating[p] {
continue
}
+ seen[p] = true
dbs = append(dbs, p)
}
return dbs, nil
diff --git a/internal/ops/ops_test.go b/internal/ops/ops_test.go
index 18400fde4..7f5fc7739 100644
--- a/internal/ops/ops_test.go
+++ b/internal/ops/ops_test.go
@@ -22,9 +22,17 @@ func TestConfigGenSkipsHibernatingRoutes(t *testing.T) {
t.Fatal(err)
}
+ // A unit tenant's data lives OUTSIDE dataDir entirely (its systemd
+ // StateDirectory, e.g. /var/lib/carlos-app/<host>) — a separate temp dir
+ // here stands in for that, deliberately not under dir, so this test
+ // actually exercises FleetDBs' DBPath-based discovery instead of
+ // accidentally passing because the file happened to sit where the glob
+ // already looks.
+ unitDir := t.TempDir()
+
awake := filepath.Join(dir, "a.example.com.db")
asleep := filepath.Join(dir, "b.example.com.db")
- unitTenant := filepath.Join(dir, "c.example.com.db")
+ unitTenant := filepath.Join(unitDir, "state.db")
for _, p := range []string{awake, asleep, unitTenant} {
if err := os.WriteFile(p, []byte("x"), 0o644); err != nil {
t.Fatal(err)
@@ -36,9 +44,10 @@ func TestConfigGenSkipsHibernatingRoutes(t *testing.T) {
if err := reg.Upsert(registry.Route{Host: "b.example.com", DBPath: asleep, Hibernate: true}); err != nil {
t.Fatal(err)
}
- // A unit-backed hibernating tenant (carlos-app@): its wake/sleep is
- // systemctl and the activator never replicates it, so this replicator
- // must keep owning its database.
+ // A unit-backed hibernating tenant (carlos-app@) with an operator-set
+ // -db: its wake/sleep is systemctl and the activator never replicates
+ // it, so this box-wide replicator must own its database — found via
+ // DBPath, not the dataDir glob (its file isn't under dataDir at all).
if err := reg.Upsert(registry.Route{Host: "c.example.com", DBPath: unitTenant,
Hibernate: true, Backing: registry.BackingUnit}); err != nil {
t.Fatal(err)
@@ -56,7 +65,7 @@ func TestConfigGenSkipsHibernatingRoutes(t *testing.T) {
t.Fatalf("hibernating route's DB must be excluded (activator owns its replication):\n%s", cfg)
}
if !strings.Contains(cfg, unitTenant) {
- t.Fatalf("unit-backed hibernating route's DB missing (nobody else replicates it):\n%s", cfg)
+ t.Fatalf("unit-backed hibernating route's DB (outside dataDir) missing (nobody else replicates it):\n%s", cfg)
}
// The registry itself lives in the data dir and is not anyone's hibernating
// instance — it must be replicated.
@@ -71,6 +80,66 @@ func TestConfigGenSkipsHibernatingRoutes(t *testing.T) {
}
}
+// TestFleetDBsUnitTenantWithoutDBPathIsInvisible pins the honest half of the
+// I1 fix: a BackingUnit route added without -db has no discoverable database
+// path (the platform enforces no filename convention inside a unit tenant's
+// state dir), so FleetDBs must not guess one — it must simply not appear in
+// the generated config. This is the documented gap operators must close by
+// hand (or cli.go's -hibernate requirement closes it for them); it must
+// never masquerade as covered.
+func TestFleetDBsUnitTenantWithoutDBPathIsInvisible(t *testing.T) {
+ dir := t.TempDir()
+ reg, err := registry.Open(filepath.Join(dir, "registry.db"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.Upsert(registry.Route{Host: "no-db.example.com", Backing: registry.BackingUnit}); err != nil {
+ t.Fatal(err)
+ }
+
+ dbs, err := FleetDBs(reg, dir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, p := range dbs {
+ if strings.Contains(p, "no-db") {
+ t.Fatalf("FleetDBs invented a path for a unit tenant with no -db: %v", dbs)
+ }
+ }
+}
+
+// TestFleetDBsDedupesUnitPathUnderDataDir: a unit tenant's DBPath can, in
+// principle, sit inside dataDir (nothing forbids it) — FleetDBs must not
+// list it twice just because both the glob and the DBPath pass found it.
+func TestFleetDBsDedupesUnitPathUnderDataDir(t *testing.T) {
+ dir := t.TempDir()
+ reg, err := registry.Open(filepath.Join(dir, "registry.db"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ p := filepath.Join(dir, "d.example.com.db")
+ if err := os.WriteFile(p, []byte("x"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.Upsert(registry.Route{Host: "d.example.com", DBPath: p, Backing: registry.BackingUnit}); err != nil {
+ t.Fatal(err)
+ }
+
+ dbs, err := FleetDBs(reg, dir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ count := 0
+ for _, got := range dbs {
+ if got == p {
+ count++
+ }
+ }
+ if count != 1 {
+ t.Fatalf("FleetDBs listed %s %d times, want exactly once: %v", p, count, dbs)
+ }
+}
+
// The round-trip is the contract that lets restore-verify read the generated
// config without a YAML parser: whatever LitestreamConfig writes,
// ParseLitestreamDBs must recover exactly.