carlos / platform Public

deploy: watch success requires 2xx; restore API door bumps serial

watchVersion's success arm matched X-Carlos-Version alone, so a tenant
that died on boot could answer 502 with the new version header (the
edge stamps it before proxying) and deploy would exit 0. Success now
requires got == ver && 2xx; a matching header on a non-2xx stays
pending with a "serving <ver> but HTTP <code>" diagnostic, and the
stale comment claiming that shape was header-less is corrected.

handleAPIRestoreApp (apicli.go) now bumps the serial after a
successful restore, matching the web door — restore re-enables
instance minting, which is fullPass leg 1, and the API door was
silently missing the announcement. The web door's own comment
overclaimed that the trash side "already announces its way out of"
the hourly wait; corrected to say restore is the first to bump and
trash still relies on the next pass.

Also rewraps an overlong comment in stage_relay_test.go.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Paul Campbell 2 months ago bd43b87ea3f4e7d9b79251281b84265b7d983ea5 parent 3dbe025

6 files changed, +75 −11

deploy.go +17 −7
deploy_test.go +26 −0
internal/console/apicli.go +6 −0
internal/console/ownership.go +3 −2
internal/console/serialbump_test.go +21 −0
internal/host/stage_relay_test.go +2 −2

diff --git a/deploy.go b/deploy.go

index 1fd55715d..61bbecb4e 100644

--- a/deploy.go

+++ b/deploy.go

@@ -274,18 +274,28 @@ func watchVersion(w io.Writer, targets []watchTarget, ver string, timeout, inter

resp.Body.Close()

got := resp.Header.Get("X-Carlos-Version")

switch {

- case got == ver:

+ case got == ver && resp.StatusCode/100 == 2:

fmt.Fprintf(w, "%s https://%s\n", colorFor(w).ok("live"), host)

delete(pending, host)

+ case got == ver:

+ // The edge stamps X-Carlos-Version before proxying to the

+ // origin, so a tenant that dies on boot can still answer

+ // with the new version header attached to a non-2xx (a 502,

+ // most commonly). That is the new build in a crash loop, not

+ // a live deploy, so the success arm has to gate on status

+ // too — matching the header alone let a dead tenant exit 0.

+ last[host] = fmt.Sprintf("serving %s but HTTP %d — the app is failing", ver, resp.StatusCode)

case got == "" && resp.StatusCode/100 != 2:

// A non-2xx with no header is not an old edge — it is an edge

// that answered and had nothing good to serve: an activator

- // 503 because the wake failed or timed out, a 404 because the

- // route has not been minted yet, a 502 from a tenant that

- // died on boot. Reporting those as "edge predates this

- // feature — the hourly pass will adopt it" sends the operator

- // off to wait for a pass that will not help (review finding

- // 2): the status code is the diagnostic, so print it.

+ // 503 because the wake failed or timed out, or a 404 because

+ // the route has not been minted yet. (A 502 from a tenant

+ // that died on boot carries the version header — the edge

+ // stamps it before proxying — so that shape lands in the

+ // case above, not here.) Reporting those as "edge predates

+ // this feature — the hourly pass will adopt it" sends the

+ // operator off to wait for a pass that will not help (review

+ // finding 2): the status code is the diagnostic, so print it.

last[host] = fmt.Sprintf("not serving (HTTP %d) — instance failing to wake, or route not live yet", resp.StatusCode)

case got == "":

// 2xx and no header: something IS being served here, it just

diff --git a/deploy_test.go b/deploy_test.go

index 114383193..1f7dac812 100644

--- a/deploy_test.go

+++ b/deploy_test.go

@@ -74,6 +74,7 @@ func TestWatchVersionSucceedsWhenHeaderMatches(t *testing.T) {

mu.Lock()

w.Header().Set("X-Carlos-Version", ver)

mu.Unlock()

+ w.WriteHeader(http.StatusOK) // the 200 path this test pins stays unchanged

}))

defer srv.Close()

go func() { time.Sleep(30 * time.Millisecond); mu.Lock(); ver = "new5678"; mu.Unlock() }()

@@ -104,6 +105,31 @@ func TestWatchVersionTimeoutDiagnostics(t *testing.T) {

}

+// TestWatchVersionMatchingHeaderNon2xxNotLive pins the fix: the edge stamps

+// X-Carlos-Version before proxying, so a tenant that dies on boot can answer

+// with the new version header attached to a 502. Matching the header alone

+// used to report that as "live" and exit 0 on a crash-looping deploy — the

+// success arm must gate on status too.

+func TestWatchVersionMatchingHeaderNon2xxNotLive(t *testing.T) {

+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

+ w.Header().Set("X-Carlos-Version", "new5678")

+ w.WriteHeader(http.StatusBadGateway)

+ }))

+ defer srv.Close()

+ var out bytes.Buffer

+ err := watchVersion(&out, []watchTarget{{Host: "x.test", Origin: srv.URL}}, "new5678",

+ 50*time.Millisecond, 10*time.Millisecond)

+ if err == nil {

+ t.Fatal("a 502 carrying the new version header must NOT be reported live")

+ }

+ if strings.Contains(out.String(), "live") {

+ t.Fatalf("must not print live: %q", out.String())

+ }

+ if !strings.Contains(err.Error(), "serving new5678 but HTTP 502") {

+ t.Fatalf("diagnostic must name the version and the failing status: %v", err)

+ }

// TestWatchVersionNoHeaderDiagnostic: 2xx with no header is the ONE shape

// that really means "this edge predates X-Carlos-Version" — something is

// being served here, it just isn't stamped.

diff --git a/internal/console/apicli.go b/internal/console/apicli.go

index 2880051b1..e75ac7e1d 100644

--- a/internal/console/apicli.go

+++ b/internal/console/apicli.go

@@ -643,6 +643,12 @@ func (s *Server) handleAPIRestoreApp(w http.ResponseWriter, r *http.Request) {

return

}

s.audit(acct, app, addr, "restore", "restored "+app+" from the trash (cli)")

+ // Box-consumed (carlos-deploy §3), same as the web door (handleRestoreApp,

+ // ownership.go): removing the trash marker re-enables instance minting for

+ // this app, and minting from records is leg 1 of the fast pass's reconcile.

+ // Without the bump a restore through this door is invisible to the fleet

+ // until the hourly floor comes round.

+ s.bumpSerial(ctx)

writeJSON(w, http.StatusOK, map[string]any{

"result": "restored",

"note": app + " is restored with its releases and channels intact, and with NO routes:" +

diff --git a/internal/console/ownership.go b/internal/console/ownership.go

index 6e6055b69..799d8f017 100644

--- a/internal/console/ownership.go

+++ b/internal/console/ownership.go

@@ -246,8 +246,9 @@ func (s *Server) handleRestoreApp(w http.ResponseWriter, r *http.Request) {

// Box-consumed (carlos-deploy §3): removing the trash marker re-enables

// instance minting for this app, and minting from records is leg 1 of the

// fast pass's reconcile. Without the bump a restore is invisible to the

- // fleet until the hourly floor comes round — the same wait the delete side

- // already announces its way out of.

+ // fleet until the hourly floor comes round. Restore is the first door on

+ // this page to announce its way out of that wait — the trash (purge) side

+ // still relies on the next hourly pass; it does not bump.

s.bumpSerial(r.Context())

// The anti-ghost rule, said where the person can read it: restoring brings

// back releases and channels and no routes at all (spec §3), so an app

diff --git a/internal/console/serialbump_test.go b/internal/console/serialbump_test.go

index bbfe715fb..af46aabff 100644

--- a/internal/console/serialbump_test.go

+++ b/internal/console/serialbump_test.go

@@ -248,6 +248,27 @@ func TestAPIRollbackBumpsSerial(t *testing.T) {

}

+// TestAPIRestoreBumpsSerial: restore through the CLI/API door re-enables

+// instance minting for the app (removing the trash marker), which is leg 1 of

+// the fast pass's reconcile — the same reason the web door (TestWebRestoreBumpsSerial)

+// bumps. Without it, an app restored over the API stays invisible to the

+// fleet until the hourly floor comes round.

+func TestAPIRestoreBumpsSerial(t *testing.T) {

+ fx, token := trashFixture(t, "code-serial-restore")

+ if status, out := postBody(t, fx.ts.URL+"/api/cli/apps/bab/titogo/delete", token, ""); status != http.StatusOK {

+ t.Fatalf("trash = %d %s", status, out)

+ }

+ before := serial(t, fx)

+ status, out := postBody(t, fx.ts.URL+"/api/cli/apps/bab/titogo/restore", token, "")

+ if status != http.StatusOK {

+ t.Fatalf("restore = %d %s", status, out)

+ }

+ if got := serial(t, fx); got != before+1 {

+ t.Fatalf("serial after an API restore = %d, want %d", got, before+1)

+ }

// TestAPIConfigPutBumpsSerial: the config write the box's exec config-sync

// leg consumes — the difference between an env var arriving in seconds and

// arriving on the hour.

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

index d9db7a4e2..41f5d5f51 100644

--- a/internal/host/stage_relay_test.go

+++ b/internal/host/stage_relay_test.go

@@ -62,8 +62,8 @@ func TestStageRequesterThrottles(t *testing.T) {

}

// Each loop gets its own budget: the fast converge loop asking must not

-// silence the hourly one. stageRequester returns a fresh closure per call, so this is a

-// property of the API shape rather than of the clock.

+// silence the hourly one. stageRequester returns a fresh closure per call,

+// so this is a property of the API shape rather than of the clock.

func TestStageRequesterBudgetIsPerRequester(t *testing.T) {

dd := t.TempDir()

marker := filepath.Join(dd, stageRequestMarker)