amadan

carlos / platform Public

console, status: tell the truth about sidecars

Paul Campbell pushed by paul@keymail.dev 95c037b2a14c3614d5fa032e5dbcc1b24b48a309 parent 14dcd8b
9 files changed, +301 −15
  • internal/activator/activator_test.go +32 −0
  • internal/activator/backend_exec.go +13 −0
  • internal/console/instances.go +10 −5
  • internal/console/instances_test.go +30 −0
  • internal/console/listquery.go +4 −2
  • internal/console/overview.go +53 −0
  • internal/console/overview_test.go +68 −0
  • internal/status/snapshot.go +34 −8
  • internal/status/status_test.go +57 −0
diff --git a/internal/activator/activator_test.go b/internal/activator/activator_test.go
index d70444402..651a4d026 100644
--- a/internal/activator/activator_test.go
+++ b/internal/activator/activator_test.go
@@ -3888,6 +3888,38 @@ func TestExecBackendStartWorkerEnvWinsOverTenantFile(t *testing.T) {
}
}
+// TestExecBackendStartWorkerEmitsExitEvent pins the fix for a gap Task 5's
+// review carried into Task 15: Start's exit goroutine tells the member
+// "instance exited" (warn on a crash, info on exit 0), but StartWorker's
+// mirrored StartSidecar's silence instead. That mattered once sidecars got
+// their own health words a member reads on the Instances tab (Crashed among
+// them) — a crash with no event is a status word with no explanation. A
+// nonzero exit must land a warn-level event naming the worker's host.
+func TestExecBackendStartWorkerEmitsExitEvent(t *testing.T) {
+ b := testExecBackend(t)
+ sink := &fakeSink{}
+ b.Logs = sink
+ host := "worker-exit.x.internal"
+ stageSeedScript(t, b, host, "v1", "exit 7\n")
+
+ rt := registry.Route{Host: host, Kind: "sidecar", App: "eleven", Args: `["sidecar","run"]`}
+ p, _, err := b.StartWorker(context.Background(), rt, nil)
+ if err != nil {
+ t.Fatalf("StartWorker: %v", err)
+ }
+ waitFor(t, "worker exit", p.Exited)
+
+ var found bool
+ for _, e := range sink.events {
+ if e.host == host && e.level == "warn" && strings.Contains(e.msg, "exited") {
+ found = true
+ }
+ }
+ if !found {
+ t.Fatalf("no member-facing exit event for a crashed worker: %+v", sink.events)
+ }
+}
+
// stageRecordingLive stages host's live binary as a shell script that dumps
// its argv and its CARLOS_CONTROL_SOCKET to two files and exits — the only way
// to see what the child was ACTUALLY handed, argv and environment both.
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go
index 39b24acc4..fd76a385e 100644
--- a/internal/activator/backend_exec.go
+++ b/internal/activator/backend_exec.go
@@ -813,6 +813,19 @@ func (b *ExecBackend) StartWorker(ctx context.Context, rt registry.Route, extra
p := &childProc{cmd: cmd, done: make(chan struct{})}
go func() {
p.err = cmd.Wait()
+ // Mirrors Start's exit record (#5 review, closed here): a worker's
+ // crash is a sidecar's health word ("Crashed") the moment it lands,
+ // so the member needs the SAME explanation an instance crash gets,
+ // not silence. This fires once per actual exit, and the caller's
+ // own doubling-backoff restart (2s up to a minute, activator.go)
+ // already bounds how often an exit can happen at all — a
+ // crash-looping worker cannot make this any noisier than the
+ // restart log line it already earns internally.
+ if p.err != nil {
+ emitEvent(b.Logs, rt, "warn", "worker exited: "+redactPaths(p.err.Error()))
+ } else {
+ emitEvent(b.Logs, rt, "info", "worker exited (exit 0)")
+ }
b.Cgroups.CleanupHost(rt.Host)
close(p.done)
}()
diff --git a/internal/console/instances.go b/internal/console/instances.go
index 4a7d700c8..412cfdce7 100644
--- a/internal/console/instances.go
+++ b/internal/console/instances.go
@@ -81,13 +81,18 @@ func newInstanceFetcher(client *http.Client, scheme string) *instanceFetcher {
}
// Report picks the app's reporting host and fetches (or reuses) its report.
-// The host is the first ready, non-static route that has NOT opted into
-// hibernation — polling a hibernatable host could wake it, and the console
-// must never wake an app just to draw a table. nil means no route is
-// pollable, which renders the same as "not reporting".
+// The host is the first ready, non-static, non-sidecar route that has NOT
+// opted into hibernation — polling a hibernatable host could wake it, and
+// the console must never wake an app just to draw a table. A sidecar route
+// is excluded for a different reason: its Host is a derived
+// .sidecar.internal label nothing can dial (design §4.1), so choosing it
+// would make a HEALTHY app read "not reporting" purely because the picker
+// landed on an unroutable row from the same app's route set (Assemble groups
+// by app NAME with no kind filter). nil means no route is pollable, which
+// renders the same as "not reporting".
func (f *instanceFetcher) Report(ctx context.Context, routes []RouteView) *InstanceReport {
for _, rt := range routes {
- if rt.Host == "" || rt.Kind == "static" || rt.Status != "ready" || rt.Hibernate {
+ if rt.Host == "" || rt.Kind == "static" || rt.Kind == "sidecar" || rt.Status != "ready" || rt.Hibernate {
continue
}
rep := f.get(ctx, rt.Host)
diff --git a/internal/console/instances_test.go b/internal/console/instances_test.go
index 8dce3583f..b8780fee2 100644
--- a/internal/console/instances_test.go
+++ b/internal/console/instances_test.go
@@ -161,3 +161,33 @@ func TestInstancesNeverPollsHibernatableOrStatic(t *testing.T) {
t.Fatalf("second host got %d hits, want 0", n)
}
}
+
+// TestInstancesNeverPollsSidecar: a sidecar row shares its app's routes
+// slice (Assemble groups by app NAME, no kind filter) but its Host is a
+// derived .sidecar.internal label nothing can dial — polling it wastes the
+// timeout and, worse, makes a HEALTHY app read "not reporting" because the
+// one route the picker chose was never routable to begin with. A ready,
+// non-hibernating, non-static sidecar must still be skipped in favor of the
+// next real instance route.
+func TestInstancesNeverPollsSidecar(t *testing.T) {
+ sidecar := newInstanceApp(t, kassReport)
+ real := newInstanceApp(t, kassReport)
+ f := newInstanceFetcher(nil, "http")
+ routes := []RouteView{
+ {Host: sidecar.host(t), Kind: "sidecar", Status: "ready"},
+ readyRoute(real.host(t)),
+ }
+ rep := f.Report(context.Background(), routes)
+ if rep == nil || rep.Host != real.host(t) {
+ t.Fatalf("report = %+v, want the real instance host — the sidecar is unroutable", rep)
+ }
+ if n := sidecar.hits.Load(); n != 0 {
+ t.Fatalf("sidecar got %d hits — it must never be polled", n)
+ }
+
+ // A sidecar-only route set: nothing pollable, same as hibernating/static.
+ f2 := newInstanceFetcher(nil, "http")
+ if rep := f2.Report(context.Background(), []RouteView{{Host: sidecar.host(t), Kind: "sidecar", Status: "ready"}}); rep != nil {
+ t.Fatalf("report = %+v, want nil — a sidecar-only app has nothing pollable", rep)
+ }
+}
diff --git a/internal/console/listquery.go b/internal/console/listquery.go
index f5d072ef9..8700e1e22 100644
--- a/internal/console/listquery.go
+++ b/internal/console/listquery.go
@@ -18,8 +18,10 @@ const instancePageSize = 50
// healthWords is every word healthWord can emit, in the order the header
// lists them. A new word joins healthWord, this list, and nothing else —
-// slugs, filters and links all derive from here.
-var healthWords = []string{"Running", "Serving", "Setting up", "Asleep", "No readings", "Not responding", "Failed"}
+// slugs, filters and links all derive from here. Waking/Crashed/Never
+// started are sidecarHealthWord's own vocabulary (overview.go); Running and
+// Asleep are shared with the instance family, not duplicated here.
+var healthWords = []string{"Running", "Serving", "Setting up", "Asleep", "No readings", "Not responding", "Failed", "Waking", "Crashed", "Never started"}
// instanceQuery is one request's list controls. Zero value = everything,
// first page.
diff --git a/internal/console/overview.go b/internal/console/overview.go
index 5a635f281..1850e1c17 100644
--- a/internal/console/overview.go
+++ b/internal/console/overview.go
@@ -42,6 +42,19 @@ type OverviewStats struct {
// minus the unknowable. Templates render facts, never compute them.
InstanceCounted int
+ // SidecarTotal is how many of this account's routes are standalone
+ // workers (Kind "sidecar") — counted apart from InstanceTotal on
+ // purpose. A sidecar never serves a request and is never static either
+ // (registry.Route.IsSidecar's doc), so folding it into InstanceTotal
+ // would make an app with only sidecars read as "static — nothing to
+ // run" once InstanceExpected (instance-kind only) stays 0, and would
+ // make a mixed app's tile count a worker as if it were a copy of the
+ // app process. It has no health rollup here, mirroring InstanceTotal's
+ // role before InstanceExpected existed — the Instances tab's per-row
+ // healthWord is where a sidecar's own state (Running/Asleep/Waking/
+ // Crashed/Never started) is read.
+ SidecarTotal int
+
// MetricsUnavailable: the sweep itself failed (nil metrics), which is
// "readings unavailable", not "zero instances reporting".
MetricsUnavailable bool
@@ -130,6 +143,10 @@ func assembleOverview(view *AppView, acct string, metrics []InstanceMetrics, ver
if rt.Account != acct {
continue
}
+ if rt.Kind == "sidecar" {
+ st.SidecarTotal++
+ continue
+ }
st.InstanceTotal++
if rt.Kind == "instance" {
st.InstanceExpected++
@@ -284,6 +301,9 @@ func expectsReadings(provisioned bool, unit, backing string) bool {
// ("Not responding") or honest ignorance about a route the sweep was never
// told how to sample ("No readings") — see expectsReadings.
func healthWord(kind, status string, hibernate, reporting, expectReadings bool) string {
+ if kind == "sidecar" {
+ return sidecarHealthWord(status, hibernate, reporting, expectReadings)
+ }
switch status {
case "pending", "creating", "starting", "issuing":
return "Setting up"
@@ -304,3 +324,36 @@ func healthWord(kind, status string, hibernate, reporting, expectReadings bool)
return "No readings"
}
}
+
+// sidecarHealthWord is healthWord's vocabulary for Kind=="sidecar" rows,
+// split out because "Serving" — the word every other non-instance kind gets
+// — is simply false for a sidecar: it is a standalone worker that never
+// binds a socket or answers a request (registry.Route.IsSidecar's doc), so
+// nothing it does is ever "serving". Its five words: Running (reporting a
+// version over the agent socket), Asleep (hibernating, the app manifest's
+// choice), Waking (mid-provisioning, mirrors "Setting up"), Crashed (the
+// platform runs its process and it has gone silent — mirrors "failed" and
+// the instance family's "Not responding", but a sidecar's silence reads as
+// a crash rather than a request timing out, since nothing dials it) and
+// Never started (nobody has ever run it: not reporting, not hibernating,
+// and not yet provisioned to run at all). Every word here is also in
+// healthWords (listquery.go), which is what makes it filterable on the
+// Running tab.
+func sidecarHealthWord(status string, hibernate, reporting, expectReadings bool) string {
+ switch status {
+ case "pending", "creating", "starting", "issuing":
+ return "Waking"
+ case "failed":
+ return "Crashed"
+ }
+ switch {
+ case reporting:
+ return "Running"
+ case hibernate:
+ return "Asleep"
+ case expectReadings:
+ return "Crashed"
+ default:
+ return "Never started"
+ }
+}
diff --git a/internal/console/overview_test.go b/internal/console/overview_test.go
index 02ab3834c..97a205cf1 100644
--- a/internal/console/overview_test.go
+++ b/internal/console/overview_test.go
@@ -361,6 +361,52 @@ func TestInstanceRowHealthWord(t *testing.T) {
}
}
+// TestSidecarHealthWordVocabulary: a sidecar never serves a request, so
+// "Serving" — the catch-all for every non-instance kind — is simply false
+// for it (the bug this pins). Sidecars get their own five words instead:
+// Running, Asleep, Waking, Crashed, Never started.
+func TestSidecarHealthWordVocabulary(t *testing.T) {
+ cases := []struct {
+ status string
+ hibernate bool
+ reporting bool
+ expect bool
+ want string
+ }{
+ {"ready", false, true, true, "Running"}, // reporting a version over the socket
+ {"ready", true, false, true, "Asleep"}, // hibernating, not reporting
+ {"ready", false, false, true, "Crashed"}, // platform runs it, silent — presumed crashed
+ {"ready", false, false, false, "Never started"}, // never provisioned to run yet
+ {"starting", false, false, false, "Waking"},
+ {"pending", false, false, false, "Waking"},
+ {"creating", false, false, false, "Waking"},
+ {"issuing", false, false, false, "Waking"},
+ {"failed", false, false, false, "Crashed"},
+ }
+ for _, c := range cases {
+ got := healthWord("sidecar", c.status, c.hibernate, c.reporting, c.expect)
+ if got != c.want {
+ t.Errorf("healthWord(sidecar,%q,hib=%v,rep=%v,expect=%v) = %q, want %q",
+ c.status, c.hibernate, c.reporting, c.expect, got, c.want)
+ }
+ if got == "Serving" {
+ t.Errorf("healthWord(sidecar,...) = Serving — a sidecar never serves a request")
+ }
+ }
+}
+
+// TestSidecarHealthWordsAreFilterable pins listquery.go's contract that
+// "a new word joins healthWord, this list, and nothing else" — a sidecar
+// word missing from healthWords would silently break the Running tab's
+// health filter and its URL slug for that word.
+func TestSidecarHealthWordsAreFilterable(t *testing.T) {
+ for _, w := range []string{"Waking", "Crashed", "Never started"} {
+ if healthFromSlug(healthSlug(w)) != w {
+ t.Errorf("healthWords is missing %q — its filter/slug round trip is broken", w)
+ }
+ }
+}
+
// expectsReadings is the one rule for whether silence from a route's process
// means something: the platform runs the process (provisioned, or a
// carlos-app@ tenant) or a stamped unit tells the sweep where to look.
@@ -565,6 +611,28 @@ func TestOverviewExpectedCountsInstanceKindOnly(t *testing.T) {
}
}
+// TestOverviewCountsSidecarsSeparately: a sidecar is not an "instance" for
+// the Running tile's purposes — it never serves a request and is never
+// static either, so lumping it into InstanceTotal makes a sidecar-bearing
+// app's tile lie (InstanceExpected stays 0, and the template reads that as
+// "static — nothing to run"). Sidecars get their own counter instead.
+func TestOverviewCountsSidecarsSeparately(t *testing.T) {
+ view := &AppView{App: "eleven", Routes: []RouteView{
+ {Host: "a", Account: "bab", Kind: "instance"},
+ {Host: "bot.eleven.bab.sidecar.internal", Account: "bab", Kind: "sidecar"},
+ {Host: "bot2.eleven.bab.sidecar.internal", Account: "bab", Kind: "sidecar"},
+ // Another account's sidecar must not count here either.
+ {Host: "bot.eleven.other.sidecar.internal", Account: "other", Kind: "sidecar"},
+ }}
+ st := assembleOverview(view, "bab", []InstanceMetrics{}, nil)
+ if st.InstanceTotal != 1 {
+ t.Errorf("InstanceTotal = %d, want 1 — sidecars must not inflate it", st.InstanceTotal)
+ }
+ if st.SidecarTotal != 2 {
+ t.Errorf("SidecarTotal = %d, want 2 (this account's sidecars only)", st.SidecarTotal)
+ }
+}
+
// TestAppPageShowsPartialMismatchNotFullAlarm guards the boundary
// AllMismatched exists to draw: when only SOME of an app's routes disagree
// with the viewed account, the page must name the disagreeing route(s), not
diff --git a/internal/status/snapshot.go b/internal/status/snapshot.go
index cf20091c7..8f5aabeda 100644
--- a/internal/status/snapshot.go
+++ b/internal/status/snapshot.go
@@ -229,14 +229,16 @@ type Replication struct {
// Box is one box's self-report: what CollectBox produces and WriteBox
// serializes. Every field here has already had its needles pulled — there is
-// no hostname, machine id, or raw ratio anywhere in this type — with exactly
-// one deliberate exception: Metrics, the raw/private block documented on the
-// package comment and on BoxMetrics itself. Every OTHER field on Box is
-// still bound by the no-raw-numbers rule; Metrics is not "one more field",
-// it's the one place that rule is intentionally suspended, gated by staying
-// bucket-private (see the package doc) and excluded from FleetBox by
-// construction (fleet.go's BuildFleet copies fields one at a time and never
-// names Metrics).
+// no hostname, machine id, or raw ratio anywhere in this type — with two
+// deliberate exceptions: Metrics, the raw/private block documented on the
+// package comment and on BoxMetrics itself, and Sidecars, an aggregate count
+// that is private for a different reason (a sidecar's existence is
+// account-visible, not public, not because the number itself is sensitive).
+// Every OTHER field on Box is still bound by the no-raw-numbers rule and IS
+// meant for the public page; Metrics and Sidecars are the two places that
+// default is intentionally suspended, gated by staying bucket-private (see
+// the package doc) and excluded from FleetBox by construction (fleet.go's
+// BuildFleet copies fields one at a time and never names either).
type Box struct {
GeneratedAt time.Time `json:"generated_at"`
Label string `json:"label"`
@@ -283,6 +285,18 @@ type Box struct {
// package wrote before Metrics existed, and a box that genuinely can't
// answer any of the six fields, produce the identical absent-key shape.
Metrics *BoxMetrics `json:"metrics,omitempty"`
+ // Sidecars is how many of this box's routes are standalone workers
+ // (registry.Route.IsSidecar) — counted by CollectBox in their own
+ // branch, never as Resident, Static or Hibernating (its Hibernate comes
+ // from the app manifest, so a sidecar may legitimately be either, and
+ // either way it does not belong in the public instance count). Present
+ // in the private per-box snapshot (bucket-private, read only by
+ // fleet-status machinery and credentialed operators) but — like
+ // Metrics — deliberately NEVER copied into FleetBox: a sidecar's
+ // existence is account-visible, not public, so even the aggregate count
+ // must not reach fleet.json. omitempty keeps a box with no sidecars
+ // byte-identical to one written before this field existed.
+ Sidecars int `json:"sidecars,omitempty"`
}
// replHealthyWithin is the lag under which replication counts as healthy.
@@ -377,6 +391,10 @@ func CollectBox(routes []registry.Route, p Probes, now time.Time) Box {
var counts RouteCounts
var awakeKnown = true
awake, asleep := 0, 0
+ // sidecars counts standalone workers (registry.Route.IsSidecar) apart
+ // from RouteCounts entirely — see Box.Sidecars' doc for why it is never
+ // public.
+ var sidecars int
// routeVersions is issue #78's staged_version chip: host -> Version for
// this box's own non-platform routes that have one stamped. Built
// alongside the counts above rather than in a second pass over routes,
@@ -404,6 +422,13 @@ func CollectBox(routes []registry.Route, p Probes, now time.Time) Box {
routeVersions[r.Host] = r.Version
}
switch {
+ case r.IsSidecar():
+ // Checked first, ahead of Hibernate: a sidecar's Hibernate comes
+ // from the app manifest and may legitimately be true, but
+ // either way it is not the public Hibernating count — its own
+ // Socket is empty by construction (no upstream to dial), so the
+ // awake/asleep probe below would be meaningless for it too.
+ sidecars++
case r.Kind == "static":
counts.Static++
case r.Hibernate:
@@ -480,6 +505,7 @@ func CollectBox(routes []registry.Route, p Probes, now time.Time) Box {
ConfigPathRunning: p.ConfigPathRunning,
RestartPathRunning: p.RestartPathRunning,
ExpiredCanaries: p.ExpiredCanaries,
+ Sidecars: sidecars,
}
// Metrics attaches only when something is known — an all-unknown Probes,
diff --git a/internal/status/status_test.go b/internal/status/status_test.go
index 85790bde9..5e61c03d0 100644
--- a/internal/status/status_test.go
+++ b/internal/status/status_test.go
@@ -87,6 +87,63 @@ func TestCollectBoxSkipsPlatformMintedRoutes(t *testing.T) {
}
}
+// TestCollectBoxSidecarNotResident: a sidecar (Kind "sidecar") is not the
+// public fleet page's business — its existence is account-visible, not
+// public. Before an explicit branch, it fell into "default: Resident++",
+// silently inflating the public resident-app-instance count. It must land
+// in neither Resident, Static nor Hibernating (even when it happens to be
+// hibernating — a sidecar's Hibernate comes from the app manifest, so it may
+// legitimately be either), and Awake/Asleep must not count it — its Socket
+// is empty by construction, so probing it would be meaningless.
+func TestCollectBoxSidecarNotResident(t *testing.T) {
+ listening := func(string) *bool { v := true; return &v }
+ routes := []registry.Route{
+ {Host: "bot.eleven.bdm.sidecar.internal", Kind: "sidecar", Status: "ready"},
+ {Host: "bot2.eleven.bdm.sidecar.internal", Kind: "sidecar", Status: "ready", Hibernate: true},
+ {Host: "real.custom", Kind: "instance", Status: "ready"},
+ }
+ b := status.CollectBox(routes, status.Probes{MachineID: "m", Listening: listening}, time.Now())
+ if b.Routes.Resident != 1 {
+ t.Errorf("Resident = %d, want 1 — a sidecar must not be counted as a public resident app instance", b.Routes.Resident)
+ }
+ if b.Routes.Hibernating != 0 {
+ t.Errorf("Hibernating = %d, want 0 — a hibernating sidecar still belongs in its own bucket, not this one", b.Routes.Hibernating)
+ }
+ if b.Sidecars != 2 {
+ t.Errorf("Sidecars = %d, want 2 — both sidecars, hibernating or not", b.Sidecars)
+ }
+}
+
+// TestSidecarsNeverReachFleetJSON is the dryrun.go pattern applied to this
+// one field: BuildFleet's per-box copy names Box's public fields one at a
+// time (fleet.go), and Sidecars must never be one of them — a sidecar's
+// existence is account-visible, not public. Proven by round-tripping through
+// the real writer/merger, not just by the FleetBox struct's shape, so a
+// future field added to FleetBox that DOES copy it would be caught here too.
+func TestSidecarsNeverReachFleetJSON(t *testing.T) {
+ s := teststore(t)
+ now := time.Now()
+ routes := []registry.Route{{Host: "bot.x.sidecar.internal", Kind: "sidecar", Status: "ready"}}
+ b := status.CollectBox(routes, status.Probes{MachineID: "leaky"}, now)
+ if b.Sidecars != 1 {
+ t.Fatalf("Sidecars = %d, want 1 in the private snapshot itself", b.Sidecars)
+ }
+ if err := status.WriteBox(context.Background(), s, b); err != nil {
+ t.Fatal(err)
+ }
+ f, err := status.BuildFleet(context.Background(), s, nil, now, 5*time.Minute)
+ if err != nil {
+ t.Fatal(err)
+ }
+ j, err := json.Marshal(f)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if strings.Contains(strings.ToLower(string(j)), "sidecar") {
+ t.Errorf("fleet.json mentions sidecars: %s", j)
+ }
+}
+
func TestLabelStableAndOpaque(t *testing.T) {
a, b := status.Label("machine-1"), status.Label("machine-1")
if a != b || a == "" || strings.Contains(a, "machine") {