carlos / platform Public

host: the fast config loop notices a route's environment rebinding (#196)

genPollPass compares appconfig.ConfigRouteFingerprints against the previous
tick alongside the generation etags, so a binding converges in ~3s instead
of waiting out configSyncLoop's hourly floor. Costs no bucket read.

Covers both doors onto the column: `carlos route -environment` on the box,
and reconcile converging Environment from an instance record — the
sanctioned door for a Provisioned row, which the fix suggested in the issue
would have missed.

A route entering or leaving scope now also earns a pass; the two re-add
tests move their silence assertion to the settled tick after it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gyhxeoXGBW4WCFRnMqBue

Paul Campbell 2 months ago 3054e2baafcb9eb3ab50f596ac2ff133a41c4ab2 parent 28f0434

2 files changed, +203 −34

internal/host/agent.go +53 −8
internal/host/agent_test.go +150 −26

diff --git a/internal/host/agent.go b/internal/host/agent.go

index 09c483b4f..b7498b778 100644

--- a/internal/host/agent.go

+++ b/internal/host/agent.go

@@ -578,10 +578,31 @@ func bounce(svc *activator.Service) func(string) error {

// DirStore's is hex).

const genAbsent = "\x00absent"

+// genCache is everything configGenLoop remembers between ticks: one

+// generation etag per config-carrying app, and the previous tick's

+// route-shape digests. One object rather than two parameters because the

+// two are remembered for exactly one reason — deciding what "changed"

+// means — and a caller holding one without the other would silently lose

+// half the loop's sensitivity.

+type genCache struct {

+ etags map[[2]string]string

+ // exec/unit are appconfig.ConfigRouteFingerprints' two digests as of

+ // the last pass that computed them; shapeSeen is false until the first

+ // such pass. The bool is not redundant with an empty string: a box with

+ // no config-carrying routes at all has a real, stable digest of the

+ // empty set, and treating that as "never seen" would make its first

+ // route look like a change on the tick after boot rather than on the

+ // tick it appeared.

+ exec, unit string

+ shapeSeen bool

+func newGenCache() *genCache { return &genCache{etags: map[[2]string]string{}} }

// genPollPass is one tick of configGenLoop, split out for tests: read the

// generation etag of every app the box can converge — exec-config apps and

-// unit-backed apps alike — and report which HALVES moved since the cache

-// was built.

+// unit-backed apps alike — plus the registry's own route shape, and report

+// which HALVES moved since the cache was built.

// Semantics, each of which is a deliberate choice:

@@ -613,6 +634,20 @@ const genAbsent = "\x00absent"

// re-added app is a first sight again rather than a comparison against

// an etag remembered from before it left (a route being re-added gets

// fresh config from the spawn path anyway).

+// - The ROUTE SHAPE is watched too, not just the bucket. A route's

+// Environment binding lives in the box's own registry and moves no etag

+// anywhere, so without this the loop cannot see a rebinding at all and

+// delivery waits out configSyncLoop's hourly floor: issue #196, where a

+// live route served the wrong merge for ten minutes and the only symptom

+// was the app reporting a credential missing — which reads as a BAD

+// credential rather than an undelivered one. First sight of the shape is

+// silent for the same reason first sight of an app is. This covers BOTH

+// doors onto the column: `carlos route -environment` on the box, and the

+// provisioning reconciler converging Environment from an instance record

+// — which is the sanctioned door for a Provisioned row, and which the

+// issue's own suggested fix (bump the generation from `carlos route`)

+// would have missed entirely. It costs no bucket read: see

+// appconfig.ConfigRouteFingerprints.

// - A read error that isn't "not found" leaves that app's cache entry

// exactly as it was and keeps evaluating the rest: the errors come back

// for the caller to aggregate into ONE log line per pass, never one per

@@ -622,7 +657,17 @@ const genAbsent = "\x00absent"

// The cache is mutated in place — it lives across ticks in the loop, which

// is the whole point (there is nowhere else to remember what "changed"

// means).

-func genPollPass(ctx context.Context, routes []registry.Route, store release.Store, cache map[[2]string]string) (execChanged, unitChanged bool, readErrs []error) {

+func genPollPass(ctx context.Context, routes []registry.Route, store release.Store, cache *genCache) (execChanged, unitChanged bool, readErrs []error) {

+ // The registry-side half (issue #196). Free — no bucket read at all —

+ // and evaluated first so a rebinding converges on the same tick a var

+ // change would, rather than one behind it.

+ execFP, unitFP := appconfig.ConfigRouteFingerprints(routes)

+ if cache.shapeSeen {

+ execChanged = execChanged || execFP != cache.exec

+ unitChanged = unitChanged || unitFP != cache.unit

+ }

+ cache.exec, cache.unit, cache.shapeSeen = execFP, unitFP, true

execApps := appconfig.ExecConfigApps(routes)

unitApps := appconfig.UnitConfigApps(routes)

// The poll set is the UNION, read once per app: the two sets overlap

@@ -654,16 +699,16 @@ func genPollPass(ctx context.Context, routes []registry.Route, store release.Sto

readErrs = append(readErrs, fmt.Errorf("%s/%s: %w", k[0], k[1], err))

continue // cache entry untouched: this app is unknown this tick, not changed

}

- prev, seen := cache[k]

- cache[k] = etag

+ prev, seen := cache.etags[k]

+ cache.etags[k] = etag

if seen && prev != etag {

execChanged = execChanged || inExec[k]

unitChanged = unitChanged || inUnit[k]

}

- for k := range cache {

+ for k := range cache.etags {

if !live[k] {

- delete(cache, k)

+ delete(cache.etags, k)

}

return execChanged, unitChanged, readErrs

@@ -808,7 +853,7 @@ func configGenLoop(reg *registry.Registry, svc *activator.Service, dd string) {

}

// One cache for the life of the process: what "changed" means is

// exactly "differs from the last tick that could read it".

- cache := map[[2]string]string{}

+ cache := newGenCache()

log.Printf("agent: config generation loop armed (3s + jitter, exec-config and unit-backed apps)")

for {

time.Sleep(3*time.Second + rand.N(time.Second))

diff --git a/internal/host/agent_test.go b/internal/host/agent_test.go

index 7ce08214e..07758c854 100644

--- a/internal/host/agent_test.go

+++ b/internal/host/agent_test.go

@@ -113,7 +113,7 @@ func TestGenPollPassFirstSightIsSilent(t *testing.T) {

ctx, s := genKit(t)

touch(t, ctx, s, "bab", "eleven")

routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")}

- cache := map[[2]string]string{}

+ cache := newGenCache()

execChanged, unitChanged, errs := genPollPass(ctx, routes, s, cache)

if execChanged || unitChanged {

@@ -123,7 +123,7 @@ func TestGenPollPassFirstSightIsSilent(t *testing.T) {

t.Fatalf("unexpected read errors: %v", errs)

}

want := genEtag(t, ctx, s, "bab", "eleven")

- if got := cache[[2]string{"bab", "eleven"}]; got != want {

+ if got := cache.etags[[2]string{"bab", "eleven"}]; got != want {

t.Fatalf("cache etag = %q, want the store's %q", got, want)

}

@@ -132,7 +132,7 @@ func TestGenPollPassUnchangedIsSilent(t *testing.T) {

ctx, s := genKit(t)

touch(t, ctx, s, "bab", "eleven")

routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")}

- cache := map[[2]string]string{}

+ cache := newGenCache()

genPollPass(ctx, routes, s, cache)

execChanged, unitChanged, errs := genPollPass(ctx, routes, s, cache)

@@ -148,7 +148,7 @@ func TestGenPollPassTouchedGenerationTriggersSync(t *testing.T) {

ctx, s := genKit(t)

touch(t, ctx, s, "bab", "eleven")

routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")}

- cache := map[[2]string]string{}

+ cache := newGenCache()

genPollPass(ctx, routes, s, cache)

touch(t, ctx, s, "bab", "eleven")

@@ -163,7 +163,7 @@ func TestGenPollPassTouchedGenerationTriggersSync(t *testing.T) {

t.Fatalf("unexpected read errors: %v", errs)

}

want := genEtag(t, ctx, s, "bab", "eleven")

- if got := cache[[2]string{"bab", "eleven"}]; got != want {

+ if got := cache.etags[[2]string{"bab", "eleven"}]; got != want {

t.Fatalf("cache etag = %q, want the new %q (a stale cache would re-sync forever)", got, want)

}

if execChanged, unitChanged, _ := genPollPass(ctx, routes, s, cache); execChanged || unitChanged {

@@ -176,7 +176,7 @@ func TestGenPollPassAbsentGenerationStaysSilent(t *testing.T) {

// No touch at all: an app last written by a pre-generation CLI. Legal

// forever — the hourly configSyncLoop is its floor.

routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")}

- cache := map[[2]string]string{}

+ cache := newGenCache()

for i := range 2 {

execChanged, unitChanged, errs := genPollPass(ctx, routes, s, cache)

@@ -187,7 +187,7 @@ func TestGenPollPassAbsentGenerationStaysSilent(t *testing.T) {

t.Fatalf("pass %d: absent is not a read error: %v", i, errs)

}

- if got := cache[[2]string{"bab", "eleven"}]; got != genAbsent {

+ if got := cache.etags[[2]string{"bab", "eleven"}]; got != genAbsent {

t.Fatalf("cache = %q, want the known-absent sentinel %q", got, genAbsent)

}

@@ -195,7 +195,7 @@ func TestGenPollPassAbsentGenerationStaysSilent(t *testing.T) {

func TestGenPollPassAbsentToPresentTriggersSync(t *testing.T) {

ctx, s := genKit(t)

routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")}

- cache := map[[2]string]string{}

+ cache := newGenCache()

genPollPass(ctx, routes, s, cache) // records known-absent

touch(t, ctx, s, "bab", "eleven") // the app's first ever config write

@@ -216,7 +216,7 @@ func TestGenPollPassOneChangedAppSyncsOnce(t *testing.T) {

execRoute("a.example.com", "bab", "eleven"),

execRoute("b.example.com", "bab", "titogo"),

}

- cache := map[[2]string]string{}

+ cache := newGenCache()

genPollPass(ctx, routes, s, cache)

touch(t, ctx, s, "bab", "titogo")

@@ -230,7 +230,7 @@ func TestGenPollPassOneChangedAppSyncsOnce(t *testing.T) {

// One flag for the whole pass: the caller runs ONE SyncExec, which

// resolves every app itself. The unchanged app's cache entry must be

// carried forward untouched so it doesn't re-fire next tick.

- if got, want := cache[[2]string{"bab", "eleven"}], genEtag(t, ctx, s, "bab", "eleven"); got != want {

+ if got, want := cache.etags[[2]string{"bab", "eleven"}], genEtag(t, ctx, s, "bab", "eleven"); got != want {

t.Fatalf("unchanged app's cache = %q, want %q", got, want)

}

if execChanged, _, _ := genPollPass(ctx, routes, s, cache); execChanged {

@@ -247,21 +247,37 @@ func TestGenPollPassDropsVanishedApps(t *testing.T) {

execRoute("b.example.com", "bab", "titogo"),

}

one := both[:1]

- cache := map[[2]string]string{}

+ cache := newGenCache()

genPollPass(ctx, both, s, cache)

genPollPass(ctx, one, s, cache) // titogo's route is gone

- if _, ok := cache[[2]string{"bab", "titogo"}]; ok {

+ if _, ok := cache.etags[[2]string{"bab", "titogo"}]; ok {

t.Fatal("a route that left the registry must leave no cache entry behind")

}

// The proof that dropping matters: while the app was unrouted its

- // config moved. Re-adding the route must record silently (first sight

- // again), not fire a sync off a remembered etag — a route being

- // re-added already gets fresh config from the spawn path.

+ // config moved. The ETAG half must record that silently (first sight

+ // again) rather than comparing against a remembered value — a route

+ // being re-added already gets fresh config from the spawn path.

+ //

+ // The tick itself is NOT silent, and that is the route-shape half

+ // speaking, not the etag one (issue #196): a route entering or leaving

+ // scope changes what belongs on disk — a file to write, or one to reap

+ // — so it earns a pass. The pass is cheap and cannot cost a restart:

+ // SyncExec bounces only a route whose rendered bytes actually changed.

+ // What this test still pins directly is the cache entry above; the

+ // etag half's silence is no longer separately observable from here,

+ // since the shape movement covers it either way.

touch(t, ctx, s, "bab", "titogo")

+ _, _, errs := genPollPass(ctx, both, s, cache)

+ if len(errs) != 0 {

+ t.Fatalf("unexpected read errors: %v", errs)

+ }

+ // Settled again: with the route set stable and no bucket movement, the

+ // next tick must be silent. A remembered etag or a mis-stored digest

+ // would still be firing here.

execChanged, unitChanged, errs := genPollPass(ctx, both, s, cache)

if execChanged || unitChanged {

- t.Fatal("a re-added app is a first sight, not a change")

+ t.Fatal("a settled tick after a re-add must be silent")

}

if len(errs) != 0 {

t.Fatalf("unexpected read errors: %v", errs)

@@ -276,9 +292,9 @@ func TestGenPollPassReadErrorLeavesCacheAndKeepsGoing(t *testing.T) {

execRoute("a.example.com", "bab", "eleven"),

execRoute("b.example.com", "bab", "titogo"),

}

- cache := map[[2]string]string{}

+ cache := newGenCache()

genPollPass(ctx, routes, dir, cache)

- elevenEtag := cache[[2]string{"bab", "eleven"}]

+ elevenEtag := cache.etags[[2]string{"bab", "eleven"}]

// eleven's generation object goes unreadable; titogo's moves.

boom := errors.New("bucket said no")

@@ -295,7 +311,7 @@ func TestGenPollPassReadErrorLeavesCacheAndKeepsGoing(t *testing.T) {

if !errors.Is(errs[0], boom) {

t.Fatalf("read error = %v, want it to wrap %v", errs[0], boom)

}

- if got := cache[[2]string{"bab", "eleven"}]; got != elevenEtag {

+ if got := cache.etags[[2]string{"bab", "eleven"}]; got != elevenEtag {

t.Fatalf("unreadable app's cache = %q, want it untouched at %q (a cleared entry would re-fire on recovery)", got, elevenEtag)

}

@@ -304,7 +320,7 @@ func TestGenPollPassUnitFirstSightIsSilent(t *testing.T) {

ctx, s := genKit(t)

touch(t, ctx, s, "bab", "unitapp")

routes := []registry.Route{unitRoute("u.example.com", "bab", "unitapp")}

- cache := map[[2]string]string{}

+ cache := newGenCache()

execChanged, unitChanged, errs := genPollPass(ctx, routes, s, cache)

if execChanged || unitChanged {

@@ -314,7 +330,7 @@ func TestGenPollPassUnitFirstSightIsSilent(t *testing.T) {

t.Fatalf("unexpected read errors: %v", errs)

}

want := genEtag(t, ctx, s, "bab", "unitapp")

- if got := cache[[2]string{"bab", "unitapp"}]; got != want {

+ if got := cache.etags[[2]string{"bab", "unitapp"}]; got != want {

t.Fatalf("cache etag = %q, want the store's %q", got, want)

}

@@ -327,7 +343,7 @@ func TestGenPollPassUnitMovementRequestsRootPassOnly(t *testing.T) {

unitRoute("u.example.com", "bab", "unitapp"),

execRoute("a.example.com", "bab", "eleven"),

}

- cache := map[[2]string]string{}

+ cache := newGenCache()

genPollPass(ctx, routes, s, cache)

touch(t, ctx, s, "bab", "unitapp")

@@ -357,7 +373,7 @@ func TestGenPollPassBothShapesSetBothHalvesOnOneRead(t *testing.T) {

unitRoute("mixed-unit.example.com", "zed", "mixed"),

}

s := &countingStore{Store: dir}

- cache := map[[2]string]string{}

+ cache := newGenCache()

genPollPass(ctx, routes, s, cache)

touch(t, ctx, dir, "zed", "mixed")

@@ -381,7 +397,7 @@ func TestGenPollPassPrunesUnitAppsToo(t *testing.T) {

unitRoute("u.example.com", "bab", "unitapp"),

}

one := both[:1]

- cache := map[[2]string]string{}

+ cache := newGenCache()

genPollPass(ctx, both, s, cache)

// The live set the cache is pruned against is the UNION: a unit app whose

@@ -389,13 +405,21 @@ func TestGenPollPassPrunesUnitAppsToo(t *testing.T) {

// remembered etag would fire a bogus root-pass request when the route

// comes back (adoption already delivers fresh config on the way in).

genPollPass(ctx, one, s, cache)

- if _, ok := cache[[2]string{"bab", "unitapp"}]; ok {

+ if _, ok := cache.etags[[2]string{"bab", "unitapp"}]; ok {

t.Fatal("a unit-backed route that left the registry must leave no cache entry behind")

}

+ // As in TestGenPollPassDropsVanishedApps: the re-add tick itself moves

+ // the route-shape digest (issue #196), so the silence worth asserting

+ // is the SETTLED tick after it. The dropped cache entry above is what

+ // pins the etag half directly.

touch(t, ctx, s, "bab", "unitapp")

+ _, _, errs := genPollPass(ctx, both, s, cache)

+ if len(errs) != 0 {

+ t.Fatalf("unexpected read errors: %v", errs)

+ }

execChanged, unitChanged, errs := genPollPass(ctx, both, s, cache)

if execChanged || unitChanged {

- t.Fatal("a re-added unit-backed app is a first sight, not a change")

+ t.Fatal("a settled tick after a unit-backed re-add must be silent")

}

if len(errs) != 0 {

t.Fatalf("unexpected read errors: %v", errs)

@@ -1666,3 +1690,103 @@ func TestStampedUnitNamesEmptyRegistry(t *testing.T) {

t.Fatalf("stampedUnitNames on an empty registry = %v, want nil", got)

}

+// --- issue #196: the registry side of "did anything change" ---

+// boundExecRoute is execRoute wired to a named config environment — the

+// binding whose whole point is that it changes which bundle merges into

+// the route's delivered config.

+func boundExecRoute(host, acct, app, env string) registry.Route {

+ rt := execRoute(host, acct, app)

+ rt.Environment = env

+ return rt

+// TestGenPollPassSyncsOnRouteRebinding is issue #196: binding a route to an

+// environment must converge from the FAST loop, not an hour later on

+// configSyncLoop's floor. The bucket does not move at all here — only the

+// registry column does, which is exactly why the etag poll could not see

+// it and titogo lost an hour to a symptom reading as a bad credential.

+func TestGenPollPassSyncsOnRouteRebinding(t *testing.T) {

+ ctx, s := genKit(t)

+ touch(t, ctx, s, "bab", "titogo")

+ cache := newGenCache()

+ genPollPass(ctx, []registry.Route{execRoute("connect.go.tito.io", "bab", "titogo")}, s, cache)

+ rebound := []registry.Route{boundExecRoute("connect.go.tito.io", "bab", "titogo", "relay")}

+ execChanged, unitChanged, errs := genPollPass(ctx, rebound, s, cache)

+ if len(errs) != 0 {

+ t.Fatalf("unexpected read errors: %v", errs)

+ }

+ if !execChanged {

+ t.Fatal("binding a route to an environment must make the exec half sync")

+ }

+ if unitChanged {

+ t.Fatal("an exec route's rebinding must not ask for a root config pass")

+ }

+ // And the pass after it is quiet again: a digest that re-fired every

+ // tick would run SyncExec forever.

+ if execChanged, _, _ := genPollPass(ctx, rebound, s, cache); execChanged {

+ t.Fatal("the pass after a rebinding must be silent again (a stale digest would re-sync forever)")

+ }

+// TestGenPollPassSyncsOnUnitRouteRebinding: the unit half converges per

+// environment too (appconfig.Sync's group is (acct, app, environment)), so

+// a unit-backed route's rebinding must ask for the root pass — and must

+// not make the agent render anything itself.

+func TestGenPollPassSyncsOnUnitRouteRebinding(t *testing.T) {

+ ctx, s := genKit(t)

+ touch(t, ctx, s, "bae", "keymail")

+ cache := newGenCache()

+ genPollPass(ctx, []registry.Route{unitRoute("keymail.dev", "bae", "keymail")}, s, cache)

+ rebound := []registry.Route{unitRoute("keymail.dev", "bae", "keymail")}

+ rebound[0].Environment = "staging"

+ execChanged, unitChanged, errs := genPollPass(ctx, rebound, s, cache)

+ if len(errs) != 0 {

+ t.Fatalf("unexpected read errors: %v", errs)

+ }

+ if !unitChanged {

+ t.Fatal("binding a unit-backed route to an environment must request a root config pass")

+ }

+ if execChanged {

+ t.Fatal("a unit route's rebinding must not run a SyncExec pass")

+ }

+// TestGenPollPassFirstSightOfRouteShapeIsSilent: a box coming up has never

+// seen the digest before, and must not treat that as a change — every

+// instance on the box would sync at once on boot. configSyncLoop's ~1min

+// first pass owns boot convergence, exactly as it does for etags.

+func TestGenPollPassFirstSightOfRouteShapeIsSilent(t *testing.T) {

+ ctx, s := genKit(t)

+ touch(t, ctx, s, "bab", "titogo")

+ routes := []registry.Route{boundExecRoute("connect.go.tito.io", "bab", "titogo", "relay")}

+ execChanged, unitChanged, errs := genPollPass(ctx, routes, s, newGenCache())

+ if len(errs) != 0 {

+ t.Fatalf("unexpected read errors: %v", errs)

+ }

+ if execChanged || unitChanged {

+ t.Fatal("the first pass must record the route shape silently")

+ }

+// TestGenPollPassRouteShapeCostsNoBucketReads: the registry half must not

+// add a single fetch to the tick — the whole economy of this loop is that

+// it reads generation objects and nothing else.

+func TestGenPollPassRouteShapeCostsNoBucketReads(t *testing.T) {

+ ctx, _ := genKit(t)

+ cs := &countingStore{Store: release.NewDirStore(t.TempDir())}

+ touch(t, ctx, cs, "bab", "titogo")

+ routes := []registry.Route{execRoute("connect.go.tito.io", "bab", "titogo")}

+ cache := newGenCache()

+ genPollPass(ctx, routes, cs, cache)

+ key := appconfig.GenKey("bab", "titogo")

+ before := cs.gets[key]

+ rebound := []registry.Route{boundExecRoute("connect.go.tito.io", "bab", "titogo", "relay")}

+ genPollPass(ctx, rebound, cs, cache)

+ if got := cs.gets[key] - before; got != 1 {

+ t.Fatalf("the rebinding tick made %d bucket reads, want exactly 1 (the app's generation object, as before)", got)

+ }