carlos / platform Public

host: the belt reads systemctl's answer not its exit status; recovery needs no store

The hourly belt parsed nothing: is-active exits 3 for inactive, so every
unarmed box read as "systemctl unavailable". --recover no longer needs a
store or a pubkey, bounds its wait for the apply lock, and the backup of
carlos.prev is no longer journaled. Promoting straight to stable warns.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QCUCpfxbCyEAVQ2m51dH5
Paul Campbell pushed by paul@keymail.dev 785f0bc3c2bfea1d23bc3a53b01dab4a0bb2d207 parent 9255142
14 files changed, +573 −65
  • docs/superpowers/specs/2026-09-06-carlos-system-update-design.md +2 −1
  • internal/host/agent.go +5 −2
  • internal/host/systemapply.go +101 −20
  • internal/host/systemapply_test.go +146 −3
  • internal/host/systemapply_unix.go +5 −5
  • internal/host/systemapply_windows.go +0 −2
  • internal/host/systemleg.go +8 −2
  • internal/host/systemprobe.go +22 −4
  • internal/host/systemprobe_test.go +52 −6
  • internal/host/systemunits.go +57 −10
  • internal/host/systemunits_test.go +110 −8
  • internal/release/system.go +8 −2
  • system.go +23 −0
  • system_test.go +34 −0
diff --git a/docs/superpowers/specs/2026-09-06-carlos-system-update-design.md b/docs/superpowers/specs/2026-09-06-carlos-system-update-design.md
index b44999bc4..7db22cae1 100644
--- a/docs/superpowers/specs/2026-09-06-carlos-system-update-design.md
+++ b/docs/superpowers/specs/2026-09-06-carlos-system-update-design.md
@@ -419,7 +419,8 @@ outcome (a no-op request, §6) is shown as such, not as an apply.
Swaps `Current` with `History[0]` — whole releases, so the previous
artifacts and hashes come back with the version — re-signs with
-`PromotedAt` now, bumps the serial. Nothing GCs `bootstrap/router/` in
+`PromotedAt` now, bumps the serial. `History[0]` becomes `Current` and is
+removed from `History`; a second rollback goes one further back. Nothing GCs `bootstrap/router/` in
v1, so the objects are there. Every box rolls back through the same path
it rolled forward on; the box-side `.prev` (§6) is for the box that
cannot come up, not for this.
diff --git a/internal/host/agent.go b/internal/host/agent.go
index b05370e64..0208f0dc7 100644
--- a/internal/host/agent.go
+++ b/internal/host/agent.go
@@ -2354,7 +2354,10 @@ func statusLoop(reg *registry.Registry) {
DiskFreeBytes: readDiskFreeBytes(),
Release: version.Build,
Platform: runtime.GOOS + "-" + runtime.GOARCH,
- SystemChannel: systemChannel(os.Getenv),
+ // The invalid-value line is newSystemLeg's to say, once at
+ // arming; this tick publishes the channel the box actually
+ // follows, five-minutely, and has nowhere to say it again.
+ SystemChannel: boxSystemChannel(os.Getenv),
System: systemStateProbe(release.OptDir()),
Tunnel: tunnelState(),
// The edge's probe table, read WHOLE every tick (never merged
@@ -2859,7 +2862,7 @@ func RunAdopt(args []string) {
// Spec §7 belt 2: arm the self-update relay if the last hand roll did
// not. Before the fatal gates below, and never fatal itself.
if !*restartOnly {
- ensureSystemUnits("/etc", filepath.Join(systemRootDir(release.OptDir()), systemLockFile), productionSysOps().systemctl, log.Printf)
+ ensureSystemUnits("/etc", filepath.Join(systemRootDir(release.OptDir()), systemLockFile), productionSysOps().systemctl, systemctlProbe, log.Printf)
}
// openServiceRegistry, not registry.Open: this pass runs as root
// (carlos-adopt/-stage/-restart/-config-sync have no User=), opening the
diff --git a/internal/host/systemapply.go b/internal/host/systemapply.go
index 0be0eff27..f37f4f537 100644
--- a/internal/host/systemapply.go
+++ b/internal/host/systemapply.go
@@ -37,6 +37,11 @@ const (
systemGateSoak = 15 * time.Second
systemVersionWait = 30 * time.Second
systemBootstrapWait = 120 * time.Second
+
+ // How long --recover waits for the apply lock, and how often it asks.
+ // See waitForLock.
+ systemRecoverLockWait = 30 * time.Second
+ systemRecoverLockPoll = 500 * time.Millisecond
)
// systemQuiesceUnits are the root units that exec /opt/carlos/carlos
@@ -82,16 +87,12 @@ func RunSystemApply(args []string) {
log.Fatal("carlos system apply is the root oneshot behind carlos-system-update.service; it is not for operators")
}
ctx := context.Background()
- store, err := openDeployStore(ctx)
- if err != nil {
- log.Fatalf("system apply: %v", err)
- }
- pub, err := release.ParsePublicKey(os.Getenv("CARLOS_RELEASE_PUBKEY"), "CARLOS_RELEASE_PUBKEY")
- if err != nil {
- log.Fatalf("system apply: %v", err)
+ channel, invalidChannel := systemChannel(os.Getenv)
+ if invalidChannel != "" {
+ log.Printf("system apply: %s=%q is not a channel; this box follows %s", systemChannelEnv, invalidChannel, channel)
}
a := &systemApplier{
- ops: productionSysOps(), store: store, pub: pub, channel: systemChannel(os.Getenv),
+ ops: productionSysOps(), openStore: openSystemStore, channel: channel,
optDir: release.OptDir(), dataDir: dataDir(), etcDir: "/etc", build: version.Build,
platform: runtime.GOOS + "-" + runtime.GOARCH, gateWait: systemGateWait, soak: systemGateSoak,
}
@@ -100,6 +101,29 @@ func RunSystemApply(args []string) {
}
}
+// openSystemStore is the production openStore: the deploy store and the
+// release public key, opened at the moment the pointer is about to be read
+// and not one step sooner.
+//
+// The lateness is the point. `carlos system apply --recover` runs at boot,
+// before the network is up (its unit deliberately carries no
+// After=network-online.target, because the whole job is to put the box's
+// FILES back so the edge can start at all), and reads nothing from any
+// store. Opening a bucket and parsing CARLOS_RELEASE_PUBKEY up front made
+// recovery — the one path that must work on a broken box — depend on two
+// things a broken box is most likely to be missing.
+func openSystemStore(ctx context.Context) (release.Store, ed25519.PublicKey, error) {
+ store, err := openDeployStore(ctx)
+ if err != nil {
+ return nil, nil, err
+ }
+ pub, err := release.ParsePublicKey(os.Getenv("CARLOS_RELEASE_PUBKEY"), "CARLOS_RELEASE_PUBKEY")
+ if err != nil {
+ return nil, nil, err
+ }
+ return store, pub, nil
+}
+
// productionSysOps is the real box behind the seam.
func productionSysOps() sysOps {
return sysOps{
@@ -150,9 +174,11 @@ func productionSysOps() sysOps {
}
type systemApplier struct {
- ops sysOps
- store release.Store
- pub ed25519.PublicKey
+ ops sysOps
+ // openStore is called once, by verifyPointer, and only on a run that
+ // has a request to answer. --recover never calls it: see
+ // openSystemStore for why that matters at boot.
+ openStore func(ctx context.Context) (release.Store, ed25519.PublicKey, error)
channel string
optDir string
dataDir string
@@ -186,9 +212,14 @@ func (a *systemApplier) Run(ctx context.Context, recover bool) error {
}
defer lock.Close()
if recover {
- if err := flockExclusive(lock); err != nil {
+ got, err := a.waitForLock(lock)
+ if err != nil {
return err
}
+ if !got {
+ a.ops.logf("system apply: an apply holds the lock; it recovers at its own start")
+ return nil
+ }
} else if err := flockExclusiveNonblock(lock); err != nil {
if errors.Is(err, errSystemUnsupported) {
a.ops.logf("system apply: %v", err)
@@ -260,6 +291,37 @@ func (a *systemApplier) Run(ctx context.Context, recover bool) error {
return a.install(ctx, req, p, staged)
}
+// waitForLock is --recover's bounded wait for the apply lock: poll
+// systemRecoverLockPoll apart for up to systemRecoverLockWait, then give up.
+// It reports whether it got the lock; an unsupported platform is the one
+// error it returns.
+//
+// The bound is the fix for a real deadlock. An update run holds this lock
+// across `systemctl restart carlos-edge`, and carlos-system-recover.service
+// is ordered Before=carlos-edge.service — so if a recover unit's start job
+// is queued while an apply is mid-restart, systemd makes the edge's restart
+// wait on the recovery, and a recovery that blocked on the lock would wait
+// on the apply that is waiting on the restart. Both then sit there until
+// their unit timeouts fire. There is nothing for a recovery to do in that
+// window anyway: the apply owns the journal it would read, and finishes by
+// recovering from it itself if it dies.
+func (a *systemApplier) waitForLock(lock *os.File) (bool, error) {
+ deadline := a.ops.now().Add(systemRecoverLockWait)
+ for {
+ err := flockExclusiveNonblock(lock)
+ if err == nil {
+ return true, nil
+ }
+ if errors.Is(err, errSystemUnsupported) {
+ return false, err
+ }
+ if !a.ops.now().Before(deadline) {
+ return false, nil
+ }
+ a.ops.sleep(systemRecoverLockPoll)
+ }
+}
+
// install is steps 6–12: decide .prev, stage both unit sets, then the
// box-changing dance — each step journaled before it happens, and any
// failure from the quiesce on going to the rollback of step 13.
@@ -371,6 +433,17 @@ func (a *systemApplier) removeJournal() {
// applied and there is no .prev (a hand-rolled box whose running binary
// passed the script's own post-condition). Otherwise it leaves .prev alone
// and reports whether there is one to fall back to at all.
+//
+// The backup is deliberately NOT journaled, though every other box-changing
+// step is. copyFileDurable is whole-or-nothing — it writes carlos.prev.tmp
+// and renames — so a kill here leaves either the old .prev or the new one,
+// never a torn file, and a stray .prev.tmp is truncated by the next write.
+// A journal would make it WORSE: a phase written before the copy is a phase
+// the exec shim in carlos-system-recover.service reads at the next boot,
+// and it would then run a .prev that is one release older than the box, for
+// a whole apply, on the strength of a backup that had nothing to recover.
+// (recoverIfNeeded still knows systemPhaseBackingUp: journals written by
+// the version that did write one outlive the upgrade to this one.)
func (a *systemApplier) decidePrev(ctx context.Context) (bool, error) {
prev := systemPrevPath(a.optDir)
_, prevErr := os.Stat(prev)
@@ -383,13 +456,9 @@ func (a *systemApplier) decidePrev(ctx context.Context) (bool, error) {
if !known {
return prevErr == nil, nil
}
- if err := a.journal(systemJournal{Phase: systemPhaseBackingUp, Target: "", PrevBuild: disk, StartedAt: a.ops.now().UTC()}); err != nil {
- return false, err
- }
if err := copyFileDurable(systemBinaryPath(a.optDir), prev, 0o755); err != nil {
return false, err
}
- a.removeJournal()
return true, nil
}
@@ -754,9 +823,12 @@ func (a *systemApplier) recoverIfNeeded(ctx context.Context) (bool, error) {
}
a.ops.logf("system apply: recovering an apply of %s interrupted at %s", j.Target, j.Phase)
if j.Phase == systemPhaseBackingUp {
- // Nothing on the box had changed yet: only a half-written .prev
- // copy can be left, and it is not the file anything reads. The run
- // carries on to the request it was started for.
+ // A legacy journal: this version no longer writes one for the
+ // backup (see decidePrev), but a box killed mid-apply by the
+ // version that did comes up running this one. Nothing on the box
+ // had changed yet — only a half-written .prev copy can be left,
+ // and it is not the file anything reads. The run carries on to the
+ // request it was started for.
os.Remove(systemPrevPath(a.optDir) + ".tmp")
a.removeJournal()
return false, nil
@@ -856,8 +928,17 @@ func (a *systemApplier) readRequest() (systemRequest, error) {
// verifyPointer fetches THIS box's channel pointer and verifies it with
// the release public key. The channel is root's own (host.env), never the
// request's. A store error is deferred, a bad pointer refused.
+//
+// This is where the store is opened and the pubkey parsed — a store the box
+// cannot reach, or an unset CARLOS_RELEASE_PUBKEY, is deferred like any
+// other "could not read the pointer", recorded in result.json where an
+// operator sees it, rather than exiting before the run began.
func (a *systemApplier) verifyPointer(ctx context.Context) (*release.SystemPointer, error) {
- p, _, err := release.GetSystemPointer(ctx, a.store, a.pub, release.SystemComponentRouter, a.channel)
+ store, pub, err := a.openStore(ctx)
+ if err != nil {
+ return nil, fmt.Errorf("%w: %v", errSystemDeferred, err)
+ }
+ p, _, err := release.GetSystemPointer(ctx, store, pub, release.SystemComponentRouter, a.channel)
switch {
case err == nil:
return p, nil
diff --git a/internal/host/systemapply_test.go b/internal/host/systemapply_test.go
index 698de007d..a16ebe196 100644
--- a/internal/host/systemapply_test.go
+++ b/internal/host/systemapply_test.go
@@ -23,6 +23,7 @@ type applyFixture struct {
a *systemApplier
store *release.DirStore
priv ed25519.PrivateKey
+ pub ed25519.PublicKey
calls []string // "systemctl is-active carlos-edge", "run <base> version", …
answer map[string]string // "version:<path>" → stdout; "plan:<path>" → JSON plan
now time.Time
@@ -41,9 +42,11 @@ func newApplyFixture(t *testing.T, channel, build string) *applyFixture {
}
f := &applyFixture{t: t, store: release.NewDirStore(t.TempDir()), priv: priv, answer: map[string]string{},
now: time.Date(2026, 9, 6, 11, 0, 0, 0, time.UTC), edgeUp: true, nrestarts: "0", activeEnter: "1000", edgeStartedThisBoot: true}
+ f.pub = pub
f.a = &systemApplier{
- store: f.store, pub: pub, channel: channel,
- optDir: t.TempDir(), dataDir: t.TempDir(), etcDir: t.TempDir(), build: build,
+ openStore: func(context.Context) (release.Store, ed25519.PublicKey, error) { return f.store, f.pub, nil },
+ channel: channel,
+ optDir: t.TempDir(), dataDir: t.TempDir(), etcDir: t.TempDir(), build: build,
platform: "linux-arm64",
}
f.a.ops = sysOps{
@@ -304,7 +307,9 @@ func TestApplyNoOpNotesAndTouchesNothing(t *testing.T) {
func TestApplyStoreErrorIsDeferredNotRefused(t *testing.T) {
f := newApplyFixture(t, "stable", "v1")
req := f.promote("v2", []byte("x"), false)
- f.a.store = unreadableStore{f.store}
+ f.a.openStore = func(context.Context) (release.Store, ed25519.PublicKey, error) {
+ return unreadableStore{f.store}, f.pub, nil
+ }
if err := f.a.Run(context.Background(), false); err != nil {
t.Fatal(err)
}
@@ -1022,3 +1027,141 @@ func TestARolledBackTupleIsNotAppliedAgain(t *testing.T) {
t.Fatalf("result = %+v, a refused tuple must stay retryable", r)
}
}
+
+// --recover runs at boot, before the network is up and on a box whose
+// environment may be no more than /etc/carlos/host.env: it must never need
+// a deploy store or CARLOS_RELEASE_PUBKEY. The seam says so out loud —
+// opening the store at all fails the test.
+func TestRecoverNeedsNeitherStoreNorPubkey(t *testing.T) {
+ f, _ := applyHappy(t)
+ f.a.openStore = func(context.Context) (release.Store, ed25519.PublicKey, error) {
+ t.Error("--recover opened the deploy store")
+ return nil, nil, errors.New("no store on this box")
+ }
+ os.WriteFile(systemPrevPath(f.a.optDir), []byte("#!/bin/sh\necho carlos v1\n"), 0o755)
+ os.WriteFile(systemBinaryPath(f.a.optDir), []byte("#!/bin/sh\necho carlos v2\n"), 0o755)
+ writeJSONDurable(filepath.Join(systemRootDir(f.a.optDir), "journal.json"),
+ systemJournal{Phase: systemPhaseUnits, Target: "v2", PrevBuild: "v1", StartedAt: f.now}, 0o600)
+ if err := f.a.Run(context.Background(), true); err != nil {
+ t.Fatal(err)
+ }
+ bin, _ := os.ReadFile(systemBinaryPath(f.a.optDir))
+ if !strings.Contains(string(bin), "carlos v1") {
+ t.Fatalf("binary = %q, want the .prev restored", bin)
+ }
+ // And a --recover with nothing to recover — the every-boot case — is
+ // just as storeless.
+ f2, _ := applyHappy(t)
+ f2.a.openStore = func(context.Context) (release.Store, ed25519.PublicKey, error) {
+ t.Error("a no-op --recover opened the deploy store")
+ return nil, nil, errors.New("no store on this box")
+ }
+ if err := f2.a.Run(context.Background(), true); err != nil {
+ t.Fatal(err)
+ }
+ if f2.result() != nil {
+ t.Fatalf("a --recover with no journal wrote a result: %+v", f2.result())
+ }
+}
+
+// --recover waits for the apply lock, but not forever. carlos-system-recover
+// is ordered Before=carlos-edge.service, so a recover start job queued while
+// an apply is mid-`systemctl restart carlos-edge` has systemd holding the
+// restart for the recovery — and a recovery blocking on the lock would hold
+// it for the apply. Both would sit there until their unit timeouts. It waits
+// half a minute and stands down; the apply recovers at its own start.
+func TestRecoverGivesUpOnALockAnApplyHolds(t *testing.T) {
+ f, _ := applyHappy(t)
+ writeJSONDurable(filepath.Join(systemRootDir(f.a.optDir), "journal.json"),
+ systemJournal{Phase: systemPhaseUnits, Target: "v2", PrevBuild: "v1", StartedAt: f.now}, 0o600)
+ os.WriteFile(systemPrevPath(f.a.optDir), []byte("#!/bin/sh\necho carlos v1\n"), 0o755)
+ held, err := os.OpenFile(f.a.lockPath(), os.O_CREATE|os.O_RDWR, 0o600)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer held.Close()
+ if err := flockExclusiveNonblock(held); err != nil {
+ t.Skipf("flock unsupported on this platform: %v", err)
+ }
+ var logged []string
+ f.a.ops.logf = func(format string, args ...any) { logged = append(logged, format) }
+ start := f.now
+ if err := f.a.Run(context.Background(), true); err != nil {
+ t.Fatalf("a locked-out --recover must exit 0: %v", err)
+ }
+ if waited := f.now.Sub(start); waited != systemRecoverLockWait {
+ t.Errorf("waited %s, want %s", waited, systemRecoverLockWait)
+ }
+ if len(logged) != 1 || !strings.Contains(logged[0], "it recovers at its own start") {
+ t.Errorf("log = %v", logged)
+ }
+ if j, _ := readSystemJournal(f.a.optDir); j == nil {
+ t.Error("a locked-out --recover closed the journal it never read")
+ }
+ if len(f.calls) != 0 {
+ t.Errorf("a locked-out --recover touched the box: %v", f.calls)
+ }
+ // The lock is free again: the next run gets it and does the work.
+ held.Close()
+ if err := f.a.Run(context.Background(), true); err != nil {
+ t.Fatal(err)
+ }
+ if j, _ := readSystemJournal(f.a.optDir); j != nil {
+ t.Error("the recovery that got the lock left the journal open")
+ }
+}
+
+// The carlos.prev backup is NOT journaled, though everything else that
+// touches the box is. copyFileDurable is whole-or-nothing, so there is
+// nothing to recover from — and a journal here is actively harmful: the
+// exec shim in carlos-system-recover.service reads it at the next boot and
+// would run a .prev one release behind the box for a whole apply.
+//
+// A sentinel journal makes the absence visible: decidePrev neither writes
+// over it nor removes it. (Nothing puts a journal there in real life —
+// recoverIfNeeded has closed any open one long before install runs.)
+func TestPrevBackupIsNotJournaled(t *testing.T) {
+ f, _ := applyHappy(t)
+ writeJSONDurable(filepath.Join(systemRootDir(f.a.optDir), "journal.json"),
+ systemJournal{Phase: "sentinel", Target: "v9", PrevBuild: "v1", StartedAt: f.now}, 0o600)
+ writeJSONDurable(filepath.Join(systemRootDir(f.a.optDir), "applied.json"),
+ systemApplied{Version: "v1", Channel: "stable", At: f.now}, 0o644)
+ ok, err := f.a.decidePrev(context.Background())
+ if err != nil || !ok {
+ t.Fatalf("decidePrev = %v, %v", ok, err)
+ }
+ prev, _ := os.ReadFile(systemPrevPath(f.a.optDir))
+ if !strings.Contains(string(prev), "carlos v1") {
+ t.Fatalf(".prev = %q, want the known-good v1 backed up", prev)
+ }
+ j, err := readSystemJournal(f.a.optDir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if j == nil || j.Phase != "sentinel" {
+ t.Fatalf("journal = %+v, want the sentinel untouched: the backup writes no journal", j)
+ }
+}
+
+// A journal written by the version that DID journal the backup outlives the
+// upgrade to the one that does not: recovery still knows the phase, cleans
+// the half-written copy, and carries on to the request it was started for.
+func TestRecoveryStillHandlesALegacyBackingUpJournal(t *testing.T) {
+ f, req := applyHappy(t)
+ tmp := systemPrevPath(f.a.optDir) + ".tmp"
+ os.WriteFile(tmp, []byte("half a binary"), 0o755)
+ writeJSONDurable(filepath.Join(systemRootDir(f.a.optDir), "journal.json"),
+ systemJournal{Phase: systemPhaseBackingUp, PrevBuild: "v1", StartedAt: f.now}, 0o600)
+ if err := f.a.Run(context.Background(), false); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := os.Stat(tmp); err == nil {
+ t.Error("the half-written .prev copy was left behind")
+ }
+ if j, _ := readSystemJournal(f.a.optDir); j != nil {
+ t.Error("the legacy journal was left open")
+ }
+ if r := f.result(); r == nil || !r.matches("stable", req.Version, req.PromotedAt) || r.Outcome != systemOutcomeApplied {
+ t.Fatalf("result = %+v, want the run to carry on to its request", r)
+ }
+}
diff --git a/internal/host/systemapply_unix.go b/internal/host/systemapply_unix.go
index 50efc63c7..15dfa714f 100644
--- a/internal/host/systemapply_unix.go
+++ b/internal/host/systemapply_unix.go
@@ -40,11 +40,11 @@ func openBeneath(root string, parts ...string) (*os.File, error) {
return os.NewFile(uintptr(fd), filepath.Join(append([]string{root}, parts...)...)), nil
}
-// flockExclusive waits for the apply lock (the recovery run must not skip
-// its work); flockExclusiveNonblock refuses when another apply holds it, so
-// a path-unit-triggered run exits quietly instead of queueing.
-func flockExclusive(f *os.File) error { return lockFileEx(f) }
-
+// flockExclusiveNonblock refuses when another apply holds the lock, so a
+// path-unit-triggered run exits quietly instead of queueing. It is the ONLY
+// way this package takes the apply lock: --recover wants it too, and wants
+// it enough to poll for half a minute, but never enough to block — see
+// systemApplier.waitForLock for the deadlock that buys.
func flockExclusiveNonblock(f *os.File) error {
return syscall.Flock(int(f.Fd()), syscall.LOCK_EX|syscall.LOCK_NB)
}
diff --git a/internal/host/systemapply_windows.go b/internal/host/systemapply_windows.go
index f28035db3..146756492 100644
--- a/internal/host/systemapply_windows.go
+++ b/internal/host/systemapply_windows.go
@@ -15,6 +15,4 @@ func openBeneath(root string, parts ...string) (*os.File, error) {
return nil, errSystemUnsupported
}
-func flockExclusive(f *os.File) error { return errSystemUnsupported }
-
func flockExclusiveNonblock(f *os.File) error { return errSystemUnsupported }
diff --git a/internal/host/systemleg.go b/internal/host/systemleg.go
index ec4785a3b..3396c851c 100644
--- a/internal/host/systemleg.go
+++ b/internal/host/systemleg.go
@@ -71,8 +71,9 @@ type systemPending struct {
const systemObserveEvery = time.Hour
func newSystemLeg(store release.Store, pub ed25519.PublicKey, getenv func(string) string, dataDir, optDir string) *systemLeg {
- return &systemLeg{
- store: store, pub: pub, channel: systemChannel(getenv),
+ channel, invalid := systemChannel(getenv)
+ l := &systemLeg{
+ store: store, pub: pub, channel: channel,
platform: runtime.GOOS + "-" + runtime.GOARCH, build: version.Build,
dataDir: dataDir, optDir: optDir,
now: time.Now,
@@ -82,6 +83,11 @@ func newSystemLeg(store release.Store, pub ed25519.PublicKey, getenv func(string
},
logf: log.Printf,
}
+ // Once, here at arming, rather than on every tick that reads the value.
+ if invalid != "" {
+ l.printf("system: %s=%q is not a channel; this box follows %s", systemChannelEnv, invalid, l.channel)
+ }
+ return l
}
func (l *systemLeg) printf(format string, args ...any) {
diff --git a/internal/host/systemprobe.go b/internal/host/systemprobe.go
index 6d088dae3..a9677bc22 100644
--- a/internal/host/systemprobe.go
+++ b/internal/host/systemprobe.go
@@ -3,6 +3,7 @@ package host
import (
"strings"
+ "github.com/carlosframework/platform/internal/release"
"github.com/carlosframework/platform/internal/status"
)
@@ -12,11 +13,28 @@ import (
// agent's request (spec §6 step 2).
const systemChannelEnv = "CARLOS_SYSTEM_CHANNEL"
-func systemChannel(getenv func(string) string) string {
- if c := strings.TrimSpace(getenv(systemChannelEnv)); c != "" {
- return c
+// A value that is not a channel at all — a typo, a shell that expanded to
+// nothing useful, a name with a slash too many — falls back to stable
+// rather than travelling into a bucket key and an on-box state file to ask
+// for a pointer nobody publishes. It comes back as invalid so the caller
+// can say so ONCE: this is read per tick, and a line per tick is a line
+// nobody reads.
+func systemChannel(getenv func(string) string) (channel, invalid string) {
+ c := strings.TrimSpace(getenv(systemChannelEnv))
+ switch {
+ case c == "":
+ return "stable", ""
+ case !release.ValidChannel(c):
+ return "stable", c
}
- return "stable"
+ return c, ""
+}
+
+// boxSystemChannel is systemChannel for the caller that has nowhere to log:
+// the status tick, which publishes this every five minutes.
+func boxSystemChannel(getenv func(string) string) string {
+ c, _ := systemChannel(getenv)
+ return c
}
// systemStateProbe is root's result.json as the heartbeat carries it, or nil
diff --git a/internal/host/systemprobe_test.go b/internal/host/systemprobe_test.go
index 0dbcb019a..508459ca5 100644
--- a/internal/host/systemprobe_test.go
+++ b/internal/host/systemprobe_test.go
@@ -1,23 +1,69 @@
package host
import (
+ "bytes"
+ "log"
"os"
"path/filepath"
+ "strings"
"testing"
"time"
)
func TestSystemChannelDefaultsToStable(t *testing.T) {
- if got := systemChannel(func(string) string { return "" }); got != "stable" {
- t.Fatalf("default = %q", got)
+ env := func(v string) func(string) string {
+ return func(k string) string {
+ if k == systemChannelEnv {
+ return v
+ }
+ return ""
+ }
+ }
+ for _, c := range []struct {
+ name string
+ set string
+ want, wantBadCh string
+ }{
+ {"unset", "", "stable", ""},
+ {"set", " canary ", "canary", ""},
+ {"a branch canary", "canary/paul", "canary/paul", ""},
+ // Not a channel: it would travel into a bucket key and an on-box
+ // state file, so the box follows stable and says which value it
+ // refused, rather than asking for a pointer nobody publishes.
+ {"a path traversal", "../../etc", "stable", "../../etc"},
+ {"a word with a space", "not a channel", "stable", "not a channel"},
+ } {
+ t.Run(c.name, func(t *testing.T) {
+ got, bad := systemChannel(env(c.set))
+ if got != c.want || bad != c.wantBadCh {
+ t.Fatalf("systemChannel(%q) = %q, %q; want %q, %q", c.set, got, bad, c.want, c.wantBadCh)
+ }
+ })
}
- if got := systemChannel(func(k string) string {
+}
+
+// The refused value is said ONCE, where the leg is armed — not on every one
+// of the two-second ticks that read the channel.
+func TestNewSystemLegSaysWhichChannelValueItRefused(t *testing.T) {
+ var buf bytes.Buffer
+ out, flags := log.Writer(), log.Flags()
+ log.SetOutput(&buf)
+ log.SetFlags(0)
+ defer func() { log.SetOutput(out); log.SetFlags(flags) }()
+
+ leg := newSystemLeg(nil, nil, func(k string) string {
if k == systemChannelEnv {
- return " canary "
+ return "canary/too/deep"
}
return ""
- }); got != "canary" {
- t.Fatalf("env = %q", got)
+ }, t.TempDir(), t.TempDir())
+ if leg.channel != "stable" {
+ t.Fatalf("channel = %q, want stable", leg.channel)
+ }
+ said := strings.Count(buf.String(), "canary/too/deep")
+ lines := len(strings.Split(strings.TrimSpace(buf.String()), "\n"))
+ if said != 1 || lines != 1 {
+ t.Fatalf("arming said the refused value %d times over %d lines:\n%s", said, lines, buf.String())
}
}
diff --git a/internal/host/systemunits.go b/internal/host/systemunits.go
index 0010e6273..c84e70561 100644
--- a/internal/host/systemunits.go
+++ b/internal/host/systemunits.go
@@ -2,6 +2,7 @@ package host
import (
"os"
+ "os/exec"
"path/filepath"
"strings"
)
@@ -25,7 +26,12 @@ var systemUnitFiles = []string{"carlos-system-update.path", "carlos-system-updat
// rather than risk tearing a unit file mid-copy. lockPath is the apply's
// lock file (systemRootDir(release.OptDir())/lock in production; a temp
// path in tests).
-func ensureSystemUnits(etcDir, lockPath string, systemctl func(args ...string) (string, error), logf func(string, ...any)) {
+//
+// Two systemctl seams, because the two uses want opposite things from an
+// exit status: systemctl runs the arming COMMANDS, where non-zero is a
+// failure to report, and probe asks the two QUESTIONS, where non-zero is
+// half the answer (see the block below).
+func ensureSystemUnits(etcDir, lockPath string, systemctl, probe func(args ...string) (string, error), logf func(string, ...any)) {
if err := os.MkdirAll(filepath.Dir(lockPath), 0o755); err != nil {
logf("adopt: system units: %v", err)
return
@@ -47,23 +53,35 @@ func ensureSystemUnits(etcDir, lockPath string, systemctl func(args ...string) (
}
unitDir := filepath.Join(etcDir, "systemd", "system")
- active, activeErr := systemctl("is-active", systemUpdatePathUnit)
- enabled, enabledErr := systemctl("is-enabled", systemRecoverUnit)
- if activeErr != nil || enabledErr != nil {
- probeErr := activeErr
- if probeErr == nil {
- probeErr = enabledErr
- }
- logf("adopt: system units: systemctl unavailable; not touching the self-update units: %v", probeErr)
+ // systemctl answers these two in stdout and spends its EXIT STATUS on
+ // the answer's polarity: is-active exits 3 for "inactive", is-enabled
+ // exits 1 for "disabled" and again for a unit file that is not there
+ // yet. Reading a non-zero exit as a broken systemctl is what made every
+ // unarmed box — the exact box this pass exists to arm — look like a box
+ // with no systemctl at all, so the printed word is the answer and the
+ // exit status is ignored, exactly as probes.go has always done it.
+ active, activeErr := probe("is-active", systemUpdatePathUnit)
+ // The is-enabled error is dropped on purpose: its cases are the words
+ // parseIsEnabled reads, and "no such unit file" is a perfectly good
+ // "not enabled" on the box we are here to fix.
+ enabled, _ := probe("is-enabled", systemRecoverUnit)
+ // is-active is the probe that tells a silent systemctl from an unarmed
+ // unit: it prints a state word even for a unit systemd has never heard
+ // of, so no word at all means the question could not be asked — no
+ // binary, or an exec that failed outright.
+ isActive := parseIsActive(active)
+ if isActive == nil {
+ logf("adopt: system units: systemctl said nothing; not touching the self-update units: %v", activeErr)
return
}
+ isEnabled := parseIsEnabled(enabled)
missing := false
for _, u := range systemUnitFiles {
if _, err := os.Stat(filepath.Join(unitDir, u)); err != nil {
missing = true
}
}
- if !missing && strings.TrimSpace(active) == "active" && strings.TrimSpace(enabled) == "enabled" {
+ if !missing && *isActive && isEnabled != nil && *isEnabled {
return
}
if err := os.MkdirAll(unitDir, 0o755); err != nil {
@@ -89,3 +107,32 @@ func ensureSystemUnits(etcDir, lockPath string, systemctl func(args ...string) (
}
logf("adopt: self-update units installed and armed")
}
+
+// parseIsEnabled maps `systemctl is-enabled`'s vocabulary onto "armed for
+// the next boot, or not", as probes.go's parseIsActive maps is-active's onto
+// "running now". enabled and enabled-runtime are the wants-symlink states;
+// static, indirect, alias and generated are units that cannot be enabled at
+// all, so an enable would be a no-op and there is nothing there to fix.
+// Everything else — disabled, masked, linked, the empty answer a unit file
+// that is not installed gets, a word a future systemd invents — is nil, and
+// the caller arms a nil, because the answer this must never invent is the
+// reassuring one.
+func parseIsEnabled(out string) *bool {
+ yes, no := true, false
+ switch strings.TrimSpace(out) {
+ case "enabled", "enabled-runtime", "static", "indirect", "alias", "generated":
+ return &yes
+ case "disabled", "masked", "masked-runtime", "linked", "linked-runtime", "transient", "bad":
+ return &no
+ }
+ return nil
+}
+
+// systemctlProbe is `systemctl <args>` asked as a QUESTION: stdout is the
+// answer and the exit status is the caller's to ignore, so it uses Output()
+// rather than productionSysOps().systemctl's fail-on-non-zero. The error
+// comes back only so a caller left with no word to read can say why.
+func systemctlProbe(args ...string) (string, error) {
+ out, err := exec.Command("systemctl", args...).Output()
+ return string(out), err
+}
diff --git a/internal/host/systemunits_test.go b/internal/host/systemunits_test.go
index 5fe204f8e..4947d965d 100644
--- a/internal/host/systemunits_test.go
+++ b/internal/host/systemunits_test.go
@@ -2,28 +2,41 @@ package host
import (
"errors"
+ "fmt"
"os"
"path/filepath"
"strings"
"testing"
)
+// systemctlExit is the shape of every answer the two probes get on a real
+// box: systemd prints the state and exits non-zero to say which state it is
+// — 3 for is-active's "inactive", 1 for is-enabled's "disabled". A fake that
+// returned a nil error here would test a systemctl that does not exist.
+func systemctlExit(status int) error {
+ return fmt.Errorf("exit status %d", status)
+}
+
func TestEnsureSystemUnitsInstallsAndArmsWhatIsMissing(t *testing.T) {
etc := t.TempDir()
os.MkdirAll(filepath.Join(etc, "systemd", "system"), 0o755)
lockPath := filepath.Join(t.TempDir(), "lock")
var calls []string
systemctl := func(args ...string) (string, error) {
+ calls = append(calls, strings.Join(args, " "))
+ return "", nil
+ }
+ probe := func(args ...string) (string, error) {
calls = append(calls, strings.Join(args, " "))
switch args[0] {
case "is-active":
- return "inactive\n", nil
+ return "inactive\n", systemctlExit(3)
case "is-enabled":
- return "disabled\n", nil
+ return "disabled\n", systemctlExit(1)
}
return "", nil
}
- ensureSystemUnits(etc, lockPath, systemctl, t.Logf)
+ ensureSystemUnits(etc, lockPath, systemctl, probe, t.Logf)
for _, u := range []string{"carlos-system-update.path", "carlos-system-update.service", "carlos-system-recover.service"} {
if _, err := os.Stat(filepath.Join(etc, "systemd", "system", u)); err != nil {
t.Errorf("%s not installed", u)
@@ -40,7 +53,7 @@ func TestEnsureSystemUnitsInstallsAndArmsWhatIsMissing(t *testing.T) {
}
// All present and armed: no writes, no systemctl beyond the two checks.
calls = nil
- systemctl2 := func(args ...string) (string, error) {
+ probe2 := func(args ...string) (string, error) {
calls = append(calls, strings.Join(args, " "))
switch args[0] {
case "is-active":
@@ -50,12 +63,73 @@ func TestEnsureSystemUnitsInstallsAndArmsWhatIsMissing(t *testing.T) {
}
return "", nil
}
- ensureSystemUnits(etc, lockPath, systemctl2, t.Logf)
+ ensureSystemUnits(etc, lockPath, systemctl, probe2, t.Logf)
if len(calls) != 2 {
t.Fatalf("an armed box got %v", calls)
}
}
+// The unit files are not on disk at all, which is what a box the roll script
+// forgot looks like: is-enabled prints NOTHING and exits 1 for a unit file
+// systemd cannot find. That silence is "not enabled", not "no systemctl".
+func TestEnsureSystemUnitsArmsAUnitSystemdHasNeverHeardOf(t *testing.T) {
+ etc := t.TempDir()
+ lockPath := filepath.Join(t.TempDir(), "lock")
+ var calls []string
+ systemctl := func(args ...string) (string, error) {
+ calls = append(calls, strings.Join(args, " "))
+ return "", nil
+ }
+ probe := func(args ...string) (string, error) {
+ switch args[0] {
+ case "is-active":
+ return "inactive\n", systemctlExit(3)
+ case "is-enabled":
+ return "", systemctlExit(1)
+ }
+ return "", nil
+ }
+ ensureSystemUnits(etc, lockPath, systemctl, probe, t.Logf)
+ for _, u := range []string{"carlos-system-update.path", "carlos-system-update.service", "carlos-system-recover.service"} {
+ if _, err := os.Stat(filepath.Join(etc, "systemd", "system", u)); err != nil {
+ t.Errorf("%s not installed", u)
+ }
+ }
+ if !strings.Contains(strings.Join(calls, "\n"), "start carlos-system-update.path") {
+ t.Errorf("the path unit was never started: %v", calls)
+ }
+}
+
+// An enabled recover unit whose path unit is enabled but never started —
+// the flagship's own failure — is armed, not left alone.
+func TestEnsureSystemUnitsStartsAnEnabledButUnstartedPathUnit(t *testing.T) {
+ etc := t.TempDir()
+ unitDir := filepath.Join(etc, "systemd", "system")
+ os.MkdirAll(unitDir, 0o755)
+ for _, u := range systemUnitFiles {
+ os.WriteFile(filepath.Join(unitDir, u), []byte("[Unit]\n"), 0o644)
+ }
+ lockPath := filepath.Join(t.TempDir(), "lock")
+ var calls []string
+ systemctl := func(args ...string) (string, error) {
+ calls = append(calls, strings.Join(args, " "))
+ return "", nil
+ }
+ probe := func(args ...string) (string, error) {
+ switch args[0] {
+ case "is-active":
+ return "inactive\n", systemctlExit(3)
+ case "is-enabled":
+ return "enabled\n", nil
+ }
+ return "", nil
+ }
+ ensureSystemUnits(etc, lockPath, systemctl, probe, t.Logf)
+ if !strings.Contains(strings.Join(calls, "\n"), "start carlos-system-update.path") {
+ t.Errorf("an enabled-but-inactive path unit was left unstarted: %v", calls)
+ }
+}
+
func TestEnsureSystemUnitsLeavesAConcurrentApplyAlone(t *testing.T) {
etc := t.TempDir()
os.MkdirAll(filepath.Join(etc, "systemd", "system"), 0o755)
@@ -70,11 +144,11 @@ func TestEnsureSystemUnitsLeavesAConcurrentApplyAlone(t *testing.T) {
t.Skipf("flock unsupported on this platform: %v", err)
}
var calls []string
- systemctl := func(args ...string) (string, error) {
+ record := func(args ...string) (string, error) {
calls = append(calls, strings.Join(args, " "))
return "", nil
}
- ensureSystemUnits(etc, lockPath, systemctl, t.Logf)
+ ensureSystemUnits(etc, lockPath, record, record, t.Logf)
if len(calls) != 0 {
t.Fatalf("a locked-out pass called systemctl: %v", calls)
}
@@ -85,6 +159,8 @@ func TestEnsureSystemUnitsLeavesAConcurrentApplyAlone(t *testing.T) {
}
}
+// No systemctl at all: the probe prints no word, and a box we cannot ask is
+// a box we do not touch.
func TestEnsureSystemUnitsTrustsNoBrokenSystemctl(t *testing.T) {
etc := t.TempDir()
os.MkdirAll(filepath.Join(etc, "systemd", "system"), 0o755)
@@ -92,10 +168,36 @@ func TestEnsureSystemUnitsTrustsNoBrokenSystemctl(t *testing.T) {
systemctl := func(args ...string) (string, error) {
return "", errors.New("systemctl: command not found")
}
- ensureSystemUnits(etc, lockPath, systemctl, t.Logf)
+ ensureSystemUnits(etc, lockPath, systemctl, systemctl, t.Logf)
for _, u := range []string{"carlos-system-update.path", "carlos-system-update.service", "carlos-system-recover.service"} {
if _, err := os.Stat(filepath.Join(etc, "systemd", "system", u)); err == nil {
t.Errorf("%s was installed despite systemctl being unavailable", u)
}
}
}
+
+func TestParseIsEnabled(t *testing.T) {
+ yes, no := true, false
+ for _, c := range []struct {
+ out string
+ want *bool
+ }{
+ {"enabled\n", &yes},
+ {"enabled-runtime\n", &yes},
+ {"static\n", &yes},
+ {"disabled\n", &no},
+ {"masked\n", &no},
+ {"", nil},
+ {"something-systemd-invents\n", nil},
+ } {
+ got := parseIsEnabled(c.out)
+ switch {
+ case got == nil && c.want != nil:
+ t.Errorf("parseIsEnabled(%q) = nil, want %v", c.out, *c.want)
+ case got != nil && c.want == nil:
+ t.Errorf("parseIsEnabled(%q) = %v, want nil", c.out, *got)
+ case got != nil && *got != *c.want:
+ t.Errorf("parseIsEnabled(%q) = %v, want %v", c.out, *got, *c.want)
+ }
+ }
+}
diff --git a/internal/release/system.go b/internal/release/system.go
index 88c60b18d..fe3eb19dc 100644
--- a/internal/release/system.go
+++ b/internal/release/system.go
@@ -278,8 +278,14 @@ func PromoteSystem(ctx context.Context, s Store, sg Signer, component, channel s
return next, nil
}
-// RollbackSystem swaps Current with History[0] and writes a NEW pointer —
-// new PromotedAt — so every box's replay guard sees a promote, not a replay.
+// RollbackSystem POPS History[0] rather than swapping with it: History[0]
+// becomes Current and is removed from History, and the release that was
+// Current is not pushed back on. So a second rollback goes one release
+// further back, not forward again to where it started — the verb means "go
+// back another one", and repeating it walks the history down.
+//
+// It writes a NEW pointer — new PromotedAt — so every box's replay guard
+// sees a promote, not a replay.
func RollbackSystem(ctx context.Context, s Store, sg Signer, component, channel, actor string) (*SystemPointer, error) {
key := SystemPointerKey(component, channel)
body, etag, err := s.Get(ctx, key)
diff --git a/system.go b/system.go
index e82b49427..faa4a6a0b 100644
--- a/system.go
+++ b/system.go
@@ -186,6 +186,9 @@ func runSystemUpdate(args []string) {
if cur.Partial {
fmt.Fprintf(stdout, " %s\n", colorFor(stdout).warn("some fleets could not be read; more boxes than these may roll"))
}
+ if msg := systemLadderWarning(cur.Pointers, *channel, *version); msg != "" {
+ fmt.Fprintf(stdout, "%s\n", colorFor(stdout).warn(msg))
+ }
if !*yes {
if !interactiveTerminal() {
log.Fatal("no terminal to confirm on; pass --yes")
@@ -234,6 +237,26 @@ func runSystemUpdate(args []string) {
colorFor(stdout).ok("promoted"), *channel, *channel)
}
+// systemLadderWarning is the canary → stable ladder, said out loud and not
+// enforced. A promote straight to stable is a legitimate thing to do — a
+// hotfix at three in the morning, a build that has already proved itself
+// somewhere else — so this is a line to read, never a refusal: the operator
+// is the one who knows whether the rung was skipped on purpose.
+//
+// It stays quiet unless it is certain. Without --version the console is
+// promoting "the latest public release", a tag only the server resolves, so
+// there is nothing here to compare against the canary pointer and a warning
+// would be a guess. Only stable has a rung below it worth naming.
+func systemLadderWarning(pointers map[string]*systemPointerRow, channel, version string) string {
+ if channel != "stable" || version == "" {
+ return ""
+ }
+ if canary := pointers["canary"]; canary != nil && canary.Version == version {
+ return ""
+ }
+ return fmt.Sprintf("warning: %s has not been on canary; promoting straight to stable", version)
+}
+
// watchSystemJob polls the job and prints each step once, as deploy's
// watchVersion does for a route. The console runs the promote off the
// request precisely so a dropped connection cannot abandon it, so this is a
diff --git a/system_test.go b/system_test.go
index 2dff1bc39..7453b8d48 100644
--- a/system_test.go
+++ b/system_test.go
@@ -113,3 +113,37 @@ func TestPrintSystemStatusShowsWhatItCouldNotRead(t *testing.T) {
t.Errorf("a complete fleet warned anyway:\n%s", quiet.String())
}
}
+
+func TestSystemLadderWarning(t *testing.T) {
+ onCanary := map[string]*systemPointerRow{"canary": {Version: "v9"}}
+ never := map[string]*systemPointerRow{}
+ older := map[string]*systemPointerRow{"canary": {Version: "v8"}}
+ const want = "warning: v9 has not been on canary; promoting straight to stable"
+ for _, c := range []struct {
+ name string
+ pointers map[string]*systemPointerRow
+ channel, version string
+ warn string
+ }{
+ {"the version soaked on canary", onCanary, "stable", "v9", ""},
+ {"canary has never been promoted", never, "stable", "v9", want},
+ {"canary is on a different version", older, "stable", "v9", want},
+ // Promoting TO canary is the rung below; nothing to skip.
+ {"promoting to canary", never, "canary", "v9", ""},
+ // No --version: the tag is "the latest public release", which only
+ // the server resolves, so there is nothing to compare and a warning
+ // would be a guess.
+ {"the latest public release", never, "stable", "", ""},
+ } {
+ t.Run(c.name, func(t *testing.T) {
+ if got := systemLadderWarning(c.pointers, c.channel, c.version); got != c.warn {
+ t.Errorf("systemLadderWarning = %q, want %q", got, c.warn)
+ }
+ })
+ }
+ // A nil map is what an unreadable status body leaves behind: it warns
+ // rather than panicking.
+ if got := systemLadderWarning(nil, "stable", "v9"); got != want {
+ t.Errorf("nil pointers = %q, want %q", got, want)
+ }
+}