amadan
server: deploy --all leaves pinned canaries alone (#797)
The pin only ever gated the hourly auto-updater; a manual fleet promote (deploy --all --version) moved every instance, pinned or not — which stomped a live canary today while its pin sat intact. The sweep now skips pinned hosts loudly (--include-pinned to override, naming a host still deploys it). Failure split on purpose: unreadable hibernating prefs fail closed like the updater; a dead running socket deploys anyway (the sweep is a crashed host's rescue path). --pin with --all now refuses (a fleet-wide pin is never right).
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
4 files changed,
+123
−3
CLAUDE.md+3 −1control.go+44 −1deploy_pin_test.go+71 −0docs/instance-router.md+5 −1
diff --git a/CLAUDE.md b/CLAUDE.md| index 0f933a9..22996f9 100644 |
| --- a/CLAUDE.md |
| +++ b/CLAUDE.md |
| @@ -512,7 +512,9 @@ sessions (they trample each other): |
| stored in 1Password — everything about them dies with the session. |
| - Corollary for fleet-wide operations (channel changes, unpin sweeps, |
| `deploy --all`): check `lchat update status` first — every `AUTO=pinned` |
| - row is someone's live canary; leave it alone. |
| + row is someone's live canary; leave it alone. (`deploy --all --version` |
| + now skips pinned rows by itself; `--include-pinned` is the deliberate |
| + override. The check still matters for channel changes and unpin sweeps.) |
| ## Dev CLI (`tools/dev`) |
diff --git a/control.go b/control.go| index 64d06c4..e962630 100644 |
| --- a/control.go |
| +++ b/control.go |
| @@ -843,9 +843,10 @@ func runDeploy(args []string) { |
| fs := flag.NewFlagSet("deploy", flag.ExitOnError) |
| registryPath := fs.String("registry", envOr("LCHAT_REGISTRY", "/data/registry.db"), "router registry SQLite path") |
| unitTmpl := fs.String("unit", envOr("LCHAT_UNIT", "lchat@"), "systemd instance unit template prefix") |
| - all := fs.Bool("all", false, "deploy every instance, one at a time") |
| + all := fs.Bool("all", false, "deploy every instance, one at a time (skips pinned canaries — see --include-pinned)") |
| verFlag := fs.String("version", "", "point the instance(s) at this staged version before restarting (see `lchat stage`)") |
| pinFlag := fs.Bool("pin", false, "also pin the instance(s) to --version, so the auto-updater leaves them alone (the canary flow)") |
| + includePinned := fs.Bool("include-pinned", false, "let --all move pinned instances too (a pinned host is someone's live canary — override deliberately)") |
| _ = fs.Parse(reorderFlagsFirst(fs, args)) |
| ver := strings.TrimSpace(*verFlag) |
| @@ -860,6 +861,9 @@ func runDeploy(args []string) { |
| if *pinFlag && ver == "" { |
| die("deploy: --pin requires --version (a pin names one exact build)") |
| } |
| + if *pinFlag && *all { |
| + die("deploy: --pin with --all would pin the whole fleet — pin one canary at a time") |
| + } |
| unlock, err := acquireDeployLock() |
| if err != nil { |
| @@ -887,6 +891,20 @@ func runDeploy(args []string) { |
| if rt.Kind != "instance" { |
| die("deploy %s: not an instance (kind %q) — only messenger instances are deployed", host, rt.Kind) |
| } |
| + // A fleet sweep leaves pinned instances alone: a pin is someone's live |
| + // canary (CLAUDE.md § per-session canaries), and until now only the |
| + // AUTO-updater honoured it — `deploy --all` was the loophole that |
| + // stomped a pinned canary onto the channel build while its pin sat |
| + // intact. Naming a host explicitly still deploys it (that's intent); |
| + // --include-pinned overrides the sweep on purpose. |
| + if *all && ver != "" && !*includePinned { |
| + if skip, why := sweepSkipsPinned(rt, ver); skip { |
| + fmt.Printf("skipped %s (%s — move it with `lchat deploy %s --version %s --pin`, or rerun with --include-pinned)\n", host, why, host, ver) |
| + continue |
| + } else if why != "" { |
| + fmt.Printf("note: %s: %s\n", host, why) |
| + } |
| + } |
| if rt.Hibernate { |
| // No unit to restart — and starting one would fight the activator's |
| // child for the socket. A --version deploy is still meaningful as a |
| @@ -963,6 +981,31 @@ func runDeploy(args []string) { |
| } |
| } |
| +// sweepSkipsPinned decides whether a fleet sweep (`deploy --all --version`) |
| +// leaves this host alone, returning the reason to print either way. Split |
| +// failure policies, both deliberate: |
| +// - a RUNNING host whose prefs can't be fetched is probably crashed — the |
| +// sweep is how it gets rescued, so deploy anyway (with a note); |
| +// - a HIBERNATING host whose parked DB can't be read fails CLOSED (skip), |
| +// matching the auto-updater: its pin may live only in the unreadable |
| +// copy, and "the one host that must not move, moves" is the failure |
| +// that matters. It converges once a wake leaves a warm local DB. |
| +func sweepSkipsPinned(rt *Route, ver string) (skip bool, why string) { |
| + var prefs updatePrefs |
| + var err error |
| + if rt.Hibernate { |
| + if prefs, err = hibernatingPrefs(rt.Host); err != nil { |
| + return true, fmt.Sprintf("hibernating and its parked prefs are unreadable (%v) — pin unknown, left alone", err) |
| + } |
| + } else if prefs, err = fetchUpdatePrefs(rt.Socket); err != nil { |
| + return false, fmt.Sprintf("couldn't read update prefs (%v) — deploying anyway", err) |
| + } |
| + if prefs.Pin != "" && prefs.Pin != ver { |
| + return true, "pinned to " + prefs.Pin + " — a canary" |
| + } |
| + return false, "" |
| +} |
| + |
| // deployHostVersion is the one per-host apply dance, shared by `deploy |
| // --version` and the auto-updater (updater.go): repoint the live symlink at a |
| // staged version, restart, verify the instance actually reports that build, |
diff --git a/deploy_pin_test.go b/deploy_pin_test.go| new file mode 100644 |
| index 0000000..b51d557 |
| --- /dev/null |
| +++ b/deploy_pin_test.go |
| @@ -0,0 +1,71 @@ |
| +package main |
| + |
| +import ( |
| + "database/sql" |
| + "os" |
| + "path/filepath" |
| + "testing" |
| +) |
| + |
| +// A fleet sweep (`deploy --all --version`) must leave pinned instances alone — |
| +// the pin used to gate only the AUTO-updater, and a manual promote stomped a |
| +// pinned canary while its pin sat intact. sweepSkipsPinned is the decision. |
| +func TestSweepSkipsPinned(t *testing.T) { |
| + dir := t.TempDir() |
| + |
| + // A hibernating host reads prefs straight from its parked DB — build one |
| + // with a pin, exactly as the instance's own settings write would. |
| + prev := instanceDataDir |
| + instanceDataDir = dir |
| + defer func() { instanceDataDir = prev }() |
| + |
| + mkDB := func(host, pin string) { |
| + db, err := sql.Open("sqlite", filepath.Join(dir, host+".db")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer db.Close() |
| + if _, err := db.Exec("CREATE TABLE config (key TEXT PRIMARY KEY, value TEXT)"); err != nil { |
| + t.Fatal(err) |
| + } |
| + if pin != "" { |
| + if _, err := db.Exec("INSERT INTO config VALUES('update_pin', ?)", pin); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + } |
| + |
| + // Hibernating + pinned to something else → skipped. |
| + mkDB("pinned.example", "aaaa111") |
| + rt := &Route{Host: "pinned.example", Hibernate: true} |
| + if skip, why := sweepSkipsPinned(rt, "bbbb222"); !skip || why == "" { |
| + t.Fatalf("pinned canary must be skipped by the sweep, got skip=%v why=%q", skip, why) |
| + } |
| + // Pinned to the sweep's own target → deploy (harmless repoint/no-op). |
| + if skip, _ := sweepSkipsPinned(rt, "aaaa111"); skip { |
| + t.Fatal("a host pinned to the sweep's target version should deploy") |
| + } |
| + |
| + // Hibernating, no pin → deploys. |
| + mkDB("plain.example", "") |
| + rt2 := &Route{Host: "plain.example", Hibernate: true} |
| + if skip, _ := sweepSkipsPinned(rt2, "bbbb222"); skip { |
| + t.Fatal("an unpinned host must not be skipped") |
| + } |
| + |
| + // Hibernating with an UNREADABLE parked DB fails closed (the updater's |
| + // rule): the pin may live only in the copy we can't read. |
| + rt3 := &Route{Host: "missing.example", Hibernate: true} |
| + if skip, why := sweepSkipsPinned(rt3, "bbbb222"); !skip || why == "" { |
| + t.Fatalf("unreadable hibernating prefs must fail closed, got skip=%v why=%q", skip, why) |
| + } |
| + |
| + // A RUNNING host whose socket is dead is probably crashed — the sweep is |
| + // its rescue, so it deploys anyway (with a note, not silence). |
| + rt4 := &Route{Host: "down.example", Socket: filepath.Join(dir, "no-such.sock")} |
| + if skip, why := sweepSkipsPinned(rt4, "bbbb222"); skip || why == "" { |
| + t.Fatalf("a dead-socket running host should deploy with a note, got skip=%v why=%q", skip, why) |
| + } |
| + |
| + _ = os.Remove(filepath.Join(dir, "pinned.example.db")) |
| +} |
diff --git a/docs/instance-router.md b/docs/instance-router.md| index 2b1d9fa..fc86ddd 100644 |
| --- a/docs/instance-router.md |
| +++ b/docs/instance-router.md |
| @@ -653,7 +653,11 @@ touching anything real. |
| - `lchat deploy <host> [--version <ver>]` — zero-downtime restart; with `--version`, |
| repoint the host at that staged version and health-check the `build` (auto-rollback |
| on mismatch). |
| -- `lchat deploy --all [--version <ver>]` — roll the fleet, one instance at a time. |
| +- `lchat deploy --all [--version <ver>]` — roll the fleet, one instance at a |
| + time. A `--version` sweep **skips pinned instances** (a pin is someone's live |
| + canary; the pin used to gate only the auto-updater, and a manual promote |
| + could stomp it) — `--include-pinned` overrides deliberately, and naming a |
| + host explicitly always deploys it. |
| - `lchat rollback <host> [--all]` — step back to the recorded previous version. |
| - `lchat init-versions` — one-time adoption: seed the store + create `live` links |
| for existing instances. |