amadan

carlos / platform Public

Refuse a channel-less sidecar, and let placement evacuate

Paul Campbell pushed by paul@keymail.dev 8c3f2df100479a9c95c861a83162af61f1c59f1a parent 509db85
2 files changed, +175 −81
  • internal/provision/sidecar_reconcile.go +97 −61
  • internal/provision/sidecar_reconcile_test.go +78 −20
diff --git a/internal/provision/sidecar_reconcile.go b/internal/provision/sidecar_reconcile.go
index c833986a0..c458f43d0 100644
--- a/internal/provision/sidecar_reconcile.go
+++ b/internal/provision/sidecar_reconcile.go
@@ -63,17 +63,6 @@ func sidecarArgsJSON(m SidecarManifest) string {
return string(b)
}
-// sidecarChannel resolves the channel a sidecar's row is pinned to: the
-// record's own if it states one, else the app manifest's. The record wins
-// because it is the per-sidecar authorization; the manifest is the app-wide
-// default a record may simply not repeat.
-func sidecarChannel(rec SidecarRecord, m SidecarManifest) string {
- if rec.Channel != "" {
- return rec.Channel
- }
- return m.Channel
-}
-
// sidecarPlacement resolves the placement policy a record is placed under
// (design §8). The record wins when it states one — it is the workload's own
// requirement — and the manifest's PlacementOrDefault answers otherwise, so
@@ -86,6 +75,16 @@ func sidecarPlacement(rec SidecarRecord, m SidecarManifest) string {
return m.PlacementOrDefault()
}
+// placeable reports whether this box satisfies a record's placement — the
+// BOX half of design §8's two independent halves. It reads as a SCOPE test,
+// beside region and pool, and is used as one: a record this box may not run
+// is not this box's business at all, so it never enters the record set and
+// the removal phase takes any row it left behind. That is how a placement
+// change takes effect — the same way a region change does.
+func placeable(rec SidecarRecord, m SidecarManifest, cfg ReconcileConfig) bool {
+ return sidecarPlacement(rec, m) != PlacementNoInbound || cfg.Inbound == InboundNone
+}
+
// sidecarRouteFor is the one answer to "what route does this sidecar record
// describe" — the mint builds from it and the converge branch compares
// against it, so a shape this pass would mint and a shape it calls converged
@@ -109,7 +108,12 @@ func sidecarRouteFor(rec SidecarRecord, m SidecarManifest, dataDir string) regis
return registry.Route{
Host: rec.Host, Slug: rec.Name, Kind: SidecarKind,
App: rec.App, Account: rec.Account, Pool: rec.Pool, Region: rec.Region,
- Channel: sidecarChannel(rec, m), Environment: rec.Environment,
+ // The RECORD's channel, never the manifest's: a record that reached
+ // this pass without one was refused above (a route with no channel
+ // resolves no release, so it has no build to exec), so there is
+ // nothing here for a manifest-side fallback to rescue. The manifest's
+ // Channel is the console door's default at CREATE time.
+ Channel: rec.Channel, Environment: rec.Environment,
DBPath: DefaultDBPath(dataDir, rec.Host),
Hibernate: m.HibernateOrDefault(),
Args: sidecarArgsJSON(m),
@@ -141,16 +145,24 @@ func coSpawnStaged(dataDir, host string) bool {
// read failure must never mass-retire a tenant's fleet. Minting is additive
// and always safe, so it proceeds regardless.
//
-// Three states decline to MINT while leaving the record in the pass's record
-// set — so removal never fires for them either, because declining to create
-// is reversible and deleting a row is not (registry.Delete purges the route's
-// capability grants, and nothing else would put the row back):
+// SCOPE — region, pool and PLACEMENT (design §8) — decides what enters the
+// record set at all, and so decides removal: a record this box may not run is
+// not this box's business, and any row it left behind is retired by the
+// ordinary removal phase. That is how a placement change takes effect, exactly
+// as a region change does through the instance pass; a member who asks that a
+// bot only ever run where there is no inbound path must not find it still
+// running on an inbound box after flipping the manifest. Evacuation is
+// governed by the same fail-safe as every other removal: if any read failed
+// this pass, nothing is evacuated.
+//
+// Two states instead decline to MINT while leaving the record in the record
+// set — so removal never fires for them — because neither is a statement
+// about where the workload belongs:
//
// - the app is in the trash (domains.Reconcile drops its routes on its own
// authority; minting here would re-raise them every tick),
// - the app's sidecars manifest is absent or unreadable (no argv to run,
-// and no ceilings to stamp),
-// - the record's placement is not one this box satisfies (design §8).
+// and no ceilings to stamp).
func ReconcileSidecars(ctx context.Context, s release.Store, reg *registry.Registry, cfg ReconcileConfig) error {
if cfg.Now == nil {
cfg.Now = time.Now
@@ -161,6 +173,47 @@ func ReconcileSidecars(ctx context.Context, s release.Store, reg *registry.Regis
return err
}
+ // --- manifest cache, per (acct, app), lazily ------------------------------
+ // known=false covers BOTH "no manifest" and "the read failed", and in both
+ // cases this pass declines to mint or converge: every manifest-owned
+ // column (argv, the ceilings, hibernation) would otherwise be stamped from
+ // a zero value — an empty argv over a working one, on a blip. This is
+ // seedFor's seedKnown discipline, applied to a family where the manifest
+ // carries the RUN instruction rather than a seed.
+ //
+ // Declared BEFORE the record scan because placement is a scope axis and
+ // placement can be stated app-wide: deciding whether a record is in scope
+ // needs the manifest in hand. An unknown manifest resolves placement from
+ // the record alone (sidecarPlacement over a zero manifest) — and a manifest
+ // whose read FAILED has already disabled the removal phase, so an
+ // unknowable placement can never evacuate anything.
+ manCache := map[acctApp]SidecarManifest{}
+ manKnown := map[acctApp]bool{}
+ manSeen := map[acctApp]bool{}
+ manifestFor := func(acct, app string) (SidecarManifest, bool) {
+ k := acctApp{acct, app}
+ if manSeen[k] {
+ return manCache[k], manKnown[k]
+ }
+ manSeen[k] = true
+ m, err := GetSidecarManifest(ctx, s, acct, app)
+ if err != nil {
+ if errors.Is(err, os.ErrNotExist) {
+ // Absence IS the answer (design §3.1): sidecars are not
+ // enabled for this app. Not a read failure — removal stays
+ // armed for the pass — but the records themselves stay in
+ // recs, so nothing of theirs is retired on it.
+ log.Printf("provision: %s/%s has sidecar records but no sidecars manifest — not minting", acct, app)
+ return SidecarManifest{}, false
+ }
+ log.Printf("provision: sidecar manifest %s/%s: read failed: %v", acct, app, err)
+ readFailed = true
+ return SidecarManifest{}, false
+ }
+ manCache[k], manKnown[k] = *m, true
+ return manCache[k], manKnown[k]
+ }
+
// --- collect in-scope records --------------------------------------------
keys, err := s.List(ctx, "apps/")
if err != nil {
@@ -224,9 +277,35 @@ func ReconcileSidecars(ctx context.Context, s release.Store, reg *registry.Regis
protect[host] = true
continue
}
+ // ValidateSidecarShape deliberately does not inspect Channel (it
+ // validates shape fields, and Channel is not one), but a channel-less
+ // record must not become — or converge — a route pinned to no channel:
+ // such a row resolves no release, so it has no build to exec, and the
+ // worker boot pass would try to run it against a symlink that does not
+ // exist. The console stamps a channel at create, so a record without
+ // one is malformed. Protected like a corrupted body: the record is
+ // there, it just cannot be used.
+ if rec.Channel == "" {
+ log.Printf("provision: skipping sidecar record %s: empty channel", key)
+ protect[host] = true
+ continue
+ }
if rec.Region != cfg.Region || !poolServed(cfg.Pools, rec.Pool) {
continue // another agent's jurisdiction, not this pass's business
}
+ // Placement is the third scope axis (design §8), and it is applied
+ // HERE, beside region and pool, precisely so it behaves like them: a
+ // record this box may not run never enters recs, and the removal phase
+ // below therefore evacuates any row it left behind. "none" is a
+ // property of the BOX's network; "no-inbound" is a requirement of the
+ // WORKLOAD — the point of the two halves being independent is that a
+ // member gets a mechanical guarantee rather than an operator's promise,
+ // which a row left running on an inbound box would not be.
+ if m, _ := manifestFor(acct, app); !placeable(rec, m, cfg) {
+ log.Printf("provision: sidecar %s is not placeable here: it asks for %q placement and this box accepts inbound work (CARLOS_INBOUND=%q)",
+ rec.Host, sidecarPlacement(rec, m), cfg.Inbound)
+ continue
+ }
// Two records claiming one host is a collision this pass cannot
// arbitrate silently: whichever this listing reaches FIRST wins, and
// the loser is skipped loudly rather than overwriting the winner in
@@ -239,40 +318,6 @@ func ReconcileSidecars(ctx context.Context, s release.Store, reg *registry.Regis
recs[rec.Host] = rec
}
- // --- manifest cache, per (acct, app), lazily ------------------------------
- // known=false covers BOTH "no manifest" and "the read failed", and in both
- // cases this pass declines to mint or converge: every manifest-owned
- // column (argv, the ceilings, hibernation) would otherwise be stamped from
- // a zero value — an empty argv over a working one, on a blip. This is
- // seedFor's seedKnown discipline, applied to a family where the manifest
- // carries the RUN instruction rather than a seed.
- manCache := map[acctApp]SidecarManifest{}
- manKnown := map[acctApp]bool{}
- manSeen := map[acctApp]bool{}
- manifestFor := func(acct, app string) (SidecarManifest, bool) {
- k := acctApp{acct, app}
- if manSeen[k] {
- return manCache[k], manKnown[k]
- }
- manSeen[k] = true
- m, err := GetSidecarManifest(ctx, s, acct, app)
- if err != nil {
- if errors.Is(err, os.ErrNotExist) {
- // Absence IS the answer (design §3.1): sidecars are not
- // enabled for this app. Not a read failure — removal stays
- // armed for the pass — but the records themselves stay in
- // recs, so nothing of theirs is retired on it.
- log.Printf("provision: %s/%s has sidecar records but no sidecars manifest — not minting", acct, app)
- return SidecarManifest{}, false
- }
- log.Printf("provision: sidecar manifest %s/%s: read failed: %v", acct, app, err)
- readFailed = true
- return SidecarManifest{}, false
- }
- manCache[k], manKnown[k] = *m, true
- return manCache[k], manKnown[k]
- }
-
trashed := trashGate(ctx, s, "sidecar")
// --- mint / converge -------------------------------------------------------
@@ -293,15 +338,6 @@ func ReconcileSidecars(ctx context.Context, s release.Store, reg *registry.Regis
if !mKnown {
continue
}
- // Placement (design §8). "none" is a property of the BOX's network;
- // "no-inbound" is a requirement of the WORKLOAD, and the point of the
- // two halves being independent is that a member gets a mechanical
- // guarantee rather than an operator's promise.
- if place := sidecarPlacement(rec, m); place == PlacementNoInbound && cfg.Inbound != InboundNone {
- log.Printf("provision: not placing sidecar %s here: it asks for %q placement and this box accepts inbound work (CARLOS_INBOUND=%q)",
- rec.Host, place, cfg.Inbound)
- continue
- }
existing, ok := byHost[rec.Host]
switch {
case !ok:
diff --git a/internal/provision/sidecar_reconcile_test.go b/internal/provision/sidecar_reconcile_test.go
index f0a8d7e17..2eeceb26f 100644
--- a/internal/provision/sidecar_reconcile_test.go
+++ b/internal/provision/sidecar_reconcile_test.go
@@ -205,7 +205,7 @@ func TestSidecarReconcileMints(t *testing.T) {
func TestSidecarReconcileConvergesArgsFromTheManifest(t *testing.T) {
f := sidecarSetup(t)
f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven", Args: []string{"sidecar", "run"}})
- f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven"})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable"})
f.reconcile(t, ReconcileConfig{})
f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven", Args: []string{"worker", "run"}})
@@ -221,7 +221,7 @@ func TestSidecarReconcileConvergesTheCeilings(t *testing.T) {
f := sidecarSetup(t)
f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven",
Args: []string{"sidecar", "run"}, IdleSeconds: 300, MaxRunSeconds: 3600})
- f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven"})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable"})
f.reconcile(t, ReconcileConfig{})
f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven",
@@ -278,7 +278,7 @@ func TestSidecarReconcileConvergesChannelEnvironmentAndHibernate(t *testing.T) {
func TestSidecarReconcileSkipsRemovalWhenAReadFailed(t *testing.T) {
f := sidecarSetup(t)
f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven", Args: []string{"sidecar", "run"}})
- f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven"})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable"})
keep := f.host("keep", "eleven")
f.mustUpsert(t, registry.Route{Host: keep, Kind: "sidecar", Slug: "keep",
App: "eleven", Account: f.sqid, Provisioned: true})
@@ -309,6 +309,35 @@ func TestSidecarReconcileProtectsAMalformedRecord(t *testing.T) {
hasRoute(t, f.reg, host, "a record that cannot be PARSED is not a record that was retired")
}
+// TestSidecarReconcileProtectsAChannellessRecord: a route with no channel
+// resolves no release, so it has no build to exec — the worker boot pass
+// would try to run it against a symlink that does not exist. The console
+// stamps a channel at create, so a record without one is malformed, and
+// malformed is exactly what protect is for: not minted, and not read as "the
+// bucket retired it" either.
+func TestSidecarReconcileProtectsAChannellessRecord(t *testing.T) {
+ f := sidecarSetup(t)
+ host := f.host("bot", "eleven")
+ f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven",
+ Args: []string{"sidecar", "run"}, Channel: "stable"})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven"}) // no channel, no row yet
+ // A channel-less record whose row already exists: protected, not retired.
+ held := f.host("held", "eleven")
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "held", App: "eleven"})
+ f.mustUpsert(t, registry.Route{Host: held, Kind: "sidecar", Slug: "held",
+ App: "eleven", Account: f.sqid, Channel: "stable", Provisioned: true})
+ // A row with no record at all, to prove the removal phase still ran.
+ old := f.host("old", "eleven")
+ f.mustUpsert(t, registry.Route{Host: old, Kind: "sidecar", Slug: "old",
+ App: "eleven", Account: f.sqid, Provisioned: true})
+
+ f.reconcile(t, ReconcileConfig{})
+
+ noRoute(t, f.reg, host, "a channel-less record must not mint a row that can never resolve a release — the manifest's channel is the console door's default at CREATE time, not a runtime rescue")
+ hasRoute(t, f.reg, held, "a record that cannot be USED is not a record that was retired")
+ noRoute(t, f.reg, old, "protect is narrow: an unusable record holds back ITS OWN host only, and the pass's removal phase still runs")
+}
+
// --- placement -------------------------------------------------------------------
// TestSidecarReconcilePlacementGate pins §8's second half: a no-inbound
@@ -317,11 +346,11 @@ func TestSidecarReconcileProtectsAMalformedRecord(t *testing.T) {
func TestSidecarReconcilePlacementGate(t *testing.T) {
f := sidecarSetup(t)
f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven", Args: []string{"sidecar", "run"}})
- f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Placement: PlacementNoInbound})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable", Placement: PlacementNoInbound})
// A second app whose MANIFEST asks for the placement, record silent.
f.putManifest(t, "relay", SidecarManifest{Schema: 1, App: "relay",
Args: []string{"relay"}, Placement: PlacementNoInbound})
- f.putRecord(t, SidecarRecord{Schema: 1, Name: "hub", App: "relay"})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "hub", App: "relay", Channel: "stable"})
f.reconcile(t, ReconcileConfig{}) // an ordinary box: accepts inbound work
noRoute(t, f.reg, f.host("bot", "eleven"), "a no-inbound workload must not land on a box that accepts inbound work")
@@ -332,24 +361,53 @@ func TestSidecarReconcilePlacementGate(t *testing.T) {
hasRoute(t, f.reg, f.host("hub", "relay"), "it must land on a box that declares CARLOS_INBOUND=none")
}
-// TestSidecarReconcilePlacementMismatchDoesNotRemove: a record this box may
-// not RUN is still a record the bucket has not retired. Declining to mint is
-// the reversible direction; deleting the row is not (registry.Delete purges
-// the route's capability grants), and nothing else would put it back.
-func TestSidecarReconcilePlacementMismatchDoesNotRemove(t *testing.T) {
+// TestSidecarReconcilePlacementChangeEvacuatesTheRow: placement is a SCOPE
+// axis, beside region and pool, so flipping a manifest to no-inbound takes the
+// row off a box that accepts inbound work — the same way repointing a
+// record's region moves an instance. Decline-but-keep would break the only
+// guarantee the axis exists to give: a member who asks that a bot only ever
+// run where there is no inbound path would find it still running on an
+// inbound box.
+func TestSidecarReconcilePlacementChangeEvacuatesTheRow(t *testing.T) {
f := sidecarSetup(t)
host := f.host("bot", "eleven")
f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven", Args: []string{"sidecar", "run"}})
- f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Placement: PlacementNoInbound})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable"})
+ f.reconcile(t, ReconcileConfig{})
+ hasRoute(t, f.reg, host, "an unplaced record mints here")
+
+ // The app's owner now requires no inbound path anywhere it runs.
+ f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven",
+ Args: []string{"sidecar", "run"}, Placement: PlacementNoInbound})
+
+ f.reconcile(t, ReconcileConfig{}) // this box accepts inbound work
+ noRoute(t, f.reg, host, "flipping placement to no-inbound must EVACUATE the row from an inbound box, not leave the bot running there")
+
+ f.reconcile(t, ReconcileConfig{Inbound: InboundNone}) // and land where it belongs
+ hasRoute(t, f.reg, host, "the same record must mint on a box that declares CARLOS_INBOUND=none")
+}
+
+// TestSidecarReconcilePlacementEvacuationObeysTheFailSafe: evacuation is a
+// removal, so it is governed by the pass-wide fail-safe like every other
+// removal. A read that failed anywhere this pass means this box cannot tell a
+// placement change from a blip, and a transient blip must never evacuate a
+// tenant's fleet.
+func TestSidecarReconcilePlacementEvacuationObeysTheFailSafe(t *testing.T) {
+ f := sidecarSetup(t)
+ host := f.host("bot", "eleven")
+ f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven",
+ Args: []string{"sidecar", "run"}, Placement: PlacementNoInbound})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable"})
f.mustUpsert(t, registry.Route{Host: host, Kind: "sidecar", Slug: "bot",
App: "eleven", Account: f.sqid, Provisioned: true})
+ // An UNRELATED app's records cannot be read this pass.
+ f.putManifest(t, "relay", SidecarManifest{Schema: 1, App: "relay", Args: []string{"relay"}})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "hub", App: "relay", Channel: "stable"})
+ f.store.failReadsFor(SidecarsPrefix(f.sqid, "relay"))
- f.reconcile(t, ReconcileConfig{}) // ordinary box: placement does not match
+ f.reconcile(t, ReconcileConfig{}) // placement does not match, but a read failed
- rt := hasRoute(t, f.reg, host, "a placement this box cannot satisfy means DON'T MINT — it is not the bucket retiring the sidecar")
- if rt.Args != "" {
- t.Fatalf("Args = %q — a record this box may not place is a record this box does not act on at all", rt.Args)
- }
+ hasRoute(t, f.reg, host, "a pass that could not read everything must evacuate nothing — the fail-safe governs placement removals too")
}
// --- the co-spawn conflict ---------------------------------------------------------
@@ -364,7 +422,7 @@ func TestSidecarReconcileRefusesHostWithAStagedCoSpawnFile(t *testing.T) {
t.Fatal(err)
}
f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven", Args: []string{"sidecar", "run"}})
- f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven"})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable"})
f.reconcile(t, ReconcileConfig{})
@@ -379,7 +437,7 @@ func TestSidecarReconcileRefusesHostWithAStagedCoSpawnFile(t *testing.T) {
func TestSidecarReconcileSkipsAnotherBoxesScope(t *testing.T) {
f := sidecarSetup(t)
f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven", Args: []string{"sidecar", "run"}})
- f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Region: "ie", Pool: "eleven"})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable", Region: "ie", Pool: "eleven"})
// A foreign box's own provisioned sidecar row, visible in this registry.
foreign := f.host("far", "eleven")
f.mustUpsert(t, registry.Route{Host: foreign, Kind: "sidecar", Slug: "far", App: "eleven",
@@ -399,7 +457,7 @@ func TestSidecarReconcileSkipsAnotherBoxesScope(t *testing.T) {
func TestSidecarReconcileDryRunWritesNothing(t *testing.T) {
f := sidecarSetup(t)
f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven", Args: []string{"sidecar", "run"}})
- f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven"})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable"})
gone := f.host("gone", "eleven")
f.mustUpsert(t, registry.Route{Host: gone, Kind: "sidecar", Slug: "gone",
App: "eleven", Account: f.sqid, Provisioned: true})
@@ -419,7 +477,7 @@ func TestSidecarReconcileLeavesAForeignRowAlone(t *testing.T) {
f := sidecarSetup(t)
host := f.host("bot", "eleven")
f.putManifest(t, "eleven", SidecarManifest{Schema: 1, App: "eleven", Args: []string{"sidecar", "run"}})
- f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven"})
+ f.putRecord(t, SidecarRecord{Schema: 1, Name: "bot", App: "eleven", Channel: "stable"})
f.mustUpsert(t, registry.Route{Host: host, Kind: "sidecar", Slug: "bot",
App: "eleven", Account: f.sqid, Args: `["hand","written"]`})