carlos / platform Public

moves: one activation guard, both locks, every rename and finish; contention defers, never activates unlocked (Task 13 fix round 3)

- activationGuard replaces guardActivation. It always takes floor.lock,
  then, with a floor armed, always freezes the held rows (also when the
  target is the running release), and runs the floor and hold check
  inside, immediately before the step.
- Every executable rename and every journal finish goes through it: the
  install rename, the restore rename, the forward and backward recovery
  finishes, and both rollback finishes. The ordinary install finish after
  its guarded rename stays outside, with a comment. An AST scan test
  (activationguard_test.go) fails on any rename or finish outside the
  guard and on any stale exemption.
- floor.lock contention (the wait runs out) defers the apply with its own
  reason; only a lock that cannot exist (EROFS, ENOSPC, EACCES, EPERM,
  EDQUOT) on an unarmed box falls back to main's step (F15).
- A restore stopped before its rename whose rearm fails records
  rearm-pending.json with the units as they were before the quiesce;
  every run starts what is pending before any early refusal.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude agent, operated by paul@keymail.dev f0e760a4a6f332f3161eec01937900a0832eeb53 parent e7eedf6
8 files changed, +559 −96
  • docs/superpowers/plans/2026-10-06-moves-phase1-copy.md +3 −0
  • internal/host/activationguard_test.go +144 −0
  • internal/host/movesfloor.go +10 −0
  • internal/host/movesfloor_linux.go +24 −5
  • internal/host/movesfloor_linux_test.go +25 −0
  • internal/host/movesfloor_other.go +8 −1
  • internal/host/systemapply.go +189 −90
  • internal/host/systemapply_test.go +156 −0
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
index d1d359459..ddeff3ade 100644
--- a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
+++ b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
@@ -144,6 +144,9 @@ Columns: id, file, text, status.
| MV-N102 | internal/release/materializeheld.go | release: held rows are not staged while CARLOS_HOST_CLAIM is on | pending |
| MV-N103 | internal/appconfig/renderheld.go | config: not a held row of this move | pending |
| MV-N104 | internal/appconfig/renderheld.go | config: held rows are not rendered while CARLOS_HOST_CLAIM is on | pending |
+| MV-N105 | internal/host/systemapply.go | moves protocol floor: another writer holds the floor lock on this box; this waits for the next run | pending |
+| MV-N106 | internal/host/systemapply.go | <error>; the root passes paused for the restore could not be started again: <error>, and the retry could not be recorded: <error> | pending |
+| MV-N107 | internal/host/systemapply.go | <error>; the journal stays: <error> | pending |
## Plan 2
diff --git a/internal/host/activationguard_test.go b/internal/host/activationguard_test.go
new file mode 100644
index 000000000..ead15f310
--- /dev/null
+++ b/internal/host/activationguard_test.go
@@ -0,0 +1,144 @@
+package host
+
+import (
+ "go/ast"
+ "go/parser"
+ "go/token"
+ "sort"
+ "strings"
+ "testing"
+)
+
+// TestEveryRenameAndFinishIsGuarded is the invariant of moves Task 13 (fix
+// round 3) as a scan of systemapply.go: every os.Rename (an executable put
+// in place), every a.finish( and every a.removeJournal() (a journal
+// finished) sits inside a function literal passed to a.activationGuard,
+// unless the table below names it and says why. A new call that is
+// neither fails here, and so does an exemption no call uses any more.
+func TestEveryRenameAndFinishIsGuarded(t *testing.T) {
+ exempt := map[string]string{
+ // The ordinary install finish follows the guarded rename: the
+ // floor protects activation, and the rename is the activation.
+ "install a.finish": "after the guarded rename",
+ // Inside finish itself; its callers are guarded or exempt.
+ "finish a.removeJournal": "finish is the guarded or exempt step",
+ // The install never reached its rename: nothing was activated.
+ "abandonInstall a.removeJournal": "no executable changed",
+ // A legacy backing-up journal: nothing on the box had changed.
+ "recoverIfNeeded a.removeJournal (legacy backing-up)": "no executable changed",
+ }
+ // Sites that must be guarded, so a guard removed from one is caught even
+ // if someone also adds it to the exemptions.
+ mustGuard := []string{
+ "install os.Rename",
+ "restoreFiles os.Rename",
+ "recoverIfNeeded a.finish",
+ "recoverIfNeeded a.removeJournal",
+ "rollback a.removeJournal",
+ }
+
+ fset := token.NewFileSet()
+ file, err := parser.ParseFile(fset, "systemapply.go", nil, parser.ParseComments)
+ if err != nil {
+ t.Fatal(err)
+ }
+ guarded := map[string]int{}
+ used := map[string]bool{}
+ var unguarded []string
+ for _, decl := range file.Decls {
+ fn, ok := decl.(*ast.FuncDecl)
+ if !ok || fn.Body == nil {
+ continue
+ }
+ var stack []ast.Node
+ ast.Inspect(fn.Body, func(n ast.Node) bool {
+ if n == nil {
+ stack = stack[:len(stack)-1]
+ return true
+ }
+ stack = append(stack, n)
+ call, ok := n.(*ast.CallExpr)
+ if !ok {
+ return true
+ }
+ name := callName(call)
+ if name != "os.Rename" && name != "a.finish" && name != "a.removeJournal" {
+ return true
+ }
+ site := fn.Name.Name + " " + name
+ if insideGuard(stack) {
+ guarded[site]++
+ return true
+ }
+ key := site
+ if _, ok := exempt[key]; !ok && fn.Name.Name == "recoverIfNeeded" && name == "a.removeJournal" && legacyBranch(stack) {
+ key = site + " (legacy backing-up)"
+ }
+ if _, ok := exempt[key]; ok {
+ used[key] = true
+ return true
+ }
+ unguarded = append(unguarded, site+" at "+fset.Position(call.Pos()).String())
+ return true
+ })
+ }
+ sort.Strings(unguarded)
+ for _, u := range unguarded {
+ t.Errorf("%s is outside activationGuard and not exempt", u)
+ }
+ for _, s := range mustGuard {
+ if guarded[s] == 0 {
+ t.Errorf("%s is not inside activationGuard", s)
+ }
+ }
+ for k := range exempt {
+ if !used[k] {
+ t.Errorf("exemption %q matches no call any more", k)
+ }
+ }
+}
+
+func callName(call *ast.CallExpr) string {
+ sel, ok := call.Fun.(*ast.SelectorExpr)
+ if !ok {
+ return ""
+ }
+ if id, ok := sel.X.(*ast.Ident); ok {
+ return id.Name + "." + sel.Sel.Name
+ }
+ return ""
+}
+
+// insideGuard: some enclosing function literal is an argument of a call to
+// a.activationGuard.
+func insideGuard(stack []ast.Node) bool {
+ for i := len(stack) - 1; i > 0; i-- {
+ lit, ok := stack[i].(*ast.FuncLit)
+ if !ok {
+ continue
+ }
+ if call, ok := stack[i-1].(*ast.CallExpr); ok && callName(call) == "a.activationGuard" {
+ for _, arg := range call.Args {
+ if arg == lit {
+ return true
+ }
+ }
+ }
+ }
+ return false
+}
+
+// legacyBranch: the call sits in the `if j.Phase == systemPhaseBackingUp`
+// branch.
+func legacyBranch(stack []ast.Node) bool {
+ for _, n := range stack {
+ if ifs, ok := n.(*ast.IfStmt); ok {
+ if be, ok := ifs.Cond.(*ast.BinaryExpr); ok {
+ if id, ok := be.Y.(*ast.Ident); ok && strings.Contains(id.Name, "BackingUp") {
+ return true
+ }
+ }
+ }
+ }
+ return false
+}
diff --git a/internal/host/movesfloor.go b/internal/host/movesfloor.go
index 37cc95f8b..ac979fe28 100644
--- a/internal/host/movesfloor.go
+++ b/internal/host/movesfloor.go
@@ -102,6 +102,16 @@ func readLocalFloor(dataDir string) (*moves.Floor, error) {
return &fl, nil
}
+// errFloorLockBusy is floor.lock still held by another process after the
+// wait: a writer is active, and may be about to publish a floor. Never a
+// reason to go ahead without the lock (fix round 3): an activation that
+// meets it is deferred.
+var errFloorLockBusy = errors.New("another writer holds the floor lock on this box")
+
+// floorLockWait bounds the wait for floor.lock (movesDir.Lock); a variable
+// so a test can make contention time out quickly.
+var floorLockWait = 2 * time.Minute
+
// movesDirName is the directory both local floor files live in, below
// the data dir.
var movesDirName = filepath.Dir(moves.FloorFile)
diff --git a/internal/host/movesfloor_linux.go b/internal/host/movesfloor_linux.go
index 29c654a22..a8916e823 100644
--- a/internal/host/movesfloor_linux.go
+++ b/internal/host/movesfloor_linux.go
@@ -30,10 +30,7 @@ type movesDir struct {
// it (root's judgement of carlos.prev and its activation guard).
const floorLockFile = "floor.lock"
-const (
- floorLockWait = 2 * time.Minute
- floorLockPoll = 100 * time.Millisecond
-)
+const floorLockPoll = 100 * time.Millisecond
// openMovesDir opens <root>/.moves. With create it makes the directory
// when missing; without, a missing directory is os.ErrNotExist.
@@ -110,10 +107,14 @@ func (d *movesDir) Lock() error {
d.lock = fd
return nil
}
- if !errors.Is(err, syscall.EWOULDBLOCK) || time.Now().After(deadline) {
+ if !errors.Is(err, syscall.EWOULDBLOCK) {
syscall.Close(fd)
return fmt.Errorf("lock %s: %w", filepath.Join(d.path, floorLockFile), err)
}
+ if time.Now().After(deadline) {
+ syscall.Close(fd)
+ return fmt.Errorf("lock %s: %w", filepath.Join(d.path, floorLockFile), errFloorLockBusy)
+ }
time.Sleep(floorLockPoll)
}
}
@@ -175,3 +176,21 @@ func fchownToService(fd int) {
}
syscall.Fchown(fd, uid, gid)
}
+
+// floorLockCannotExist reports whether err, from opening or locking the
+// floor lock, means the lock cannot exist on this box at all: .moves or
+// floor.lock cannot be created (a read-only, full or unwritable data dir).
+// Then no writer can arm a floor either, which is what makes the F15
+// fallback safe. Contention (errFloorLockBusy) is never this: a writer is
+// active.
+func floorLockCannotExist(err error) bool {
+ if errors.Is(err, errFloorLockBusy) {
+ return false
+ }
+ for _, e := range []syscall.Errno{syscall.EROFS, syscall.ENOSPC, syscall.EACCES, syscall.EPERM, syscall.EDQUOT} {
+ if errors.Is(err, e) {
+ return true
+ }
+ }
+ return false
+}
diff --git a/internal/host/movesfloor_linux_test.go b/internal/host/movesfloor_linux_test.go
index 2b99c3b4f..d378da768 100644
--- a/internal/host/movesfloor_linux_test.go
+++ b/internal/host/movesfloor_linux_test.go
@@ -4,6 +4,8 @@ package host
import (
"errors"
+ "fmt"
+ "syscall"
"testing"
"github.com/carlosframework/platform/internal/moves"
@@ -32,3 +34,26 @@ func TestEveryFloorWriteSyncsTheDataDir(t *testing.T) {
t.Fatalf("floor after a failed fsync = %+v", f)
}
}
+
+// Only a lock that cannot be created counts as "cannot exist" (the F15
+// fallback). Contention never does: a writer is active (fix round 3).
+func TestFloorLockCannotExistIsOnlyForAnUncreatableLock(t *testing.T) {
+ for _, tc := range []struct {
+ err error
+ want bool
+ }{
+ {fmt.Errorf("mkdir /data/.moves: %w", syscall.EROFS), true},
+ {fmt.Errorf("mkdir /data/.moves: %w", syscall.ENOSPC), true},
+ {fmt.Errorf("open floor.lock: %w", syscall.EACCES), true},
+ {fmt.Errorf("mkdir: %w", syscall.EPERM), true},
+ {fmt.Errorf("mkdir: %w", syscall.EDQUOT), true},
+ {fmt.Errorf("lock /data/.moves/floor.lock: %w", errFloorLockBusy), false},
+ {fmt.Errorf("open /data/.moves: %w", syscall.ELOOP), false},
+ {fmt.Errorf("fsync /data: %w", syscall.EIO), false},
+ {errors.Join(syscall.EROFS, errFloorLockBusy), false},
+ } {
+ if got := floorLockCannotExist(tc.err); got != tc.want {
+ t.Errorf("floorLockCannotExist(%v) = %v, want %v", tc.err, got, tc.want)
+ }
+ }
+}
diff --git a/internal/host/movesfloor_other.go b/internal/host/movesfloor_other.go
index 0390a1d66..1937948ec 100644
--- a/internal/host/movesfloor_other.go
+++ b/internal/host/movesfloor_other.go
@@ -2,7 +2,10 @@
package host
-import "os"
+import (
+ "errors"
+ "os"
+)
// The floor files are written by root's apply and the agent, both of which
// run on a Linux box only (see systemapply_other.go). Off Linux every
@@ -19,3 +22,7 @@ func (d *movesDir) Lock() error { return errSystemUnsupported }
func (d *movesDir) WriteDurable(name string, data []byte, mode os.FileMode) error {
return errSystemUnsupported
}
+
+// floorLockCannotExist: off Linux the lock never exists (openMovesDir
+// refuses), and neither can a floor.
+func floorLockCannotExist(err error) bool { return errors.Is(err, errSystemUnsupported) }
diff --git a/internal/host/systemapply.go b/internal/host/systemapply.go
index fd2856e2e..cb5ac6357 100644
--- a/internal/host/systemapply.go
+++ b/internal/host/systemapply.go
@@ -225,7 +225,7 @@ type systemApplier struct {
// store is the deploy store verifyPointer opened, kept for armFloor.
// Nil on --recover, which reads no store.
store release.Store
- // beforeSwap, when set, runs inside guardActivation after the rule has
+ // beforeSwap, when set, runs inside activationGuard after the rule has
// allowed and just before the step it guards: a test's schedule hook.
// Nil in production.
beforeSwap func()
@@ -445,15 +445,16 @@ func (a *systemApplier) install(ctx context.Context, req systemRequest, p *relea
// passes are idempotent. Which of their triggers were active is kept, so
// a refused rename can put them back.
prior := a.quiesce()
- // The activation rule again, at the step it guards (moves spec §11; fix
- // round 1): the check before staging can have gone stale, since the
- // agent may have raised the floor or held a row since. Here the floor
- // and the holds are held still across the rename.
- if err := a.guardActivation(target, running, func() error {
+ // The activation rule again, at the step it guards (moves spec §11): the
+ // check before staging can have gone stale, since the agent may have
+ // raised the floor or held a row since. activationGuard holds the floor
+ // and the holds still across the rename.
+ renamed := false
+ if err := a.activationGuard(target, running, func() error {
+ renamed = true
return os.Rename(staged, systemBinaryPath(a.optDir))
}); err != nil {
- var refusal activationRefused
- if errors.As(err, &refusal) {
+ if !renamed {
return a.abandonInstall(req, err, prior)
}
return fail("install", err)
@@ -487,6 +488,11 @@ func (a *systemApplier) install(ctx context.Context, req systemRequest, p *relea
if err := a.journal(j); err != nil {
return fail("journal", err)
}
+ // Outside activationGuard, deliberately: the floor protects activation,
+ // and the guarded rename above was this install's activation. A floor
+ // raised since applies to the next one. (Recovery's forward finish is
+ // different: it has no rename of its own, so the finish is its
+ // activation, and it is guarded.)
return a.finish(req, target, newPlan)
}
@@ -505,16 +511,23 @@ func (a *systemApplier) install(ctx context.Context, req systemRequest, p *relea
// interrupted install and restores itself over a binary that never changed.
// Only when even the retry state cannot be written does the journal stay.
func (a *systemApplier) abandonInstall(req systemRequest, refusal error, prior []string) error {
+ // A guard that could not get the floor lock is deferred (retried at the
+ // next run); every other stop is a refusal.
+ kind := errSystemRefused
+ var deferred activationDeferred
+ if errors.As(refusal, &deferred) {
+ kind = errSystemDeferred
+ }
rerr := a.rearm(prior)
if rerr == nil {
a.removeJournal()
- return a.record(req, fmt.Errorf("%w: %v", errSystemRefused, refusal))
+ return a.record(req, fmt.Errorf("%w: %v", kind, refusal))
}
reason := fmt.Errorf("%w: %v; the root passes paused for the install could not be started again: %v. the next apply or recovery starts them",
- errSystemRefused, refusal, rerr)
+ kind, refusal, rerr)
if err := a.notePendingRearm(prior); err != nil {
reason = fmt.Errorf("%w: %v; the root passes paused for the install could not be started again: %v, and the retry could not be recorded: %v. the journal stays for the next recovery",
- errSystemRefused, refusal, rerr, err)
+ kind, refusal, rerr, err)
} else {
a.removeJournal()
}
@@ -778,7 +791,7 @@ func (a *systemApplier) armFloor(ctx context.Context) error {
//
// This is the EARLY check, made before anything is staged so a refused
// build is never copied in or run. It can go stale by the time the build is
-// put in place, so the step that puts it there runs under guardActivation,
+// put in place, so the step that puts it there runs under activationGuard,
// which applies the same rule again with its inputs held still.
//
// How the hold half relates to systemparticipation.go (region HA H14): that
@@ -791,12 +804,11 @@ func (a *systemApplier) armFloor(ctx context.Context) error {
// (this check before staging, H14's marker just before the journal) and
// either refusal stands. Neither reads or relaxes the other.
func (a *systemApplier) activationAllowed(ctx context.Context, version string) error {
- return a.activationRule(version, func() string { return a.onDiskBuild(ctx) }, nil)
+ return a.activationRule(version, func() string { return a.onDiskBuild(ctx) })
}
// activationRefused is a refusal by the activation rule, told apart from a
-// failure of the step it guards: the install abandons on one and rolls
-// back on the other.
+// failure of the step it guards.
type activationRefused struct{ error }
func (e activationRefused) Unwrap() error { return e.error }
@@ -805,93 +817,144 @@ func refuse(format string, args ...any) error {
return activationRefused{fmt.Errorf(format, args...)}
}
-// guardActivation runs swap, the step that makes version this box's
-// executable, with the rule's inputs held still across it (fix round 1):
-// the floor files under the floor lock, which every floor writer and every
-// judgement of carlos.prev also takes, and, when the rule needs them, the
-// held rows under the registry's write lock (WithHoldsFrozen: a SetHold
-// waits). running is the release on disk, read by the caller before any
-// lock, since reading it runs a binary. A refusal is an activationRefused;
-// swap's own error comes back as it is.
-func (a *systemApplier) guardActivation(version, running string, swap func() error) error {
- guarded := func() error {
+// activationDeferred is the guard unable to get floor.lock because another
+// process holds it: a writer is active and may be about to publish a floor.
+// The step is put off to the next run, never taken unlocked (fix round 3).
+type activationDeferred struct{ error }
+
+func (e activationDeferred) Unwrap() error { return e.error }
+
+// activationGuard is the ONE guard for every step that makes an executable
+// this box's (moves spec §11; fix round 3): the install's rename, the
+// restore's rename, and each finish of a journal that has no rename of its
+// own (recovery forward and backward, and rollback's). Nothing else renames
+// an executable into place or finishes a journal; the scan in
+// activationguard_test.go fails if something does.
+//
+// It always takes floor.lock first, creating .moves for it on a box with
+// no floor yet, so no writer can arm or raise a floor until fn has run.
+// Then, with a floor armed, it always freezes the held rows
+// (registry.WithHoldsFrozen), whether or not target is the running release,
+// and checks the floor and the hold rule inside, immediately before fn.
+// With no floor armed, under the lock none can appear, the hold rule does
+// not apply, and the registry is not consulted (index D13: an unarmed
+// deployment behaves as main).
+//
+// Lock failures:
+// - contention (the wait ran out): activationDeferred. Never unlocked: a
+// writer holding the lock may not have published yet.
+// - the lock cannot exist (.moves or floor.lock cannot be created:
+// read-only, full or unwritable data dir) and no floor is armed: one
+// log line and fn runs as main's step would (F15). No writer can arm a
+// floor on such a box either.
+// - anything else, or a floor armed: a refusal.
+//
+// running is the release on disk, read by the caller before any lock,
+// since reading it runs a binary. A refusal or deferral comes back typed;
+// fn's own error comes back as it is.
+func (a *systemApplier) activationGuard(target, running string, fn func() error) error {
+ step := func() error {
if a.beforeSwap != nil {
a.beforeSwap()
}
- return swap()
+ return fn()
}
- ran := false
- // create: the lock is taken even on a box with no floor yet, so an
- // agent arming the first floor waits for this step rather than landing
- // between the rule and the swap (fix round 2).
+ locked := false
err := withFloorLock(a.dataDir, true, func(*movesDir) error {
- ran = true
- return a.activationRule(version, func() string { return running }, guarded)
+ locked = true
+ floor, err := readLocalFloor(a.dataDir)
+ if err != nil {
+ return refuse("moves protocol floor: the floor on this box cannot be read: %v", err)
+ }
+ if floor == nil {
+ return step()
+ }
+ if err := floorRefusal(target, floor); err != nil {
+ return err
+ }
+ frozen := false
+ err = a.freezeHolds(func(held []registry.Route) error {
+ frozen = true
+ if err := holdRefusal(target, running, held); err != nil {
+ return err
+ }
+ return step()
+ })
+ if !frozen {
+ return refuse("a participant in a held operation does not downgrade, and the registry cannot say whether this box is one: %v", err)
+ }
+ return err
})
- if ran {
+ if locked {
return err
}
- // The lock could not be had. With a floor armed (or one that cannot be
- // read) that is a refusal. With none, the F15 dormancy rule: one log
- // line and the step goes ahead as main's would; an agent that cannot
- // create the lock cannot arm a floor either.
- if a.floorArmed() {
- return refuse("moves protocol floor: the floor on this box cannot be read: %v", err)
+ if errors.Is(err, errFloorLockBusy) {
+ return activationDeferred{fmt.Errorf("moves protocol floor: %v; this waits for the next run", errFloorLockBusy)}
+ }
+ if floorLockCannotExist(err) && !a.floorArmed() {
+ a.ops.logf("system apply: moves protocol floor lock: %v (no floor is armed, and none can be on this box; continuing)", err)
+ return step()
}
- a.ops.logf("system apply: moves protocol floor lock: %v (no floor is armed; continuing)", err)
- return guarded()
+ return refuse("moves protocol floor: the floor on this box cannot be read: %v", err)
}
-// activationRule is the rule itself. swap, when not nil, runs only once the
-// rule allows, inside whatever the rule had to hold to decide: guardActivation
-// passes the step, and the early check (activationAllowed) passes nil.
-func (a *systemApplier) activationRule(version string, running func() string, swap func() error) error {
- early := swap == nil
- if early {
- swap = func() error { return nil }
+// floorRefusal is the floor half of the rule.
+func floorRefusal(version string, floor *moves.Floor) error {
+ if version == "" {
+ return refuse("moves protocol floor: carlos.prev records no version, so it is below %s", floor.MinRelease)
}
+ if ok, _ := moves.ReleaseAtOrAbove(version, floor.MinRelease); !ok {
+ return refuse("moves protocol floor: %s is below %s", version, floor.MinRelease)
+ }
+ return nil
+}
+
+// holdRefusal is the hold half: while any row is held, nothing below the
+// running release. The same release, or a later one, is not a downgrade.
+func holdRefusal(version, running string, held []registry.Route) error {
+ if len(held) == 0 || version == running {
+ return nil
+ }
+ if ok, _ := moves.ReleaseAtOrAbove(version, running); ok {
+ return nil
+ }
+ if running == "" {
+ return refuse("a participant in a held operation does not downgrade, and the running release is unknown")
+ }
+ return refuse("a participant in a held operation does not downgrade: %s is below the running %s", version, running)
+}
+
+// activationRule is the early, unlocked check (activationAllowed, and
+// restoreFiles before its quiesce): the same two halves, read once, so a
+// refused build is never staged and a refused restore never quiesces. It
+// guards nothing by itself; the step it precedes runs under
+// activationGuard.
+func (a *systemApplier) activationRule(version string, running func() string) error {
floor, err := readLocalFloor(a.dataDir)
if err != nil {
return refuse("moves protocol floor: the floor on this box cannot be read: %v", err)
}
if floor == nil {
- if !early {
- return swap()
- }
a.ops.logf("system apply: no moves protocol floor is armed on this box; %q is not checked against one", version)
- return swap()
- }
- if version == "" {
- return refuse("moves protocol floor: carlos.prev records no version, so it is below %s", floor.MinRelease)
+ return nil
}
- if ok, _ := moves.ReleaseAtOrAbove(version, floor.MinRelease); !ok {
- return refuse("moves protocol floor: %s is below %s", version, floor.MinRelease)
+ if err := floorRefusal(version, floor); err != nil {
+ return err
}
run := running()
if version == run {
- return swap()
+ return nil
}
if ok, _ := moves.ReleaseAtOrAbove(version, run); ok {
- return swap() // not a downgrade: the held rows do not matter
+ return nil
}
- // A downgrade, or a running release that cannot be compared: allowed
- // only while no row is held, and the rows stay as they were read until
- // swap has run.
+ var held []registry.Route
called := false
- err = a.freezeHolds(func(held []registry.Route) error {
- called = true
- if len(held) == 0 {
- return swap()
- }
- if run == "" {
- return refuse("a participant in a held operation does not downgrade, and the running release is unknown")
- }
- return refuse("a participant in a held operation does not downgrade: %s is below the running %s", version, run)
- })
+ err = a.freezeHolds(func(h []registry.Route) error { called = true; held = h; return nil })
if !called {
return refuse("a participant in a held operation does not downgrade, and the registry cannot say whether this box is one: %v", err)
}
- return err
+ return holdRefusal(version, run, held)
}
// floorAllowsForward is the floor half alone, for recovery's forward path:
@@ -1306,22 +1369,35 @@ func (a *systemApplier) rollback(ctx context.Context, req systemRequest, reason
if err != nil {
return failed("restore", err) // journal kept: the restore is half done
}
+ // carlos.prev, now on disk: what this rollback's journal finish makes
+ // current, checked again under activationGuard at that finish.
+ restored, _ := readLine(prevVersionPath(a.optDir))
if err := a.restartEdge(); err != nil {
return failed("restart", err) // journal kept: the edge is still to come back
}
a.restartRelay()
if err := a.gate(ctx, j.PrevBuild); err != nil {
err := failed("previous build failed its gate too", err)
- a.removeJournal()
+ if gerr := a.activationGuard(restored, restored, func() error {
+ a.removeJournal()
+ return nil
+ }); gerr != nil {
+ return fmt.Errorf("%v; the journal stays: %v", err, gerr)
+ }
return err
}
if err := a.arm(plan); err != nil {
a.ops.logf("system apply: arm previous plan: %v", err)
}
- if err := a.writeResult(req, systemOutcomeRolledBack, reason); err != nil {
- return err
+ if err := a.activationGuard(restored, restored, func() error {
+ if err := a.writeResult(req, systemOutcomeRolledBack, reason); err != nil {
+ return err
+ }
+ a.removeJournal()
+ return nil
+ }); err != nil {
+ return failed("finish", err) // journal kept
}
- a.removeJournal()
return fmt.Errorf("system apply: %s rolled back: %s", j.Target, reason)
}
@@ -1342,7 +1418,7 @@ func (a *systemApplier) restoreFiles(ctx context.Context, j *systemJournal) (boo
// box stays on the build it is on, and the caller reports it.
prevVersion, _ := readLine(prevVersionPath(a.optDir))
running := a.onDiskBuild(ctx)
- if err := a.activationRule(prevVersion, func() string { return running }, nil); err != nil {
+ if err := a.activationRule(prevVersion, func() string { return running }); err != nil {
return bootstrapPlan{}, err
}
// The same quiesce the install did: a root pass that is mid-flight
@@ -1355,20 +1431,27 @@ func (a *systemApplier) restoreFiles(ctx context.Context, j *systemJournal) (boo
if err := copyFileDurable(prev, restore, 0o755); err != nil {
return bootstrapPlan{}, err
}
- if err := a.guardActivation(prevVersion, running, func() error {
+ renamed := false
+ if err := a.activationGuard(prevVersion, running, func() error {
+ renamed = true
if err := os.Rename(restore, systemBinaryPath(a.optDir)); err != nil {
return err
}
return syncDir(a.optDir)
}); err != nil {
os.Remove(restore)
- // Refused before the rename (the floor or a hold changed during the
- // copy): nothing was restored, so the paused triggers go back to how
- // they were. The caller keeps the journal.
- var refusal activationRefused
- if errors.As(err, &refusal) {
+ // Stopped before the rename (refused or deferred: the floor or a hold
+ // changed during the copy, or a writer holds the floor lock): nothing
+ // was restored, so the paused triggers go back to how they were. If
+ // they cannot be started, they go to rearm-pending.json, captured as
+ // they were before the quiesce, and every run starts them first,
+ // before any early refusal. The caller keeps the journal.
+ if !renamed {
if rerr := a.rearm(prior); rerr != nil {
err = fmt.Errorf("%w; the root passes paused for the restore could not be started again: %v", err, rerr)
+ if nerr := a.notePendingRearm(prior); nerr != nil {
+ err = fmt.Errorf("%w, and the retry could not be recorded: %v", err, nerr)
+ }
}
}
return bootstrapPlan{}, err
@@ -1500,11 +1583,18 @@ func (a *systemApplier) recoverIfNeeded(ctx context.Context) (bool, error) {
// Finishing the journal is the forward path's activation: the
// rule again, under the guard, at the step itself, since the
// floor can rise while the plan is staged (fix round 2).
- err = a.guardActivation(j.Target, j.Target, func() error { return a.finish(req, j.Target, plan) })
- var refusal activationRefused
- if !errors.As(err, &refusal) {
+ finished := false
+ err = a.activationGuard(j.Target, j.Target, func() error {
+ finished = true
+ return a.finish(req, j.Target, plan)
+ })
+ if finished {
return true, err
}
+ var deferred activationDeferred
+ if errors.As(err, &deferred) {
+ return false, fmt.Errorf("system apply: recovery: %w", err) // journal kept
+ }
a.ops.logf("system apply: recovery: %s passed its gate but %v; rolling back instead", j.Target, err)
}
}
@@ -1512,6 +1602,8 @@ func (a *systemApplier) recoverIfNeeded(ctx context.Context) (bool, error) {
if err != nil {
return false, fmt.Errorf("system apply: recovery: %w", err) // journal kept: the next run recovers again
}
+ // carlos.prev, now on disk: what the finish below makes current.
+ restored, _ := readLine(prevVersionPath(a.optDir))
if a.ops.edgeStartedThisBoot() {
a.ops.systemctl("reset-failed", systemEdgeUnit)
// --no-block: this can be running as the edge's own OnFailure=
@@ -1530,10 +1622,17 @@ func (a *systemApplier) recoverIfNeeded(ctx context.Context) (bool, error) {
a.ops.systemctl(append([]string{"start"}, armed...)...)
}
}
- if err := a.writeResult(req, systemOutcomeRolledBack, "interrupted at "+j.Phase); err != nil {
- return false, err
+ // The backward finish is guarded like every other: a floor raised since
+ // the restore's rename keeps the journal, and the next run judges again.
+ if err := a.activationGuard(restored, restored, func() error {
+ if err := a.writeResult(req, systemOutcomeRolledBack, "interrupted at "+j.Phase); err != nil {
+ return err
+ }
+ a.removeJournal()
+ return nil
+ }); err != nil {
+ return false, fmt.Errorf("system apply: recovery: %w", err) // journal kept
}
- a.removeJournal()
return true, nil
}
diff --git a/internal/host/systemapply_test.go b/internal/host/systemapply_test.go
index 894cd5080..b90fb8fdb 100644
--- a/internal/host/systemapply_test.go
+++ b/internal/host/systemapply_test.go
@@ -2281,3 +2281,159 @@ func TestUnarmedBoxWithoutALockDirUpdatesAsMain(t *testing.T) {
t.Fatalf("result %+v, on disk %q", r, f.onDisk())
}
}
+
+// ---- fix round 3: one guard, both locks, no unlocked path on contention ----
+
+// shortFloorLockWait makes a contended floor.lock time out quickly.
+func shortFloorLockWait(t *testing.T) {
+ old := floorLockWait
+ floorLockWait = 200 * time.Millisecond
+ t.Cleanup(func() { floorLockWait = old })
+}
+
+// holdFloorLock takes floor.lock as a writer would, without publishing
+// anything, until the returned func is called.
+func holdFloorLock(t *testing.T, dataDir string) func() {
+ t.Helper()
+ held, free, done := make(chan struct{}), make(chan struct{}), make(chan struct{})
+ go func() {
+ defer close(done)
+ if err := withFloorLock(dataDir, true, func(*movesDir) error {
+ close(held)
+ <-free
+ return nil
+ }); err != nil {
+ t.Error(err)
+ }
+ }()
+ <-held
+ return func() { close(free); <-done }
+}
+
+// A writer holding floor.lock has not necessarily published yet: a reader
+// that times out on it must not go ahead unlocked. The apply is deferred,
+// and nothing is activated.
+func TestContendedFloorLockDefersTheInstall(t *testing.T) {
+ shortFloorLockWait(t)
+ f, _ := floorFixture(t, "v0.31.0", "v0.32.0")
+ unlock := holdFloorLock(t, f.a.dataDir)
+ defer unlock()
+ if err := f.a.Run(context.Background(), false); err != nil {
+ t.Fatal(err)
+ }
+ r := f.result()
+ if r == nil || r.Outcome != systemOutcomeDeferred || !strings.Contains(r.Reason, "another writer holds the floor lock") {
+ t.Fatalf("result = %+v", r)
+ }
+ if f.onDisk() != "v0.31.0" {
+ t.Fatalf("activated under a contended lock: on disk %q", f.onDisk())
+ }
+ if j, _ := readSystemJournal(f.a.optDir); j != nil {
+ t.Fatalf("journal left: %+v", j)
+ }
+}
+
+// With a floor armed, the guard freezes the holds even where the version
+// being made current is the one already on disk (a journal finish).
+func TestForwardFinishFreezesTheHolds(t *testing.T) {
+ f, _ := floorFixture(t, "v0.30.0", "v0.31.0")
+ f.brokenBox("v0.31.0", "v0.30.0", "v0.30.0")
+ f.a.journal(systemJournal{Phase: systemPhaseGated, Target: "v0.31.0", PrevBuild: "v0.30.0", StartedAt: f.now})
+ armLocalFloor(t, f.a.dataDir, "v0.30.0")
+ frozen := 0
+ open := f.a.openRegistry
+ f.a.openRegistry = func() (*registry.Registry, error) { frozen++; return open() }
+ f.edgeStartedThisBoot = false
+ if err := f.a.Run(context.Background(), true); err != nil {
+ t.Fatal(err)
+ }
+ if ap, _ := readSystemApplied(f.a.optDir); ap == nil || ap.Version != "v0.31.0" {
+ t.Fatalf("not finished forward: %+v", ap)
+ }
+ if frozen == 0 {
+ t.Fatal("the forward finish did not freeze the holds")
+ }
+}
+
+// The backward recovery's own finish (result and journal removal) is
+// guarded too: a floor raised after the restore's rename, before the
+// finish, keeps the journal.
+func TestBackwardRecoveryFinishRechecksTheFloor(t *testing.T) {
+ f, _ := floorFixture(t, "v0.31.0", "v0.32.0")
+ armLocalFloor(t, f.a.dataDir, "v0.30.0")
+ f.brokenBox("v0.31.0", "v0.30.0", "v0.30.0")
+ systemctl := f.a.ops.systemctl
+ f.a.ops.systemctl = func(args ...string) (string, error) {
+ if args[0] == "restart" {
+ armLocalFloor(t, f.a.dataDir, "v0.31.0")
+ }
+ return systemctl(args...)
+ }
+ f.edgeStartedThisBoot = true
+ err := f.a.Run(context.Background(), true)
+ if err == nil || !strings.Contains(err.Error(), "moves protocol floor: v0.30.0 is below v0.31.0") {
+ t.Fatalf("err = %v", err)
+ }
+ if j, _ := readSystemJournal(f.a.optDir); j == nil {
+ t.Fatal("the journal was finished below a floor raised before the finish")
+ }
+}
+
+// A restore refused before its rename whose rearm then fails records the
+// units it paused (as they were before the quiesce) in rearm-pending.json.
+// The next recovery starts them first, and only then refuses again.
+func TestRefusedRestoreWhoseRearmFailsIsRetriedFirst(t *testing.T) {
+ f, _ := floorFixture(t, "v0.32.0", "v0.33.0")
+ armLocalFloor(t, f.a.dataDir, "v0.30.0")
+ j := f.brokenBox("v0.32.0", "v0.31.0", "v0.31.0")
+ active := map[string]bool{"carlos-adopt.timer": true, "carlos-adopt.path": true, "carlos-stage.path": false, "carlos-restart.path": true}
+ before := copyActivity(active)
+ f.unitActivity(active)
+ failStart := true
+ systemctl := f.a.ops.systemctl
+ var order []string
+ f.a.ops.systemctl = func(args ...string) (string, error) {
+ if args[0] == "start" {
+ order = append(order, "start")
+ if failStart {
+ return "", errors.New("unit masked")
+ }
+ }
+ out, err := systemctl(args...)
+ if args[0] == "stop" && failStart {
+ armLocalFloor(t, f.a.dataDir, "v0.33.0")
+ }
+ return out, err
+ }
+ if _, err := f.a.restoreFiles(context.Background(), &j); err == nil || !strings.Contains(err.Error(), "could not be started again") {
+ t.Fatalf("err = %v", err)
+ }
+ var p rearmPending
+ if err := readJSONFile(f.a.file(rearmPendingFile), &p); err != nil {
+ t.Fatalf("no retry state: %v", err)
+ }
+ if want := []string{"carlos-adopt.timer", "carlos-adopt.path", "carlos-restart.path"}; !slices.Equal(p.Units, want) {
+ t.Fatalf("pending units = %v, want %v (as they were before the quiesce)", p.Units, want)
+ }
+
+ failStart = false
+ f.edgeStartedThisBoot = false
+ err := f.a.Run(context.Background(), true)
+ if err == nil || !strings.Contains(err.Error(), "moves protocol floor") {
+ t.Fatalf("recovery err = %v", err)
+ }
+ if len(order) < 2 || order[len(order)-1] != "start" {
+ t.Fatalf("the recovery did not start the pending passes: %v", order)
+ }
+ if _, err := os.Stat(f.a.file(rearmPendingFile)); err == nil {
+ t.Fatal("retry state left after the passes started")
+ }
+ for u, was := range before {
+ if active[u] != was {
+ t.Errorf("%s active = %v, was %v", u, active[u], was)
+ }
+ }
+ if j, _ := readSystemJournal(f.a.optDir); j == nil {
+ t.Fatal("journal dropped")
+ }
+}