amadan

carlos / platform Public

Merge pull request #117 from carlosframework/route-unit

Routes learn the systemd unit that owns their process; adoption cycles it (#110)
Paul Campbell c8b2eed22520fc660e53acf8ec5eccfc7cdabba6 parent 4046963parent 11d71e9
8 files changed, +1153 −70
  • cli.go +188 −20
  • cli_test.go +248 −8
  • internal/caps/converge.go +1 −1
  • internal/host/agent.go +112 −23
  • internal/host/agent_test.go +246 −2
  • internal/registry/registry.go +126 −14
  • internal/registry/registry_test.go +231 −1
  • internal/release/adopt.go +1 −1
diff --git a/cli.go b/cli.go
index 6fa9272bc..e49017e48 100644
--- a/cli.go
+++ b/cli.go
@@ -79,6 +79,7 @@ func runAdd(args []string) {
hibernate := fs.Bool("hibernate", false, "hibernate when idle")
statusNamed := fs.Bool("status-named", false, "opt this app into being named on the public status page")
backing := fs.String("backing", "", `process backing: "" (agent exec child) | "unit" (carlos-app@<host> systemd unit)`)
+ unit := fs.String("unit", "", "systemd unit that owns this route's process when it is neither an exec child nor a carlos-app@ tenant (e.g. console.service)")
dbPath := fs.String("db", "", "instance database path")
fs.Parse(args)
if fs.NArg() != 1 {
@@ -91,6 +92,25 @@ func runAdd(args []string) {
if *backing != "" && *backing != registry.BackingUnit {
log.Fatalf("backing %q: want \"\" or %q", *backing, registry.BackingUnit)
}
+ // Checked here, before anything is opened or written, for the same reason
+ // -backing is: registry.SetUnit would refuse it too, but a route created
+ // with a typo'd unit is a route whose adoption silently restarts nothing
+ // — the failure this column exists to end.
+ if !registry.ValidUnitName(*unit) {
+ log.Fatalf("add %s: unit %q: want a systemd service unit name (e.g. console.service)", host, *unit)
+ }
+ // The same combination `carlos route -unit` refuses (unitRail), refused at
+ // the OTHER door onto these two columns — and this is the door that could
+ // mint the row from nothing. A route that is both unit-backed and
+ // unit-named is two answers to one question, and the dispatch resolves it
+ // by honouring the name: the carlos-app@<host> unit would then never be
+ // cycled by adoption again, which is issue #110 recreated for an ordinary
+ // tenant. Same wording as unitRail's, so an operator who meets it at one
+ // door recognises it at the other.
+ if *unit != "" && *backing == registry.BackingUnit {
+ log.Fatalf("add %s: -backing unit with -unit %s: a unit-backed route's process is the carlos-app@%s unit the platform derives and already restarts, "+
+ "so a second unit name here would be two answers to one question — drop one of the two flags", host, *unit, host)
+ }
if *socket == "" && *addr == "" && *kind == "instance" {
if *backing == registry.BackingUnit {
// The carlos-app@<host>.socket unit's path — enable that unit
@@ -154,7 +174,7 @@ func runAdd(args []string) {
Host: host, Slug: *slug, Kind: *kind, Socket: *socket, Addr: *addr,
App: *app, Account: account,
Channel: *channel, Hibernate: *hibernate, StatusNamed: *statusNamed, Backing: *backing,
- DBPath: *dbPath, Status: "ready",
+ Unit: *unit, DBPath: *dbPath, Status: "ready",
}); err != nil {
log.Fatal(err)
}
@@ -213,14 +233,19 @@ func runInstances(args []string) {
log.Fatal(err)
}
w := newTable(os.Stdout)
- fmt.Fprintln(w, "HOST\tAPP\tKIND\tCHANNEL\tSTATUS\tVERSION\tHIB\tBACKING")
+ // BACKING and UNIT are the two halves of one answer — who owns this
+ // route's process — so they print side by side: a route with a UNIT is
+ // cycled by that unit on adoption, whatever BACKING says (and BACKING is
+ // empty for every such route, which is the shape that made issue #110
+ // invisible from this listing).
+ fmt.Fprintln(w, "HOST\tAPP\tKIND\tCHANNEL\tSTATUS\tVERSION\tHIB\tBACKING\tUNIT")
for _, t := range routes {
hib := ""
if t.Hibernate {
hib = "yes"
}
- fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n",
- t.Host, t.App, t.Kind, t.Channel, t.Status, t.Version, hib, t.Backing)
+ fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n",
+ t.Host, t.App, t.Kind, t.Channel, t.Status, t.Version, hib, t.Backing, t.Unit)
}
w.Flush()
}
@@ -321,14 +346,22 @@ func runInstances(args []string) {
// make that canary's own `followed` check false, which mints a fresh canary
// host next pass. Additive, not reverted, but worth knowing.)
//
-// This verb also carries the three SINGLE-ROUTE axes, each of which stands
-// alone and short-circuits everything above (see runRouteGrant and
-// runRouteHibernate):
+// This verb also carries the four SINGLE-ROUTE axes, each of which stands
+// alone and short-circuits everything above (see runRouteGrant,
+// runRouteHibernate, runRouteBacking and runRouteUnit):
//
// carlos route -host keymail.dev -grant port:25
// carlos route -host platform.carlosframework.com -grant host-role
// carlos route -host keymail.dev -revoke port:25
// carlos route -host keymail.dev -hibernate=false
+// carlos route -host console.carloku.com -unit console.service
+//
+// -unit is how an EXISTING route — the production consoles, addr-backed under
+// a bespoke console.service — tells the adopt pass which unit to cycle when
+// its version moves. Without it adoption stamped Version (and therefore the
+// edge's X-Carlos-Version header) over a process nothing had restarted, and
+// the deploy needed a hand-run `systemctl restart console.service` to become
+// true (issue #110). Like the other three it is -host only.
//
// -grant/-revoke are the operator's door to the two operator-consented
// capabilities (spec §2 "Grants"): a manifest DECLARES `ports`/`host_role`,
@@ -351,6 +384,8 @@ func runRoute(args []string) {
revoke := fs.String("revoke", "", "capability token to revoke from this route on this box")
hibernate := fs.Bool("hibernate", false, "hibernate this route when idle; -hibernate=false stops it hibernating")
backing := fs.String("backing", "", `who owns this route's process: "unit" (a carlos-app@<host> systemd unit) | "" (an exec child of the agent)`)
+ unit := fs.String("unit", "",
+ `systemd unit that owns this route's process (e.g. console.service), so adoption can cycle it; "-" clears the stamp (host only, no -app)`)
tunnel := fs.String("tunnel", "",
`name of the reverse channel backing this route at the edge; "-" clears to a Socket/Addr-routed upstream (host only, no -app)`)
allowVersionChange := fs.Bool("allow-version-change", false,
@@ -366,14 +401,21 @@ func runRoute(args []string) {
// `-hibernate=false` rather than a quoted tri-state string.
// -backing is tri-state for the same reason, and more sharply: "" is a
// LEGAL VALUE of that column (an exec child), so a flag left unmentioned
- // and a flag set to "" cannot be told apart by the value alone.
- hibernateSet, backingSet := false, false
+ // and a flag set to "" cannot be told apart by the value alone. -unit is
+ // tri-state on the same grounds — "" is that column's default, meaning
+ // "no unit owns this route's process" — though its CLEAR spelling is
+ // "-unit -", the sentinel -environment and -tunnel already use, so that a
+ // bare `-unit ""` reads as the mistake it almost certainly is rather than
+ // as a silent unstamp.
+ hibernateSet, backingSet, unitSet := false, false, false
fs.Visit(func(f *flag.Flag) {
switch f.Name {
case "hibernate":
hibernateSet = true
case "backing":
backingSet = true
+ case "unit":
+ unitSet = true
}
})
@@ -382,7 +424,8 @@ func runRoute(args []string) {
" (-account-id names ONE account by sqid and supersedes -account, \"all\" included; -tunnel is -host only)\n" +
" or: carlos route -host <host> (-grant | -revoke) <port:n|host-role> [-registry <db>] [-dry-run]\n" +
" or: carlos route -host <host> -hibernate[=false] [-registry <db>] [-dry-run]\n" +
- ` or: carlos route -host <host> -backing <unit|""> [-registry <db>] [-dry-run]`
+ " or: carlos route -host <host> -backing <unit|\"\"> [-registry <db>] [-dry-run]\n" +
+ ` or: carlos route -host <host> -unit <name.service|-> [-registry <db>] [-dry-run]`
if *host == "" && *app == "" {
log.Fatal(usage)
}
@@ -414,11 +457,15 @@ func runRoute(args []string) {
log.Fatal("route: -backing is mutually exclusive with -grant/-revoke and -hibernate — run them as two commands, so each meets its own rail alone. " +
"The order is not arbitrary either: -backing unit comes FIRST, because a grant onto a route that is not unit-backed is refused (it would materialize nothing)")
}
- if (grantAxis || hibernateSet || backingSet) && (*channel != "" || *environment != "" || *tunnel != "") {
- log.Fatal("route: -grant/-revoke/-hibernate/-backing cannot be combined with -channel, -environment or -tunnel — a capability grant and a repoint are different operations with different rails, and one command that did both would commit them together")
+ if unitSet && (grantAxis || hibernateSet || backingSet) {
+ log.Fatal("route: -unit is mutually exclusive with -grant/-revoke, -hibernate and -backing — run them as two commands, so each meets its own rail alone. " +
+ "-unit and -backing in particular are two different answers to the same question (who owns this route's process), and a command that wrote both would leave the row saying both")
+ }
+ if (grantAxis || hibernateSet || backingSet || unitSet) && (*channel != "" || *environment != "" || *tunnel != "") {
+ log.Fatal("route: -grant/-revoke/-hibernate/-backing/-unit cannot be combined with -channel, -environment or -tunnel — a capability grant and a repoint are different operations with different rails, and one command that did both would commit them together")
}
- if (grantAxis || hibernateSet || backingSet) && *host == "" {
- log.Fatal("route: -grant/-revoke/-hibernate/-backing require -host, never -app: grants and process backing are per route per box by design, and a bulk grant is precisely the operation nobody should be able to type by accident\n\n" + usage)
+ if (grantAxis || hibernateSet || backingSet || unitSet) && *host == "" {
+ log.Fatal("route: -grant/-revoke/-hibernate/-backing/-unit require -host, never -app: grants, process backing and the owning unit are per route per box by design, and a bulk grant is precisely the operation nobody should be able to type by accident\n\n" + usage)
}
// The token is checked BEFORE the registry is opened for writing: a typo
// should cost nothing and reach nothing.
@@ -443,11 +490,31 @@ func runRoute(args []string) {
if backingSet && *backing != "" && *backing != registry.BackingUnit {
log.Fatalf("route: backing %q: want %q (a carlos-app@<host> systemd unit) or \"\" (an exec child of the agent)", *backing, registry.BackingUnit)
}
+ // -unit's value gets the same treatment at the same moment, and for the
+ // reason the column exists: an unvalidated name reaches the row verbatim
+ // and is discovered months later as an adopt pass that restarted nothing.
+ // "-" is the clear sentinel (checked first, exactly as envClear precedes
+ // ValidEnvironmentName), and it resolves to "" — the column's own default,
+ // which ValidUnitName accepts. A bare `-unit ""` is refused rather than
+ // treated as a clear: the doors that CLEAR say so out loud.
+ unitClear := *unit == "-"
+ resolvedUnit := *unit
+ if unitClear {
+ resolvedUnit = ""
+ }
+ if unitSet && !unitClear {
+ if *unit == "" {
+ log.Fatal(`route: -unit "" is refused — use "-unit -" to clear the stamp, or name the unit that owns this route's process (e.g. -unit console.service)`)
+ }
+ if !registry.ValidUnitName(*unit) {
+ log.Fatalf("route: unit %q: want a systemd service unit name (e.g. console.service), or \"-\" to clear the stamp", *unit)
+ }
+ }
if *tunnel != "" && *app != "" {
log.Fatal("route: -tunnel is only supported with -host — bulk -app tunnel repointing is not implemented\n\n" + usage)
}
- if *channel == "" && *environment == "" && *tunnel == "" && !grantAxis && !hibernateSet && !backingSet {
- log.Fatal(`route: at least one of -channel, -environment, -tunnel, -grant, -revoke, -hibernate or -backing is required. "" only means stable-by-default at route CREATION (carlos add) — ` +
+ if *channel == "" && *environment == "" && *tunnel == "" && !grantAxis && !hibernateSet && !backingSet && !unitSet {
+ log.Fatal(`route: at least one of -channel, -environment, -tunnel, -grant, -revoke, -hibernate, -backing or -unit is required. "" only means stable-by-default at route CREATION (carlos add) — ` +
"this verb sets an explicit binding, and needs an explicit target to set it to.")
}
if *channel != "" && !release.ValidChannel(*channel) {
@@ -509,7 +576,7 @@ func runRoute(args []string) {
// The single-route axes short-circuit here, before the channel/environment
// machinery below: they write one column of one row, through one narrow
// setter, and share none of that batch's all-or-nothing plumbing.
- if grantAxis || hibernateSet || backingSet {
+ if grantAxis || hibernateSet || backingSet || unitSet {
rt, err := r.Get(*host)
if err != nil {
log.Fatal(err)
@@ -522,8 +589,10 @@ func runRoute(args []string) {
runRouteGrant(r, *rt, *grant, *revoke, *dryRun)
case hibernateSet:
runRouteHibernate(r, *rt, *hibernate, *dryRun)
- default:
+ case backingSet:
runRouteBacking(r, *rt, *backing, *dryRun)
+ default:
+ runRouteUnit(r, *rt, resolvedUnit, *dryRun)
}
return
}
@@ -1133,6 +1202,10 @@ func runRouteHibernate(r *registry.Registry, rt registry.Route, on, dryRun bool)
// single-route axes.
// - kind != "instance" is refused: only an instance has a process for
// anything to back. A service or static route has no ExecStart to move.
+// - a UNIT-STAMPED row may not be moved TO unit backing. That is the same
+// forbidden state unitRail guards from the other side, reached in the
+// other order (`-unit X` first, `-backing unit` second), and it has to be
+// refused at both ends or the pair is only half-guarded.
// - the value is "" or BackingUnit, checked at the flag before the
// registry is even opened.
//
@@ -1142,7 +1215,7 @@ func runRouteHibernate(r *registry.Registry, rt registry.Route, on, dryRun bool)
// and it prints what the next adopt pass will do with the new value so the
// operator can decide whether they are ready for it.
func runRouteBacking(r *registry.Registry, rt registry.Route, backing string, dryRun bool) {
- if err := backingRail(rt); err != nil {
+ if err := backingRail(rt, backing); err != nil {
log.Fatalf("route: %s: %v", rt.Host, err)
}
prefix := ""
@@ -1181,7 +1254,18 @@ func runRouteBacking(r *registry.Registry, rt registry.Route, backing string, dr
// rows — it is meaningless. A platform alias is a second name for one
// process, and a second carlos-app@ unit under it would claim ports and a
// state directory the real route already owns.
-func backingRail(rt registry.Route) error {
+//
+// It takes the TARGET value as well as the row because one of its refusals
+// is directional: moving a unit-stamped row TO unit backing creates the state
+// unitRail forbids from the other side, while moving one AWAY from unit
+// backing is always fine (it can only reduce the number of things claiming to
+// own the process). Refusing both directions would strand any row that
+// somehow reached the forbidden state with no way back out.
+func backingRail(rt registry.Route, backing string) error {
+ if backing == registry.BackingUnit && rt.Unit != "" {
+ return fmt.Errorf("route names the unit %s — a unit-backed route's process is the carlos-app@%s unit the platform derives and already restarts, "+
+ "so a second unit name would be two answers to one question. Clear the stamp first: `carlos route -host %s -unit -`", rt.Unit, rt.Host, rt.Host)
+ }
if rt.Platform {
return fmt.Errorf("Platform route — a platform alias is a second name for one process, so a unit of its own would " +
"claim ports and a state directory the real route already holds; and the domains reconciler rebuilds these rows " +
@@ -1193,6 +1277,90 @@ func backingRail(rt registry.Route) error {
return nil
}
+// runRouteUnit stamps the systemd unit that owns a route's process — the door
+// issue #110 needed. A console is an addr-backed route (`-addr
+// 127.0.0.1:8090`) under an operator-run console.service: the adopt pass's
+// restart dispatch could reach it by neither of its arms, so promote → adopt
+// stamped a new Version, and with it the edge's X-Carlos-Version header,
+// behind a process still running the old build. The rule was "promote →
+// adopt → ssh in and systemctl restart console.service", which is exactly the
+// kind of follow-up step a `carlos` command is not allowed to require.
+//
+// This is a stamp on an EXISTING row on a live box, deliberately: the
+// production consoles already exist, and the fix must not need them recreated.
+//
+// Two rails, both structural, neither overridable:
+//
+// - a unit-backed route is refused. Its process is a carlos-app@<host>
+// unit the platform derives and already restarts; a second unit name on
+// the same row is two answers to one question, and the dispatch would
+// honour this one and stop cycling the tenant.
+// - the name's shape is checked at the flag, before the registry is even
+// opened (registry.ValidUnitName), because a typo here is invisible
+// afterwards — a restart that reaches nothing looks exactly like a
+// restart that worked.
+//
+// It does NOT check that the unit exists. It cannot: on the box where it
+// matters the command may well run before the unit is installed, and a probe
+// that has to succeed would make the stamp unavailable exactly when it is
+// most needed. What it does instead is print what the next adopt pass will do
+// with the value, at the moment of typing.
+func runRouteUnit(r *registry.Registry, rt registry.Route, unit string, dryRun bool) {
+ if err := unitRail(rt); err != nil {
+ log.Fatalf("route: %s: %v", rt.Host, err)
+ }
+ prefix := ""
+ if dryRun {
+ prefix = colorFor(os.Stdout).warn("(dry run)") + " "
+ }
+ if rt.Unit == unit {
+ mutedf(os.Stdout, "%s: unit already %s — nothing to do\n", rt.Host, unitWord(unit))
+ return
+ }
+ if !dryRun {
+ if err := r.SetUnit(rt.Host, unit); err != nil {
+ log.Fatalf("route: write: %v", err)
+ }
+ }
+ fmt.Printf("%s%s: unit %s -> %s\n", prefix, rt.Host, unitWord(rt.Unit), unitWord(unit))
+ if unit != "" {
+ fmt.Printf(" every adopt pass that moves this route's version now runs `systemctl try-restart %s` instead of cycling nothing — try-restart, so a unit you have stopped stays stopped\n", unit)
+ } else {
+ fmt.Printf(" adoption goes back to the backing dispatch for this route: a carlos-app@%s unit if it is unit-backed, the activator's own child otherwise — and neither exists for an addr-backed route, which is what leaves its process running the old build\n", rt.Host)
+ }
+}
+
+// unitRail is the row-shape half of runRouteUnit's refusals, split out as a
+// pure function for the same reason backingRail is: a rail nobody can
+// exercise without running the process is a rail nobody tests.
+//
+// Deliberately NARROWER than backingRail. It does not refuse Platform rows:
+// this column is not derived by the domains reconciler (mintedRoute never
+// sets it) and registry.Upsert preserves a non-empty value, so a stamp there
+// survives a re-mint rather than being silently reverted — and the
+// deployment's own console can legitimately be a platform-minted row. It does
+// not refuse non-instance kinds either: a `service` route is precisely a
+// sibling process the platform only proxies, and naming the unit that owns it
+// is a true statement, even though adoption has no version to move for one
+// today.
+func unitRail(rt registry.Route) error {
+ if rt.Backing == registry.BackingUnit {
+ return fmt.Errorf("unit-backed route — its process is the carlos-app@%s unit the platform derives and already restarts, "+
+ "so a second unit name here would be two answers to one question (drop the backing first with `carlos route -backing \"\"` if this route really is owned by a bespoke service)", rt.Host)
+ }
+ return nil
+}
+
+// unitWord renders a unit column value for an operator — the empty string is
+// this column's default and a real answer ("nothing named"), and printing it
+// as "" reads as a bug.
+func unitWord(unit string) string {
+ if unit == "" {
+ return `"" (no unit named — adoption dispatches on backing)`
+ }
+ return unit
+}
+
// backingWord renders a backing column value for an operator — the empty
// string is a real value of this column, and printing it as "" reads as a
// bug rather than as an answer.
diff --git a/cli_test.go b/cli_test.go
index ee98ebea2..714015d05 100644
--- a/cli_test.go
+++ b/cli_test.go
@@ -413,27 +413,267 @@ func TestBackingRailRefusals(t *testing.T) {
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
- err := backingRail(c.rt)
- if err == nil {
- t.Fatalf("backingRail(%+v) accepted; want a refusal", c.rt)
- }
- if !strings.Contains(err.Error(), c.says) {
- t.Fatalf("refusal %q does not say %q", err, c.says)
+ // Row-shape refusals hold whichever value is being written, so
+ // both targets are exercised.
+ for _, target := range []string{registry.BackingUnit, ""} {
+ err := backingRail(c.rt, target)
+ if err == nil {
+ t.Fatalf("backingRail(%+v, %q) accepted; want a refusal", c.rt, target)
+ }
+ if !strings.Contains(err.Error(), c.says) {
+ t.Fatalf("refusal %q does not say %q", err, c.says)
+ }
}
})
}
// Both directions of the migration are the same verb on the same row, so
// both must clear the rail from either starting value.
- if err := backingRail(instance); err != nil {
+ if err := backingRail(instance, registry.BackingUnit); err != nil {
t.Fatalf("backingRail(exec-backed instance) = %v, want accepted", err)
}
unit := instance
unit.Backing = registry.BackingUnit
- if err := backingRail(unit); err != nil {
+ if err := backingRail(unit, ""); err != nil {
t.Fatalf("backingRail(unit-backed instance) = %v, want accepted", err)
}
}
+// The pair rail, from backingRail's side: `carlos route -unit X` followed by
+// `carlos route -backing unit` reaches exactly the state unitRail forbids in
+// the other order, and a rail guarded at one end only is not guarded. The
+// refusal is DIRECTIONAL — moving a stamped row AWAY from unit backing stays
+// legal, or a row that somehow reached the forbidden state would have no way
+// back out.
+func TestBackingRailRefusesUnitBackingOnAStampedRow(t *testing.T) {
+ stamped := registry.Route{
+ Host: "console.carloku.com", Kind: "instance", App: "console",
+ Addr: "127.0.0.1:8090", Unit: "console.service",
+ }
+ err := backingRail(stamped, registry.BackingUnit)
+ if err == nil {
+ t.Fatal("backingRail(unit-stamped row, backing=unit) accepted; want a refusal")
+ }
+ // The operator has to be told which flag to undo, or the refusal is a
+ // dead end.
+ if !strings.Contains(err.Error(), "-unit -") {
+ t.Errorf("refusal %q does not name the clear command", err)
+ }
+ if err := backingRail(stamped, ""); err != nil {
+ t.Fatalf("backingRail(unit-stamped row, backing=\"\") = %v, want accepted — the way out must stay open", err)
+ }
+}
+
+// TestUnitRailRefusals: `carlos route -unit` is the stamp issue #110 needed,
+// and its one structural refusal is a unit-backed route — that route's
+// process is the carlos-app@<host> unit the platform derives and already
+// restarts, so a second unit name on the row is two answers to one question.
+//
+// Everything else clears the rail on purpose, and the omissions are the
+// interesting half: unlike backingRail this one does NOT refuse Platform rows
+// (the domains reconciler never derives this column and Upsert preserves it,
+// so a stamp survives a re-mint) and does NOT refuse non-instance kinds (a
+// `service` route is exactly a sibling process the platform only proxies, and
+// naming the unit that owns it is a true statement).
+func TestUnitRailRefusals(t *testing.T) {
+ console := registry.Route{
+ Host: "console.carloku.com", Kind: "instance", App: "console",
+ Addr: "127.0.0.1:8090",
+ }
+ unitBacked := console
+ unitBacked.Backing = registry.BackingUnit
+ err := unitRail(unitBacked)
+ if err == nil {
+ t.Fatal("unitRail(unit-backed route) accepted; want a refusal")
+ }
+ if !strings.Contains(err.Error(), "carlos-app@") {
+ t.Fatalf("refusal %q does not name the unit that already owns the process", err)
+ }
+
+ for _, c := range []struct {
+ name string
+ rt registry.Route
+ }{
+ {"addr-backed console", console},
+ {"platform apex", func() registry.Route { rt := console; rt.Platform = true; return rt }()},
+ {"platform+canary row", func() registry.Route {
+ rt := console
+ rt.Platform, rt.Canary = true, true
+ return rt
+ }()},
+ {"service route", func() registry.Route { rt := console; rt.Kind = "service"; return rt }()},
+ } {
+ t.Run(c.name, func(t *testing.T) {
+ if err := unitRail(c.rt); err != nil {
+ t.Fatalf("unitRail(%+v) = %v, want accepted", c.rt, err)
+ }
+ })
+ }
+}
+
+// TestRouteUnitStampsAnExistingRoute drives the whole door as a subprocess,
+// because that is the shape the fix ships in: the production consoles already
+// exist, and `carlos route -unit console.service` has to stamp a LIVE
+// registry row rather than need the route recreated. It also pins the
+// refusals an operator can actually type — a typo'd unit name above all,
+// since a bad name is invisible afterwards (a restart that reaches nothing
+// looks exactly like one that worked).
+func TestRouteUnitStampsAnExistingRoute(t *testing.T) {
+ bin := buildCarlos(t)
+ tmp := t.TempDir()
+ scratch := t.TempDir()
+ regPath := filepath.Join(tmp, "registry.db")
+
+ run := func(args ...string) (string, error) {
+ t.Helper()
+ cmd := exec.Command(bin, args...)
+ cmd.Env = carlosTestEnv(t)
+ // pick() walks cwd ancestors for .carlos/config, so the subprocess must not run from the real checkout.
+ cmd.Dir = scratch
+ out, err := cmd.CombinedOutput()
+ return string(out), err
+ }
+
+ // The console as it exists on a box today: addr-backed, no unit.
+ if out, err := run("add", "-registry", regPath, "-app", "console",
+ "-addr", "127.0.0.1:8090", "console.carloku.com"); err != nil {
+ t.Fatalf("add console route: %v\n%s", err, out)
+ }
+ unitOf := func(host string) string {
+ t.Helper()
+ reg, err := registry.Open(regPath)
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ defer reg.Close()
+ rt, err := reg.Get(host)
+ if err != nil || rt == nil {
+ t.Fatalf("get %s: %+v, %v", host, rt, err)
+ }
+ return rt.Unit
+ }
+ if got := unitOf("console.carloku.com"); got != "" {
+ t.Fatalf("a fresh route already names a unit: %q", got)
+ }
+
+ // A typo must cost nothing and reach nothing.
+ out, err := run("route", "-registry", regPath, "-host", "console.carloku.com", "-unit", "console.servcie")
+ if err == nil {
+ t.Fatalf("route -unit console.servcie: want non-zero exit, got 0\n%s", out)
+ }
+ if got := unitOf("console.carloku.com"); got != "" {
+ t.Fatalf("a refused -unit still wrote %q", got)
+ }
+
+ // -dry-run runs the rail and prints the plan, writing nothing.
+ out, err = run("route", "-registry", regPath, "-host", "console.carloku.com",
+ "-unit", "console.service", "-dry-run")
+ if err != nil {
+ t.Fatalf("route -unit -dry-run: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "console.service") {
+ t.Errorf("dry run output = %q, want it to name the unit", out)
+ }
+ if got := unitOf("console.carloku.com"); got != "" {
+ t.Fatalf("-dry-run wrote %q", got)
+ }
+
+ // The real stamp — the command the deploy runs on each box.
+ if out, err := run("route", "-registry", regPath, "-host", "console.carloku.com",
+ "-unit", "console.service"); err != nil {
+ t.Fatalf("route -unit console.service: %v\n%s", err, out)
+ }
+ if got := unitOf("console.carloku.com"); got != "console.service" {
+ t.Fatalf("Unit = %q after the stamp, want console.service", got)
+ }
+
+ // It survives a re-`carlos add` of the same row (registry.Upsert's
+ // preservation): dropping it would silently restore issue #110.
+ if out, err := run("add", "-registry", regPath, "-app", "console",
+ "-addr", "127.0.0.1:8090", "console.carloku.com"); err != nil {
+ t.Fatalf("re-add console route: %v\n%s", err, out)
+ }
+ if got := unitOf("console.carloku.com"); got != "console.service" {
+ t.Fatalf("Unit = %q after a re-add, want the stamp preserved", got)
+ }
+
+ // The listing shows it, so an operator can see which routes adoption can
+ // cycle without opening the DB.
+ out, err = run("instances", "-registry", regPath)
+ if err != nil {
+ t.Fatalf("instances: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "UNIT") || !strings.Contains(out, "console.service") {
+ t.Errorf("instances output = %q, want a UNIT column carrying console.service", out)
+ }
+
+ // And the clear door, so a mis-stamp is reversible.
+ if out, err := run("route", "-registry", regPath, "-host", "console.carloku.com", "-unit", "-"); err != nil {
+ t.Fatalf("route -unit -: %v\n%s", err, out)
+ }
+ if got := unitOf("console.carloku.com"); got != "" {
+ t.Fatalf("Unit = %q after -unit -, want it cleared", got)
+ }
+
+ // The pair rail through the real binary, in both orders. Neither door may
+ // mint a row that is unit-BACKED and unit-NAMED at once: the dispatch
+ // honours the name, so the carlos-app@<host> unit would never be cycled
+ // by adoption again — issue #110 recreated for an ordinary tenant.
+ out, err = run("add", "-registry", regPath, "-app", "kass", "-backing", "unit",
+ "-unit", "bespoke.service", "-db", filepath.Join(tmp, "t.db"), "pair.example.com")
+ if err == nil {
+ t.Fatalf("add -backing unit -unit bespoke.service: want non-zero exit, got 0\n%s", out)
+ }
+ if !strings.Contains(out, "carlos-app@") {
+ t.Errorf("add refusal = %q, want it to name the unit that already owns the process", out)
+ }
+ if out, err := run("add", "-registry", regPath, "-app", "kass", "-backing", "unit",
+ "-db", filepath.Join(tmp, "t.db"), "tenant.example.com"); err != nil {
+ t.Fatalf("add unit tenant: %v\n%s", err, out)
+ }
+ out, err = run("route", "-registry", regPath, "-host", "tenant.example.com", "-unit", "bespoke.service")
+ if err == nil {
+ t.Fatalf("route -unit on a unit-backed route: want non-zero exit, got 0\n%s", out)
+ }
+ if out, err := run("route", "-registry", regPath, "-host", "console.carloku.com",
+ "-unit", "console.service"); err != nil {
+ t.Fatalf("re-stamp console: %v\n%s", err, out)
+ }
+ out, err = run("route", "-registry", regPath, "-host", "console.carloku.com", "-backing", "unit")
+ if err == nil {
+ t.Fatalf("route -backing unit on a unit-stamped route: want non-zero exit, got 0\n%s", out)
+ }
+ if !strings.Contains(out, "-unit -") {
+ t.Errorf("backing refusal = %q, want it to name the clear command", out)
+ }
+ if out, err := run("route", "-registry", regPath, "-host", "console.carloku.com", "-unit", "-"); err != nil {
+ t.Fatalf("clear the stamp again: %v\n%s", err, out)
+ }
+
+ // The exclusions: each of these is a command that would otherwise do part
+ // of what was typed and report success.
+ for _, c := range []struct {
+ name string
+ args []string
+ }{
+ {"with -channel", []string{"-host", "console.carloku.com", "-unit", "console.service", "-channel", "stable"}},
+ {"with -backing", []string{"-host", "console.carloku.com", "-unit", "console.service", "-backing", "unit"}},
+ {"with -hibernate", []string{"-host", "console.carloku.com", "-unit", "console.service", "-hibernate"}},
+ {"bulk -app", []string{"-app", "console", "-unit", "console.service"}},
+ {"empty value", []string{"-host", "console.carloku.com", "-unit", ""}},
+ {"unknown host", []string{"-host", "nosuch.example.com", "-unit", "console.service"}},
+ } {
+ t.Run(c.name, func(t *testing.T) {
+ out, err := run(append([]string{"route", "-registry", regPath}, c.args...)...)
+ if err == nil {
+ t.Fatalf("route %v: want non-zero exit, got 0\n%s", c.args, out)
+ }
+ })
+ }
+ if got := unitOf("console.carloku.com"); got != "" {
+ t.Fatalf("a refused command wrote %q", got)
+ }
+}
+
// --- release versioning: the CLI surface -------------------------------
//
// (release versioning spec, 2026-08-08.) Driven through the real binary in
diff --git a/internal/caps/converge.go b/internal/caps/converge.go
index b931cbb0e..a7998b317 100644
--- a/internal/caps/converge.go
+++ b/internal/caps/converge.go
@@ -948,7 +948,7 @@ func Restore(o Ops, prev Snapshot) error {
// Restart is the daemon route's restart: `systemctl restart`, and its error
// is REAL — it is what makes adoptHost's rollback fire. Non-daemon routes are
-// not this function's business: restartRoute's try-restart keeps owning them,
+// not this function's business: rootRestartRoute's try-restart keeps owning them,
// byte-identically.
func Restart(o Ops, p Plan) error {
if err := validHost(p.Host); err != nil {
diff --git a/internal/host/agent.go b/internal/host/agent.go
index 84e7e0f50..8b6928fb7 100644
--- a/internal/host/agent.go
+++ b/internal/host/agent.go
@@ -266,7 +266,7 @@ func adoptLoop(reg *registry.Registry, svc *activator.Service) {
return
}
optDir := release.OptDir()
- restart := makeRestartRoute(reg, systemctlTryRestart, bounceFn(svc))
+ restart := makeRestartRoute(reg, systemctlTryRestart, systemctlTryRestartUnit, bounceFn(svc))
log.Printf("agent: adopt loop armed (hourly + jitter)")
// First pass shortly after boot (Eleven's OnBootSec=5min, compressed —
// the edge is already up by the time this fires), then hourly + jitter.
@@ -946,7 +946,7 @@ func fastConvergeLoop(reg *registry.Registry, svc *activator.Service) {
}
dd := dataDir()
optDir := release.OptDir()
- restart := makeRestartRoute(reg, systemctlTryRestart, bounceFn(svc))
+ restart := makeRestartRoute(reg, systemctlTryRestart, systemctlTryRestartUnit, bounceFn(svc))
// Built once, used by both pass shapes below — see fastPassOpts for the
// DO-NOT list and why the stage requester is per-loop, not per-tick.
opts := fastPassOpts(dd, stageRequester(dd), restart)
@@ -1496,15 +1496,25 @@ func statusTick(ctx context.Context, s release.Store, reg *registry.Registry, p
}
// makeRestartRoute builds the adopt pass's restart callback, dispatched on
-// the route's backing: a unit tenant is try-restarted (a no-op when the unit
-// isn't running — hibernators are never woken by this), while an exec child
-// is Bounced through the activator, the same graceful cycle config sync
-// uses. systemctl cannot reach a child of THIS process, which is why
-// exec-backed warm instances used to keep serving the old binary until they
-// happened to hibernate: the symlink was repointed under them and nothing
-// cycled the process.
+// what owns the route's process. THREE arms, in this order:
//
-// Hosts the registry cannot answer for take the unit path: try-restart of a
+// 1. a route that NAMES its own unit (registry.Route.Unit — an operator-run
+// bespoke service, console.service) is try-restarted by that name, and
+// returns. This arm comes first because it is the only one that can be
+// right when it applies: such a route is addr-backed, so the activator
+// owns no child to Bounce and there is no carlos-app@<host> unit to
+// try-restart — both of the arms below would run and cycle nothing,
+// which is issue #110 (adoption stamped Version, and therefore
+// X-Carlos-Version, behind a console still serving the old build).
+// 2. a carlos-app@<host> unit tenant is try-restarted by host (a no-op when
+// the unit isn't running — hibernators are never woken by this).
+// 3. an exec child is Bounced through the activator, the same graceful
+// cycle config sync uses. systemctl cannot reach a child of THIS
+// process, which is why exec-backed warm instances used to keep serving
+// the old binary until they happened to hibernate: the symlink was
+// repointed under them and nothing cycled the process.
+//
+// Hosts the registry cannot answer for take arm 2: try-restart of a
// nonexistent unit is a no-op, whereas a bounce of a process we might not own
// is not.
//
@@ -1518,19 +1528,48 @@ func statusTick(ctx context.Context, s release.Store, reg *registry.Registry, p
// only fires when the version actually moves (adoptHost early-returns on
// cur == ver), so it is one line per version change per host, never per tick.
//
-// Neither path can WAKE anything, which is what lets release.Pass hand this
+// The named-unit arm takes NO part in that fallthrough, in either direction.
+// It does not fall through (a route that names its unit has said where its
+// process is; trying the other two afterwards could only cycle something
+// else), and nothing falls through INTO it (the other arms are reached only
+// when Unit is empty, i.e. when there is no name to try).
+//
+// No path can WAKE anything, which is what lets release.Pass hand this
// callback hibernating routes (RestartHibernating): Bounce returns false
// without starting anything when the host is not awake, and try-restart is
-// defined as a no-op on a unit that is not running.
+// defined as a no-op on a unit that is not running — including the named one,
+// which is why it is try-restart and not restart (a console an operator has
+// deliberately stopped stays stopped).
+//
+// ERRORS split by arm, and the split is the point:
+//
+// Arms 2 and 3 log and return nil, unchanged: a failed try-restart of a
+// carlos-app@<host> unit must not poison the adopted version for the other
+// hosts in the pass (release/adopt.go's rule), and it cannot be told apart
+// from the ordinary case anyway — a sleeping exec child reaches arm 2 under
+// RestartHibernating precisely because there is nothing there to cycle.
+// Fire-and-forget is what makes that safe.
//
-// Errors are logged, never returned: a failed restart must not poison the
-// adopted version for the other hosts in the pass (release/adopt.go's rule).
-func makeRestartRoute(reg *registry.Registry, tryRestart func(host string), bounce func(host string) bool) func(host string) error {
+// Arm 1 RETURNS the failure, because for a named unit the ordinary case is
+// not indistinguishable: try-restart of a unit that is loaded and stopped
+// exits 0, so a non-zero exit means the name is wrong (exit 5, "not loaded")
+// or the service would not come back. Both are "the new build is NOT
+// running", and swallowing them would hand adoptHost a success it did not
+// have: it would stamp Version, the edge would serve the new
+// X-Carlos-Version, and `carlos deploy` would report live over the old
+// process — issue #110 again, now with the stamp actively lying rather than
+// merely unattended. Returned, adoptHost rolls the symlink back to the
+// version that IS running, skips SetVersions, prints the failure, and the
+// next pass retries; the header stays honest on the old version throughout.
+func makeRestartRoute(reg *registry.Registry, tryRestart func(host string), tryRestartUnit func(unit string) error, bounce func(host string) bool) func(host string) error {
return func(host string) error {
rt, err := reg.Get(host)
if err != nil {
log.Printf("adopt: registry lookup %s: %v (restarting as a unit)", host, err)
}
+ if err == nil && rt != nil && rt.Unit != "" {
+ return tryRestartUnit(rt.Unit)
+ }
if err == nil && rt != nil && rt.Backing != registry.BackingUnit {
if bounce != nil && bounce(host) {
log.Printf("adopt: bounced exec instance %s onto its new version", host)
@@ -1558,15 +1597,52 @@ func systemctlTryRestart(host string) {
}
}
-// restartRoute is the root adopt pass's restart callback (RunAdopt, out of
-// carlos-adopt.service): pure systemctl, no bounce. That process has no
-// activator and no children of its own — the exec instances belong to the
-// agent, and another process's children are not its to stop.
-func restartRoute(host string) error {
- systemctlTryRestart(host)
+// systemctlTryRestartUnit is makeRestartRoute's NAMED-unit path: cycle the
+// unit the route itself names, iff it's running. Same try-restart-not-restart
+// reasoning as systemctlTryRestart above — a service an operator has stopped
+// stays stopped, and try-restart on a loaded-but-stopped unit exits 0.
+//
+// It LOGS AND RETURNS, where its carlos-app@ sibling only logs, and the
+// asymmetry is deliberate: this name came from an operator rather than from
+// activator.UnitName, so a non-zero exit here means the stamp is wrong or the
+// service will not come back — never the routine "nothing was running"
+// systemctlTryRestart has to tolerate. The journal line is kept as well as
+// the error because the error's own destination is a pass that prints one
+// summary line per host; the exit status and systemctl's own words belong in
+// the journal where an operator greps for the unit name.
+//
+// The unit reaches exec.Command as ONE argv element, so there is no shell for
+// a name to escape into; the shape rail lives at the write (registry's
+// ValidUnitName), where a typo can still be reported to the person who typed
+// it.
+func systemctlTryRestartUnit(unit string) error {
+ out, err := exec.Command("systemctl", "try-restart", unit).CombinedOutput()
+ if err != nil {
+ log.Printf("adopt: try-restart %s: %v: %s", unit, err, strings.TrimSpace(string(out)))
+ return fmt.Errorf("try-restart %s: %w: %s", unit, err, strings.TrimSpace(string(out)))
+ }
return nil
}
+// rootRestartRoute is the root adopt pass's restart callback (RunAdopt, out of
+// carlos-adopt.service): no bounce. That process has no activator and no
+// children of its own — the exec instances belong to the agent, and another
+// process's children are not its to stop. With bounce nil the dispatch's arm
+// 3 is unreachable and every route it used to try-restart by host still is,
+// byte-identically; what it gains is arm 1.
+//
+// Arm 1 has to be HERE, not only on the agent's in-process loops, and the
+// reason is the sandbox. carlos-edge.service runs as User=carlos with
+// ProtectSystem=strict and ReadWritePaths=/data /run/carlos, so the agent
+// cannot write /opt/carlos: on a real box its pass reports the stage as
+// blocked and relays to this verb (carlos-stage.path), which does the
+// staging, the repoint, the restart and the version stamp. A restart arm
+// wired only into the agent would therefore be a restart arm the consoles
+// never reach.
+func rootRestartRoute(reg *registry.Registry) func(host string) error {
+ return makeRestartRoute(reg, systemctlTryRestart, systemctlTryRestartUnit, nil)
+}
+
// bounceFn adapts an activator to makeRestartRoute's bounce parameter,
// nil-safe: a box with no activator armed gets a nil callback, which
// makeRestartRoute reads as "nothing running to cycle" rather than falling
@@ -1628,6 +1704,19 @@ func RunAdopt(args []string) {
if err != nil {
log.Fatalf("adopt: %v", err)
}
+ // One callback for both halves below, built once here now that it needs
+ // the registry — it reads each route's `unit` column to decide what to
+ // cycle. See rootRestartRoute for why the named-unit arm has to exist on
+ // THIS pass.
+ //
+ // Only the RELEASE pass can actually reach that arm. appconfig.Sync skips
+ // every route whose Backing is not BackingUnit before it restarts anything
+ // (an exec-backed route's config is delivered by the agent's in-process
+ // loop instead), and a unit-backed route is exactly what `carlos route
+ // -unit` refuses to stamp — so config sync sees no unit-named route by
+ // construction. It is handed the same callback anyway because there should
+ // be ONE restart dispatch on this pass, not two that could drift.
+ restart := rootRestartRoute(reg)
// Parsed here, before config sync, exactly as the full pass always has
// (a box with an unusable release key must die before it converges
// anything), but SKIPPED entirely for -config-only: that mode never
@@ -1649,7 +1738,7 @@ func RunAdopt(args []string) {
secretsPriv, err := appconfig.ParsePrivateKey(raw, "CARLOS_SECRETS_KEY")
if err != nil {
log.Printf("adopt: config sync disabled: %v", err)
- } else if err := appconfig.Sync(ctx, reg, store, secretsPriv, appconfig.DefaultEnvDir, restartRoute); err != nil {
+ } else if err := appconfig.Sync(ctx, reg, store, secretsPriv, appconfig.DefaultEnvDir, restart); err != nil {
log.Printf("adopt: config sync: %v", err)
}
}
@@ -1709,7 +1798,7 @@ func RunAdopt(args []string) {
if !*stageOnly {
passOpts = append(passOpts, release.NagExpiredCanaries(os.Stdout))
}
- if err := release.Pass(ctx, reg, store, pub, release.OptDir(), restartRoute, passOpts...); err != nil {
+ if err := release.Pass(ctx, reg, store, pub, release.OptDir(), restart, passOpts...); err != nil {
log.Fatalf("adopt: %v", err)
}
diff --git a/internal/host/agent_test.go b/internal/host/agent_test.go
index 96f6c8c8a..00917f7a4 100644
--- a/internal/host/agent_test.go
+++ b/internal/host/agent_test.go
@@ -1,14 +1,18 @@
package host
import (
+ "bytes"
"context"
"crypto/ed25519"
"encoding/base64"
"errors"
+ "log"
+ "os"
"path/filepath"
"runtime"
"slices"
"sort"
+ "strings"
"testing"
"time"
@@ -517,6 +521,10 @@ func TestMakeRestartRouteDispatchesOnBacking(t *testing.T) {
var restarted, bounced []string
restart := makeRestartRoute(reg,
func(h string) { restarted = append(restarted, h) },
+ func(u string) error {
+ t.Fatalf("named-unit arm fired for %q on a route that names no unit", u)
+ return nil
+ },
func(h string) bool { bounced = append(bounced, h); return true })
if err := restart("unit.test"); err != nil {
t.Fatalf("restart unit.test: %v", err)
@@ -538,7 +546,11 @@ func TestMakeRestartRouteDispatchesOnBacking(t *testing.T) {
func TestMakeRestartRouteUnknownHostFallsBackToUnit(t *testing.T) {
reg := restartTestRegistry(t)
var restarted []string
- restart := makeRestartRoute(reg, func(h string) { restarted = append(restarted, h) }, nil)
+ restart := makeRestartRoute(reg, func(h string) { restarted = append(restarted, h) },
+ func(u string) error {
+ t.Fatalf("named-unit arm fired for %q on a host the registry cannot answer for", u)
+ return nil
+ }, nil)
if err := restart("ghost.test"); err != nil {
t.Fatalf("restart ghost.test: %v", err)
}
@@ -573,7 +585,11 @@ func TestMakeRestartRouteExecFallsThroughWhenNothingBounced(t *testing.T) {
t.Fatalf("upsert exec route: %v", err)
}
var restarted []string
- restart := makeRestartRoute(reg, func(h string) { restarted = append(restarted, h) }, tc.bounce)
+ restart := makeRestartRoute(reg, func(h string) { restarted = append(restarted, h) },
+ func(u string) error {
+ t.Fatalf("named-unit arm fired for %q on a route that names no unit", u)
+ return nil
+ }, tc.bounce)
if err := restart("exec.test"); err != nil {
t.Fatalf("restart exec.test: %v", err)
}
@@ -584,6 +600,226 @@ func TestMakeRestartRouteExecFallsThroughWhenNothingBounced(t *testing.T) {
}
}
+// TestMakeRestartRouteRestartsTheUnitARouteNames pins issue #110's fix: an
+// addr-backed route that names its own unit (the consoles, `-addr
+// 127.0.0.1:8090` under a bespoke console.service) is cycled BY THAT NAME,
+// and neither of the other two arms runs — both would cycle nothing here,
+// which is exactly how adoption came to stamp a Version the running process
+// did not have.
+func TestMakeRestartRouteRestartsTheUnitARouteNames(t *testing.T) {
+ reg := restartTestRegistry(t)
+ if err := reg.Upsert(registry.Route{
+ Host: "console.carloku.com", Kind: "instance", App: "console",
+ Addr: "127.0.0.1:8090", Status: "ready", Unit: "console.service",
+ }); err != nil {
+ t.Fatalf("upsert console route: %v", err)
+ }
+ var units, restarted, bounced []string
+ restart := makeRestartRoute(reg,
+ func(h string) { restarted = append(restarted, h) },
+ func(u string) error { units = append(units, u); return nil },
+ func(h string) bool { bounced = append(bounced, h); return true })
+ if err := restart("console.carloku.com"); err != nil {
+ t.Fatalf("restart console.carloku.com: %v", err)
+ }
+ if len(units) != 1 || units[0] != "console.service" {
+ t.Fatalf("named-unit arm: %v, want [console.service]", units)
+ }
+ if len(restarted) != 0 {
+ t.Fatalf("a route that names its unit must not also be try-restarted as carlos-app@<host>: %v", restarted)
+ }
+ if len(bounced) != 0 {
+ t.Fatalf("a route that names its unit must not be offered to the activator: %v", bounced)
+ }
+}
+
+// The named-unit arm wins over BACKING, not merely over its absence: a route
+// carrying both a unit name and unit backing names where its process is, and
+// the name is the more specific answer. Pinned so a later reordering of the
+// dispatch has to argue with a test rather than silently cycle
+// carlos-app@<host> for a route whose process is somewhere else.
+func TestMakeRestartRouteUnitNameBeatsBacking(t *testing.T) {
+ reg := restartTestRegistry(t)
+ if err := reg.Upsert(registry.Route{
+ Host: "odd.test", Kind: "instance", App: "a", Status: "ready",
+ Backing: registry.BackingUnit, Unit: "bespoke.service",
+ }); err != nil {
+ t.Fatalf("upsert: %v", err)
+ }
+ var units, restarted []string
+ restart := makeRestartRoute(reg,
+ func(h string) { restarted = append(restarted, h) },
+ func(u string) error { units = append(units, u); return nil },
+ nil)
+ if err := restart("odd.test"); err != nil {
+ t.Fatalf("restart odd.test: %v", err)
+ }
+ if len(units) != 1 || units[0] != "bespoke.service" || len(restarted) != 0 {
+ t.Fatalf("units = %v, host try-restarts = %v; want the named unit alone", units, restarted)
+ }
+}
+
+// The root pass (RunAdopt, carlos-adopt.service) must carry the named-unit
+// arm, because on a real box it is the pass that actually adopts a console:
+// the agent runs inside carlos-edge.service's ProtectSystem=strict sandbox
+// and cannot write /opt/carlos, so its stage is blocked and relayed here.
+// Everything else about it is unchanged — no activator, so every route it
+// used to try-restart by host still is.
+func TestRootRestartRouteCarriesTheNamedUnitArm(t *testing.T) {
+ reg := restartTestRegistry(t)
+ if err := reg.Upsert(registry.Route{
+ Host: "console.carloku.com", Kind: "instance", App: "console",
+ Addr: "127.0.0.1:8090", Status: "ready", Unit: "console.service",
+ }); err != nil {
+ t.Fatalf("upsert: %v", err)
+ }
+ // A fake systemctl on PATH records the real argv this callback builds —
+ // the same recording trick TestBootstrapStagingRunsNoSystemctl uses — so
+ // this covers the production path (systemctlTryRestartUnit's exec.Command)
+ // and not just the constructor.
+ bin := t.TempDir()
+ callLog := filepath.Join(t.TempDir(), "systemctl-calls")
+ if err := os.WriteFile(filepath.Join(bin, "systemctl"),
+ []byte("#!/bin/sh\necho \"$@\" >> "+callLog+"\n"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH"))
+
+ if err := rootRestartRoute(reg)("console.carloku.com"); err != nil {
+ t.Fatalf("root restart: %v", err)
+ }
+ got, err := os.ReadFile(callLog)
+ if err != nil {
+ t.Fatalf("the root pass ran no systemctl at all: %v", err)
+ }
+ if want := "try-restart console.service\n"; string(got) != want {
+ t.Fatalf("systemctl calls = %q, want %q", got, want)
+ }
+}
+
+// TestAdoptPassRestartsANamedUnitOnAdoption is the end-to-end: a promote the
+// pass adopts must reach the route's own unit, not just repoint a symlink and
+// stamp a version. This is the shape of the deploy that produced issue #110.
+func TestAdoptPassRestartsANamedUnitOnAdoption(t *testing.T) {
+ dir := t.TempDir()
+ optDir := t.TempDir()
+ s := release.NewDirStore(dir)
+ ctx := context.Background()
+ plat := runtime.GOOS + "-" + runtime.GOARCH
+
+ pub, priv, err := ed25519.GenerateKey(nil)
+ if err != nil {
+ t.Fatalf("generate key: %v", err)
+ }
+ reg := restartTestRegistry(t)
+ if err := reg.Upsert(registry.Route{
+ Host: "console.test", Kind: "instance", App: "console", Channel: "canary/console",
+ Addr: "127.0.0.1:8090", Status: "ready", Unit: "console.service",
+ }); err != nil {
+ t.Fatalf("upsert console.test: %v", err)
+ }
+ m := release.Manifest{
+ App: "console", Version: "v1", Kind: "binary",
+ Artifacts: map[string]release.Artifact{plat: {}},
+ }
+ if err := release.Ship(ctx, s, "bab", m, map[string][]byte{plat: []byte("build one")}); err != nil {
+ t.Fatalf("Ship: %v", err)
+ }
+ if err := release.Promote(ctx, s, priv, "bab", "console", "canary/console", "v1"); err != nil {
+ t.Fatalf("Promote: %v", err)
+ }
+
+ var units []string
+ restart := makeRestartRoute(reg,
+ func(h string) { t.Fatalf("carlos-app@ arm fired for %q — the console has no such unit", h) },
+ func(u string) error { units = append(units, u); return nil },
+ func(h string) bool {
+ t.Fatalf("bounce arm fired for %q — the activator owns no console child", h)
+ return false
+ })
+
+ if err := release.Pass(ctx, reg, s, pub, optDir, restart, release.RestartHibernating(restart)); err != nil {
+ t.Fatalf("Pass: %v", err)
+ }
+ if len(units) != 1 || units[0] != "console.service" {
+ t.Fatalf("adoption cycled %v, want [console.service] — the header would claim v1 over the old process", units)
+ }
+ if v, err := release.LiveVersion(optDir, "console.test"); err != nil || v != "v1" {
+ t.Fatalf("console.test live = %q, %v; want v1", v, err)
+ }
+}
+
+// TestAdoptPassRefusesToStampAVersionAWrongUnitDidNotStart is the honesty
+// half of issue #110, and the reason the named-unit arm returns its error
+// where the other two swallow theirs. A well-formed but WRONG unit name
+// (`systemctl try-restart` exits 5, "not loaded") restarted nothing — exactly
+// the state the column exists to end — and if that were swallowed adoption
+// would stamp Version, the edge would serve the new X-Carlos-Version, and
+// `carlos deploy` would report live over the old process.
+//
+// So: no Version stamp, and the failure is in the journal under the unit name
+// an operator would grep for. The next pass retries.
+func TestAdoptPassRefusesToStampAVersionAWrongUnitDidNotStart(t *testing.T) {
+ dir := t.TempDir()
+ optDir := t.TempDir()
+ s := release.NewDirStore(dir)
+ ctx := context.Background()
+ plat := runtime.GOOS + "-" + runtime.GOARCH
+
+ pub, priv, err := ed25519.GenerateKey(nil)
+ if err != nil {
+ t.Fatalf("generate key: %v", err)
+ }
+ reg := restartTestRegistry(t)
+ // The stamp is well-formed — ValidUnitName cannot tell a real unit from an
+ // absent one — but no such unit is loaded on the box.
+ if err := reg.Upsert(registry.Route{
+ Host: "console.test", Kind: "instance", App: "console", Channel: "canary/console",
+ Addr: "127.0.0.1:8090", Status: "ready", Unit: "consoel.service",
+ }); err != nil {
+ t.Fatalf("upsert console.test: %v", err)
+ }
+ m := release.Manifest{
+ App: "console", Version: "v1", Kind: "binary",
+ Artifacts: map[string]release.Artifact{plat: {}},
+ }
+ if err := release.Ship(ctx, s, "bab", m, map[string][]byte{plat: []byte("build one")}); err != nil {
+ t.Fatalf("Ship: %v", err)
+ }
+ if err := release.Promote(ctx, s, priv, "bab", "console", "canary/console", "v1"); err != nil {
+ t.Fatalf("Promote: %v", err)
+ }
+
+ // A fake systemctl that answers the way the real one does for a unit that
+ // is not loaded: exit 5, with systemd's own words on stdout.
+ bin := t.TempDir()
+ if err := os.WriteFile(filepath.Join(bin, "systemctl"),
+ []byte("#!/bin/sh\necho \"Failed to try-restart $2: Unit $2 not found.\"\nexit 5\n"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH"))
+
+ var journal bytes.Buffer
+ log.SetOutput(&journal)
+ t.Cleanup(func() { log.SetOutput(os.Stderr) })
+
+ restart := rootRestartRoute(reg)
+ if err := release.Pass(ctx, reg, s, pub, optDir, restart); err != nil {
+ t.Fatalf("Pass: %v", err)
+ }
+
+ rt, err := reg.Get("console.test")
+ if err != nil || rt == nil {
+ t.Fatalf("get console.test: %+v, %v", rt, err)
+ }
+ if rt.Version != "" {
+ t.Fatalf("Version = %q after a restart that started nothing — the header would claim a build the console is not running", rt.Version)
+ }
+ if !strings.Contains(journal.String(), "consoel.service") {
+ t.Fatalf("journal = %q, want the failed try-restart logged under the unit name", journal.String())
+ }
+}
+
// TestAdoptPassBouncesAWarmHibernatingInstance is the end-to-end this task
// exists for, through the real release.Pass with the real closure: an exec
// instance minted the way provision does it (Hibernate: true, Backing "") is
@@ -632,6 +868,10 @@ func TestAdoptPassBouncesAWarmHibernatingInstance(t *testing.T) {
// it never starts anything.
restart := makeRestartRoute(reg,
func(h string) { tried = append(tried, h) },
+ func(u string) error {
+ t.Fatalf("named-unit arm fired for %q on a route that names no unit", u)
+ return nil
+ },
func(h string) bool {
bounced = append(bounced, h)
return h == "warm.test"
@@ -937,6 +1177,10 @@ func TestFastSweepBouncesAWarmHibernatingCanary(t *testing.T) {
var bounced []string
restart := makeRestartRoute(env.reg,
func(string) {},
+ func(u string) error {
+ t.Fatalf("named-unit arm fired for %q on a route that names no unit", u)
+ return nil
+ },
func(h string) bool { bounced = append(bounced, h); return true })
d := env.deps(nil)
d.restart = restart
diff --git a/internal/registry/registry.go b/internal/registry/registry.go
index 24192db6d..fc5bf7520 100644
--- a/internal/registry/registry.go
+++ b/internal/registry/registry.go
@@ -107,6 +107,31 @@ type Route struct {
// DynamicUser, StateDirectory) that the activator drives via systemctl.
Backing string
+ // Unit is the systemd unit that owns this route's process when it is
+ // neither an exec child nor a carlos-app@ tenant — an operator-run
+ // bespoke service (console.service is the case that forced the column:
+ // an addr-backed route, `-addr 127.0.0.1:8090`, whose process no
+ // backing-dispatched restart could reach). Empty for everything else,
+ // which is every route on every box today.
+ //
+ // The adopt pass's restart dispatch tries this unit FIRST when it is set
+ // (internal/host's makeRestartRoute): before the change, adoption
+ // repointed the symlink, stamped Version — and therefore the edge's
+ // X-Carlos-Version header — and cycled nothing, so the old process went
+ // on serving a build the header already claimed was live (issue #110).
+ //
+ // Written by SetUnit, which validates the name, and by Upsert, which does
+ // not — Upsert validates no column at all (SetCaps's rule: the rails live
+ // at the door), so the convention is that a full-row caller has already
+ // run ValidUnitName. `carlos add -unit` is the only such caller today and
+ // does exactly that, at the flag, before the registry is opened. Upsert
+ // preserves a non-empty existing value — see Environment's comment for
+ // why.
+ //
+ // Deliberately NOT in Fingerprint(): unit changes who restarts the
+ // process, not what the edge serves.
+ Unit string
+
// Platform marks a route the domains reconcile pass minted
// (<app>.<sqid>.<apps-domain>). Only platform routes are ever
// auto-removed by reconciliation; custom domains are operator/owner
@@ -181,6 +206,40 @@ func ValidTunnelName(name string) bool {
return tunnelNameRe.MatchString(name)
}
+// unitNameRe: a systemd unit name that must END in .service — the only unit
+// type this column means anything for (a route's process). The character
+// class is systemd's own alphabet for the part before the suffix, including
+// the '@' of a template instance, so `console.service` and
+// `titogo-canary@demo.service` both pass and a typo does not.
+//
+// The FIRST character is narrower than the rest, and bounded like
+// ValidTunnelName's 32 (64 here, because unit names legitimately carry a
+// prefix and an instance): a leading '-' is how a name gets mistaken for a
+// flag by anything that ever builds an argv from this column, and a leading
+// '.' admits `..service`, `..`-shaped names and other things that read as
+// paths rather than units. Neither is a name anybody means to type, and both
+// are cheaper to refuse than to reason about later.
+//
+// Lives here rather than in the CLI for the same reason ValidTunnelName does:
+// `unit` is this package's column, and this is the shape SetUnit enforces on
+// every write, not merely the shape one door happens to check.
+var unitNameRe = regexp.MustCompile(`^[A-Za-z0-9@_][A-Za-z0-9@_.\-]{0,63}\.service$`)
+
+// ValidUnitName reports whether name is usable as a route's Unit binding.
+// Unlike ValidTunnelName, "" is VALID here: it is the column's default and
+// its only "no unit" value, so every caller that can clear a binding
+// (`carlos route -unit -`, SetUnit(host, "")) passes it through this same
+// gate rather than around it.
+//
+// The rail is about a typo, not an injection: these strings reach systemctl
+// as one argv element of an exec.Command, so there is no shell to escape.
+// What there IS, without the rail, is `console.servcie` written silently to
+// the column and discovered only as a restart that quietly did nothing —
+// exactly the failure the column exists to end.
+func ValidUnitName(name string) bool {
+ return name == "" || unitNameRe.MatchString(name)
+}
+
func Open(path string) (*Registry, error) {
d, err := sql.Open("sqlite", path)
if err != nil {
@@ -257,6 +316,7 @@ CREATE TABLE IF NOT EXISTS routes (
`ALTER TABLE routes ADD COLUMN tunnel TEXT NOT NULL DEFAULT ''`,
`ALTER TABLE routes ADD COLUMN provisioned INTEGER NOT NULL DEFAULT 0`,
`ALTER TABLE routes ADD COLUMN seed TEXT NOT NULL DEFAULT ''`,
+ `ALTER TABLE routes ADD COLUMN unit TEXT NOT NULL DEFAULT ''`,
} {
if _, err := r.sql.Exec(stmt); err != nil && !strings.Contains(err.Error(), "duplicate column") {
return err
@@ -420,7 +480,7 @@ func (r *Registry) Accounts() ([]Account, error) {
}
// routeCols is the column list every route read and the one route write
-// share — 29 names, in this exact order. scanRoute's destinations, Upsert's
+// share — 32 names, in this exact order. scanRoute's destinations, Upsert's
// placeholder row and Upsert's argument list all count from here: adding a
// column means adding it in FOUR places, and a miscount is a silent column
// shift rather than an error (TestUpsertRoundTripsAllColumns is what catches
@@ -429,7 +489,7 @@ const routeCols = `host, slug, kind, socket, addr, status, last_error, db_path,
version, prev_version, app, pool, region, channel, hibernate, canary,
replication, next_due, backing, status_named, platform, dns_status,
dns_checked_at, cert_expires_at, account, environment, caps, tunnel,
-provisioned, seed, created_at`
+provisioned, seed, unit, created_at`
func scanRoute(scan func(...any) error) (Route, error) {
var t Route
@@ -438,7 +498,7 @@ func scanRoute(scan func(...any) error) (Route, error) {
&t.LastError, &t.DBPath, &t.Version, &t.PrevVersion, &t.App, &t.Pool,
&t.Region, &t.Channel, &hib, &can, &t.Replication, &t.NextDue, &t.Backing,
&sn, &plat, &t.DNSStatus, &t.DNSCheckedAt, &t.CertExpiresAt, &t.Account,
- &t.Environment, &t.Caps, &t.Tunnel, &prov, &t.Seed, &t.CreatedAt)
+ &t.Environment, &t.Caps, &t.Tunnel, &prov, &t.Seed, &t.Unit, &t.CreatedAt)
t.Hibernate = hib != 0
t.Canary = can != 0
t.StatusNamed = sn != 0
@@ -499,10 +559,10 @@ func (r *Registry) Delete(host string) error {
return err
}
-// Upsert inserts or updates a route by host. Five columns are deliberately
-// not plain overwrites — `environment`, `caps`, `tunnel`, `provisioned` and
-// `seed` all keep the existing row's value when the incoming Route carries
-// the zero one.
+// Upsert inserts or updates a route by host. Six columns are deliberately
+// not plain overwrites — `environment`, `caps`, `tunnel`, `provisioned`,
+// `seed` and `unit` all keep the existing row's value when the incoming
+// Route carries the zero one.
//
// The reason is the same for all five, and it is structural: a full-row
// Upsert caller (`carlos add`, the domains reconciler's two mint paths, the
@@ -520,11 +580,17 @@ func (r *Registry) Delete(host string) error {
// over that route FOREVER — deleting the bucket record could no longer
// retire it, and only a human noticing would.
//
+// `unit` is the same shape as `tunnel` with the blast radius of `caps`: a
+// re-`carlos add` on the console's row would drop the stamp that is the only
+// thing making adoption cycle that process, and nothing would report it —
+// the next promote would stamp X-Carlos-Version onto a console still serving
+// the old build, which is issue #110 returning silently.
+//
// An explicit non-zero value still wins in every case; clearing keeps
// exactly one door per column, none of which goes through Upsert:
// SetEnvironment (`carlos route -environment -`), SetCaps (`carlos route
-// -revoke`), SetTunnel (`carlos route -tunnel -`), SetSeed, and for
-// `provisioned` the row's own deletion.
+// -revoke`), SetTunnel (`carlos route -tunnel -`), SetUnit (`carlos route
+// -unit -`), SetSeed, and for `provisioned` the row's own deletion.
func (r *Registry) Upsert(t Route) error {
if t.CreatedAt == 0 {
t.CreatedAt = time.Now().Unix()
@@ -543,7 +609,7 @@ func (r *Registry) Upsert(t Route) error {
}
_, err := r.sql.Exec(`
INSERT INTO routes (`+routeCols+`)
-VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
+VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
ON CONFLICT(host) DO UPDATE SET
slug=excluded.slug, kind=excluded.kind, socket=excluded.socket,
addr=excluded.addr, status=excluded.status, last_error=excluded.last_error,
@@ -560,13 +626,14 @@ ON CONFLICT(host) DO UPDATE SET
caps=CASE WHEN excluded.caps = '' THEN routes.caps ELSE excluded.caps END,
tunnel=CASE WHEN excluded.tunnel = '' THEN routes.tunnel ELSE excluded.tunnel END,
provisioned=CASE WHEN excluded.provisioned = 0 THEN routes.provisioned ELSE excluded.provisioned END,
- seed=CASE WHEN excluded.seed = '' THEN routes.seed ELSE excluded.seed END`,
+ seed=CASE WHEN excluded.seed = '' THEN routes.seed ELSE excluded.seed END,
+ unit=CASE WHEN excluded.unit = '' THEN routes.unit ELSE excluded.unit END`,
t.Host, t.Slug, t.Kind, t.Socket, t.Addr, t.Status, t.LastError,
t.DBPath, t.Version, t.PrevVersion, t.App, t.Pool, t.Region, t.Channel,
b2i(t.Hibernate), b2i(t.Canary), t.Replication, t.NextDue, t.Backing,
b2i(t.StatusNamed), b2i(t.Platform), t.DNSStatus, t.DNSCheckedAt,
t.CertExpiresAt, t.Account, t.Environment, t.Caps, t.Tunnel,
- b2i(t.Provisioned), t.Seed, t.CreatedAt)
+ b2i(t.Provisioned), t.Seed, t.Unit, t.CreatedAt)
return err
}
@@ -728,6 +795,49 @@ func (r *Registry) SetBacking(host, backing string) error {
return nil
}
+// SetUnit stamps the systemd unit that owns a route's process — the door
+// issue #110 needed and did not have: a console is an addr-backed route under
+// an operator-run `console.service`, which the adopt pass's restart dispatch
+// could reach by neither of its two arms (Bounce owns no such process, and
+// there is no carlos-app@<host> unit to try-restart), so promote → adopt
+// stamped a new Version behind a process still running the old build.
+//
+// Narrow UPDATE with an error on an unmatched host, the SetBacking contract
+// and for the same reason: a human types the hostname into this one directly
+// (`carlos route -unit console.service` against a live registry is how the
+// production consoles get theirs), and a silent no-op on a typo would be
+// worse than a refusal.
+//
+// Deliberately NOT a compare-and-swap, again following SetBacking: unit is
+// not a set an operator merges into (SetCaps's case) and not a value a pass
+// rewrites hourly (SetDBPath's) — it is one operator naming the unit that
+// owns one process.
+//
+// It DOES validate, which SetBacking does not, and the difference is the
+// point: backing has two legal values and the doors compare against both, so
+// a bad value cannot survive the flag. A unit name is free text an operator
+// types once, on the row whose whole purpose is to be handed to systemctl
+// months later — a typo would be discovered as a restart that silently did
+// nothing, i.e. as the very bug this column closes. "" is accepted, because
+// clearing the binding is a legitimate write (see ValidUnitName).
+func (r *Registry) SetUnit(host, unit string) error {
+ if !ValidUnitName(unit) {
+ return fmt.Errorf("unit %q: want a systemd service unit name (e.g. console.service) or \"\" to clear", unit)
+ }
+ res, err := r.sql.Exec(`UPDATE routes SET unit = ? WHERE host = ?`, unit, host)
+ if err != nil {
+ return err
+ }
+ n, err := res.RowsAffected()
+ if err != nil {
+ return err
+ }
+ if n == 0 {
+ return fmt.Errorf("no route for host %q", host)
+ }
+ return nil
+}
+
// SetChannels repoints every host in hosts to channel as ONE transaction —
// every row moves or none do. `carlos route -app` uses this for its bulk
// form: the caller runs every route's safety-rail check to completion FIRST
@@ -991,8 +1101,10 @@ WHERE kind = 'instance' AND next_due > 0 AND next_due <= ? ORDER BY host`, now)
// two DIFFERENT values of the SAME length (channel canary/1 -> canary/2, a
// git short-sha version bump) change the string exactly like any other
// write. `grants` stays deliberately excluded (registry.go:93's rule: grants
-// change unit shape, not routing). Row count is folded into the result so an
-// empty table and a never-opened one still differ from a populated one.
+// change unit shape, not routing), and so does `unit` (it changes who
+// restarts the process, not what the edge serves). Row count is folded into
+// the result so an empty table and a never-opened one still differ from a
+// populated one.
// Still a fresh connection per poll — it must see other processes' commits
// (same WAL-snapshot quirk as List).
//
diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go
index 82ff11a3a..dc6a24ff2 100644
--- a/internal/registry/registry_test.go
+++ b/internal/registry/registry_test.go
@@ -73,7 +73,8 @@ func TestUpsertRoundTripsAllColumns(t *testing.T) {
App: "kass", Pool: "shared", Region: "ie", Channel: "stable",
Environment: "demo", Caps: "host-role,port:25",
Hibernate: true, Canary: true, Replication: "batched",
- NextDue: 4200, Backing: BackingUnit, CreatedAt: 1700000000,
+ NextDue: 4200, Backing: BackingUnit, Unit: "console.service",
+ CreatedAt: 1700000000,
}
if err := reg.Upsert(want); err != nil {
t.Fatal(err)
@@ -1376,6 +1377,235 @@ func TestValidTunnelName(t *testing.T) {
}
}
+// A registry file written BEFORE the unit column existed must still open, its
+// rows must read as "no unit" (which is what every route on every box is
+// today), and the new column must be writable on that same migrated row. The
+// pre-change file is simulated the way TestPreCapsRegistryOpensAndReadsUngranted
+// simulates its own: drop the column from a fresh registry and write a row
+// through raw SQL, which is the shape an older binary left behind.
+func TestPreUnitRegistryOpensAndReadsNoUnit(t *testing.T) {
+ path := filepath.Join(t.TempDir(), "registry.db")
+ reg, err := Open(path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.Close(); err != nil {
+ t.Fatal(err)
+ }
+
+ old, err := sql.Open("sqlite", path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, err := old.Exec(`ALTER TABLE routes DROP COLUMN unit`); err != nil {
+ t.Fatalf("simulate a pre-unit registry: %v", err)
+ }
+ if _, err := old.Exec(`INSERT INTO routes (host, kind, addr, status, created_at)
+VALUES ('console.carloku.com', 'instance', '127.0.0.1:8090', 'ready', 1700000000)`); err != nil {
+ t.Fatal(err)
+ }
+ if err := old.Close(); err != nil {
+ t.Fatal(err)
+ }
+
+ reg, err = Open(path)
+ if err != nil {
+ t.Fatalf("a pre-unit registry must still open: %v", err)
+ }
+ defer reg.Close()
+ got, err := reg.Get("console.carloku.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got == nil {
+ t.Fatal("the pre-existing row vanished across the migration")
+ }
+ if got.Unit != "" {
+ t.Fatalf("Unit = %q on a row written before the column existed, want %q", got.Unit, "")
+ }
+ if got.Host != "console.carloku.com" || got.Kind != "instance" ||
+ got.Addr != "127.0.0.1:8090" || got.CreatedAt != 1700000000 {
+ t.Fatalf("the migrated row shifted columns: %+v", *got)
+ }
+ // The stamp the live consoles need, on exactly that migrated row.
+ if err := reg.SetUnit("console.carloku.com", "console.service"); err != nil {
+ t.Fatal(err)
+ }
+ got, err = reg.Get("console.carloku.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "console.service" {
+ t.Fatalf("Unit = %q after SetUnit on a migrated row", got.Unit)
+ }
+ // And clearing it again has a door, so a mis-stamp is reversible.
+ if err := reg.SetUnit("console.carloku.com", ""); err != nil {
+ t.Fatal(err)
+ }
+ got, err = reg.Get("console.carloku.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "" {
+ t.Fatalf("SetUnit clear failed: %q", got.Unit)
+ }
+}
+
+// SetUnit is narrow (it touches no other column) and refuses an unknown host
+// — the SetBacking contract, for the same reason: a human types the hostname
+// in directly.
+func TestSetUnitIsNarrowAndErrorsOnAnUnknownHost(t *testing.T) {
+ reg := openTestRegistry(t)
+ base := Route{
+ Host: "console.example.com", Kind: "instance", App: "console",
+ Addr: "127.0.0.1:8090", Status: "ready", Version: "abc1234", Channel: "canary/console",
+ }
+ if err := reg.Upsert(base); err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.SetUnit("console.example.com", "console.service"); err != nil {
+ t.Fatal(err)
+ }
+ got, err := reg.Get("console.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "console.service" {
+ t.Fatalf("Unit = %q, want console.service", got.Unit)
+ }
+ if got.Version != "abc1234" || got.Channel != "canary/console" || got.Addr != "127.0.0.1:8090" {
+ t.Fatalf("SetUnit touched another column: %+v", *got)
+ }
+ if err := reg.SetUnit("nope.example.com", "console.service"); err == nil {
+ t.Fatal("SetUnit on an unknown host: got nil error, want one")
+ }
+}
+
+// A typo'd unit must fail at WRITE time, loudly, rather than at restart time
+// silently — the whole reason the column exists is that a restart which
+// reaches nothing looks exactly like a restart that worked.
+func TestSetUnitRefusesAMalformedName(t *testing.T) {
+ reg := openTestRegistry(t)
+ if err := reg.Upsert(Route{Host: "console.example.com", Kind: "instance", Status: "ready"}); err != nil {
+ t.Fatal(err)
+ }
+ for _, bad := range []string{"console.servcie", "console", "console.timer", "console service", "console.service extra"} {
+ if err := reg.SetUnit("console.example.com", bad); err == nil {
+ t.Fatalf("SetUnit(%q) accepted; want a refusal", bad)
+ }
+ }
+ got, err := reg.Get("console.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "" {
+ t.Fatalf("a refused SetUnit still wrote %q", got.Unit)
+ }
+}
+
+func TestValidUnitName(t *testing.T) {
+ valid := []string{
+ "", // the column's default: no unit
+ "console.service", //
+ "keymail-relay.service", // hyphens, just not leading
+ "titogo-canary@demo.service", // template instance
+ "carlos_app.service", // underscore
+ "a.b.c.service", // dots, just not leading
+ "a.service", // one-character stem
+ strings.Repeat("a", 64) + ".service", // the length bound itself
+ }
+ invalid := []string{
+ "console", // no suffix
+ "console.servcie", // the typo this rail is for
+ "carlos-adopt.timer", // a unit, but not one that owns a process
+ "console.socket", //
+ "has space.service", // space
+ "../../etc/x.service", // path traversal
+ "console.service; reboot", // shell-shaped, and refused as a shape
+ ".service", // suffix alone
+ "-x.service", // leading dash: reads as a flag in any argv built from this column
+ "..service", // dot-only stem
+ "...service", //
+ ".x.service", // leading dot
+ strings.Repeat("a", 65) + ".service", // one over the bound
+ }
+ for _, n := range valid {
+ if !ValidUnitName(n) {
+ t.Errorf("ValidUnitName(%q) = false, want true", n)
+ }
+ }
+ for _, n := range invalid {
+ if ValidUnitName(n) {
+ t.Errorf("ValidUnitName(%q) = true, want false", n)
+ }
+ }
+}
+
+// Same rationale as TestUpsertPreservesTunnel, with the stakes of the caps
+// column: a full-row Upsert caller (`carlos add`, a reconciler's mint) knows
+// nothing about which unit owns a route's process, and dropping the stamp
+// would silently restore issue #110 on the one row it matters for.
+func TestUpsertPreservesUnit(t *testing.T) {
+ reg := openTestRegistry(t)
+ base := Route{Host: "console.example.com", Kind: "instance", App: "console", Status: "ready"}
+ if err := reg.Upsert(base); err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.SetUnit("console.example.com", "console.service"); err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.Upsert(base); err != nil {
+ t.Fatal(err)
+ }
+ got, err := reg.Get("console.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "console.service" {
+ t.Fatalf("Upsert clobbered Unit: %q", got.Unit)
+ }
+ // Preservation is a floor, not a freeze: an explicit value still wins.
+ base.Unit = "console-next.service"
+ if err := reg.Upsert(base); err != nil {
+ t.Fatal(err)
+ }
+ got, err = reg.Get("console.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Unit != "console-next.service" {
+ t.Fatalf("Unit = %q, want an explicit Upsert value to win", got.Unit)
+ }
+}
+
+// unit changes who RESTARTS the process, not what the edge serves — so the
+// edge must not reload for one. Same rule and same test shape as
+// TestFingerprintIgnoresAGrant, and this pins the exclusion so a later
+// "hash every column" tidy-up has to argue with a test.
+func TestFingerprintIgnoresAUnitStamp(t *testing.T) {
+ reg := openTestRegistry(t)
+ if err := reg.Upsert(Route{
+ Host: "console.example.com", Kind: "instance", App: "console",
+ Addr: "127.0.0.1:8090", Status: "ready",
+ }); err != nil {
+ t.Fatal(err)
+ }
+ before, err := reg.Fingerprint()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := reg.SetUnit("console.example.com", "console.service"); err != nil {
+ t.Fatal(err)
+ }
+ after, err := reg.Fingerprint()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if after != before {
+ t.Fatalf("Fingerprint moved on a unit stamp (%s -> %s) — the edge would reload for a change that does not touch routing", before, after)
+ }
+}
+
func TestValidTunnelNameLengthBoundary(t *testing.T) {
ok32 := "01234567890123456789012345678901" // 32 chars
if len(ok32) != 32 {
diff --git a/internal/release/adopt.go b/internal/release/adopt.go
index 4ad5453ab..46f6925ff 100644
--- a/internal/release/adopt.go
+++ b/internal/release/adopt.go
@@ -765,7 +765,7 @@ func adoptHost(ctx context.Context, reg *registry.Registry, s Store, optDir stri
if restart != nil {
// A converged DAEMON's restart is caps.Restart, not the caller's
// callback: this is the only call site that knows the plan, and it is
- // the only restart whose error must be real. restartRoute's
+ // the only restart whose error must be real. rootRestartRoute's
// try-restart (and its swallow-and-continue) keeps owning every other
// route, byte-identically to before.
//