carlos / platform Public

cli: multi-upstream create/set-upstreams/list; carlos add carries upstream columns

carlos instances create gains -addr/-health-path/-health-status, mapping onto
the console's addrs/probe POST body. New carlos instances set-upstreams
verb PATCHes an instance's upstream set and/or probe through the console
(apimode.go's instanceSetUpstreams + patchJSON, mirroring postJSON/putJSON).
carlos instances list gains an UPSTREAMS column rendering up/total from the
joined per-upstream state, "-" when absent.

Carries Task 3's review forward: carlos add re-run over an existing host no
longer blanks a multi-upstream route's Addrs/Health (and Addr, when the
invocation named none of its own) — the registry.Upsert hazard the
registry.go doc named at cli.go:194-198.
Paul Campbell e96e28ffe1527e0bc390d4c1a476b02ba3882907 parent b483d1e
9 files changed, +708 −29
  • apimode.go +111 −5
  • apimode_test.go +189 −0
  • cli.go +24 −2
  • cli_test.go +84 −0
  • instances_cli.go +152 −10
  • instances_cli_test.go +72 −0
  • instances_e2e_test.go +59 −0
  • internal/registry/registry.go +15 −11
  • main.go +2 −1
diff --git a/apimode.go b/apimode.go
index a26cd81bb..981bc9160 100644
--- a/apimode.go
+++ b/apimode.go
@@ -1041,6 +1041,38 @@ func (s *apiSession) putJSON(ctx context.Context, path string, in any) (int, []b
return res.StatusCode, body, nil
}
+// patchJSON runs one JSON PATCH and hands back the status and raw body
+// without interpreting a non-200 as an error itself — mirrors postJSON and
+// putJSON exactly (apimode.go), only the HTTP method differs: the instance
+// upstream-repoint door is PATCH (`PATCH
+// /api/cli/apps/{account}/{app}/instances/{host}`, handleAPIInstanceUpdate)
+// — "here is what changed about this record", not the whole state (PUT) or a
+// new one (POST).
+func (s *apiSession) patchJSON(ctx context.Context, path string, in any) (int, []byte, error) {
+ raw, err := json.Marshal(in)
+ if err != nil {
+ return 0, nil, err
+ }
+ req, err := http.NewRequestWithContext(ctx, http.MethodPatch, s.c.base+path, bytes.NewReader(raw))
+ if err != nil {
+ return 0, nil, err
+ }
+ req.Header.Set("Content-Type", "application/json")
+ if s.c.token != "" {
+ req.Header.Set("Authorization", "Bearer "+s.c.token)
+ }
+ res, err := s.c.httpc.Do(req)
+ if err != nil {
+ return 0, nil, err
+ }
+ defer res.Body.Close()
+ body, err := io.ReadAll(io.LimitReader(res.Body, 1<<20))
+ if err != nil {
+ return res.StatusCode, nil, err
+ }
+ return res.StatusCode, body, nil
+}
+
// apiConfigVar is one row of a config bundle read back through the console —
// GET /api/cli/apps/{account}/{app}/config's "vars" entries. Value is only
// ever populated for Kind=="env": a secret row's value is structurally
@@ -1268,6 +1300,36 @@ type apiInstanceRecord struct {
// predates the field. Always empty on a create response, same reason
// StagedVersion is.
Health string `json:"health"`
+ // Upstreams is the console's per-upstream probe join (Task 8's
+ // instanceListRecord.Upstreams, apiinstances.go) — what lets the LIST's
+ // UPSTREAMS column say "2/3" without a box session. Nil for the
+ // overwhelming majority of records (single upstream, never probed) and
+ // always nil on a create or set-upstreams response: the join is a
+ // list-only best-effort read of the fleet's own snapshots, not something
+ // either write path computes.
+ Upstreams []apiUpstreamState `json:"upstreams,omitempty"`
+}
+
+// apiUpstreamState mirrors status.UpstreamState's wire shape (internal/status)
+// — redeclared here for the same reason apiInstanceRecord is: this client
+// renders the join, never writes it, and Addr/Up are the only fields the
+// UPSTREAMS column reads. Since/Fails/LastError exist on the wire but have no
+// CLI reader yet, so they are left off rather than pinning a contract nothing
+// here uses.
+type apiUpstreamState struct {
+ Addr string `json:"addr"`
+ Up bool `json:"up"`
+}
+
+// instanceProbeReq is the CLI's request-side spelling of provision.Health —
+// redeclared for the same reason apiInstanceRecord is (this client never
+// imports internal/provision): only the two fields `carlos instances
+// create`/`set-upstreams` expose (-health-path, -health-status) cross the
+// wire. Everything else the server-side Health struct carries keeps its
+// platform default, exactly as an omitted probe block always has.
+type instanceProbeReq struct {
+ Path string `json:"path,omitempty"`
+ Status int `json:"status,omitempty"`
}
// instanceCreate asks the console to create one instance record — the
@@ -1275,12 +1337,21 @@ type apiInstanceRecord struct {
// is what brings the host live. The console stamps pool from the manifest
// and validates host against the manifest's domain; this side sends only
// what a caller may choose.
-func (s *apiSession) instanceCreate(ctx context.Context, a apiAcct, app, host, region, channel string) (apiInstanceRecord, error) {
+//
+// addrs and probe are the route-shape fields handleAPIInstanceCreate accepts
+// alongside host/region/channel (Task 8's -addr/-health-path/-health-status
+// on `carlos instances create`): addrs nil means the platform's own default
+// upstream (kind defaults to "instance", which needs no addr at all), and
+// probe nil means unprobed — the same "absent, not zero" contract the wire
+// carries for every optional field on this door.
+func (s *apiSession) instanceCreate(ctx context.Context, a apiAcct, app, host, region, channel string, addrs []string, probe *instanceProbeReq) (apiInstanceRecord, error) {
in := struct {
- Host string `json:"host"`
- Region string `json:"region,omitempty"`
- Channel string `json:"channel,omitempty"`
- }{host, region, channel}
+ Host string `json:"host"`
+ Region string `json:"region,omitempty"`
+ Channel string `json:"channel,omitempty"`
+ Addrs []string `json:"addrs,omitempty"`
+ Probe *instanceProbeReq `json:"probe,omitempty"`
+ }{host, region, channel, addrs, probe}
status, body, err := s.postJSON(ctx, apiPath(a, app, "/instances"), in)
if err != nil {
return apiInstanceRecord{}, err
@@ -1295,6 +1366,41 @@ func (s *apiSession) instanceCreate(ctx context.Context, a apiAcct, app, host, r
return out, nil
}
+// instanceSetUpstreams asks the console to repoint one instance's upstream
+// set and/or its probe — `carlos instances set-upstreams`, the CLI front
+// over handleAPIInstanceUpdate's PATCH (Task 7, issue #152).
+//
+// addrs and probe are independently optional, exactly as the door documents
+// (apiinstances.go): a nil addrs leaves the record's upstream set alone, and
+// a nil probe leaves its probe config alone — the caller may repoint one
+// without touching the other. addrs is a POINTER, not a plain slice, so
+// "flag not given" (nil, omitted from the wire) and an explicitly empty
+// -addrs (a pointer to an empty slice, sent as `"addrs":[]`, which clears
+// the set) stay two
+// different requests the way the door's own *[]string distinguishes them —
+// the same precedent apiConfigApply's optional fields set. This client has
+// no way to spell `"probe":null` (an explicit probe CLEAR): nothing here
+// exposes it, since set-upstreams' only probe-side flags ADD or replace a
+// probe, never retract one.
+func (s *apiSession) instanceSetUpstreams(ctx context.Context, a apiAcct, app, host string, addrs *[]string, probe *instanceProbeReq) (apiInstanceRecord, error) {
+ in := struct {
+ Addrs *[]string `json:"addrs,omitempty"`
+ Probe *instanceProbeReq `json:"probe,omitempty"`
+ }{addrs, probe}
+ status, body, err := s.patchJSON(ctx, apiPath(a, app, "/instances/"+url.PathEscape(host)), in)
+ if err != nil {
+ return apiInstanceRecord{}, err
+ }
+ if status != http.StatusOK {
+ return apiInstanceRecord{}, apiErrorFromBody(status, body)
+ }
+ var out apiInstanceRecord
+ if err := json.Unmarshal(body, &out); err != nil {
+ return apiInstanceRecord{}, fmt.Errorf("instances set-upstreams: unreadable response: %w", err)
+ }
+ return out, nil
+}
+
// instanceList reads the app's instance records through the console, honest
// about the server's 500-record cap. match and health are the server-side
// filters Task 9 added (q and health query params) — empty strings send
diff --git a/apimode_test.go b/apimode_test.go
index 3c3f98f69..cb3f56a7c 100644
--- a/apimode_test.go
+++ b/apimode_test.go
@@ -646,6 +646,195 @@ func TestInstanceListSendsFilters(t *testing.T) {
}
}
+// TestInstanceListDecodesUpstreams pins the LIST join's wire shape (Task 8):
+// each record's "upstreams" array decodes into apiInstanceRecord.Upstreams,
+// and a record carrying no such key (the ordinary, single-upstream majority)
+// decodes to nil rather than an empty-but-present slice — the distinction
+// upstreamsCell's "-" rendering depends on.
+func TestInstanceListDecodesUpstreams(t *testing.T) {
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ json.NewEncoder(w).Encode(map[string]any{
+ "instances": []map[string]any{
+ {"host": "up.example.com", "upstreams": []map[string]any{
+ {"addr": "10.0.0.1:8090", "up": true},
+ {"addr": "10.0.0.2:8090", "up": false},
+ }},
+ {"host": "solo.example.com"},
+ },
+ "truncated": false, "total": 2,
+ })
+ }))
+ defer ts.Close()
+
+ s := newTestSession(ts.URL)
+ recs, _, _, err := s.instanceList(context.Background(), apiAcct{Seg: "bah"}, "app", "", "")
+ if err != nil {
+ t.Fatalf("instanceList: %v", err)
+ }
+ if len(recs) != 2 {
+ t.Fatalf("recs = %+v, want 2", recs)
+ }
+ if len(recs[0].Upstreams) != 2 || !recs[0].Upstreams[0].Up || recs[0].Upstreams[1].Up {
+ t.Fatalf("recs[0].Upstreams = %+v, want [10.0.0.1:8090 up, 10.0.0.2:8090 down]", recs[0].Upstreams)
+ }
+ if recs[1].Upstreams != nil {
+ t.Fatalf("recs[1].Upstreams = %+v, want nil for a record with no upstreams key", recs[1].Upstreams)
+ }
+}
+
+// TestInstanceCreateSendsAddrsAndProbe pins instanceCreate's request body
+// (Task 8): -addr/-health-path/-health-status land as addrs/probe on the
+// POST, alongside host/region/channel exactly as before, and are OMITTED
+// entirely (not sent as null or empty) when the caller passes neither —
+// today's create request, byte for byte, for a caller that never heard of
+// the new flags.
+func TestInstanceCreateSendsAddrsAndProbe(t *testing.T) {
+ var gotBody string
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if r.Method != http.MethodPost {
+ t.Errorf("method = %s, want POST", r.Method)
+ }
+ b, _ := io.ReadAll(r.Body)
+ gotBody = string(b)
+ w.WriteHeader(http.StatusCreated)
+ json.NewEncoder(w).Encode(map[string]any{"host": "a.example.com", "channel": "stable"})
+ }))
+ defer ts.Close()
+
+ s := newTestSession(ts.URL)
+ rec, err := s.instanceCreate(context.Background(), apiAcct{Seg: "bah"}, "app",
+ "a.example.com", "", "", []string{"10.0.0.1:8090", "10.0.0.2:8090"},
+ &instanceProbeReq{Path: "/healthz", Status: 200})
+ if err != nil {
+ t.Fatalf("instanceCreate: %v", err)
+ }
+ if rec.Host != "a.example.com" || rec.Channel != "stable" {
+ t.Fatalf("rec = %+v", rec)
+ }
+ var got struct {
+ Addrs []string `json:"addrs"`
+ Probe struct {
+ Path string `json:"path"`
+ Status int `json:"status"`
+ } `json:"probe"`
+ }
+ if err := json.Unmarshal([]byte(gotBody), &got); err != nil {
+ t.Fatalf("request body unmarshal: %v\nbody: %s", err, gotBody)
+ }
+ if len(got.Addrs) != 2 || got.Addrs[0] != "10.0.0.1:8090" || got.Addrs[1] != "10.0.0.2:8090" {
+ t.Fatalf("request addrs = %+v", got.Addrs)
+ }
+ if got.Probe.Path != "/healthz" || got.Probe.Status != 200 {
+ t.Fatalf("request probe = %+v", got.Probe)
+ }
+
+ // No addrs/probe given at all: neither key crosses the wire.
+ gotBody = ""
+ if _, err := s.instanceCreate(context.Background(), apiAcct{Seg: "bah"}, "app",
+ "b.example.com", "", "", nil, nil); err != nil {
+ t.Fatalf("instanceCreate (no addrs/probe): %v", err)
+ }
+ if strings.Contains(gotBody, "addrs") || strings.Contains(gotBody, "probe") {
+ t.Fatalf("request body = %q, want no addrs/probe key when neither was given", gotBody)
+ }
+}
+
+// TestInstanceSetUpstreamsPatches pins instanceSetUpstreams' whole contract
+// (Task 8, the CLI front over Task 7's PATCH): the HTTP method, the path
+// (mirrors instanceDelete's /instances/<host> escaping), the addrs/probe
+// wire shape, AND the three-way "absent vs explicit-empty vs a real value"
+// distinction the PATCH door depends on — a nil *[]string must send no
+// "addrs" key at all, while a non-nil pointer to an empty slice must send
+// "addrs":[] (the door's own clear spelling, apiinstances.go).
+func TestInstanceSetUpstreamsPatches(t *testing.T) {
+ var gotMethod, gotPath, gotBody string
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ gotMethod, gotPath = r.Method, r.URL.Path
+ b, _ := io.ReadAll(r.Body)
+ gotBody = string(b)
+ json.NewEncoder(w).Encode(map[string]any{"host": "a.example.com"})
+ }))
+ defer ts.Close()
+
+ s := newTestSession(ts.URL)
+ addrs := []string{"10.0.0.1:8090", "10.0.0.2:8090"}
+ rec, err := s.instanceSetUpstreams(context.Background(), apiAcct{Seg: "bah"}, "app", "a.example.com",
+ &addrs, &instanceProbeReq{Path: "/up", Status: 204})
+ if err != nil {
+ t.Fatalf("instanceSetUpstreams: %v", err)
+ }
+ if rec.Host != "a.example.com" {
+ t.Fatalf("rec = %+v", rec)
+ }
+ if gotMethod != http.MethodPatch {
+ t.Fatalf("method = %s, want PATCH", gotMethod)
+ }
+ if gotPath != "/api/cli/apps/bah/app/instances/a.example.com" {
+ t.Fatalf("path = %q", gotPath)
+ }
+ var got struct {
+ Addrs []string `json:"addrs"`
+ Probe struct {
+ Path string `json:"path"`
+ Status int `json:"status"`
+ } `json:"probe"`
+ }
+ if err := json.Unmarshal([]byte(gotBody), &got); err != nil {
+ t.Fatalf("request body unmarshal: %v\nbody: %s", err, gotBody)
+ }
+ if len(got.Addrs) != 2 || got.Addrs[1] != "10.0.0.2:8090" {
+ t.Fatalf("request addrs = %+v", got.Addrs)
+ }
+ if got.Probe.Path != "/up" || got.Probe.Status != 204 {
+ t.Fatalf("request probe = %+v", got.Probe)
+ }
+
+ // addrs=nil (flag not given): no "addrs" key at all — "leave it alone".
+ gotBody = ""
+ if _, err := s.instanceSetUpstreams(context.Background(), apiAcct{Seg: "bah"}, "app", "a.example.com",
+ nil, nil); err != nil {
+ t.Fatalf("instanceSetUpstreams (nil addrs): %v", err)
+ }
+ if strings.Contains(gotBody, "addrs") {
+ t.Fatalf("request body = %q, want no addrs key for a nil pointer", gotBody)
+ }
+
+ // addrs=&[]string{} (an explicit -addrs '' clear): "addrs":[] on the wire.
+ gotBody = ""
+ empty := []string{}
+ if _, err := s.instanceSetUpstreams(context.Background(), apiAcct{Seg: "bah"}, "app", "a.example.com",
+ &empty, nil); err != nil {
+ t.Fatalf("instanceSetUpstreams (empty addrs): %v", err)
+ }
+ if !strings.Contains(gotBody, `"addrs":[]`) {
+ t.Fatalf("request body = %q, want an explicit \"addrs\":[]", gotBody)
+ }
+}
+
+// TestInstanceSetUpstreamsSurfacesRefusalVerbatim pins the self-review rule
+// this task carries: the PATCH door's own refusal text (ValidateShape's or
+// the "nothing to update" 400) reaches the caller byte-for-byte — it names
+// the offending upstream or the missing field, which is the only thing an
+// operator can act on, and paraphrasing it here would throw that away.
+func TestInstanceSetUpstreamsSurfacesRefusalVerbatim(t *testing.T) {
+ const wantMsg = "addr \"garbage\": want a TCP upstream as host:port (e.g. 127.0.0.1:8090)"
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.WriteHeader(http.StatusBadRequest)
+ json.NewEncoder(w).Encode(map[string]any{"error": wantMsg})
+ }))
+ defer ts.Close()
+
+ s := newTestSession(ts.URL)
+ addrs := []string{"garbage"}
+ _, err := s.instanceSetUpstreams(context.Background(), apiAcct{Seg: "bah"}, "app", "a.example.com", &addrs, nil)
+ if err == nil {
+ t.Fatal("instanceSetUpstreams: want an error, got nil")
+ }
+ if err.Error() != wantMsg {
+ t.Fatalf("error = %q, want the door's own message %q verbatim", err.Error(), wantMsg)
+ }
+}
+
// ---- Task 5: the four verbs wired to resolveMode --------------------------
// loginTo saves one live credential for origin/address and returns it, so a
diff --git a/cli.go b/cli.go
index 15cd17b63..df95762dc 100644
--- a/cli.go
+++ b/cli.go
@@ -191,12 +191,31 @@ func runAdd(args []string) {
if err != nil {
log.Fatal(err)
}
- if err := r.Upsert(registry.Route{
+ route := registry.Route{
Host: host, Slug: *slug, Kind: *kind, Socket: *socket, Addr: *addr,
App: *app, Account: account,
Channel: *channel, Hibernate: *hibernate, StatusNamed: *statusNamed, Backing: *backing,
Unit: *unit, DBPath: *dbPath, Status: "ready",
- }); err != nil {
+ }
+ if existing != nil {
+ // Carried forward from Task 3's review (registry.go's Upsert doc names
+ // this exact gap at cli.go:194-198): this verb has no flags for a
+ // multi-upstream set or its probe, so it must not be the thing that
+ // blanks one — Upsert's addrs/health columns are plain overwrites, and
+ // a Route literal built above with no Addrs/Health always overwrites
+ // with "". Preserving what this box-local verb doesn't manage mirrors
+ // Upsert's own CASE-preserved columns (environment, caps, tunnel, ...):
+ // an explicit value from a flag still wins, an absent one keeps what
+ // was already there. Addr is the one exception among the three upstream
+ // columns that IS a flag here, so it only carries forward when this
+ // invocation named none of its own — an explicit -addr still replaces
+ // the legacy single-upstream field exactly as it always has.
+ route.Addrs, route.Health = existing.Addrs, existing.Health
+ if *addr == "" {
+ route.Addr = existing.Addr
+ }
+ }
+ if err := r.Upsert(route); err != nil {
log.Fatal(err)
}
if existing != nil {
@@ -244,6 +263,9 @@ func runInstances(args []string) {
case "delete":
runInstancesDelete(args[1:])
return
+ case "set-upstreams":
+ runInstancesSetUpstreams(args[1:])
+ return
}
}
fs := flag.NewFlagSet("instances", flag.ExitOnError)
diff --git a/cli_test.go b/cli_test.go
index 714015d05..d4e2f8472 100644
--- a/cli_test.go
+++ b/cli_test.go
@@ -674,6 +674,90 @@ func TestRouteUnitStampsAnExistingRoute(t *testing.T) {
}
}
+// TestAddPreservesUpstreamsOnReAdd carries Task 3's review forward (registry
+// doc at cli.go:194-198, now closed by runAdd): a bare re-`carlos add` over a
+// host that already carries a multi-upstream set must not blank it. Upsert's
+// addrs/health columns are plain overwrites, and until this fix a Route
+// literal built from add's own flags — none of which name Addrs/Health —
+// would clobber whatever `carlos instances set-upstreams` (or a converged
+// reconcile pass) had established, the same shape of hazard a plain
+// excluded.environment once caused.
+//
+// SetUpstreams (internal/registry) is used directly, not through a CLI verb,
+// to establish the set: it is the narrow setter reconcile itself converges
+// through, and no `carlos` command exposes it standalone — set-upstreams
+// (Task 8, instances_cli.go) speaks to a console, and this test wants
+// nothing else about the write path in flight.
+func TestAddPreservesUpstreamsOnReAdd(t *testing.T) {
+ bin := buildCarlos(t)
+ tmp := t.TempDir()
+ scratch := t.TempDir()
+ regPath := filepath.Join(tmp, "registry.db")
+
+ run := func(args ...string) (string, error) {
+ t.Helper()
+ cmd := exec.Command(bin, args...)
+ cmd.Env = carlosTestEnv(t)
+ // pick() walks cwd ancestors for .carlos/config, so the subprocess must not run from the real checkout.
+ cmd.Dir = scratch
+ out, err := cmd.CombinedOutput()
+ return string(out), err
+ }
+
+ const host = "kass.example.com"
+ if out, err := run("add", "-registry", regPath, "-app", "kass",
+ "-addr", "127.0.0.1:9001", host); err != nil {
+ t.Fatalf("add: %v\n%s", err, out)
+ }
+
+ reg, err := registry.Open(regPath)
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ defer reg.Close()
+ const wantAddrs = "127.0.0.1:9001,127.0.0.1:9002"
+ const wantHealth = `{"path":"/healthz","status":200}`
+ if err := reg.SetUpstreams(host, "", wantAddrs, wantHealth); err != nil {
+ t.Fatalf("SetUpstreams: %v", err)
+ }
+
+ // A re-add naming -addr again (an operator's ordinary "the same command I
+ // used the first time") must still carry the established set forward.
+ if out, err := run("add", "-registry", regPath, "-app", "kass",
+ "-addr", "127.0.0.1:9001", host); err != nil {
+ t.Fatalf("re-add: %v\n%s", err, out)
+ }
+ rt, err := reg.Get(host)
+ if err != nil || rt == nil {
+ t.Fatalf("get after re-add: %+v, %v", rt, err)
+ }
+ if rt.Addrs != wantAddrs {
+ t.Errorf("Addrs after re-add = %q, want %q preserved", rt.Addrs, wantAddrs)
+ }
+ if rt.Health != wantHealth {
+ t.Errorf("Health after re-add = %q, want %q preserved", rt.Health, wantHealth)
+ }
+ if rt.Addr != "127.0.0.1:9001" {
+ t.Errorf("Addr after re-add = %q, want the explicit -addr to still win", rt.Addr)
+ }
+
+ // A re-add naming NO -addr at all must not blank the legacy Addr column
+ // either — this box-local verb only carries what its own flags set.
+ if out, err := run("add", "-registry", regPath, "-app", "kass", host); err != nil {
+ t.Fatalf("re-add with no -addr: %v\n%s", err, out)
+ }
+ rt, err = reg.Get(host)
+ if err != nil || rt == nil {
+ t.Fatalf("get after flagless re-add: %+v, %v", rt, err)
+ }
+ if rt.Addr != "127.0.0.1:9001" {
+ t.Errorf("Addr after a flagless re-add = %q, want the established addr preserved", rt.Addr)
+ }
+ if rt.Addrs != wantAddrs || rt.Health != wantHealth {
+ t.Errorf("Addrs/Health after a flagless re-add = %q/%q, want both still preserved", rt.Addrs, rt.Health)
+ }
+}
+
// --- release versioning: the CLI surface -------------------------------
//
// (release versioning spec, 2026-08-08.) Driven through the real binary in
diff --git a/instances_cli.go b/instances_cli.go
index 04b37bca0..92c79a9c1 100644
--- a/instances_cli.go
+++ b/instances_cli.go
@@ -8,12 +8,17 @@
// domain to the account's own platform subdomain — so a member can
// bring an app live on <app>.<sqid>.<apps-domain> with a login alone,
// and custom domains stay an operator act.
-// - `create` / `list` / `delete` — the CLI front over the console's
-// instance-record verbs (spec §9's "natural follow-up slice", built
-// here because the records were otherwise API-only: the one
-// day-to-day surface with no carlos command). API mode ONLY: a
+// - `create` / `list` / `delete` / `set-upstreams` — the CLI front over
+// the console's instance-record verbs (spec §9's "natural follow-up
+// slice", built here because the records were otherwise API-only: the
+// one day-to-day surface with no carlos command). API mode ONLY: a
// record is a console-authorized write, and the box-side operator
-// equivalent has always been `carlos add`.
+// equivalent has always been `carlos add`. set-upstreams is the CLI
+// front over the multi-upstream repoint PATCH (HA design, issue #152):
+// `carlos add` remains the box-local, registry-direct way to place a
+// route by hand — this is its console-authorized, member-facing
+// counterpart for repointing an already-provisioned instance's upstream
+// set without a box shell.
package main
import (
@@ -197,8 +202,11 @@ func instancesEnable(ctx context.Context, s release.Store, acct, app string, m p
}
const instancesRecordsUsage = `usage: carlos instances create -app <a> -host <h> [-region <r>] [-channel <c>]
- carlos instances list -app <a> [-match <substr>] [-health <slug>]
- carlos instances delete -app <a> -host <h>
+ [-addr <a:p,...>] [-health-path <p>] [-health-status <n>]
+ carlos instances list -app <a> [-match <substr>] [-health <slug>]
+ carlos instances delete -app <a> -host <h>
+ carlos instances set-upstreams -app <a> -host <h> [-addrs <a:p,...>]
+ [-health-path <p>] [-health-status <n>]
Signed-in only (carlos auth login): instance records are console-authorized
writes — the console checks the host against the app's manifest and the
@@ -208,12 +216,33 @@ box's reconcile pass materializes the route. Common flags:
-account-id <id> account sqid, matched as a sqid ONLY
-console <origin> console to act through (default: the only logged-in one)
+create-only flags:
+
+ -addr <a:p,...> comma-separated TCP upstream set, host:port each
+ (e.g. 10.0.0.1:8090,10.0.0.2:8090); omitted means
+ the platform's own default upstream
+ -health-path <p> probe request path (default "/"); only meaningful
+ alongside more than one -addr
+ -health-status <n> probe's expected response status (default 200)
+
list-only flags:
-match <substr> only hosts containing this substring
-health <slug> only this state: running, serving, setting-up, asleep,
no-readings, not-responding, failed
+set-upstreams-only flags:
+
+ -addrs <a:p,...> comma-separated TCP upstream set, host:port each,
+ replacing the record's whole upstream set;
+ omitted leaves the existing set alone
+ -health-path <p> replaces the probe's request path
+ -health-status <n> replaces the probe's expected response status
+ (-health-path and -health-status may be given
+ together or alone; either one, given at all,
+ writes a probe block — omitting both leaves the
+ existing probe alone)
+
Operators placing routes by hand on a box use carlos add, not this.
`
@@ -240,6 +269,9 @@ func runInstancesCreate(args []string) {
host := fs.String("host", "", "instance host — one DNS label + \".\" + the app's instance domain")
region := fs.String("region", "", "region code for the record (subject to the manifest's allow-list)")
channel := fs.String("channel", "", "release channel the instance follows (default: the manifest's, then stable)")
+ addr := fs.String("addr", "", "comma-separated TCP upstream set, host:port each (e.g. 10.0.0.1:8090,10.0.0.2:8090); omitted means the platform's own default upstream")
+ healthPath := fs.String("health-path", "", "probe request path (default \"/\"); only meaningful alongside more than one -addr")
+ healthStatus := fs.Int("health-status", 0, "probe's expected response status (default 200)")
consoleFlag := fs.String("console", "", "console to create through (default: the only logged-in one)")
acctRaw := fs.String("account", "ops", "account (sqid or name) the app belongs to")
acctIDRaw := fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes -account")
@@ -247,10 +279,14 @@ func runInstancesCreate(args []string) {
if *app == "" || *host == "" {
log.Fatal(instancesRecordsUsage)
}
+ set := map[string]bool{}
+ fs.Visit(func(f *flag.Flag) { set[f.Name] = true })
+ addrs := parseAddrList(*addr, set["addr"])
+ probe := parseProbeFlags(*healthPath, *healthStatus, set["health-path"], set["health-status"])
sess := requireAPISession("create", *consoleFlag)
printUsing(stdout, sess)
rec, err := sess.instanceCreate(context.Background(), apiAccount(acctRaw, acctIDRaw), *app,
- strings.TrimSpace(*host), strings.TrimSpace(*region), strings.TrimSpace(*channel))
+ strings.TrimSpace(*host), strings.TrimSpace(*region), strings.TrimSpace(*channel), addrs, probe)
if err != nil {
log.Fatalf("instances create: %v", err)
}
@@ -258,6 +294,56 @@ func runInstancesCreate(args []string) {
mutedf(stdout, " the host comes live on the next reconcile pass on its box")
}
+// parseAddrList splits a comma-separated -addr/-addrs flag into the wire's
+// []string, trimming whitespace and dropping empty elements the same way
+// -regions' comma-split does (instances_cli.go's runInstancesEnable). given
+// is fs.Visit's "was this flag on the command line at all" — an absent flag
+// must return nil (the wire's "leave/default", not an empty, explicit list),
+// which a bare `raw == ""` check cannot tell apart from `-addr ""`.
+func parseAddrList(raw string, given bool) []string {
+ if !given {
+ return nil
+ }
+ var out []string
+ for _, a := range strings.Split(raw, ",") {
+ if a = strings.TrimSpace(a); a != "" {
+ out = append(out, a)
+ }
+ }
+ return out
+}
+
+// parseProbeFlags builds the wire's optional probe block from -health-path/
+// -health-status: nil when NEITHER flag was given (leave/default, matching
+// parseAddrList's given-vs-absent split), populated when either was — a
+// caller may set only the path, only the status, or both, and this is where
+// that distinction is made before it reaches the request struct's `omitempty`
+// pointer.
+func parseProbeFlags(path string, status int, pathGiven, statusGiven bool) *instanceProbeReq {
+ if !pathGiven && !statusGiven {
+ return nil
+ }
+ return &instanceProbeReq{Path: strings.TrimSpace(path), Status: status}
+}
+
+// upstreamsCell renders the joined per-upstream probe state as "2/3"
+// (up/total) — "-" when the record carries none, which is the ordinary case
+// for a single-upstream record (the join is omitempty on the wire, so a
+// pre-Task-8 console and a plain single-upstream record both decode to a nil
+// slice here) and for a talking-to-an-old-console read.
+func upstreamsCell(ups []apiUpstreamState) string {
+ if len(ups) == 0 {
+ return "-"
+ }
+ up := 0
+ for _, u := range ups {
+ if u.Up {
+ up++
+ }
+ }
+ return fmt.Sprintf("%d/%d", up, len(ups))
+}
+
func runInstancesList(args []string) {
fs := flag.NewFlagSet("instances list", flag.ExitOnError)
app := fs.String("app", "", "app whose instance records to list")
@@ -276,13 +362,13 @@ func runInstancesList(args []string) {
log.Fatalf("instances list: %v", err)
}
w := newTable(stdout)
- fmt.Fprintln(w, "HOST\tREGION\tCHANNEL\tPOOL\tVERSION\tHEALTH\tCREATED\tBY")
+ fmt.Fprintln(w, "HOST\tREGION\tCHANNEL\tPOOL\tVERSION\tHEALTH\tUPSTREAMS\tCREATED\tBY")
for _, r := range recs {
rowHealth := r.Health
if rowHealth == "" {
rowHealth = "—" // an old console: the field predates it, not "unhealthy"
}
- fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n", r.Host, r.Region, r.Channel, r.Pool, r.StagedVersion, rowHealth, r.CreatedAt, r.CreatedBy)
+ fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n", r.Host, r.Region, r.Channel, r.Pool, r.StagedVersion, rowHealth, upstreamsCell(r.Upstreams), r.CreatedAt, r.CreatedBy)
}
w.Flush()
if truncated {
@@ -318,3 +404,59 @@ func runInstancesDelete(args []string) {
fmt.Fprintf(stdout, "%s %s — its route retires on the next reconcile pass; parked data stays\n",
colorFor(stdout).ok("deleted instance"), *host)
}
+
+// runInstancesSetUpstreams is `carlos instances set-upstreams` — the CLI
+// front over handleAPIInstanceUpdate's PATCH (Task 7, issue #152): repoint a
+// live instance's upstream set and/or its probe without a box shell. Unlike
+// create/list/delete this is an update to an ALREADY-provisioned record, so
+// there is deliberately no -region/-channel here — those have their own
+// doors (placement, and there is none for channel via this record; see
+// spec §9).
+//
+// Neither -addrs nor a -health-* flag is required by this parser: the
+// console's own refusal ("nothing to update — send addrs, probe, or both")
+// is left to fire when a caller gives neither, so that refusal's wording
+// reaches the operator verbatim rather than being paraphrased here — the
+// self-review rule this task carries (ValidateShape's own sentences are
+// operator guidance, not something to summarize).
+func runInstancesSetUpstreams(args []string) {
+ fs := flag.NewFlagSet("instances set-upstreams", flag.ExitOnError)
+ app := fs.String("app", "", "app the instance belongs to")
+ host := fs.String("host", "", "instance host whose upstream set to repoint")
+ addrsFlag := fs.String("addrs", "", "comma-separated TCP upstream set, host:port each, replacing the record's whole upstream set; omitted leaves the existing set alone")
+ healthPath := fs.String("health-path", "", "replaces the probe's request path")
+ healthStatus := fs.Int("health-status", 0, "replaces the probe's expected response status")
+ consoleFlag := fs.String("console", "", "console to update through (default: the only logged-in one)")
+ acctRaw := fs.String("account", "ops", "account (sqid or name) the app belongs to")
+ acctIDRaw := fs.String("account-id", "", "account sqid, matched as a sqid ONLY — supersedes -account")
+ fs.Parse(args)
+ if *app == "" || *host == "" {
+ log.Fatal(instancesRecordsUsage)
+ }
+ set := map[string]bool{}
+ fs.Visit(func(f *flag.Flag) { set[f.Name] = true })
+ var addrs *[]string
+ if set["addrs"] {
+ list := parseAddrList(*addrsFlag, true)
+ if list == nil {
+ // -addrs given but empty (or all-whitespace) — the wire's explicit
+ // "clear the set" spelling ("addrs":[]), not "leave it alone"
+ // (which is what a nil *[]string would mean). parseAddrList's
+ // given=true already tells nil from absent for this caller;
+ // this turns that nil into the non-nil empty slice the PATCH
+ // door requires to see a clear rather than a no-op.
+ list = []string{}
+ }
+ addrs = &list
+ }
+ probe := parseProbeFlags(*healthPath, *healthStatus, set["health-path"], set["health-status"])
+ sess := requireAPISession("set-upstreams", *consoleFlag)
+ printUsing(stdout, sess)
+ rec, err := sess.instanceSetUpstreams(context.Background(), apiAccount(acctRaw, acctIDRaw), *app,
+ strings.TrimSpace(*host), addrs, probe)
+ if err != nil {
+ log.Fatalf("instances set-upstreams: %v", err)
+ }
+ fmt.Fprintf(stdout, "%s %s\n", colorFor(stdout).ok("updated upstreams for"), rec.Host)
+ mutedf(stdout, " the change lands on its box within seconds of the next converge tick")
+}
diff --git a/instances_cli_test.go b/instances_cli_test.go
index 1a45822e2..9a3c80ccf 100644
--- a/instances_cli_test.go
+++ b/instances_cli_test.go
@@ -53,6 +53,78 @@ func TestInstancesEnableWritesManifest(t *testing.T) {
}
}
+// TestUpstreamsCell pins the LIST column's rendering (Task 8): "up/total"
+// when the record carries a joined upstream table, and the literal "-" the
+// task brief specifies for the absent case — deliberately not HEALTH's em
+// dash "—", since an absent upstream join means "not a multi-upstream
+// record" (the overwhelming majority), not "an old console".
+func TestUpstreamsCell(t *testing.T) {
+ cases := []struct {
+ name string
+ ups []apiUpstreamState
+ want string
+ }{
+ {"nil", nil, "-"},
+ {"empty", []apiUpstreamState{}, "-"},
+ {"all up", []apiUpstreamState{{Addr: "a:1", Up: true}, {Addr: "b:1", Up: true}}, "2/2"},
+ {"one down", []apiUpstreamState{{Addr: "a:1", Up: true}, {Addr: "b:1", Up: false}, {Addr: "c:1", Up: true}}, "2/3"},
+ {"all down", []apiUpstreamState{{Addr: "a:1", Up: false}}, "0/1"},
+ }
+ for _, c := range cases {
+ if got := upstreamsCell(c.ups); got != c.want {
+ t.Errorf("upstreamsCell(%q) = %q, want %q", c.name, got, c.want)
+ }
+ }
+}
+
+// TestParseAddrList pins the given-vs-absent split -addr/-addrs share
+// (Task 8): a flag never named on the command line must return nil (the
+// wire's "leave/default"), never mistaken for the empty list an explicitly
+// blank value produces.
+func TestParseAddrList(t *testing.T) {
+ if got := parseAddrList("anything", false); got != nil {
+ t.Errorf("parseAddrList(_, given=false) = %v, want nil", got)
+ }
+ if got := parseAddrList("", true); got != nil {
+ t.Errorf("parseAddrList(\"\", given=true) = %v, want nil (no elements)", got)
+ }
+ if got := parseAddrList(" , ", true); got != nil {
+ t.Errorf("parseAddrList(whitespace-only, given=true) = %v, want nil", got)
+ }
+ got := parseAddrList(" a:1 , b:2 ,,c:3", true)
+ want := []string{"a:1", "b:2", "c:3"}
+ if len(got) != len(want) {
+ t.Fatalf("parseAddrList = %v, want %v", got, want)
+ }
+ for i := range want {
+ if got[i] != want[i] {
+ t.Fatalf("parseAddrList = %v, want %v", got, want)
+ }
+ }
+}
+
+// TestParseProbeFlags pins the same given-vs-absent split for
+// -health-path/-health-status: nil unless at least one was named, so a
+// caller touching neither never sends a "probe" key at all — the door's own
+// "leave the probe alone" contract (apiinstances.go).
+func TestParseProbeFlags(t *testing.T) {
+ if got := parseProbeFlags("", 0, false, false); got != nil {
+ t.Errorf("parseProbeFlags(neither given) = %+v, want nil", got)
+ }
+ got := parseProbeFlags("/healthz", 0, true, false)
+ if got == nil || got.Path != "/healthz" || got.Status != 0 {
+ t.Errorf("parseProbeFlags(path only) = %+v, want {Path: /healthz, Status: 0}", got)
+ }
+ got = parseProbeFlags("", 204, false, true)
+ if got == nil || got.Path != "" || got.Status != 204 {
+ t.Errorf("parseProbeFlags(status only) = %+v, want {Path: \"\", Status: 204}", got)
+ }
+ got = parseProbeFlags(" /up ", 200, true, true)
+ if got == nil || got.Path != "/up" || got.Status != 200 {
+ t.Errorf("parseProbeFlags(both) = %+v, want {Path: /up, Status: 200}", got)
+ }
+}
+
func TestParseSeed(t *testing.T) {
cases := []struct {
in string
diff --git a/instances_e2e_test.go b/instances_e2e_test.go
index b23d3d13d..57c13f0f5 100644
--- a/instances_e2e_test.go
+++ b/instances_e2e_test.go
@@ -136,6 +136,65 @@ func TestE2EInstancesThroughConsole(t *testing.T) {
t.Fatalf("record survived delete")
}
+ // --- create with -addr/-health-*, then set-upstreams (Task 8) ----------
+ haHost := "ha." + domain
+ buf.Reset()
+ runInstancesCreate([]string{"-app", "titogo", "-account", "ops", "-host", haHost,
+ "-addr", "10.0.0.1:8090,10.0.0.2:8090", "-health-path", "/healthz", "-health-status", "204"})
+ if want := "created instance " + haHost; !strings.Contains(buf.String(), want) {
+ t.Fatalf("create -addr stdout = %q, want %q", buf.String(), want)
+ }
+ haRec, err := provision.GetRecord(context.Background(), relStore, release.OpsAccount, "titogo", haHost)
+ if err != nil {
+ t.Fatalf("record after create -addr: %v", err)
+ }
+ if len(haRec.Addrs) != 2 || haRec.Addrs[0] != "10.0.0.1:8090" || haRec.Addrs[1] != "10.0.0.2:8090" {
+ t.Fatalf("record.Addrs = %v, want the two -addr entries", haRec.Addrs)
+ }
+ if haRec.Health == nil || haRec.Health.Path != "/healthz" || haRec.Health.Status != 204 {
+ t.Fatalf("record.Health = %+v, want {Path: /healthz, Status: 204}", haRec.Health)
+ }
+
+ // list must show the UPSTREAMS column, "-" here since no box has
+ // reported this fleet-fresh host through the status join yet.
+ buf.Reset()
+ runInstancesList([]string{"-app", "titogo", "-account", "ops"})
+ listOut := buf.String()
+ if !strings.Contains(listOut, "UPSTREAMS") {
+ t.Fatalf("list stdout = %q, want an UPSTREAMS column header", listOut)
+ }
+ if !strings.Contains(listOut, haHost) {
+ t.Fatalf("list stdout = %q, want the multi-upstream host listed", listOut)
+ }
+
+ // set-upstreams repoints the whole set and probe in one PATCH.
+ buf.Reset()
+ runInstancesSetUpstreams([]string{"-app", "titogo", "-account", "ops", "-host", haHost,
+ "-addrs", "10.0.0.3:8090", "-health-path", "/up", "-health-status", "200"})
+ if want := "updated upstreams for " + haHost; !strings.Contains(buf.String(), want) {
+ t.Fatalf("set-upstreams stdout = %q, want %q", buf.String(), want)
+ }
+ haRec, err = provision.GetRecord(context.Background(), relStore, release.OpsAccount, "titogo", haHost)
+ if err != nil {
+ t.Fatalf("record after set-upstreams: %v", err)
+ }
+ if len(haRec.Addrs) != 1 || haRec.Addrs[0] != "10.0.0.3:8090" {
+ t.Fatalf("record.Addrs after set-upstreams = %v, want [10.0.0.3:8090]", haRec.Addrs)
+ }
+ if haRec.Health == nil || haRec.Health.Path != "/up" || haRec.Health.Status != 200 {
+ t.Fatalf("record.Health after set-upstreams = %+v, want {Path: /up, Status: 200}", haRec.Health)
+ }
+
+ // set-upstreams naming NEITHER -addrs nor a -health-* flag reaches the
+ // console's own refusal, surfaced verbatim rather than a client-side
+ // message that would paraphrase it (self-review rule this task carries).
+ // That path is log.Fatal, same as every other malformed-invocation path
+ // in this file (instances_cli_test.go's own doc comment: no argv test
+ // harness exists here for os.Exit paths) — pinned instead at the client
+ // layer, where the request/response round trip is actually observable:
+ // TestInstanceSetUpstreamsSurfacesRefusalVerbatim (apimode_test.go)
+ // proves instanceSetUpstreams returns the door's error text unmodified.
+
// The whole flow ran through the console: nothing here may have touched
// a local registry.
checkNoRegistry()
diff --git a/internal/registry/registry.go b/internal/registry/registry.go
index 18ccb5aa2..f01da1c55 100644
--- a/internal/registry/registry.go
+++ b/internal/registry/registry.go
@@ -654,12 +654,16 @@ func (r *Registry) Delete(host string) error {
// branch (reconcile.go, the `existing.Provisioned && ...` case) converges
// through narrow setters (convergeOwned, SetHibernate, SetSeed), never a
// full-row Upsert, so a reconcile tick on an already-owned host cannot blank
-// what SetUpstreams wrote. The one remaining plain-overwrite blanker is
-// `carlos add` re-run over an existing host (cli.go:194-198): its Upsert
-// builds a Route literal with no Addrs/Health fields, so re-adding a host
-// with an established multi-upstream set still blanks it, the same way a
-// plain excluded.environment once silently unbound a route. That gap is
-// this CLI verb's, not routeFor's, and is the next task's job to close.
+// what SetUpstreams wrote. The one remaining plain-overwrite risk was
+// `carlos add` re-run over an existing host: its Upsert builds a Route
+// literal from flags alone, none of which name Addrs/Health, so re-adding a
+// host with an established multi-upstream set would blank it, the same way a
+// plain excluded.environment once silently unbound a route. Closed at the
+// CLI's own door (cli.go's runAdd, Task 8): when a route already exists for
+// the host, its Addrs/Health (and Addr, when the invocation named none of its
+// own) are copied onto the literal before this call, the same
+// preserve-what-you-don't-manage shape as the CASE columns above, just done
+// by the caller instead of the ON CONFLICT clause.
func (r *Registry) Upsert(t Route) error {
if t.CreatedAt == 0 {
t.CreatedAt = time.Now().Unix()
@@ -919,11 +923,11 @@ func (r *Registry) SetUnit(host, unit string) error {
// — but provision.routeFor now carries Record.Addrs/Health onto every route
// it mints, and the reconcile pass's owned-route branch never calls Upsert
// (it converges through narrow setters, this one included), so a reconcile
-// tick cannot blank what a prior SetUpstreams call wrote. The surviving
-// plain-overwrite hazard is `carlos add` re-run over an existing host
-// (cli.go:194-198): its Upsert carries no Addrs/Health, so re-adding a host
-// still blanks an established multi-upstream set — a CLI-side gap, not a
-// SetUpstreams or reconcile one, and the next task's job to close.
+// tick cannot blank what a prior SetUpstreams call wrote. The one other
+// plain-overwrite path, `carlos add` re-run over an existing host, is closed
+// at the CLI's own door (cli.go's runAdd, Task 8) rather than here: that verb
+// now copies an existing route's Addrs/Health onto the literal it builds
+// before calling Upsert, so a re-add cannot blank what this setter wrote.
//
// All THREE upstream columns move together, and that is the point: addr is
// the legacy single-upstream spelling of the same fact addrs carries, and
diff --git a/main.go b/main.go
index b7a85b745..b996e27c1 100644
--- a/main.go
+++ b/main.go
@@ -146,7 +146,8 @@ Your apps — ship and run software on a CARLOS deployment
it — "carlos version target -app <a> 0.5.0"
env plain per-app config vars: set | unset | list
secrets sealed per-app secrets: set | unset | list | genkey
- instances an app's instances: enable | create | list | delete
+ instances an app's instances: enable | create | list | delete |
+ set-upstreams
logs an app's merged log timeline: its own output plus platform
events (wakes, restarts, failures); -f follows live
ledger open ledgers: append | head | list | publish | blob | verify