carlos / platform Public

appconfig: SyncExec delivers .config.env to sidecars

Paul Campbell pushed by paul@keymail.dev b2d1b5fd6fcdaec6b2889ec6e5a32bd3a9710a81 parent 15321d1
2 files changed, +179 −17
  • internal/appconfig/syncexec.go +64 −17
  • internal/appconfig/syncexec_test.go +115 −0
diff --git a/internal/appconfig/syncexec.go b/internal/appconfig/syncexec.go
index 2c7f9a381..3cda209a6 100644
--- a/internal/appconfig/syncexec.go
+++ b/internal/appconfig/syncexec.go
@@ -242,22 +242,36 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
return c
}
// inScope is the single door both loops below use to decide whether a
- // route is in scope this pass, and — for the addr shape — that it must
- // never be bounced. One function so the two loops structurally cannot
- // diverge on either question (see the "if they ever diverge" comment
- // on the second loop's map lookup, which depends on the same key
- // having been computed the same way here).
- inScope := func(rt registry.Route) (ok, addrShape bool) {
+ // route is in scope this pass, and — for the addr and sidecar shapes —
+ // that it must never be bounced. One function so the two loops
+ // structurally cannot diverge on either question (see the "if they
+ // ever diverge" comment on the second loop's map lookup, which depends
+ // on the same key having been computed the same way here).
+ //
+ // sidecarShape never bounces for a DIFFERENT reason than addrShape
+ // does: carlos owns a sidecar's process (unlike the addr shape, whose
+ // app-supervised process carlos never touches), but Bounce alone has
+ // no way back for a worker — nothing dials one the way a request
+ // dials an instance, so an automatic bounce here would either strand
+ // an always-on sidecar until the agent restarts or leave a
+ // hibernating one silent until its schedule next comes due, on every
+ // ordinary config-sync pass. Restarting a sidecar on purpose is
+ // `carlos restart -app X` (host/restart.go), which pairs the stop
+ // with an explicit wake; this pass only ever writes the file.
+ inScope := func(rt registry.Route) (ok, addrShape, sidecarShape bool) {
if execSyncable(rt) {
- return true, false
+ return true, false, false
+ }
+ if execSidecarCandidate(rt) {
+ return true, false, true
}
if execAddrCandidate(rt) && checkOptedIn(release.RouteAccount(rt), rt.App, rt.Environment).v {
- return true, true
+ return true, true, false
}
- return false, false
+ return false, false, false
}
for _, rt := range routes {
- if in, _ := inScope(rt); !in {
+ if in, _, _ := inScope(rt); !in {
continue
}
k := groupKey{release.RouteAccount(rt), rt.App, rt.Environment}
@@ -341,7 +355,7 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
}
for _, rt := range routes {
- in, addrShape := inScope(rt)
+ in, addrShape, sidecarShape := inScope(rt)
if !in {
continue
}
@@ -398,11 +412,20 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
// routes get execAddrConfigHeader, socket routes get "" (no
// header at all) — see execAddrConfigHeader's doc comment for why
// this must never apply to the socket shape.
+ //
+ // sidecarShape ALSO gets nil in bounce's place, but for the
+ // opposite reason addrShape does: carlos owns this process, but
+ // Bounce cannot safely stand in for a wake here — see inScope's
+ // doc comment. No header either; a sidecar's file is read by
+ // ExecBackend.instanceEnv, the same reader an instance's is.
hostBounce := bounce
header := ""
- if addrShape {
+ switch {
+ case addrShape:
hostBounce = nil
header = execAddrConfigHeader
+ case sidecarShape:
+ hostBounce = nil
}
syncExecHost(rt, r.m, Key(k.acct, k.app), priv, dataDir, hostBounce, header)
}
@@ -461,13 +484,22 @@ func SyncExec(ctx context.Context, reg *registry.Registry, s release.Store, priv
return nil
}
-// execRouteBase is the lifecycle-managed-instance shape both exec route
-// kinds share: the unit path's syncable() exclusions, narrowed to
+// execRouteBase is the lifecycle-managed-instance shape every exec route
+// kind shares: the unit path's syncable() exclusions, narrowed to
// non-unit backing (unit routes belong to the root adopt path). Neither
-// SyncExec eligibility case on its own — see execSyncable and
-// execAddrCandidate, which each narrow it by upstream shape.
+// SyncExec eligibility case on its own — see execSyncable, execAddrCandidate
+// and execSidecarCandidate, which each narrow it by upstream shape.
+//
+// Kind "sidecar" is included alongside "instance" (Task 11, design §9.1):
+// this is the direct replacement for the hand-staged
+// <host>-sidecar.env — `carlos env set` must reach a sidecar, secrets are
+// sealed and delivered the same way an instance's are, and the file this
+// predicate gates is exactly what ExecBackend.instanceEnv reads at
+// StartWorker's spawn. Every other clause is untouched: a sidecar row is
+// still excluded by Platform, a non-ready Status, or Backing ==
+// BackingUnit, same as an instance would be.
func execRouteBase(rt registry.Route) bool {
- return rt.Kind == "instance" && rt.App != "" && !rt.Platform &&
+ return (rt.Kind == "instance" || rt.IsSidecar()) && rt.App != "" && !rt.Platform &&
(rt.Status == "" || rt.Status == "ready") &&
rt.Backing != registry.BackingUnit
}
@@ -501,6 +533,21 @@ func execAddrCandidate(rt registry.Route) bool {
return execRouteBase(rt) && rt.Socket == "" && rt.Addr != ""
}
+// execSidecarCandidate: the standalone-worker exec shape (Task 11, design
+// §9.1) — a sidecar row carries neither Socket nor Addr by construction
+// (sidecarRouteFor's contract: a sidecar binds nothing and nothing dials
+// it), so it can never satisfy execSyncable or execAddrCandidate; this is
+// its own narrowing of execRouteBase, keyed on Kind alone.
+//
+// Unconditionally in scope, like execSyncable and UNLIKE execAddrCandidate:
+// a sidecar's process is spawned and owned by THIS box's activator
+// (ExecBackend.StartWorker), the same as a socket-exec instance, so there
+// is no "app-supervised, may not read the file" case here to opt into —
+// carlos always owns delivery for a route it owns the process of.
+func execSidecarCandidate(rt registry.Route) bool {
+ return execRouteBase(rt) && rt.IsSidecar()
+}
+
// ExecConfigApps is the distinct (account, app) set the agent's fast
// generation loop (host package) polls: every app with at least one route
// SyncExec could deliver config to. Accounts are resolved through
diff --git a/internal/appconfig/syncexec_test.go b/internal/appconfig/syncexec_test.go
index 9daa680cf..75805e66a 100644
--- a/internal/appconfig/syncexec_test.go
+++ b/internal/appconfig/syncexec_test.go
@@ -1829,3 +1829,118 @@ func TestSyncExecEnvBundleFetchErrorLeavesFileAlone(t *testing.T) {
t.Fatalf("a fetch blip against the bound bundle must say so loudly and name the environment; got %q", out)
}
}
+
+// ---- Task 11: sidecar config delivery -----------------------------------
+
+// TestSidecarGetsItsConfigEnvFile: execRouteBase is the shape predicate both
+// exec eligibility cases narrow. A sidecar row (no Socket, no Addr) must
+// pass it — this is the direct replacement for the hand-staged
+// <host>-sidecar.env, and the whole reason the file path existed.
+func TestSidecarGetsItsConfigEnvFile(t *testing.T) {
+ rt := registry.Route{Host: "bot.eleven.bdm.sidecar.internal", Kind: "sidecar",
+ App: "eleven", Status: "ready"}
+ if !execRouteBase(rt) {
+ t.Fatal("a sidecar must receive config delivery — this is the direct replacement for the hand-staged <host>-sidecar.env, and the whole reason the file path existed")
+ }
+}
+
+// TestSidecarConfigStillExcludesPlatformAndUnitRoutes: widening the Kind
+// test must not loosen any of execRouteBase's other clauses.
+func TestSidecarConfigStillExcludesPlatformAndUnitRoutes(t *testing.T) {
+ base := registry.Route{Host: "bot.x.sidecar.internal", Kind: "sidecar", App: "eleven", Status: "ready"}
+ plat := base
+ plat.Platform = true
+ if execRouteBase(plat) {
+ t.Fatal("a Platform alias is still excluded")
+ }
+ unit := base
+ unit.Backing = registry.BackingUnit
+ if execRouteBase(unit) {
+ t.Fatal("unit-backed routes are still excluded")
+ }
+}
+
+// TestSyncExecWritesConfigFileForASidecarRoute is the end-to-end proof that
+// execRouteBase's widening actually reaches the file: a sidecar route
+// carries neither Socket nor Addr (sidecarRouteFor's contract — nothing
+// dials a sidecar), so execRouteBase alone is not sufficient to bring it
+// into SyncExec's resolution loop; inScope must recognize the shape too.
+// StartWorker (backend_exec.go's instanceEnv) reads exactly this file at
+// spawn, so a sidecar that never gets it here never gets its env at all.
+func TestSyncExecWritesConfigFileForASidecarRoute(t *testing.T) {
+ ctx, s, reg, dd, priv, _ := execKit(t)
+ pk, err := ParsePrivateKey(priv, "test")
+ if err != nil {
+ t.Fatalf("ParsePrivateKey: %v", err)
+ }
+ if err := SetEnv(ctx, s, "bdm", "eleven", map[string]string{"BOT_TOKEN": "shh"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ if err := reg.Upsert(registry.Route{
+ Host: "bot.eleven.bdm.sidecar.internal", Kind: "sidecar", App: "eleven", Account: "bdm",
+ Status: "ready",
+ }); err != nil {
+ t.Fatalf("Upsert: %v", err)
+ }
+ if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil {
+ t.Fatalf("SyncExec: %v", err)
+ }
+ got, err := os.ReadFile(filepath.Join(dd, "bot.eleven.bdm.sidecar.internal.config.env"))
+ if err != nil {
+ t.Fatalf("sidecar never got a config.env file: %v", err)
+ }
+ if string(got) != "BOT_TOKEN=shh\n" {
+ t.Fatalf("config env = %q, want BOT_TOKEN=shh", got)
+ }
+}
+
+// TestSyncExecSidecarRouteIsNeverBounced pins the safety decision behind
+// sidecarShape in inScope: Bounce alone cannot stand in for a sidecar's
+// wake (nothing dials a worker the way a request dials an instance — see
+// inScope's doc comment), so an automatic bounce on every ordinary
+// config-sync pass would risk stranding an always-on sidecar or leaving a
+// hibernating one silent until its schedule next comes due. The bounce
+// callback must never be invoked for a sidecar route, on either the
+// initial write or a changed-content pass — restarting one on purpose is
+// `carlos restart -app X` (host/restart.go), which pairs the stop with an
+// explicit wake.
+func TestSyncExecSidecarRouteIsNeverBounced(t *testing.T) {
+ ctx, s, reg, dd, priv, _ := execKit(t)
+ pk, err := ParsePrivateKey(priv, "test")
+ if err != nil {
+ t.Fatalf("ParsePrivateKey: %v", err)
+ }
+ if err := SetEnv(ctx, s, "bdm", "eleven", map[string]string{"BOT_TOKEN": "one"}); err != nil {
+ t.Fatalf("SetEnv: %v", err)
+ }
+ if err := reg.Upsert(registry.Route{
+ Host: "bot.eleven.bdm.sidecar.internal", Kind: "sidecar", App: "eleven", Account: "bdm",
+ Status: "ready",
+ }); err != nil {
+ t.Fatalf("Upsert: %v", err)
+ }
+ var bounced []string
+ bounce := func(h string) error { bounced = append(bounced, h); return nil }
+ if err := SyncExec(ctx, reg, s, pk, dd, bounce); err != nil {
+ t.Fatalf("SyncExec (initial write): %v", err)
+ }
+ if len(bounced) != 0 {
+ t.Fatalf("bounced = %v, want none — Bounce alone cannot wake a worker back up", bounced)
+ }
+ if err := SetEnv(ctx, s, "bdm", "eleven", map[string]string{"BOT_TOKEN": "two"}); err != nil {
+ t.Fatalf("SetEnv (change): %v", err)
+ }
+ if err := SyncExec(ctx, reg, s, pk, dd, bounce); err != nil {
+ t.Fatalf("SyncExec (changed content): %v", err)
+ }
+ if len(bounced) != 0 {
+ t.Fatalf("bounced = %v after a content change, want none", bounced)
+ }
+ got, err := os.ReadFile(filepath.Join(dd, "bot.eleven.bdm.sidecar.internal.config.env"))
+ if err != nil {
+ t.Fatalf("read config env: %v", err)
+ }
+ if string(got) != "BOT_TOKEN=two\n" {
+ t.Fatalf("config env = %q, want BOT_TOKEN=two — the changed content, no header — the write path must still run", got)
+ }
+}