amadan
host: carlos restart -app X cycles sidecars; document Bounce for workers
4 files changed,
+252
−17
internal/activator/activator.go+26 −1internal/host/agent.go+13 −1internal/host/restart.go+86 −12internal/host/restart_test.go+127 −3
diff --git a/internal/activator/activator.go b/internal/activator/activator.go| index 8342a704e..4ff0c1f2f 100644 |
| --- a/internal/activator/activator.go |
| +++ b/internal/activator/activator.go |
| @@ -977,6 +977,27 @@ func (a *Service) Owns(host string) bool { |
| // caveat that in-flight requests are not waited for. It never starts |
| // anything: not-awake (waking, stopping, asleep, unknown) is a no-op, |
| // and the instance picks the new config up at its natural next wake. |
| +// |
| +// NOT kind-gated (Task 11 item 4, carried from Task 6's review) — e's |
| +// entry has no branch on e.worker, so this stops an awake WORKER exactly |
| +// as it stops an instance, and stop() itself is worker-aware (it skips |
| +// the socket-only next-due probe, co-stops any attached sidecar first, |
| +// etc.), so nothing here needs a kind check to behave correctly. The |
| +// EXPLICIT decision this makes is what it deliberately does NOT do: start |
| +// anything back up. For an instance that is exactly right — the next |
| +// HTTP request is the wake half of the cycle, and Bounce owns only the |
| +// stop half. A worker has no such request path; nothing dials one. A |
| +// caller that wants a WORKER actually cycled — not just stopped — must |
| +// pair this call with its own WakeWorker once Bounce returns true (see |
| +// host/restart.go's cycleWorkers, the one caller that does): an |
| +// always-on (Hibernate: false) worker left unpaired is stranded until the |
| +// agent restarts (the boot pass that starts one runs once, at arming, |
| +// never again), and a hibernating one stays silent until its schedule |
| +// next comes due — neither of which "carlos restart" asked for. This is |
| +// why SyncExec (appconfig/syncexec.go) never calls Bounce for a sidecar |
| +// route at all: an automatic, unpaired bounce on every ordinary |
| +// config-sync pass would carry exactly that risk with no operator command |
| +// behind it. |
| func (a *Service) Bounce(host string) bool { |
| a.mu.Lock() |
| defer a.mu.Unlock() |
| @@ -984,7 +1005,11 @@ func (a *Service) Bounce(host string) bool { |
| if e == nil || e.state != actAwake { |
| return false |
| } |
| - emitEvent(a.Logs, e.route, "info", "instance restarting (carlos restart)") |
| + msg := "instance restarting (carlos restart)" |
| + if e.worker { |
| + msg = "sidecar restarting (carlos restart)" |
| + } |
| + emitEvent(a.Logs, e.route, "info", msg) |
| a.beginStopLocked(e) |
| return true |
| } |
diff --git a/internal/host/agent.go b/internal/host/agent.go| index 9c196c2f2..7b14e2087 100644 |
| --- a/internal/host/agent.go |
| +++ b/internal/host/agent.go |
| @@ -1340,6 +1340,7 @@ func fastConvergeLoop(reg *registry.Registry, svc *activator.Service) { |
| restartCache := map[[2]string]string{} |
| restartSeed(ctx, reg, store, dd, restartCache) |
| restartBounce := bounceFn(svc) |
| + restartWake := wakeFn(svc) |
| d := &fastDeps{ |
| reg: reg, store: store, pub: pub, optDir: optDir, |
| restart: restart, passOpts: opts, |
| @@ -1372,7 +1373,7 @@ func fastConvergeLoop(reg *registry.Registry, svc *activator.Service) { |
| log.Printf("agent: restart check: list routes: %v", err) |
| return |
| } |
| - restartRequestLeg(ctx, routes, store, dd, restartBounce, restartCache) |
| + restartRequestLeg(ctx, routes, store, dd, restartBounce, restartWake, restartCache) |
| }, |
| } |
| log.Printf("agent: fast converge loop armed (2s + jitter: serial poll + canary/* + unstaged routes)") |
| @@ -2321,6 +2322,17 @@ func bounceFn(svc *activator.Service) func(host string) bool { |
| return svc.Bounce |
| } |
| +// wakeFn adapts an activator to cycleWorkers' wake parameter, nil-safe like |
| +// bounceFn: a box with no activator armed gets a nil callback, which |
| +// cycleWorkers reads as "nothing to wake" rather than panicking on a nil |
| +// activator. |
| +func wakeFn(svc *activator.Service) func(registry.Route) error { |
| + if svc == nil { |
| + return nil |
| + } |
| + return svc.WakeWorker |
| +} |
| + |
| // openDeployStore opens the release store the box adopts from: |
| // CARLOS_DEPLOYMENT_BUCKET (S3), CARLOS_DEPLOYMENT_DIR (a directory — tests |
| // and single-box no-AWS deployments), or — on a fleet box with neither set — |
diff --git a/internal/host/restart.go b/internal/host/restart.go| index 7ebe61726..fd2e7febd 100644 |
| --- a/internal/host/restart.go |
| +++ b/internal/host/restart.go |
| @@ -59,24 +59,49 @@ import ( |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| +// restartEligible widens release.Adoptable BY NAME, for this feature alone: |
| +// a route release.Adoptable would cycle, plus a sidecar. release.Adoptable |
| +// itself is deliberately NOT changed — it is the most reused predicate in |
| +// the repo (adopt membership, capability convergence, socket and backing |
| +// advisories, status pages), and every one of those is correct to exclude |
| +// a sidecar. Widening it centrally would silently enrol sidecars in |
| +// capability drop-ins and adopt bookkeeping that make no sense for them. |
| +// `carlos restart -app X` genuinely wants sidecars too (design §9.3), so it |
| +// opts in here, explicitly, rather than through Adoptable itself. |
| +func restartEligible(rt registry.Route) bool { |
| + return release.Adoptable(rt) || rt.IsSidecar() |
| +} |
| + |
| +// restartCandidates filters routes to the ones the restart feature may |
| +// cycle: restartEligible, applied. Exported to this package's tests as the |
| +// single predicate restartApps, bounceExecRoutes' membership check and |
| +// cycleWorkers all trace back to, so the three cannot silently diverge on |
| +// who counts. |
| +func restartCandidates(routes []registry.Route) []registry.Route { |
| + var out []registry.Route |
| + for _, rt := range routes { |
| + if restartEligible(rt) { |
| + out = append(out, rt) |
| + } |
| + } |
| + return out |
| +} |
| + |
| // restartApps lists the distinct (account, app) pairs with at least one |
| // restartable route on this box — the apps whose restart generation this box |
| // reads when a serial bump (or an hourly pass) gives it a reason to. |
| // |
| -// The membership predicate is release.Adoptable, deliberately the SAME one the |
| -// adopt pass moves and the fast sweep counts: a route a pass would not adopt |
| -// is not one whose process this feature has any business cycling, and a second |
| -// copy of that judgement here is exactly the drift that would let the two |
| -// disagree. It admits addr-backed routes that name their own unit (issue |
| -// #110's consoles) — which is half of why #113 exists — because Adoptable |
| -// filters on lifecycle and app, never on backing. |
| +// The membership predicate is restartCandidates (release.Adoptable plus a |
| +// sidecar — see its doc comment): a route neither would cycle is not one |
| +// whose process this feature has any business reading a restart signal |
| +// for, and a second copy of that judgement here is exactly the drift that |
| +// would let them disagree. It admits addr-backed routes that name their |
| +// own unit (issue #110's consoles) — which is half of why #113 exists — |
| +// because Adoptable filters on lifecycle and app, never on backing. |
| func restartApps(routes []registry.Route) [][2]string { |
| seen := map[[2]string]bool{} |
| var out [][2]string |
| - for _, rt := range routes { |
| - if !release.Adoptable(rt) { |
| - continue |
| - } |
| + for _, rt := range restartCandidates(routes) { |
| k := [2]string{release.RouteAccount(rt), rt.App} |
| if seen[k] { |
| continue |
| @@ -203,6 +228,49 @@ func bounceExecRoutes(routes []registry.Route, moved [][2]string, bounce func(ho |
| return n |
| } |
| +// cycleWorkers is bounceExecRoutes' sidecar counterpart, and deliberately a |
| +// separate function rather than a widened bounceExecRoutes: a sidecar is |
| +// activator-managed exactly like a socket-exec instance, but Bounce alone |
| +// cannot cycle one the way it cycles an instance. Nothing dials a worker |
| +// the way a request dials an instance — an instance's next request supplies |
| +// the wake half of "stop now, respawn with fresh config"; a worker stopped |
| +// here has no other wake source at all (an always-on one is stranded until |
| +// the agent restarts, since the boot pass that starts it runs once, at |
| +// arming, never again; a hibernating one stays silent until its schedule |
| +// next comes due, which "carlos restart" did not ask for). Stop and wake |
| +// are therefore ONE operation for a sidecar — see |
| +// activator.Service.Bounce's own doc comment, which states the same |
| +// contract from the activator's side. |
| +// |
| +// wake is called ONLY for a host bounce actually stopped (true): a sidecar |
| +// that was already asleep is left asleep, the same "never wake a sleeper" |
| +// rule bounceExecRoutes' own HIBERNATE handling already gives an instance. |
| +// A nil bounce or wake (no activator armed) is a working no-op. |
| +func cycleWorkers(routes []registry.Route, moved [][2]string, bounce func(host string) bool, wake func(rt registry.Route) error) int { |
| + if bounce == nil || wake == nil || len(moved) == 0 { |
| + return 0 |
| + } |
| + want := make(map[[2]string]bool, len(moved)) |
| + for _, k := range moved { |
| + want[k] = true |
| + } |
| + n := 0 |
| + for _, rt := range routes { |
| + if !rt.IsSidecar() || !want[[2]string{release.RouteAccount(rt), rt.App}] { |
| + continue |
| + } |
| + if !bounce(rt.Host) { |
| + continue |
| + } |
| + n++ |
| + log.Printf("agent: restart requested: bounced sidecar %s", rt.Host) |
| + if err := wake(rt); err != nil { |
| + log.Printf("agent: restart requested: re-wake sidecar %s: %v", rt.Host, err) |
| + } |
| + } |
| + return n |
| +} |
| + |
| // restartRequestMarker is the file the agent's restart leg touches to ask a |
| // root pass to run now: carlos-restart.path watches it (PathChanged) and starts |
| // carlos-restart.service (`carlos adopt -restart-only`), which owns the half |
| @@ -598,8 +666,13 @@ func restartPass(ctx context.Context, routes []registry.Route, store release.Sto |
| // The bounce goes first, and not only for tidiness: it is in-process and |
| // instant, while the marker write can block on a busy /data, and the children |
| // bounced here are the ones cycled here or nowhere. |
| +// |
| +// wake is svc.WakeWorker in production (wakeFn), nil-safe like bounce: it is |
| +// cycleWorkers' other half, called only for the sidecars this leg actually |
| +// stops (see cycleWorkers' doc comment for why a sidecar needs both calls, |
| +// where an instance needs only bounce). |
| func restartRequestLeg(ctx context.Context, routes []registry.Route, store release.Store, dd string, |
| - bounce func(host string) bool, cache map[[2]string]string) bool { |
| + bounce func(host string) bool, wake func(rt registry.Route) error, cache map[[2]string]string) bool { |
| moved, readErrs := restartGenPass(ctx, routes, store, cache) |
| if len(readErrs) > 0 { |
| // ONE line per pass, never one per app: the count carries the scale, |
| @@ -610,6 +683,7 @@ func restartRequestLeg(ctx context.Context, routes []registry.Route, store relea |
| return false |
| } |
| bounceExecRoutes(routes, moved, bounce) |
| + cycleWorkers(routes, moved, bounce, wake) |
| if err := requestRootRestart(dd); err != nil { |
| log.Printf("agent: request root restart pass: %v", err) |
| return false |
diff --git a/internal/host/restart_test.go b/internal/host/restart_test.go| index bf59e7a98..30582937e 100644 |
| --- a/internal/host/restart_test.go |
| +++ b/internal/host/restart_test.go |
| @@ -264,7 +264,7 @@ func TestRestartRequestLegBouncesAndRelays(t *testing.T) { |
| // Baseline (what restartSeed does at arming): silent, no marker. |
| askRestart(t, ctx, s, "bab", "eleven") |
| - if asked := restartRequestLeg(ctx, routes, s, dd, bounce, cache); asked { |
| + if asked := restartRequestLeg(ctx, routes, s, dd, bounce, nil, cache); asked { |
| t.Fatal("the baselining leg asked for a root pass — first sight must be silent") |
| } |
| if _, err := os.Stat(marker); !errors.Is(err, os.ErrNotExist) { |
| @@ -272,7 +272,7 @@ func TestRestartRequestLegBouncesAndRelays(t *testing.T) { |
| } |
| askRestart(t, ctx, s, "bab", "eleven") |
| - if asked := restartRequestLeg(ctx, routes, s, dd, bounce, cache); !asked { |
| + if asked := restartRequestLeg(ctx, routes, s, dd, bounce, nil, cache); !asked { |
| t.Fatal("a moved restart generation must ask for a root pass") |
| } |
| if !slices.Equal(bounced, []string{"exec.example.com"}) { |
| @@ -284,7 +284,7 @@ func TestRestartRequestLegBouncesAndRelays(t *testing.T) { |
| // And a leg with nothing moved neither bounces nor re-asks. |
| bounced = nil |
| - if asked := restartRequestLeg(ctx, routes, s, dd, bounce, cache); asked { |
| + if asked := restartRequestLeg(ctx, routes, s, dd, bounce, nil, cache); asked { |
| t.Fatal("an unmoved generation asked for a root pass") |
| } |
| if len(bounced) != 0 { |
| @@ -704,3 +704,127 @@ func TestSeededBaselineIsWhatTheRootPassTrusts(t *testing.T) { |
| t.Fatalf("the first root pass cycled %v, want the requested route", rec.hosts) |
| } |
| } |
| + |
| +// ---- Task 11: sidecars in the restart pass ------------------------------- |
| + |
| +// TestRestartPassCyclesSidecars: release.Adoptable requires Kind == |
| +// "instance" and deliberately stays that way (adopt membership, capability |
| +// convergence, socket/backing advisories, status pages all correctly |
| +// exclude a sidecar) — restartCandidates is the widened, NAMED opt-in |
| +// `carlos restart -app X` needs instead. |
| +func TestRestartPassCyclesSidecars(t *testing.T) { |
| + routes := []registry.Route{ |
| + {Host: "bot.x.sidecar.internal", Kind: "sidecar", App: "eleven", Account: "bdm", Status: "ready"}, |
| + } |
| + if len(restartCandidates(routes)) != 1 { |
| + t.Fatal("carlos restart -app X must cycle that app's sidecars too") |
| + } |
| +} |
| + |
| +// TestRestartCandidatesStillExcludesNonAdoptableNonSidecarRoutes: the |
| +// widening is additive — everything release.Adoptable already excluded for |
| +// a non-sidecar route (a Platform alias, a mid-provision route) stays |
| +// excluded. |
| +func TestRestartCandidatesStillExcludesNonAdoptableNonSidecarRoutes(t *testing.T) { |
| + alias := execRoute("alias.oncarlos.com", "bab", "eleven") |
| + alias.Platform = true |
| + pending := execRoute("new.example.com", "bab", "titogo") |
| + pending.Status = "creating" |
| + if got := restartCandidates([]registry.Route{alias, pending}); len(got) != 0 { |
| + t.Fatalf("restartCandidates = %v, want none", got) |
| + } |
| +} |
| + |
| +// TestRestartAppsIncludesASidecarOnlyApp: an app whose only route on this |
| +// box is a sidecar must still be tracked — restartApps (via |
| +// restartCandidates) is what both restartGenPass and restartPass use to |
| +// decide which apps' restart generation to read at all. Without this, a |
| +// sidecar-only app's `carlos restart` request is never even noticed. |
| +func TestRestartAppsIncludesASidecarOnlyApp(t *testing.T) { |
| + routes := []registry.Route{ |
| + {Host: "bot.x.sidecar.internal", Kind: "sidecar", App: "eleven", Account: "bdm", Status: "ready"}, |
| + } |
| + apps := restartApps(routes) |
| + if len(apps) != 1 || apps[0] != [2]string{"bdm", "eleven"} { |
| + t.Fatalf("restartApps = %v, want [[bdm eleven]]", apps) |
| + } |
| +} |
| + |
| +// ---- Task 11: cycling a sidecar's own child ------------------------------ |
| + |
| +// alwaysOnSidecarRoute and hibernatingSidecarRoute are cycleWorkers' |
| +// fixtures — Hibernate governs nothing in cycleWorkers itself (bounce's own |
| +// awake check does the filtering), but the two shapes exist because |
| +// activator.Service.Bounce's stop half behaves identically for both and |
| +// cycleWorkers must too. |
| +func sidecarRoute(host, acct, app string) registry.Route { |
| + return registry.Route{Host: host, Kind: "sidecar", App: app, Account: acct, Status: "ready"} |
| +} |
| + |
| +// TestCycleWorkersBouncesAndWakesARunningSidecar: nothing dials a worker |
| +// the way a request dials an instance, so Bounce's stop half — reused |
| +// verbatim from bounceExecRoutes' own bounce callback — must be paired |
| +// with an explicit wake here, or the sidecar stays down (see |
| +// activator.Service.Bounce's doc comment). Only a HOST BOUNCE ACTUALLY |
| +// STOPPED gets woken. |
| +func TestCycleWorkersBouncesAndWakesARunningSidecar(t *testing.T) { |
| + routes := []registry.Route{sidecarRoute("bot.x.sidecar.internal", "bdm", "eleven")} |
| + moved := [][2]string{{"bdm", "eleven"}} |
| + var bounced, woken []string |
| + bounce := func(h string) bool { bounced = append(bounced, h); return true } |
| + wake := func(rt registry.Route) error { woken = append(woken, rt.Host); return nil } |
| + n := cycleWorkers(routes, moved, bounce, wake) |
| + if n != 1 || !slices.Equal(bounced, []string{"bot.x.sidecar.internal"}) { |
| + t.Fatalf("bounced %v (n=%d), want exactly the sidecar", bounced, n) |
| + } |
| + if !slices.Equal(woken, []string{"bot.x.sidecar.internal"}) { |
| + t.Fatalf("woken %v, want the bounced sidecar re-woken", woken) |
| + } |
| +} |
| + |
| +// TestCycleWorkersLeavesASleepingSidecarAsleep: Bounce is a no-op on |
| +// anything not awake, and cycleWorkers must never wake a sleeper just |
| +// because a restart was requested — the same "restarting a process that is |
| +// not running is meaningless" rule restartPass applies to a hibernating |
| +// instance. |
| +func TestCycleWorkersLeavesASleepingSidecarAsleep(t *testing.T) { |
| + routes := []registry.Route{sidecarRoute("bot.x.sidecar.internal", "bdm", "eleven")} |
| + moved := [][2]string{{"bdm", "eleven"}} |
| + var woken []string |
| + bounce := func(h string) bool { return false } // asleep: Bounce's own contract |
| + wake := func(rt registry.Route) error { woken = append(woken, rt.Host); return nil } |
| + if n := cycleWorkers(routes, moved, bounce, wake); n != 0 { |
| + t.Fatalf("cycleWorkers = %d, want 0 — nothing was running", n) |
| + } |
| + if len(woken) != 0 { |
| + t.Fatalf("woken %v, want none — a sleeping sidecar must stay asleep", woken) |
| + } |
| +} |
| + |
| +// TestCycleWorkersOnlyTouchesMovedAppsAndSidecarRoutes: the same |
| +// app-scoping bounceExecRoutes gives an instance, and never an ordinary |
| +// instance route even for a moved app (that is bounceExecRoutes' job, not |
| +// this one's). |
| +func TestCycleWorkersOnlyTouchesMovedAppsAndSidecarRoutes(t *testing.T) { |
| + routes := []registry.Route{ |
| + sidecarRoute("bot.eleven.sidecar.internal", "bdm", "eleven"), |
| + sidecarRoute("bot.titogo.sidecar.internal", "bdm", "titogo"), // different app |
| + execRoute("inst.example.com", "bdm", "eleven"), // not a sidecar |
| + } |
| + moved := [][2]string{{"bdm", "eleven"}} |
| + var bounced []string |
| + bounce := func(h string) bool { bounced = append(bounced, h); return true } |
| + wake := func(rt registry.Route) error { return nil } |
| + if n := cycleWorkers(routes, moved, bounce, wake); n != 1 || !slices.Equal(bounced, []string{"bot.eleven.sidecar.internal"}) { |
| + t.Fatalf("bounced %v (n=%d), want only the moved app's sidecar", bounced, n) |
| + } |
| +} |
| + |
| +// TestCycleWorkersWithoutAnActivatorOrWakeIsANoop: a box with no activator |
| +// armed gets nil for both callbacks — a working no-op, not a panic. |
| +func TestCycleWorkersWithoutAnActivatorOrWakeIsANoop(t *testing.T) { |
| + routes := []registry.Route{sidecarRoute("bot.x.sidecar.internal", "bdm", "eleven")} |
| + if n := cycleWorkers(routes, [][2]string{{"bdm", "eleven"}}, nil, nil); n != 0 { |
| + t.Fatalf("cycleWorkers = %d with no activator armed, want 0 and no panic", n) |
| + } |
| +} |