amadan

carlos / platform Public

appconfig: ExecConfigApps polls sidecars on the fast cadence too

Paul Campbell pushed by paul@keymail.dev c47079f7671143a46c9509aefc50b44650b1d035 parent 5945601
2 files changed, +48 −1
  • internal/appconfig/syncexec.go +13 −1
  • internal/appconfig/syncexec_test.go +35 −0
diff --git a/internal/appconfig/syncexec.go b/internal/appconfig/syncexec.go
index 3cda209a6..2c9a12da8 100644
--- a/internal/appconfig/syncexec.go
+++ b/internal/appconfig/syncexec.go
@@ -561,6 +561,18 @@ func execSidecarCandidate(rt registry.Route) bool {
// small read per tick, and the SyncExec pass a change triggers applies the
// opt-in properly before writing anything.
//
+// execSidecarCandidate is included alongside execSyncable and
+// execAddrCandidate (Task 11 fix round 1): left out, a sidecar-only app's
+// config — delivered by a direct-bucket write that never bumps
+// control/serial, exactly what the pinfra scripts do — would fall back to
+// the hourly floor while an instance next to it converges on this loop's
+// fast cadence. Not an error, just work happening late; the same
+// half-migrated-consumer shape this branch has repeatedly found. Costs
+// nothing extra to include unconditionally: a sidecar's process is spawned
+// and owned by carlos exactly like a socket-exec instance's (see
+// execSidecarCandidate's own doc comment), so there is no opt-in question
+// to defer here the way there is for the addr shape.
+//
// Exported from THIS file, beside the predicates it composes, so the loop's
// notion of eligibility can never drift from SyncExec's own — an app the
// loop watched but SyncExec skipped would burn reads forever, and an app it
@@ -569,7 +581,7 @@ func ExecConfigApps(routes []registry.Route) [][2]string {
seen := map[[2]string]bool{}
var out [][2]string
for _, rt := range routes {
- if !execSyncable(rt) && !execAddrCandidate(rt) {
+ if !execSyncable(rt) && !execAddrCandidate(rt) && !execSidecarCandidate(rt) {
continue
}
k := [2]string{release.RouteAccount(rt), rt.App}
diff --git a/internal/appconfig/syncexec_test.go b/internal/appconfig/syncexec_test.go
index 75805e66a..7300feb87 100644
--- a/internal/appconfig/syncexec_test.go
+++ b/internal/appconfig/syncexec_test.go
@@ -1944,3 +1944,38 @@ func TestSyncExecSidecarRouteIsNeverBounced(t *testing.T) {
t.Fatalf("config env = %q, want BOT_TOKEN=two — the changed content, no header — the write path must still run", got)
}
}
+
+// TestExecConfigAppsIncludesSidecars (fix round 1, coordinator ruling): a
+// sidecar route must be in scope for the fast generation-poll loop exactly
+// as an instance is. Left unwidened, a sidecar-only app's config delivered
+// by a direct-bucket write — never bumping control/serial, exactly what the
+// pinfra scripts do — would fall back to the hourly floor while an instance
+// next to it converges within the fast loop's cadence: a predicate widened
+// in SyncExec's inScope and not in this sibling, where the symptom is not
+// an error but work happening late.
+func TestExecConfigAppsIncludesSidecars(t *testing.T) {
+ routes := []registry.Route{
+ // In, and the whole point of this test: a SIDECAR-ONLY app — no
+ // socket, no addr route at all — must still be polled. An earlier
+ // version of this test paired the sidecar with an ordinary instance
+ // of the same app, which made the assertion pass whether or not
+ // ExecConfigApps actually looked at the sidecar route at all.
+ {Host: "bot.eleven.bdm.sidecar.internal", Kind: "sidecar", App: "eleven", Account: "bdm", Status: "ready"},
+ // In BOTH via one dedup: a second app with a sidecar AND a
+ // socket-exec instance must still report as ONE (account, app) pair.
+ {Host: "bot.mixed.bdm.sidecar.internal", Kind: "sidecar", App: "mixed", Account: "bdm", Status: "ready"},
+ {Host: "inst.mixed.example.com", Kind: "instance", App: "mixed", Account: "bdm",
+ Status: "ready", Socket: "/run/carlos/inst.sock"},
+ // Out, same exclusions execRouteBase already gives an instance:
+ // Platform and a non-ready Status.
+ {Host: "plat.bot.sidecar.internal", Kind: "sidecar", App: "platapp", Account: "bdm",
+ Status: "ready", Platform: true},
+ {Host: "creating.bot.sidecar.internal", Kind: "sidecar", App: "creatingapp", Account: "bdm",
+ Status: "creating"},
+ }
+ got := ExecConfigApps(routes)
+ want := [][2]string{{"bdm", "eleven"}, {"bdm", "mixed"}}
+ if !slices.Equal(got, want) {
+ t.Fatalf("ExecConfigApps = %v, want %v — a sidecar-only app must be polled by the fast loop too, and a mixed-shape app must dedupe to one entry", got, want)
+ }
+}