amadan
console: instances API reports health, filters and paginates (additive)
`carlos instances list`'s door grows the controls the Running tab already
has, computed server-side so the API, the CLI and the page can never
disagree about an instance's state: each record carries the same one-word
`health` the table shows, and the response carries `total`, `counts` and
`next_cursor` alongside the `instances`/`truncated` it always had.
q/health/limit/cursor come from the shared parseInstanceQuery + cursor
helpers, so a filter means the same thing on the page and on the wire.
ADDITIVE by construction: no new param means the same records, the same
default cap (instanceListMax) and every pre-existing field byte-identical —
the CLI's current decoder keeps parsing this body unchanged. The gate,
timeout and error paths are untouched; the new work all happens after the
records and staged versions are in hand.
instanceHealth joins the app's OWN records against the registry and the
metrics sweep. The tenant boundary is the record set: every lookup is keyed
on a host drawn from records already scoped to one account+app, and a
registry row stamped to another app or account is refused rather than read —
the box's reconcile pass disowns such a row too ("exists but was not minted
by this pass"). Without that refusal a neighbour's bespoke unit reaches
OwnerAppMetrics' allowlist and the caller reads someone else's process as
its own instance "Running"; the test pins exactly that. An unstamped row
still joins, so hand-added and pre-column routes don't read as forever
"Setting up". Every failure below — registry, sweep — degrades to "Setting
up" rather than erroring the list: health is an annotation on the record,
not the record.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 files changed,
+387
−5
internal/console/apiinstances.go+162 −5internal/console/apiinstances_test.go+225 −0
diff --git a/internal/console/apiinstances.go b/internal/console/apiinstances.go| index 68bd42843..59c25f09f 100644 |
| --- a/internal/console/apiinstances.go |
| +++ b/internal/console/apiinstances.go |
| @@ -26,10 +26,12 @@ import ( |
| "net/http" |
| "os" |
| "slices" |
| + "strconv" |
| "strings" |
| "time" |
| "github.com/carlosframework/platform/internal/provision" |
| + "github.com/carlosframework/platform/internal/registry" |
| "github.com/carlosframework/platform/internal/release" |
| "github.com/carlosframework/platform/internal/status" |
| ) |
| @@ -173,6 +175,106 @@ type instanceListRecord struct { |
| // rather than going on serving that box's last-known version as if it |
| // were current. The deploy watch does NOT read this field. |
| StagedVersion string `json:"staged_version"` |
| + // Health is the same one-word state the Running tab's column shows, |
| + // computed by instanceHealth below from the registry row joined with the |
| + // metrics sweep. Additive on the wire (a client that never heard of it |
| + // keeps parsing the record it always parsed) and annotation-grade: a |
| + // record whose health could not be computed still lists, reading |
| + // "Setting up" rather than failing the call. |
| + Health string `json:"health"` |
| +} |
| + |
| +// instanceHealth computes each record host's health word the same way the |
| +// Running tab does — registry row joined with the metrics sweep — so the |
| +// API, the CLI and the page can never disagree about an instance's state. |
| +// A record with no registry row yet is still provisioning: "Setting up". |
| +// Failures degrade to that same word rather than erroring the list: health |
| +// here is an annotation on the record, not the record. |
| +// |
| +// The tenant boundary is the recs slice and nothing else. recs arrives |
| +// already scoped to one account+app (provision.ListRecords under that app's |
| +// own prefix), and every lookup below is keyed on a host drawn FROM recs — |
| +// the registry read returns the whole box's routes, including other tenants', |
| +// and none of them can enter hosts, unitByHost or the answer. That matters |
| +// twice over: OwnerAppMetrics' allowlist is exactly the hosts and stamped |
| +// units it is handed, so a foreign unit name reaching unitByHost would widen |
| +// the metrics view past the caller's own app. |
| +func (s *Server) instanceHealth(ctx context.Context, recs []provision.Record) map[string]string { |
| + out := make(map[string]string, len(recs)) |
| + for _, rec := range recs { |
| + out[rec.Host] = "Setting up" |
| + } |
| + reg, err := openRegistry(s.cfg.Registry) |
| + if err != nil { |
| + log.Printf("console: api instances health: registry %s: %v", s.cfg.Registry, err) |
| + return out |
| + } |
| + routes, err := reg.List() |
| + if err != nil { |
| + log.Printf("console: api instances health: registry list: %v", err) |
| + return out |
| + } |
| + byHost := make(map[string]registry.Route, len(routes)) |
| + for _, rt := range routes { |
| + byHost[rt.Host] = rt |
| + } |
| + // routeFor is the join reduced to the routes that ANSWER for these |
| + // records, built once and used by both loops so the two can never drift. |
| + routeFor := make(map[string]registry.Route, len(recs)) |
| + var hosts []string |
| + unitByHost := map[string]string{} |
| + for _, rec := range recs { |
| + rt, ok := byHost[rec.Host] |
| + if !ok { |
| + continue // no route minted yet: the record stays "Setting up" |
| + } |
| + // A row stamped to another app or account is not this record's route. |
| + // The box's own reconcile pass says so — it refuses to converge such |
| + // a row ("exists but was not minted by this pass — leaving it |
| + // alone", reconcile.go) — and reporting it here would be worse than |
| + // useless twice: the caller would read a neighbour's health word as |
| + // its own instance's, and the neighbour's stamped Unit would enter |
| + // unitByHost, widening OwnerAppMetrics' allowlist past this app's own |
| + // units. An UNSTAMPED row (either column empty: hand-added, or minted |
| + // before the column existed) still joins — refusing those would read |
| + // live instances as forever "Setting up". |
| + if (rt.App != "" && rt.App != rec.App) || (rt.Account != "" && rt.Account != rec.Account) { |
| + continue |
| + } |
| + routeFor[rec.Host] = rt |
| + hosts = append(hosts, rec.Host) |
| + if rt.Unit != "" { |
| + unitByHost[rec.Host] = rt.Unit |
| + } |
| + } |
| + am, err := s.OwnerAppMetrics(ctx, hosts, unitByHost) |
| + if err != nil { |
| + // An unreadable sweep is not knowledge that these instances are |
| + // down: every host reads as not-reporting below, which healthWord |
| + // turns into "No readings" or "Not responding" per expectsReadings — |
| + // the same answer the page gives when the sweep is unavailable. |
| + log.Printf("console: api instances health: metrics: %v", err) |
| + am = nil |
| + } |
| + rep := make(map[string]bool, len(am)) |
| + for _, m := range am { |
| + if m.Reporting() { |
| + rep[m.Host] = true |
| + } |
| + } |
| + for _, rec := range recs { |
| + rt, ok := routeFor[rec.Host] |
| + if !ok { |
| + continue |
| + } |
| + status := rt.Status |
| + if status == "" { |
| + status = "ready" // registry contract: empty status means ready |
| + } |
| + out[rec.Host] = healthWord(rt.Kind, status, rt.Hibernate, rep[rec.Host], |
| + expectsReadings(rt.Provisioned, rt.Unit, rt.Backing)) |
| + } |
| + return out |
| } |
| // stagedVersions builds a host -> version map from every NON-STALE box's own |
| @@ -223,7 +325,16 @@ func stagedVersions(ctx context.Context, s release.Store, now time.Time) map[str |
| // handleAPIInstanceList answers `carlos instances list`: this app's records, |
| // capped and honest about the cap, each carrying its best-effort |
| // staged_version (Task 8, issue #78) — fresh-ish while some box keeps |
| -// reporting the host, dropped rather than stale once none does. |
| +// reporting the host, dropped rather than stale once none does — and its |
| +// health word, plus the same q/health/limit/cursor controls the Running tab |
| +// has, applied SERVER-side so the CLI and the page filter identically. |
| +// |
| +// Every addition is additive on the wire and defaults to today's answer: no |
| +// params means the full `instanceListMax` page, `instances` and `truncated` |
| +// exactly as before, with `health` on each record and `total`/`counts`/ |
| +// `next_cursor` alongside. Counts cover every record regardless of the |
| +// filter — they are the whole app's health story, and a filtered count could |
| +// not lead anywhere. |
| func (s *Server) handleAPIInstanceList(w http.ResponseWriter, r *http.Request) { |
| _, acct, app, ok := s.instanceGate(w, r) |
| if !ok { |
| @@ -238,11 +349,57 @@ func (s *Server) handleAPIInstanceList(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| versions := stagedVersions(ctx, s.cfg.Releases, time.Now()) |
| - out := make([]instanceListRecord, len(recs)) |
| - for i, rec := range recs { |
| - out[i] = instanceListRecord{Record: rec, StagedVersion: versions[rec.Host]} |
| + params := r.URL.Query() |
| + q := parseInstanceQuery(params) |
| + // The default stays the whole cap, so a client that sends no limit gets |
| + // the answer it always got. A limit outside (0, instanceListMax) is |
| + // ignored rather than refused — a bad page size is not worth an error |
| + // where the honest full page is right there. |
| + limit := instanceListMax |
| + if raw := params.Get("limit"); raw != "" { |
| + if n, err := strconv.Atoi(raw); err == nil && n > 0 && n < instanceListMax { |
| + limit = n |
| + } |
| + } |
| + health := s.instanceHealth(ctx, recs) |
| + counts := map[string]int{} |
| + matched := make([]instanceListRecord, 0, len(recs)) |
| + for _, rec := range recs { |
| + h := health[rec.Host] |
| + counts[h]++ |
| + if q.Q != "" && !strings.Contains(rec.Host, q.Q) { |
| + continue |
| + } |
| + if q.Health != "" && h != q.Health { |
| + continue |
| + } |
| + matched = append(matched, instanceListRecord{Record: rec, StagedVersion: versions[rec.Host], Health: h}) |
| + } |
| + total := len(matched) |
| + // provision.ListRecords returns records host-sorted and nothing above |
| + // re-orders them, which is the whole basis of the cursor: "resume after |
| + // this host" only means one page in a stable order. |
| + start := 0 |
| + if after, ok := decodeInstanceCursor(q.Cursor); ok { |
| + for i, rec := range matched { |
| + if rec.Host > after { |
| + start = i |
| + break |
| + } |
| + start = i + 1 |
| + } |
| + } |
| + next := "" |
| + end := start + limit |
| + if end < total { |
| + next = encodeInstanceCursor(matched[end-1].Host) |
| + } else { |
| + end = total |
| } |
| - writeJSON(w, http.StatusOK, map[string]any{"instances": out, "truncated": truncated}) |
| + writeJSON(w, http.StatusOK, map[string]any{ |
| + "instances": matched[start:end], "truncated": truncated, |
| + "total": total, "counts": counts, "next_cursor": next, |
| + }) |
| } |
| // handleAPIInstanceGet answers one record. A host with no record is the |
diff --git a/internal/console/apiinstances_test.go b/internal/console/apiinstances_test.go| index b394ad5ad..fd2454918 100644 |
| --- a/internal/console/apiinstances_test.go |
| +++ b/internal/console/apiinstances_test.go |
| @@ -11,17 +11,24 @@ package console |
| // AFTER the gate so a 429 can never confirm what a 404 hides. |
| import ( |
| + "bytes" |
| "context" |
| "encoding/json" |
| "errors" |
| "fmt" |
| "io" |
| + "maps" |
| "net/http" |
| "net/http/httptest" |
| + "net/url" |
| + "slices" |
| + "sort" |
| "strings" |
| "testing" |
| "time" |
| + "github.com/carlosframework/platform/internal/activator" |
| + "github.com/carlosframework/platform/internal/metrics" |
| "github.com/carlosframework/platform/internal/provision" |
| "github.com/carlosframework/platform/internal/registry" |
| "github.com/carlosframework/platform/internal/release" |
| @@ -663,6 +670,224 @@ func TestAPIInstanceListStagedVersion(t *testing.T) { |
| } |
| } |
| +// TestAPIInstanceListHealthAndFilters (Task 9): the list door grows the same |
| +// controls the Running tab has — a health word per record, a substring |
| +// filter, a health filter, and an opaque cursor — computed server-side so the |
| +// CLI, the API and the page can never disagree about an instance's state. The |
| +// growth is ADDITIVE: a client that knows only `instances` + `truncated` |
| +// still parses this body, every pre-existing field byte-identical, and a |
| +// request carrying none of the new params still returns every record it did |
| +// before. |
| +func TestAPIInstanceListHealthAndFilters(t *testing.T) { |
| + fx, token := instanceFixture(t, "code-inst-health") |
| + enableInstances(t, fx, "titogo", titoManifest()) |
| + u := instancesURL(fx, "bab", "titogo") |
| + // "a" and "ab" share a prefix so q=a proves a SUBSTRING match rather than |
| + // an accidental equality, and "b" proves it excludes. |
| + for _, h := range []string{"a.go.tito.io", "ab.go.tito.io", "b.go.tito.io", "run.go.tito.io"} { |
| + if code, _, out := rawReq(t, http.MethodPost, u, token, |
| + fmt.Sprintf(`{"host":%q,"region":"ie"}`, h)); code != http.StatusCreated { |
| + t.Fatalf("create %s = %d %s", h, code, out) |
| + } |
| + } |
| + // a's record has been materialized into a route: ready, instance-kind, |
| + // not hibernating, no stamped unit and not provisioned — nothing the |
| + // sweep was ever told how to sample, so its silence is honest ignorance |
| + // ("No readings"), not an outage. |
| + fx.addRoute(t, registry.Route{Host: "a.go.tito.io", Kind: "instance", Status: "ready", |
| + App: "titogo", Account: fx.sqid, Channel: "stable"}) |
| + // ab has no registry row at all: still provisioning, "Setting up". |
| + // |
| + // b's host carries a route stamped to ANOTHER account and app, with a |
| + // bespoke unit that IS reporting in the sweep below. That row is not this |
| + // record's route — the box's reconcile pass refuses to converge it too — |
| + // so b must read "Setting up", and the neighbour's unit must never enter |
| + // the metrics allowlist. If the tenant check were dropped, b would read |
| + // "Running" off someone else's process. |
| + fx.addRoute(t, registry.Route{Host: "b.go.tito.io", Kind: "instance", Status: "ready", |
| + App: "neighbour", Account: "zzz", Unit: "neighbour-canary@b.service"}) |
| + // run is the live case: provisioned, and its carlos-app@ unit is in a |
| + // fresh sweep — "Running", which is what proves the sweep join is wired |
| + // rather than every word coming from the registry alone. |
| + fx.addRoute(t, registry.Route{Host: "run.go.tito.io", Kind: "instance", Status: "ready", |
| + App: "titogo", Account: fx.sqid, Channel: "stable", Provisioned: true}) |
| + at := time.Now().UTC().Add(-time.Minute) |
| + if err := metrics.WriteRaw(context.Background(), fx.rel, metrics.RawChunk{ |
| + V: 1, Box: "box-health", Hour: metrics.HourOf(at), |
| + Samples: []metrics.Sample{{At: at, Units: map[string]metrics.UnitSample{ |
| + activator.UnitName("run.go.tito.io"): {}, |
| + "neighbour-canary@b.service": {}, |
| + }}}, |
| + }); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + type listBody struct { |
| + Instances []struct { |
| + Host string `json:"host"` |
| + Health string `json:"health"` |
| + StagedVersion string `json:"staged_version"` |
| + } `json:"instances"` |
| + Truncated bool `json:"truncated"` |
| + Total int `json:"total"` |
| + Counts map[string]int `json:"counts"` |
| + NextCursor string `json:"next_cursor"` |
| + } |
| + get := func(t *testing.T, query string) (listBody, string) { |
| + t.Helper() |
| + code, _, out := rawReq(t, http.MethodGet, u+query, token, "") |
| + if code != http.StatusOK { |
| + t.Fatalf("list%s = %d %s, want 200", query, code, out) |
| + } |
| + var body listBody |
| + if err := json.Unmarshal([]byte(out), &body); err != nil { |
| + t.Fatalf("list%s body %s: %v", query, out, err) |
| + } |
| + return body, out |
| + } |
| + hostsOf := func(b listBody) []string { |
| + var out []string |
| + for _, r := range b.Instances { |
| + out = append(out, r.Host) |
| + } |
| + return out |
| + } |
| + |
| + order := []string{"a.go.tito.io", "ab.go.tito.io", "b.go.tito.io", "run.go.tito.io"} |
| + wantCounts := map[string]int{"No readings": 1, "Setting up": 2, "Running": 1} |
| + |
| + // 1) No params: every record, each carrying its health word, plus the |
| + // whole-app totals the header states. |
| + all, _ := get(t, "") |
| + if got := hostsOf(all); !slices.Equal(got, order) { |
| + t.Fatalf("unfiltered list = %v, want all four in host order", got) |
| + } |
| + wantHealth := map[string]string{ |
| + "a.go.tito.io": "No readings", |
| + "ab.go.tito.io": "Setting up", |
| + "b.go.tito.io": "Setting up", // a neighbour's route answers for nobody |
| + "run.go.tito.io": "Running", |
| + } |
| + for _, r := range all.Instances { |
| + if r.Health != wantHealth[r.Host] { |
| + t.Errorf("%s health = %q, want %q", r.Host, r.Health, wantHealth[r.Host]) |
| + } |
| + } |
| + if all.Total != 4 { |
| + t.Errorf("total = %d, want 4", all.Total) |
| + } |
| + if !maps.Equal(all.Counts, wantCounts) { |
| + t.Errorf("counts = %v, want %v", all.Counts, wantCounts) |
| + } |
| + if all.NextCursor != "" { |
| + t.Errorf("next_cursor = %q on a single-page list, want empty", all.NextCursor) |
| + } |
| + |
| + // 2) q is a substring match on the host. |
| + q, _ := get(t, "?q=a") |
| + if got := hostsOf(q); !slices.Equal(got, []string{"a.go.tito.io", "ab.go.tito.io"}) { |
| + t.Errorf("q=a = %v, want a and ab", got) |
| + } |
| + if q.Total != 2 { |
| + t.Errorf("q=a total = %d, want 2", q.Total) |
| + } |
| + // Counts are the app's WHOLE health story regardless of the filter — |
| + // they are the header's chips, and a filtered count would make the chips |
| + // unable to lead anywhere. |
| + if !maps.Equal(q.Counts, wantCounts) { |
| + t.Errorf("q=a counts = %v, want the unfiltered %v", q.Counts, wantCounts) |
| + } |
| + |
| + // 3) limit + cursor: one record a page, in host order, no overlap, and |
| + // the cursor ends on the last page. |
| + cursor := "" |
| + for i, want := range order { |
| + page, _ := get(t, "?limit=1&cursor="+url.QueryEscape(cursor)) |
| + if got := hostsOf(page); !slices.Equal(got, []string{want}) { |
| + t.Fatalf("limit=1 page %d = %v, want %q alone", i+1, got, want) |
| + } |
| + if page.Total != 4 { |
| + t.Errorf("limit=1 page %d total = %d, want 4 (the match count, not the page length)", i+1, page.Total) |
| + } |
| + last := i == len(order)-1 |
| + if last && page.NextCursor != "" { |
| + t.Errorf("next_cursor = %q on the last page, want empty", page.NextCursor) |
| + } |
| + if !last && page.NextCursor == "" { |
| + t.Fatalf("limit=1 page %d has no next_cursor with records left", i+1) |
| + } |
| + cursor = page.NextCursor |
| + } |
| + |
| + // 4) health filters on the computed word — and a record with no route of |
| + // its own reads as "Setting up". |
| + h, _ := get(t, "?health=setting-up") |
| + if got := hostsOf(h); !slices.Equal(got, []string{"ab.go.tito.io", "b.go.tito.io"}) { |
| + t.Errorf("health=setting-up = %v, want the two records with no route of their own", got) |
| + } |
| + if h.Total != 2 { |
| + t.Errorf("health=setting-up total = %d, want 2", h.Total) |
| + } |
| + running, _ := get(t, "?health=running") |
| + if got := hostsOf(running); !slices.Equal(got, []string{"run.go.tito.io"}) { |
| + t.Errorf("health=running = %v, want only the reporting instance", got) |
| + } |
| + // A mistyped word is no filter at all (healthFromSlug), never an empty |
| + // list a caller would read as "you have no instances". |
| + unknown, _ := get(t, "?health=nonsense") |
| + if got := hostsOf(unknown); !slices.Equal(got, order) { |
| + t.Errorf("health=nonsense = %v, want everything — an unknown word is no filter", got) |
| + } |
| + |
| + // 5) ADDITIVE: the OLD client's shape still parses, and every field a |
| + // record already had is byte-identical. The only key the wire gained |
| + // is "health". |
| + _, raw := get(t, "") |
| + var old struct { |
| + Instances []provision.Record `json:"instances"` |
| + Truncated bool `json:"truncated"` |
| + } |
| + if err := json.Unmarshal([]byte(raw), &old); err != nil { |
| + t.Fatalf("old client shape no longer parses %s: %v", raw, err) |
| + } |
| + if len(old.Instances) != len(order) || old.Truncated { |
| + t.Fatalf("old shape = %d records truncated=%v, want %d and false", |
| + len(old.Instances), old.Truncated, len(order)) |
| + } |
| + var wire struct { |
| + Instances []map[string]json.RawMessage `json:"instances"` |
| + } |
| + if err := json.Unmarshal([]byte(raw), &wire); err != nil { |
| + t.Fatal(err) |
| + } |
| + for i, rec := range old.Instances { |
| + b, err := json.Marshal(rec) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var want map[string]json.RawMessage |
| + if err := json.Unmarshal(b, &want); err != nil { |
| + t.Fatal(err) |
| + } |
| + for k, v := range want { |
| + if !bytes.Equal(wire.Instances[i][k], v) { |
| + t.Errorf("%s field %q = %s, want the record's own %s", |
| + rec.Host, k, wire.Instances[i][k], v) |
| + } |
| + } |
| + var extra []string |
| + for k := range wire.Instances[i] { |
| + if _, ok := want[k]; !ok { |
| + extra = append(extra, k) |
| + } |
| + } |
| + sort.Strings(extra) |
| + if !slices.Equal(extra, []string{"health", "staged_version"}) { |
| + t.Errorf("%s carries extra keys %v, want exactly health and staged_version", rec.Host, extra) |
| + } |
| + } |
| +} |
| + |
| // TestAPIInstanceDelete: deletion is idempotent (the caller's goal is "not |
| // there"), survives the manifest being retired, and still refuses a host |
| // that isn't one. |