carlos / platform Public

cli: deploy e2e, and a watch diagnostic that names the status code

Two review findings, both on the deploy verb's last leg.

The acceptance debt (spec §9): `carlos deploy` is a composition, and
nothing exercised the composition. deploy_e2e_test.go drives the real
verb against a real console.Server over a DirStore, with a real
device-flow login — the shape instances/config already use — and proves
the whole line: ship lands a signed manifest, promote moves the channel
the INSTANCE RECORD named (never typed on the command line), and the
watch keeps polling until the edge serves the new version rather than
trusting the promote. The edge stand-in serves the old version to the
first poll for exactly that reason. A re-run of the same version — the
CI-retry case — is asserted too: no-op, no-op, still exits 0.

The watch leg has to dial a loopback stand-in for the edge, so
watchOrigin (deploy.go) becomes an unexported package var. Production
is unchanged and unchangeable from outside: https://<host>, the URL a
person visits. Credential isolation is useTempCredentials — the
in-process equivalent of carlosTestEnv + cmd.Dir, masking the same four
inputs (CARLOS_CREDENTIALS, HOME, CARLOS_CONSOLE, cwd).

Finding 2: a non-2xx WITHOUT the version header was being reported as
"edge predates this feature — the hourly pass will adopt it". That is
what an activator 503 (wake failed), a 404 (route not minted yet) and a
502 (tenant died on boot) all look like, and it sends the operator off
to wait an hour for a pass that cannot help. Those now read "not
serving (HTTP <code>) — instance failing to wake, or route not live
yet"; the old-edge wording is kept for the one shape that really means
it, 2xx with no header.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell ff327b694c3c85d6edb97da216db38a8f1552bdc parent ada7373
3 files changed, +290 −1
  • deploy.go +28 −1
  • deploy_e2e_test.go +231 −0
  • deploy_test.go +31 −0
diff --git a/deploy.go b/deploy.go
index 1aa54df07..1fd55715d 100644
--- a/deploy.go
+++ b/deploy.go
@@ -105,7 +105,7 @@ func runDeploy(args []string) {
}
targets := make([]watchTarget, 0, len(hosts))
for _, h := range hosts {
- targets = append(targets, watchTarget{Host: h, Origin: "https://" + h})
+ targets = append(targets, watchTarget{Host: h, Origin: watchOrigin(h)})
}
if err := watchVersion(stdout, targets, *ver, *timeout, 2*time.Second); err != nil {
log.Fatalf("deploy: %v", err)
@@ -228,6 +228,20 @@ func filterChannelHosts(recs []apiInstanceRecord, channel, hostFlag string) ([]s
// unit tests, which is never the same string as Host).
type watchTarget struct{ Host, Origin string }
+// watchOrigin maps an instance host to the origin the watch dials. In
+// production this is always https://<host> — the public URL the operator
+// will visit, with no scheme or port to configure, because that is the only
+// thing an instance record's host ever means.
+//
+// It is a var solely so the deploy e2e (deploy_e2e_test.go) can point the
+// watch leg at a loopback httptest stand-in for the edge: a test server
+// lives at 127.0.0.1:<port>, which is never a routable instance hostname,
+// so without this seam the composed verb could only be tested up to the
+// promote. Nothing in the binary reassigns it, and nothing should: an
+// operator-configurable scheme here would be a way to deploy while watching
+// something other than the URL people actually visit.
+var watchOrigin = func(host string) string { return "https://" + host }
+
// watchVersion polls each target's origin until it serves ver on
// X-Carlos-Version — the deploy verb's exit criterion (spec §6 step 5, §7).
// Every poll appends a query parameter that changes from the previous poll
@@ -263,7 +277,20 @@ func watchVersion(w io.Writer, targets []watchTarget, ver string, timeout, inter
case got == ver:
fmt.Fprintf(w, "%s https://%s\n", colorFor(w).ok("live"), host)
delete(pending, host)
+ 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.
+ 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
+ // isn't stamped — the only shape that really is an edge from
+ // before X-Carlos-Version existed.
last[host] = "no X-Carlos-Version header (edge predates this feature — the hourly pass will adopt it)"
default:
last[host] = "still serving " + got
diff --git a/deploy_e2e_test.go b/deploy_e2e_test.go
new file mode 100644
index 000000000..64a09c242
--- /dev/null
+++ b/deploy_e2e_test.go
@@ -0,0 +1,231 @@
+// End-to-end acceptance test for `carlos deploy` (spec §9's acceptance
+// debt): the one member-facing verb that is nothing but a composition —
+// ship, promote to the channel this app's instances already follow, then
+// watch the URL until it serves the new version — driven against a REAL
+// console.Server over a DirStore, with a real device-flow login, exactly
+// like TestE2EInstancesThroughConsole (instances_e2e_test.go) and
+// TestConfigE2EThroughConsole (config_e2e_test.go).
+//
+// Two deliberate choices, both worth stating:
+//
+// 1. The verb runs IN-PROCESS (runDeploy), not as a subprocess of the
+// built binary. The watch leg has to be pointed at a loopback stand-in
+// for the edge, which needs the watchOrigin seam (deploy.go) — an
+// unexported package var a subprocess could not reach without turning
+// it into an operator-visible env knob, which is exactly the thing that
+// seam's comment refuses. This is the same in-process shape the other
+// two console e2es use.
+// 2. The credential-isolation rule that governs the subprocess e2es
+// (carlosTestEnv + cmd.Dir, e2e_test.go:66 — a logged-in laptop running
+// `go test ./...` once minted 40+ accounts on the production console) is
+// honoured here by its in-process equivalent, useTempCredentials
+// (auth_test.go): it points CARLOS_CREDENTIALS, HOME, CARLOS_CONSOLE
+// *and* the working directory (t.Chdir — the .carlos/config ancestor
+// walk is why cmd.Dir exists) at scratch space, which is the same four
+// inputs carlosTestEnv+cmd.Dir masks for a subprocess. Nothing in this
+// test can reach the operator's real login, config, console or bucket.
+package main
+
+import (
+ "context"
+ "net/http"
+ "net/http/httptest"
+ "os"
+ "path/filepath"
+ "strings"
+ "sync"
+ "testing"
+ "time"
+
+ "github.com/carlosframework/platform/internal/console"
+ "github.com/carlosframework/platform/internal/provision"
+ "github.com/carlosframework/platform/internal/release"
+)
+
+// TestE2EDeployThroughConsole is the full verb: one command takes an
+// unshipped binary to a URL that serves it.
+func TestE2EDeployThroughConsole(t *testing.T) {
+ useTempCredentials(t)
+ clearDeploymentEnv(t)
+ checkNoRegistry := requireNoRegistryTouch(t)
+
+ const (
+ addr = "paul@keymail.dev"
+ app = "titogo"
+ channel = "canary/e2e"
+ version = "abc1234"
+ )
+
+ tmp := t.TempDir()
+ relStore := release.NewDirStore(filepath.Join(tmp, "store"))
+ seed, pubB64, err := release.GenerateKey()
+ if err != nil {
+ t.Fatal(err)
+ }
+ priv, err := release.ParseSigningKey(seed, "e2e")
+ if err != nil {
+ t.Fatal(err)
+ }
+ pub, err := release.ParsePublicKey(pubB64, "e2e")
+ if err != nil {
+ t.Fatal(err)
+ }
+ cst, err := console.Open(filepath.Join(tmp, "console.db"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { cst.Close() })
+
+ var srv *console.Server
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ srv.ServeHTTP(w, r)
+ }))
+ t.Cleanup(ts.Close)
+ srv, err = console.NewServer(console.Config{
+ Origin: ts.URL,
+ Keymail: "http://keymail.invalid", // never dialled — see e2eDeviceLogin
+ Registry: filepath.Join(tmp, "registry.db"),
+ Releases: relStore,
+ ReleasePub: pub,
+ Key: priv,
+ AppScheme: "http",
+ AppsDomain: "oncarlos.test",
+ }, cst)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ if err := cst.AddMember(console.OpsAccountID, addr, console.RoleOwner, time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := cst.CreateAppIn(console.OpsAccountID, app, addr, time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ token, expiresAt := e2eDeviceLogin(t, ts.URL, cst, addr, "e2e@laptop")
+ credFile, err := loadCredentials()
+ if err != nil {
+ t.Fatal(err)
+ }
+ credFile.put(credential{Console: ts.URL, Address: addr, Token: token, ExpiresAt: expiresAt})
+ if err := credFile.save(); err != nil {
+ t.Fatal(err)
+ }
+
+ // One instance, on a channel of its own: deploy resolves the promote
+ // target from THIS record, so nothing in the command line below names a
+ // channel at all.
+ host := app + ".bab.oncarlos.test"
+ buf := captureStdout(t)
+ runInstancesEnable([]string{"-app", app, "-account", "ops", "-channel", channel})
+ runInstancesCreate([]string{"-app", app, "-account", "ops", "-host", host})
+ rec, err := provision.GetRecord(context.Background(), relStore, release.OpsAccount, app, host)
+ if err != nil {
+ t.Fatalf("record after create: %v", err)
+ }
+ if rec.Channel != channel {
+ t.Fatalf("record channel = %q, want %q", rec.Channel, channel)
+ }
+
+ // The instance's edge, on loopback. It serves the PREVIOUS version to
+ // the first poll and the new one after that — so the watch leg is proven
+ // to keep polling rather than passing on whatever the first response
+ // happened to be, which is the whole reason the verb waits at all.
+ var mu sync.Mutex
+ polls := 0
+ probes := []string{}
+ edgeSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ mu.Lock()
+ polls++
+ probes = append(probes, r.URL.RawQuery)
+ served := "old0000"
+ if polls > 1 {
+ served = version // the adopt pass "landed" between polls
+ }
+ mu.Unlock()
+ w.Header().Set("X-Carlos-Version", served)
+ }))
+ t.Cleanup(edgeSrv.Close)
+
+ // The seam (deploy.go): production dials https://<host>; here the one
+ // host this deploy watches resolves to the loopback edge above. Any
+ // other host would be the verb watching something it was not asked to.
+ origOrigin := watchOrigin
+ watchOrigin = func(h string) string {
+ if h != host {
+ t.Errorf("watch dialled an unexpected host %q, want %q", h, host)
+ }
+ return edgeSrv.URL
+ }
+ t.Cleanup(func() { watchOrigin = origOrigin })
+
+ artifact := filepath.Join(tmp, "titogo-linux-arm64")
+ if err := os.WriteFile(artifact, []byte("#!/bin/true\n"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+
+ // --- the verb ------------------------------------------------------
+ buf.Reset()
+ runDeploy([]string{"-app", app, "-account", "ops", "-version", version, "-timeout", "30s", artifact})
+ out := buf.String()
+
+ // Ship happened, against the bucket, signed.
+ m, err := release.GetManifest(context.Background(), relStore, release.OpsAccount, app, version)
+ if err != nil {
+ t.Fatalf("manifest after deploy: %v", err)
+ }
+ if m.Version != version || m.Kind != "binary" {
+ t.Fatalf("manifest = %+v, want %s binary", m, version)
+ }
+ // Promote happened, onto the channel the RECORD named — never typed.
+ ptr, err := release.Channel(context.Background(), relStore, pub, release.OpsAccount, app, channel)
+ if err != nil {
+ t.Fatalf("channel after deploy: %v", err)
+ }
+ if ptr.Version != version {
+ t.Fatalf("%s points at %q, want %q", channel, ptr.Version, version)
+ }
+ // And the watch waited for the edge rather than trusting the promote:
+ // the first poll's answer was the old version.
+ mu.Lock()
+ gotPolls, gotProbes := polls, append([]string{}, probes...)
+ mu.Unlock()
+ if gotPolls < 2 {
+ t.Fatalf("edge polled %d times — the watch passed on a stale answer", gotPolls)
+ }
+ for _, q := range gotProbes {
+ if !strings.Contains(q, "carlos-deploy-probe=") {
+ t.Fatalf("poll without the cache-bust param: %v", gotProbes)
+ }
+ }
+
+ // The three lines an operator reads, in order, and the "live" line names
+ // the PUBLIC url — not the loopback origin the watch happened to dial.
+ for _, want := range []string{
+ "shipped " + app + " " + version,
+ "promoted " + app + " " + version + " -> " + channel,
+ "live https://" + host,
+ } {
+ if !strings.Contains(out, want) {
+ t.Fatalf("deploy stdout missing %q:\n%s", want, out)
+ }
+ }
+ if i, j := strings.Index(out, "shipped"), strings.Index(out, "promoted"); i > j {
+ t.Fatalf("promoted printed before shipped:\n%s", out)
+ }
+ if strings.Contains(out, edgeSrv.URL) {
+ t.Fatalf("deploy printed the test's loopback origin instead of the public URL:\n%s", out)
+ }
+
+ // A re-run of the same version is the CI-retry case: ship is a no-op,
+ // promote is a no-op, and the verb still exits 0 with the URL confirmed
+ // (the edge already serves it, so the first poll passes).
+ buf.Reset()
+ runDeploy([]string{"-app", app, "-account", "ops", "-version", version, "-timeout", "30s", artifact})
+ rerun := buf.String()
+ if !strings.Contains(rerun, "already shipped") || !strings.Contains(rerun, "live https://"+host) {
+ t.Fatalf("re-run of an identical deploy:\n%s", rerun)
+ }
+
+ // Nothing here may have opened a local registry: this is API mode.
+ checkNoRegistry()
+}
diff --git a/deploy_test.go b/deploy_test.go
index 12d291e45..114383193 100644
--- a/deploy_test.go
+++ b/deploy_test.go
@@ -2,6 +2,7 @@ package main
import (
"bytes"
+ "fmt"
"net/http"
"net/http/httptest"
"strings"
@@ -103,6 +104,9 @@ func TestWatchVersionTimeoutDiagnostics(t *testing.T) {
}
}
+// 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.
func TestWatchVersionNoHeaderDiagnostic(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
defer srv.Close()
@@ -114,6 +118,33 @@ func TestWatchVersionNoHeaderDiagnostic(t *testing.T) {
}
}
+// TestWatchVersionNonOKNoHeaderDiagnostic pins the split (review finding 2):
+// an activator 503 (wake failed) and a 404 (route not minted yet) also arrive
+// with no X-Carlos-Version, and calling those "the edge predates this
+// feature — the hourly pass will adopt it" sends the operator off to wait an
+// hour for a pass that cannot help. A non-2xx must name its status code and
+// the two things that actually produce it.
+func TestWatchVersionNonOKNoHeaderDiagnostic(t *testing.T) {
+ for _, code := range []int{http.StatusServiceUnavailable, http.StatusNotFound} {
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.WriteHeader(code)
+ }))
+ var out bytes.Buffer
+ err := watchVersion(&out, []watchTarget{{Host: "x.test", Origin: srv.URL}}, "new5678",
+ 50*time.Millisecond, 10*time.Millisecond)
+ srv.Close()
+ if err == nil {
+ t.Fatalf("HTTP %d: must time out", code)
+ }
+ if !strings.Contains(err.Error(), fmt.Sprintf("not serving (HTTP %d)", code)) {
+ t.Fatalf("HTTP %d diagnostic must name the status: %v", code, err)
+ }
+ if strings.Contains(err.Error(), "predates this feature") {
+ t.Fatalf("HTTP %d must not be reported as an old edge: %v", code, err)
+ }
+ }
+}
+
// TestWatchVersionCacheBustsEveryPoll pins the fix for the edge's up-to-5min
// cache TTL on X-Carlos-Version (internal/edge/edge.go, versionHeader): a
// watch with no cache-bust could poll the same cached response for the whole