amadan

carlos / platform Public

host: adopt restart dispatches on backing — warm exec children bounce (spec §4)

The adopt pass repointed an exec-backed route's live symlink and then ran
`systemctl try-restart carlos-app@<host>` — a unit that does not exist for an
exec route. systemctl cannot reach a child of the agent process, so a warm
exec instance kept serving its old binary until it happened to hibernate.

makeRestartRoute now dispatches on the route's Backing: unit tenants take the
unchanged try-restart path, exec children are Bounced through the activator —
the same graceful cycle config sync already uses. Hosts the registry cannot
answer for take the unit path (try-restart of a nonexistent unit is a logged
no-op; a bounce of a process we might not own is not), and a nil activator
makes the exec path a no-op rather than a pointless systemctl call.

Both in-process loops (adoptLoop, canaryAdoptLoop) take the activator and
build the closure. RunAdopt is deliberately unchanged: it is a separate root
process with no children of its own to bounce.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 4fb01ce20a27bd4a8fe91cafcd020ba29fcf44f1 parent 440c7e5
2 files changed, +157 −14
  • internal/host/agent.go +72 −14
  • internal/host/agent_test.go +85 −0
diff --git a/internal/host/agent.go b/internal/host/agent.go
index e03ee76da..5eedb0a43 100644
--- a/internal/host/agent.go
+++ b/internal/host/agent.go
@@ -157,8 +157,8 @@ func RunAgent(args []string) {
log.Printf("agent: hibernation activator armed (bucket %s, idle %s)", cfg.Bucket, cfg.Idle)
}
- go adoptLoop(reg)
- go canaryAdoptLoop(reg)
+ go adoptLoop(reg, svc)
+ go canaryAdoptLoop(reg, svc)
go statusLoop(reg)
go domainsLoop(reg, peekAcmeCache(args))
go instancesLoop(reg)
@@ -240,7 +240,10 @@ func materializeHook(reg *registry.Registry, dd string) func(context.Context, st
// config disables the loop with one log line — an agent with no release store
// is a valid box (routes managed by hand, adopted by the timer unit, or not
// versioned at all).
-func adoptLoop(reg *registry.Registry) {
+// The activator is taken so the pass can cycle exec-backed instances the way
+// only this process can (see makeRestartRoute); nil is fine — a box with no
+// hibernation armed simply has no children to bounce.
+func adoptLoop(reg *registry.Registry, svc *activator.Service) {
ctx := context.Background()
store, err := openDeployStore(ctx)
if err != nil {
@@ -253,6 +256,7 @@ func adoptLoop(reg *registry.Registry) {
return
}
optDir := release.OptDir()
+ restart := makeRestartRoute(reg, systemctlTryRestart, 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.
@@ -279,7 +283,7 @@ func adoptLoop(reg *registry.Registry) {
// OnStageBlocked, by contrast, is exactly right here: it fires only
// when this sandboxed process cannot write the version store, and all
// it does is ask the root pass to do the part this one cannot.
- if err := release.Pass(ctx, reg, store, pub, optDir, restartRoute,
+ if err := release.Pass(ctx, reg, store, pub, optDir, restart,
release.BakeDir(dataDir()), release.OnStageBlocked(stageRequester(dataDir()))); err != nil {
log.Printf("agent: adopt pass: %v", err)
}
@@ -658,7 +662,10 @@ func configGenLoop(reg *registry.Registry, svc *activator.Service, dd string) {
// Stable/beta routes stay on adoptLoop's hourly cadence: they're
// bake-gated for a day or three regardless, so faster polling would buy
// nothing there.
-func canaryAdoptLoop(reg *registry.Registry) {
+// Takes the activator for the same reason adoptLoop does: a promoted canary
+// on a warm exec instance must bounce onto its new binary, not wait for
+// hibernation. Nil is fine.
+func canaryAdoptLoop(reg *registry.Registry, svc *activator.Service) {
ctx := context.Background()
store, err := openDeployStore(ctx)
if err != nil {
@@ -669,6 +676,7 @@ func canaryAdoptLoop(reg *registry.Registry) {
return // ditto
}
optDir := release.OptDir()
+ restart := makeRestartRoute(reg, systemctlTryRestart, bounceFn(svc))
log.Printf("agent: canary adopt loop armed (2s + jitter, canary/* routes only)")
for {
time.Sleep(2*time.Second + rand.N(time.Second))
@@ -695,7 +703,7 @@ func canaryAdoptLoop(reg *registry.Registry) {
// the one step of that pickup this process cannot perform. Throttled
// inside stageRequester — this tick is 2-3 seconds, the request is at
// most one a minute.
- if err := release.PassFilter(ctx, reg, store, pub, optDir, restartRoute, isCanaryRoute,
+ if err := release.PassFilter(ctx, reg, store, pub, optDir, restart, isCanaryRoute,
release.BakeDir(dataDir()), release.OnStageBlocked(stageRequester(dataDir()))); err != nil {
log.Printf("agent: canary adopt pass: %v", err)
}
@@ -1219,21 +1227,71 @@ func statusTick(ctx context.Context, s release.Store, reg *registry.Registry, p
return status.PublishFleet(ctx, s, fleet)
}
-// restartRoute cycles a route's process onto its freshly-repointed live
-// symlink. Only systemd-managed routes need anything done here: hibernating
-// hosts never reach this (release.Pass gives them the symlink-only path), and
-// an awake activator child is converged by the sweep's ConvergeDecision — so
-// for those returning nil is the signal. For unit-managed hosts, try-restart
-// cycles the unit iff it's running, best-effort: a missing or stopped unit is
-// not this pass's problem, and must not poison the build for other hosts.
-func restartRoute(host string) error {
+// 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.
+//
+// Hosts the registry cannot answer for take the unit path: try-restart of a
+// nonexistent unit is a logged no-op, whereas a bounce of a process we might
+// not own is not. A nil bounce (no activator armed on this box) makes the
+// exec path a no-op rather than a fallthrough to systemctl — there is no
+// carlos-app@ unit behind an exec route to try-restart.
+//
+// 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 {
+ 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.Backing != registry.BackingUnit {
+ if bounce != nil && bounce(host) {
+ log.Printf("adopt: bounced exec instance %s onto its new version", host)
+ }
+ return nil
+ }
+ tryRestart(host)
+ return nil
+ }
+}
+
+// systemctlTryRestart is makeRestartRoute's production unit path: cycle the
+// route's carlos-app@ unit iff it's running, best-effort. A missing or
+// stopped unit is not this pass's problem — hibernating hosts never reach
+// here at all (release.Pass gives them the symlink-only path).
+func systemctlTryRestart(host string) {
out, err := exec.Command("systemctl", "try-restart", activator.UnitName(host)).CombinedOutput()
if err != nil {
log.Printf("adopt: try-restart carlos-app@%s: %v: %s (continuing)", host, err, strings.TrimSpace(string(out)))
}
+}
+
+// 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)
return 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
+// back to a systemctl path that could not work for an exec route anyway.
+func bounceFn(svc *activator.Service) func(host string) bool {
+ if svc == nil {
+ return nil
+ }
+ return svc.Bounce
+}
+
// openDeployStore opens the release store the box adopts from:
// CARLOS_DEPLOYMENT_BUCKET (S3) or CARLOS_DEPLOYMENT_DIR (a directory —
// tests and single-box no-AWS deployments). Mirrors cli.go's openStore, but
diff --git a/internal/host/agent_test.go b/internal/host/agent_test.go
index 66e320903..1a11066e6 100644
--- a/internal/host/agent_test.go
+++ b/internal/host/agent_test.go
@@ -479,3 +479,88 @@ func TestMaterializeHookStagesAColdRoute(t *testing.T) {
t.Fatalf("live = %q, %v; want abc1234", v, err)
}
}
+
+// restartTestRegistry is the bolt-backed registry the restart-dispatch tests
+// reason over — makeRestartRoute's whole job is to ask the registry what
+// backs a host, so a real registry (not a fake) is what the test needs.
+func restartTestRegistry(t *testing.T) *registry.Registry {
+ t.Helper()
+ reg, err := registry.Open(filepath.Join(t.TempDir(), "registry.db"))
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ return reg
+}
+
+// TestMakeRestartRouteDispatchesOnBacking pins the correction this split
+// exists for: a unit tenant is try-restarted, an exec-backed instance is
+// Bounced. Before the split both took the systemctl path — and systemctl
+// cannot reach a child of the agent process, so a warm exec instance kept
+// serving its old binary until it happened to hibernate.
+func TestMakeRestartRouteDispatchesOnBacking(t *testing.T) {
+ reg := restartTestRegistry(t)
+ if err := reg.Upsert(registry.Route{
+ Host: "unit.test", Kind: "instance", App: "a", Status: "ready",
+ Backing: registry.BackingUnit,
+ }); err != nil {
+ t.Fatalf("upsert unit route: %v", err)
+ }
+ if err := reg.Upsert(registry.Route{
+ Host: "exec.test", Kind: "instance", App: "a", Status: "ready",
+ }); err != nil {
+ t.Fatalf("upsert exec route: %v", err)
+ }
+ var restarted, bounced []string
+ restart := makeRestartRoute(reg,
+ func(h string) { restarted = append(restarted, h) },
+ func(h string) bool { bounced = append(bounced, h); return true })
+ if err := restart("unit.test"); err != nil {
+ t.Fatalf("restart unit.test: %v", err)
+ }
+ if err := restart("exec.test"); err != nil {
+ t.Fatalf("restart exec.test: %v", err)
+ }
+ if len(restarted) != 1 || restarted[0] != "unit.test" {
+ t.Fatalf("unit path: %v", restarted)
+ }
+ if len(bounced) != 1 || bounced[0] != "exec.test" {
+ t.Fatalf("exec path: %v", bounced)
+ }
+}
+
+// TestMakeRestartRouteUnknownHostFallsBackToUnit: a host the registry cannot
+// answer for gets the OLD behavior (try-restart, a safe no-op for a
+// nonexistent unit) — never a bounce of a process we might not own.
+func TestMakeRestartRouteUnknownHostFallsBackToUnit(t *testing.T) {
+ reg := restartTestRegistry(t)
+ var restarted []string
+ restart := makeRestartRoute(reg, func(h string) { restarted = append(restarted, h) }, nil)
+ if err := restart("ghost.test"); err != nil {
+ t.Fatalf("restart ghost.test: %v", err)
+ }
+ if len(restarted) != 1 || restarted[0] != "ghost.test" {
+ t.Fatalf("fallback: %v", restarted)
+ }
+}
+
+// TestMakeRestartRouteExecWithNoActivatorIsANoOp: an exec-backed route on a
+// box with no activator armed must NOT fall through to systemctl — there is
+// no carlos-app@ unit behind an exec route, so try-restart would only log a
+// failure every pass. Nothing running, nothing to restart; and a nil bounce
+// must not panic.
+func TestMakeRestartRouteExecWithNoActivatorIsANoOp(t *testing.T) {
+ reg := restartTestRegistry(t)
+ if err := reg.Upsert(registry.Route{
+ Host: "exec.test", Kind: "instance", App: "a", Status: "ready",
+ }); err != nil {
+ t.Fatalf("upsert exec route: %v", err)
+ }
+ var restarted []string
+ restart := makeRestartRoute(reg, func(h string) { restarted = append(restarted, h) }, nil)
+ if err := restart("exec.test"); err != nil {
+ t.Fatalf("restart exec.test: %v", err)
+ }
+ if len(restarted) != 0 {
+ t.Fatalf("exec route must not take the systemctl path: %v", restarted)
+ }
+}