amadan

carlos / platform Public

host: the hourly belt takes the apply lock and trusts no broken systemctl

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 bb811495b89d7b8be2464f1d9abc570af3e02c25 parent 0e5640a
3 files changed, +98 −8
  • internal/host/agent.go +1 −1
  • internal/host/systemunits.go +49 −5
  • internal/host/systemunits_test.go +48 −2
diff --git a/internal/host/agent.go b/internal/host/agent.go
index b53148eb6..b05370e64 100644
--- a/internal/host/agent.go
+++ b/internal/host/agent.go
@@ -2859,7 +2859,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", productionSysOps().systemctl, log.Printf)
+ ensureSystemUnits("/etc", filepath.Join(systemRootDir(release.OptDir()), systemLockFile), productionSysOps().systemctl, 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/systemunits.go b/internal/host/systemunits.go
index 237e9bcb3..0010e6273 100644
--- a/internal/host/systemunits.go
+++ b/internal/host/systemunits.go
@@ -6,26 +6,70 @@ import (
"strings"
)
-var systemUnitFiles = []string{"carlos-system-update.path", "carlos-system-update.service", "carlos-system-recover.service"}
+// systemRecoverUnit is carlos-system-recover.service, spelled out once
+// rather than repeated as a string literal at each call site below.
+const systemRecoverUnit = "carlos-system-recover.service"
+
+var systemUnitFiles = []string{"carlos-system-update.path", "carlos-system-update.service", systemRecoverUnit}
// ensureSystemUnits is the second belt of spec §7: a box the roll script
// forgot to arm is armed within the hour by the root adopt pass. It
// installs the embedded units when missing and arms them through
// systemctlPlan (never starting a service). Non-fatal: it runs before
// RunAdopt's registry and pubkey gates, and logs instead of exiting.
-func ensureSystemUnits(etcDir string, systemctl func(args ...string) (string, error), logf func(string, ...any)) {
+//
+// It writes into the same <etc>/systemd/system unit files that `carlos
+// system apply`'s installUnits copies, using the same durable-write
+// staging name — so it takes the apply's own lock, non-blocking, before
+// touching anything. An apply in flight wins; this pass leaves it alone
+// 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)) {
+ if err := os.MkdirAll(filepath.Dir(lockPath), 0o755); err != nil {
+ logf("adopt: system units: %v", err)
+ return
+ }
+ lock, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0o600)
+ if err != nil {
+ logf("adopt: system units: %v", err)
+ return
+ }
+ defer lock.Close()
+ // Any error here — real contention, or errSystemUnsupported on the
+ // windows build that has no flock — means don't touch the units: an
+ // apply may be mid-copy, or this platform can't tell, and either way
+ // leaving it alone is the safe default. The lock's cadence takes care
+ // of trying again next tick.
+ if err := flockExclusiveNonblock(lock); err != nil {
+ logf("adopt: system units: an apply is running; leaving the self-update units to it")
+ return
+ }
+
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)
+ return
+ }
missing := false
for _, u := range systemUnitFiles {
if _, err := os.Stat(filepath.Join(unitDir, u)); err != nil {
missing = true
}
}
- active, _ := systemctl("is-active", systemUpdatePathUnit)
- enabled, _ := systemctl("is-enabled", "carlos-system-recover.service")
if !missing && strings.TrimSpace(active) == "active" && strings.TrimSpace(enabled) == "enabled" {
return
}
+ if err := os.MkdirAll(unitDir, 0o755); err != nil {
+ logf("adopt: system units: %v", err)
+ return
+ }
for _, u := range systemUnitFiles {
b, err := unitsFS.ReadFile("units/" + u)
if err != nil {
@@ -37,7 +81,7 @@ func ensureSystemUnits(etcDir string, systemctl func(args ...string) (string, er
return
}
}
- for _, args := range systemctlPlan([]string{systemUpdatePathUnit, "carlos-system-recover.service"}) {
+ for _, args := range systemctlPlan([]string{systemUpdatePathUnit, systemRecoverUnit}) {
if _, err := systemctl(args...); err != nil {
logf("adopt: system units: systemctl %s: %v", strings.Join(args, " "), err)
return
diff --git a/internal/host/systemunits_test.go b/internal/host/systemunits_test.go
index 96f1408b3..5fe204f8e 100644
--- a/internal/host/systemunits_test.go
+++ b/internal/host/systemunits_test.go
@@ -1,6 +1,7 @@
package host
import (
+ "errors"
"os"
"path/filepath"
"strings"
@@ -10,6 +11,7 @@ import (
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, " "))
@@ -21,7 +23,7 @@ func TestEnsureSystemUnitsInstallsAndArmsWhatIsMissing(t *testing.T) {
}
return "", nil
}
- ensureSystemUnits(etc, systemctl, t.Logf)
+ ensureSystemUnits(etc, lockPath, 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 not installed", u)
@@ -48,8 +50,52 @@ func TestEnsureSystemUnitsInstallsAndArmsWhatIsMissing(t *testing.T) {
}
return "", nil
}
- ensureSystemUnits(etc, systemctl2, t.Logf)
+ ensureSystemUnits(etc, lockPath, systemctl2, t.Logf)
if len(calls) != 2 {
t.Fatalf("an armed box got %v", calls)
}
}
+
+func TestEnsureSystemUnitsLeavesAConcurrentApplyAlone(t *testing.T) {
+ etc := t.TempDir()
+ os.MkdirAll(filepath.Join(etc, "systemd", "system"), 0o755)
+ lockPath := filepath.Join(t.TempDir(), "lock")
+ // Hold the lock as `carlos system apply` would.
+ held, err := os.OpenFile(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 calls []string
+ systemctl := func(args ...string) (string, error) {
+ calls = append(calls, strings.Join(args, " "))
+ return "", nil
+ }
+ ensureSystemUnits(etc, lockPath, systemctl, t.Logf)
+ if len(calls) != 0 {
+ t.Fatalf("a locked-out pass called systemctl: %v", calls)
+ }
+ 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 by a locked-out pass", u)
+ }
+ }
+}
+
+func TestEnsureSystemUnitsTrustsNoBrokenSystemctl(t *testing.T) {
+ etc := t.TempDir()
+ os.MkdirAll(filepath.Join(etc, "systemd", "system"), 0o755)
+ lockPath := filepath.Join(t.TempDir(), "lock")
+ systemctl := func(args ...string) (string, error) {
+ return "", errors.New("systemctl: command not found")
+ }
+ ensureSystemUnits(etc, lockPath, 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)
+ }
+ }
+}