amadan

carlos / platform Public

host: create the relay's state dir on every apply, and arm what the target build asks for

Two fixes for discussion #118, both seen taking the relay live on the
Ireland box on 2026-09-07.

/data/relay is in bootstrap's dirs list, and that list only ever runs
against a fresh box or a staging root. What a self-update apply re-runs is
systemd-tmpfiles, so a box bootstrapped before the relay existed got the
unit, the binary and the master file and still failed to start with
"Failed to set up mount namespacing: /data/relay: No such file or
directory". The directory now has a tmpfiles line beside the other
persistent /data dirs, so every apply and every boot creates it.

The second is why the unit was never started at all. bootstrapPlan carried
the target build's Enabled list but not which of those units to START --
that was decided by systemctlPlan compiled into the OLD, applying binary,
which cannot know about a unit the new build just introduced. The plan now
carries an Armed list and arm() prefers it, falling back to the local rule
when it is absent, which is what a rollback stages from carlos.prev.

Because the armed list now comes from another build, arm() no longer
trusts it structurally: it refuses to start the edge whatever the plan
says, and drops a unit the plan arms but does not enable, since the single
batched `start` would otherwise lose every unit in it to one bad name.

Note the second fix does not pay off on the roll that delivers it: arm()
is the applying binary's code throughout install and rollback, so it
starts mattering from the next update on. For the relay specifically
restartRelay already covers it on main.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev a193165a2e551f8720604a3baecd1955dd05b640 parent e6773f5
4 files changed, +277 −14
  • internal/host/bootstrap.go +55 −8
  • internal/host/host_test.go +81 −0
  • internal/host/systemapply.go +41 −6
  • internal/host/systemapply_test.go +100 −0

CI passed — run details

diff --git a/internal/host/bootstrap.go b/internal/host/bootstrap.go
index 39aead0ff..89d232ffc 100644
--- a/internal/host/bootstrap.go
+++ b/internal/host/bootstrap.go
@@ -74,6 +74,15 @@ const tmpfilesConf = "d /run/carlos 2750 " + serviceUser + " " + socketsGroup +
// line just set. Both are 0700 so re-runs converge instead of racing.
"d /data/instances 0700 " + serviceUser + " " + serviceUser + " -\n" +
"d /data/layout 0700 " + serviceUser + " " + serviceUser + " -\n" +
+ // The relay's one writable state dir (carlos-relay.service's
+ // ReadWritePaths). It is in bootstrap's dirs list too, but that list
+ // only ever runs against a FRESH box or a staging root — a self-update
+ // apply creates no directories, it re-runs tmpfiles. Without this line
+ // a box bootstrapped before the relay existed gets the unit, the
+ // binary and the master file and still fails to start with "Failed to
+ // set up mount namespacing: /data/relay: No such file or directory"
+ // (discussion #118, hit on the Ireland box 2026-09-07).
+ "d /data/relay 0750 " + serviceUser + " " + serviceUser + " -\n" +
"d /run/carlos/.instances 2750 " + serviceUser + " " + socketsGroup + " -\n" +
"d /run/carlos/instances 2750 " + serviceUser + " " + socketsGroup + " -\n" +
// The readiness probe's own instance directory (design §8.5): a name
@@ -161,13 +170,50 @@ var inertUnits = []string{"carlos-restore-verify.timer", "carlos-hibernate-verif
// bootstrapPlan is what `bootstrap -root <dir> -plan-json` prints: the
// arming list of THIS binary, so a self-update apply arms what the new
// binary's bootstrap would arm and never a list an older binary remembers.
+//
+// Armed is the other half of that promise, and it was missing until
+// discussion #118. Enabled alone says which units to `enable`; WHICH of them
+// also get a `start` was decided by the applier's own systemctlPlan — code
+// from the OLD binary, which cannot know about a unit the new build just
+// introduced. That is how the relay landed on the Ireland box enabled but
+// never started on 2026-09-07: the applying build predated
+// carlos-relay.service, so its systemctlPlan had no reason to start it.
+// Carrying the list the TARGET build would start closes that gap for every
+// future unit, not just the relay.
+//
+// Additive on purpose: a plan.json written by a build older than this field
+// (a rollback stages carlos.prev's plan) carries no "armed" key, and arm()
+// then decides locally exactly as it always did. The key is NOT omitempty:
+// nil ("this build is too old to say") and empty ("start nothing") are
+// different answers, and omitempty would collapse them into the same absent
+// key, making "start nothing" impossible for a build to express.
type bootstrapPlan struct {
Enabled []string `json:"enabled"`
Inert []string `json:"inert"`
+ Armed []string `json:"armed"`
}
func currentBootstrapPlan() bootstrapPlan {
- return bootstrapPlan{Enabled: append([]string(nil), enabledUnits...), Inert: append([]string(nil), inertUnits...)}
+ return bootstrapPlan{
+ Enabled: append([]string(nil), enabledUnits...),
+ Inert: append([]string(nil), inertUnits...),
+ Armed: armedUnits(enabledUnits),
+ }
+}
+
+// armedUnits is the "and start these too" rule in one place: the unit types
+// whose enable-without-start is a silent no-op until the next boot, plus
+// carlos-relay.service, whose ConditionPathExists makes starting it safe on a
+// box with no region master. See systemctlPlan for the long version.
+func armedUnits(units []string) []string {
+ var armed []string
+ for _, u := range units {
+ if strings.HasSuffix(u, ".timer") || strings.HasSuffix(u, ".path") ||
+ strings.HasSuffix(u, ".socket") || strings.HasSuffix(u, ".mount") || u == relayUnitName {
+ armed = append(armed, u)
+ }
+ }
+ return armed
}
// systemctlPlan is the exact systemctl sequence a live bootstrap runs — and
@@ -206,17 +252,18 @@ func currentBootstrapPlan() bootstrapPlan {
// tree with no protection, until the next reboot. Starting either is
// idempotent.
func systemctlPlan(units []string) [][]string {
+ return systemctlPlanArming(units, armedUnits(units))
+}
+
+// systemctlPlanArming is systemctlPlan with the start list supplied rather
+// than derived — the shape a self-update apply needs, where the units come
+// from the target build's plan.json and so does the decision about which of
+// them to start (discussion #118).
+func systemctlPlanArming(units, armed []string) [][]string {
plan := [][]string{
{"daemon-reload"},
append([]string{"enable"}, units...),
}
- var armed []string
- for _, u := range units {
- if strings.HasSuffix(u, ".timer") || strings.HasSuffix(u, ".path") ||
- strings.HasSuffix(u, ".socket") || strings.HasSuffix(u, ".mount") || u == relayUnitName {
- armed = append(armed, u)
- }
- }
if len(armed) > 0 {
plan = append(plan, append([]string{"start"}, armed...))
}
diff --git a/internal/host/host_test.go b/internal/host/host_test.go
index 38b6ebd3e..77d9f305d 100644
--- a/internal/host/host_test.go
+++ b/internal/host/host_test.go
@@ -6,6 +6,7 @@ import (
"crypto/ed25519"
"encoding/base64"
"encoding/json"
+ "fmt"
"log"
"net"
"os"
@@ -1675,3 +1676,83 @@ func TestBootstrapWritesJournaldCap(t *testing.T) {
t.Errorf("drop-in gone after a re-run: %v", err)
}
}
+
+// TestTmpfilesCarriesRelayStateDir is discussion #118 as a test. The relay's
+// unit declares ReadWritePaths=/data/relay, and systemd refuses to start a
+// unit whose ReadWritePaths names a directory that is not there — "Failed to
+// set up mount namespacing: /data/relay: No such file or directory", which is
+// what the Ireland box answered on 2026-09-07.
+//
+// bootstrap's own dirs list creates it, and that is not enough: the dirs list
+// runs on a FRESH box or against a staging root, never against the live root
+// of a self-update apply. What an apply re-runs is systemd-tmpfiles. So the
+// directory has to be in tmpfilesConf, and this test is what keeps a future
+// unit's writable state dir from being added to one list and not the other.
+func TestTmpfilesCarriesRelayStateDir(t *testing.T) {
+ b, err := unitsFS.ReadFile("units/" + relayUnitName)
+ if err != nil {
+ t.Fatal(err)
+ }
+ var rw string
+ for _, line := range strings.Split(string(b), "\n") {
+ if v, ok := strings.CutPrefix(line, "ReadWritePaths="); ok {
+ rw = v
+ break
+ }
+ }
+ if rw == "" {
+ t.Fatal("carlos-relay.service declares no ReadWritePaths")
+ }
+ for _, p := range strings.Fields(rw) {
+ if !strings.HasPrefix(p, "/data/") {
+ continue // /run/carlos is tmpfs and already has its own line
+ }
+ want := "d " + p + " "
+ if !strings.Contains(tmpfilesConf, want) {
+ t.Errorf("tmpfilesConf has no %q line, so an apply onto a box that predates the unit leaves it unable to start:\n%s", want, tmpfilesConf)
+ }
+ }
+ // The mode must match bootstrap's dirs list, or a re-run of one flips
+ // what the other just set (the /data/instances lesson, one line up).
+ // Read the mode bootstrap actually uses rather than restating it: a
+ // hardcoded "0750" here passes happily while the two lists disagree.
+ root := t.TempDir()
+ if err := bootstrapWith(root, nil, false); err != nil {
+ t.Fatal(err)
+ }
+ fi, err := os.Stat(filepath.Join(root, "data", "relay"))
+ if err != nil {
+ t.Fatalf("bootstrap did not create /data/relay: %v", err)
+ }
+ want := fmt.Sprintf("d /data/relay %04o ", fi.Mode().Perm())
+ if !strings.Contains(tmpfilesConf, want) {
+ t.Errorf("tmpfilesConf must carry %q to match the mode bootstrap's dirs list creates:\n%s", want, tmpfilesConf)
+ }
+}
+
+// TestPlanCarriesArmedList pins the other half of discussion #118: the plan a
+// build prints says which units to START, not just which to enable, so an
+// applier running an OLDER binary arms what the new build wants rather than
+// what its own systemctlPlan happens to know about.
+func TestPlanCarriesArmedList(t *testing.T) {
+ plan := currentBootstrapPlan()
+ if len(plan.Armed) == 0 {
+ t.Fatal("plan carries no armed list")
+ }
+ if !slices.Contains(plan.Armed, relayUnitName) {
+ t.Errorf("armed list omits the relay: %v", plan.Armed)
+ }
+ if slices.Contains(plan.Armed, "carlos-edge.service") {
+ t.Errorf("edge must stay start-by-hand: %v", plan.Armed)
+ }
+ for _, u := range plan.Armed {
+ if !slices.Contains(plan.Enabled, u) {
+ t.Errorf("armed unit %s is not in the enabled list", u)
+ }
+ }
+ // The armed list and the local rule must agree for THIS build; they
+ // diverge only across builds, which is the point of carrying it.
+ if got, want := plan.Armed, armedUnits(enabledUnits); !slices.Equal(got, want) {
+ t.Errorf("plan.Armed = %v, systemctlPlan would start %v", got, want)
+ }
+}
diff --git a/internal/host/systemapply.go b/internal/host/systemapply.go
index 5ad62425e..198a3a074 100644
--- a/internal/host/systemapply.go
+++ b/internal/host/systemapply.go
@@ -710,13 +710,48 @@ func (a *systemApplier) gateOnce(ctx context.Context, target string) error {
return nil
}
-// arm is systemctlPlan verbatim: enable all, start timers and paths, and of
-// the services only carlos-relay, whose ConditionPathExists makes the start
-// a no-op on a box with no region master (relay spec §3). It starts nothing
-// else — in particular not the edge, which is running this very code — which
-// is why apply can call it from inside itself.
+// arm enables every unit in the plan and starts the ones the plan says to
+// start: the unit types whose enable-without-start is a silent no-op until
+// the next boot, and of the services only carlos-relay, whose
+// ConditionPathExists makes the start a no-op on a box with no region master
+// (relay spec §3).
+//
+// It never starts the edge, which is running this very code — that is why
+// apply can call it from inside itself. Before #118 that was structural: the
+// applier derived the start list itself and could not name a service other
+// than the relay. Now the list comes from the target build, so the guarantee
+// is enforced here instead, explicitly.
func (a *systemApplier) arm(plan bootstrapPlan) error {
- for _, args := range systemctlPlan(plan.Enabled) {
+ // plan.Armed is the TARGET build's answer to "and which of these get a
+ // start" (discussion #118). Preferring it is what lets a build introduce
+ // a unit that must be started at apply time rather than at the next
+ // boot; falling back to the local rule keeps a plan.json from an older
+ // build — which is what a rollback stages — working unchanged.
+ armed := plan.Armed
+ if armed == nil {
+ armed = armedUnits(plan.Enabled)
+ }
+ // Two guards on a list that now comes from another build. The edge is
+ // never started here, whatever the plan says. And a unit that is not in
+ // Enabled is dropped rather than passed to systemctl: `start` is ONE
+ // batched call, so a single name this box has no unit file for would
+ // fail the whole call and leave nothing armed at all.
+ enabled := make(map[string]bool, len(plan.Enabled))
+ for _, u := range plan.Enabled {
+ enabled[u] = true
+ }
+ kept := armed[:0:0]
+ for _, u := range armed {
+ switch {
+ case u == systemEdgeUnit || u == systemEdgeUnit+".service":
+ a.ops.logf("system apply: arm: refusing to start %s, which is running this code", u)
+ case !enabled[u]:
+ a.ops.logf("system apply: arm: %s is armed by the plan but not enabled by it; skipping", u)
+ default:
+ kept = append(kept, u)
+ }
+ }
+ for _, args := range systemctlPlanArming(plan.Enabled, kept) {
if _, err := a.ops.systemctl(args...); err != nil {
return fmt.Errorf("systemctl %s: %w", strings.Join(args, " "), err)
}
diff --git a/internal/host/systemapply_test.go b/internal/host/systemapply_test.go
index bf642a8a0..baf7b426a 100644
--- a/internal/host/systemapply_test.go
+++ b/internal/host/systemapply_test.go
@@ -5,9 +5,11 @@ import (
"crypto/ed25519"
"crypto/sha256"
"encoding/hex"
+ "encoding/json"
"errors"
"os"
"path/filepath"
+ "slices"
"strings"
"testing"
"time"
@@ -1253,3 +1255,101 @@ func TestRecoveryStillHandlesALegacyBackingUpJournal(t *testing.T) {
t.Fatalf("result = %+v, want the run to carry on to its request", r)
}
}
+
+// TestApplyArmsTheTargetBuildsList is the forward-compatibility half of
+// discussion #118. The applier is the OLD binary: when the target build
+// introduces a unit that must be started at apply time rather than at the
+// next boot, only the target build's own plan knows that. So arm() starts
+// what plan.Armed names, even a unit this binary's own rule would skip.
+//
+// Without this, a unit new to the box is enabled and left inactive — which
+// is exactly how carlos-relay.service reached the Ireland box on 2026-09-07
+// enabled, gated, armed with a masters file, and not running.
+func TestApplyArmsTheTargetBuildsList(t *testing.T) {
+ _, started := armCalls(t, bootstrapPlan{
+ Enabled: []string{"carlos-edge.service", "carlos-future.service"},
+ // A service this binary's armedUnits would never pick.
+ Armed: []string{"carlos-future.service"},
+ })
+ if !slices.Contains(started, "carlos-future.service") {
+ t.Errorf("apply did not start the unit the target build asked for: started %v", started)
+ }
+ if slices.Contains(started, "carlos-edge.service") {
+ t.Errorf("apply started the edge, which runs this very code: started %v", started)
+ }
+}
+
+// The plan's armed list comes from another build, so arm() does not take it
+// on trust: the edge is never started whatever the plan says, and a unit the
+// plan arms but does not enable is dropped rather than passed to systemctl,
+// because one bad name in the single batched `start` would lose every unit
+// in it.
+func TestApplyRefusesToArmTheEdgeOrAnUnenabledUnit(t *testing.T) {
+ _, started := armCalls(t, bootstrapPlan{
+ Enabled: []string{"carlos-adopt.timer", "carlos-edge.service"},
+ Armed: []string{"carlos-adopt.timer", "carlos-edge.service", "carlos-never-installed.service"},
+ })
+ if slices.Contains(started, "carlos-edge.service") {
+ t.Errorf("the edge was started from the plan's armed list: started %v", started)
+ }
+ if slices.Contains(started, "carlos-never-installed.service") {
+ t.Errorf("a unit the plan never enabled was handed to systemctl: started %v", started)
+ }
+ if !slices.Contains(started, "carlos-adopt.timer") {
+ t.Errorf("the good unit was lost along with the bad ones: started %v", started)
+ }
+}
+
+// A plan.json from a build older than the Armed field has no armed key. That
+// is what a rollback stages (carlos.prev's plan), so the fallback is not a
+// theoretical path — arm() must decide locally exactly as it did before.
+func TestApplyArmsLocallyWhenPlanPredatesArmed(t *testing.T) {
+ var plan bootstrapPlan
+ if err := json.Unmarshal([]byte(`{"enabled":["carlos-adopt.timer","carlos-edge.service","carlos-relay.service"]}`), &plan); err != nil {
+ t.Fatal(err)
+ }
+ if plan.Armed != nil {
+ t.Fatalf("armed should be absent, got %v", plan.Armed)
+ }
+ _, started := armCalls(t, plan)
+ // Membership, not substring: the relay is batched onto the same start
+ // line as the timer, and its bare name also appears on the enable line.
+ // A substring check is how the first version of this test passed
+ // against a fallback changed to drop the relay — the exact #118 failure
+ // mode it exists to catch.
+ for _, want := range []string{"carlos-adopt.timer", "carlos-relay.service"} {
+ if !slices.Contains(started, want) {
+ t.Errorf("fallback did not arm %s: started %v", want, started)
+ }
+ }
+ if slices.Contains(started, "carlos-edge.service") {
+ t.Errorf("fallback started the edge: started %v", started)
+ }
+}
+
+// armCalls runs arm(plan) against a recording systemd and returns the units
+// it enabled and the units it started. Both are read as SETS out of the two
+// batched systemctl lines: arm puts every unit on one `enable` and every
+// started unit on one `start`, so a substring check over the joined log
+// cannot tell "start carlos-relay.service" from the same name appearing in
+// the enable line, and cannot see a name that is merely batched beside
+// another. The first version of these tests got both wrong.
+func armCalls(t *testing.T, plan bootstrapPlan) (enabled, started []string) {
+ t.Helper()
+ a := &systemApplier{ops: sysOps{
+ systemctl: func(args ...string) (string, error) {
+ switch args[0] {
+ case "enable":
+ enabled = append(enabled, args[1:]...)
+ case "start":
+ started = append(started, args[1:]...)
+ }
+ return "", nil
+ },
+ logf: func(string, ...any) {},
+ }}
+ if err := a.arm(plan); err != nil {
+ t.Fatal(err)
+ }
+ return enabled, started
+}