amadan
moves: the floor holds across staging, forward recovery, concurrent refreshes and a crash (Task 13 fix round 1)
- The install and restoreFiles put the executable in place under guardActivation: the floor lock (.moves/floor.lock, taken by every floor writer and every judgement of carlos.prev) and, for a downgrade, the held rows frozen by registry.WithHoldsFrozen (BEGIN IMMEDIATE). A refusal at the rename abandons the install: journal removed, quiesced passes re-armed, recorded refused. - Forward recovery from a gated journal checks the target against the armed floor; below it, recovery goes back to carlos.prev, which restoreFiles judges in turn. - Floor writers serialise on .moves/floor.lock and never lower a floor (a lower min or min_release is refused; lowering is a manual step on each box). floor.min-release is written first and floor.json last; every Go reader, the edges file included, trusts floor.json alone. - Creating .moves fsyncs its parent before any file is written in it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8 files changed,
+787
−123
internal/host/movesfloor.go+168 −59internal/host/movesfloor_linux.go+96 −29internal/host/movesfloor_other.go+13 −4internal/host/movesfloor_test.go+112 −6internal/host/systemapply.go+152 −25internal/host/systemapply_test.go+158 −0internal/registry/hold_test.go+46 −0internal/registry/registry.go+42 −0
diff --git a/internal/host/movesfloor.go b/internal/host/movesfloor.go| index c168a96c5..37cc95f8b 100644 |
| --- a/internal/host/movesfloor.go |
| +++ b/internal/host/movesfloor.go |
| @@ -31,8 +31,12 @@ import ( |
| // - <DataDir>/.moves/floor.json and the one-line floor.min-release: the |
| // floor as this box last read it. Root's apply writes them from the |
| // store before it stages anything (armFloor); the agent refreshes them |
| -// every hour (movesFloorLoop). Never deleted once written: a store that |
| -// no longer answers, or no longer holds the object, leaves them alone. |
| +// every hour (movesFloorLoop). Both write under .moves/floor.lock, and a |
| +// floor only rises: a lower published floor is refused, and lowering |
| +// one is an operator's manual step on each box (floorRegression). Never |
| +// deleted by this code: a store that no longer answers, or no longer |
| +// holds the object, leaves them alone. floor.json is the one every Go |
| +// reader trusts; floor.min-release is for the units (writeFloorFiles). |
| // - <optDir>/carlos.prev.version: carlos.prev's release, written by root |
| // whenever it replaces carlos.prev (decidePrev), or learned by running |
| // `carlos.prev version` when it is missing and a floor is armed. |
| @@ -98,25 +102,76 @@ func readLocalFloor(dataDir string) (*moves.Floor, error) { |
| return &fl, nil |
| } |
| -// writeLocalFloor writes both local floor files, floor.json first. The |
| -// order matters only across a crash between the two: floor.json alone arms |
| -// the Go checks (activationAllowed, judgePrev) while the units still pick |
| -// carlos.prev as main does, and that carlos.prev then refuses itself in |
| -// restoreFiles if it is below the floor. The next apply or refresh writes |
| -// the second file. |
| +// movesDirName is the directory both local floor files live in, below |
| +// the data dir. |
| +var movesDirName = filepath.Dir(moves.FloorFile) |
| + |
| +// withFloorLock runs fn holding .moves/floor.lock (see floorLockFile). With |
| +// create, .moves is made when missing (a writer arming the first floor). |
| +// Without it, a missing .moves means no floor is armed and nothing can be |
| +// locked: fn runs with a nil dir. An error opening or locking is returned |
| +// without running fn. |
| +// |
| +// Lock order, everywhere both are taken: the carlos.prev lock (root only), |
| +// then this one, then the registry's write lock (WithHoldsFrozen). The |
| +// agent takes only this one and holds nothing else while it does. |
| +func withFloorLock(dataDir string, create bool, fn func(d *movesDir) error) error { |
| + d, err := openMovesDir(dataDir, create) |
| + if errors.Is(err, os.ErrNotExist) && !create { |
| + return fn(nil) |
| + } |
| + if err != nil { |
| + return err |
| + } |
| + defer d.Close() |
| + if err := d.Lock(); err != nil { |
| + return err |
| + } |
| + return fn(d) |
| +} |
| + |
| +// writeLocalFloor writes both local floor files under the floor lock. |
| func writeLocalFloor(dataDir string, f moves.Floor) error { |
| if !validFloorRelease(f.MinRelease) { |
| return fmt.Errorf("min_release %q is not a release", f.MinRelease) |
| } |
| + return withFloorLock(dataDir, true, func(d *movesDir) error { return writeFloorFiles(d, f) }) |
| +} |
| + |
| +// writeFloorFiles writes floor.min-release FIRST and floor.json LAST, under |
| +// the floor lock. Every Go reader (activationAllowed, judgePrev, the edges |
| +// file) trusts floor.json alone; floor.min-release exists for the units' |
| +// shell, which can only compare files. So after a crash between the two, |
| +// floor.min-release can only be AHEAD of floor.json (floors only rise, see |
| +// floorRegression), and ahead means it matches no verdict judged against |
| +// the JSON: the units run the current binary, whose own check then uses the |
| +// JSON. The two can never disagree in the direction that would let the |
| +// units pick a carlos.prev the Go checks would refuse. |
| +func writeFloorFiles(d *movesDir, f moves.Floor) error { |
| body, err := json.Marshal(f) |
| if err != nil { |
| return err |
| } |
| - dir := filepath.Dir(moves.FloorFile) |
| - if err := writeBeneathDurable(dataDir, dir, filepath.Base(moves.FloorFile), append(body, '\n'), 0o644); err != nil { |
| + if err := d.WriteDurable(filepath.Base(moves.FloorMinReleaseFile), []byte(f.MinRelease+"\n"), 0o644); err != nil { |
| return err |
| } |
| - return writeBeneathDurable(dataDir, dir, filepath.Base(moves.FloorMinReleaseFile), []byte(f.MinRelease+"\n"), 0o644) |
| + return d.WriteDurable(filepath.Base(moves.FloorFile), append(body, '\n'), 0o644) |
| +} |
| + |
| +// floorRegression says why remote may not replace local, or nil. A floor |
| +// only rises on a box: a lower protocol minimum, or a min_release that is |
| +// lower or does not compare, is refused. Lowering a floor is an operator's |
| +// manual step, box by box: delete <DataDir>/.moves/floor.json and |
| +// floor.min-release (and publish the lower object), and the next apply or |
| +// refresh arms what is published. |
| +func floorRegression(local, remote *moves.Floor) error { |
| + if remote.Min < local.Min { |
| + return fmt.Errorf("the published floor's protocol minimum %d is below this box's %d; lowering a floor is done by hand on each box", remote.Min, local.Min) |
| + } |
| + if ok, _ := moves.ReleaseAtOrAbove(remote.MinRelease, local.MinRelease); !ok { |
| + return fmt.Errorf("the published min_release %q is below this box's %s; lowering a floor is done by hand on each box", remote.MinRelease, local.MinRelease) |
| + } |
| + return nil |
| } |
| // errFloorWrite marks a floor the store holds that this box could not |
| @@ -128,34 +183,62 @@ var errFloorWrite = errors.New("could not write it on this box") |
| // read the published floor and make the local files say the same. It |
| // returns the floor on disk afterwards, nil when none is armed. |
| // |
| -// - published and usable: written (skipped when already identical); |
| +// - published, usable and not lower: written, under the floor lock, with |
| +// the comparison made under the same lock, so an older observation that |
| +// finishes after a newer one cannot lower the floor; |
| +// - lower than the floor on disk: refused (floorRegression), logged; |
| // - absent from the store: what is on disk stays (once armed, armed); |
| // - the store unreadable, or the object unusable: what is on disk stays, |
| // and the error is returned to be logged. |
| +// |
| +// A local floor.json that cannot be read is replaced by a usable published |
| +// floor: it holds no value to compare, and the published one is the |
| +// authority it was copied from. |
| func syncLocalFloor(ctx context.Context, s release.Store, dataDir string) (*moves.Floor, error) { |
| - local, lerr := readLocalFloor(dataDir) |
| if s == nil { |
| + local, lerr := readLocalFloor(dataDir) |
| return local, errors.Join(errors.New("no deployment store to read the floor from"), lerr) |
| } |
| remote, rerr := moves.ReadFloor(ctx, s) |
| switch { |
| case rerr != nil: |
| + local, lerr := readLocalFloor(dataDir) |
| return local, errors.Join(rerr, lerr) |
| case remote == nil: |
| - return local, lerr |
| + return readLocalFloor(dataDir) |
| case !validFloorRelease(remote.MinRelease): |
| + local, lerr := readLocalFloor(dataDir) |
| return local, errors.Join(fmt.Errorf("the published min_release %q is not a release; the floor on this box stays as it was", remote.MinRelease), lerr) |
| } |
| - if lerr == nil && local != nil && *local == *remote { |
| - if line, ok := readBeneathLine(dataDir, moves.FloorMinReleaseFile); ok && line == remote.MinRelease { |
| - return local, nil |
| + var out *moves.Floor |
| + ran := false |
| + err := withFloorLock(dataDir, true, func(d *movesDir) error { |
| + ran = true |
| + local, lerr := readLocalFloor(dataDir) |
| + if lerr == nil && local != nil { |
| + if err := floorRegression(local, remote); err != nil { |
| + out = local |
| + return err |
| + } |
| + if *local == *remote { |
| + if line, ok := readBeneathLine(dataDir, moves.FloorMinReleaseFile); ok && line == remote.MinRelease { |
| + out = local |
| + return nil |
| + } |
| + } |
| } |
| + if err := writeFloorFiles(d, *remote); err != nil { |
| + out, _ = readLocalFloor(dataDir) |
| + return fmt.Errorf("%w: %v", errFloorWrite, err) |
| + } |
| + out = remote |
| + return nil |
| + }) |
| + if !ran { |
| + out, _ = readLocalFloor(dataDir) |
| + return out, fmt.Errorf("%w: %v", errFloorWrite, err) |
| } |
| - if err := writeLocalFloor(dataDir, *remote); err != nil { |
| - local, _ = readLocalFloor(dataDir) |
| - return local, fmt.Errorf("%w: %v", errFloorWrite, err) |
| - } |
| - return remote, nil |
| + return out, err |
| } |
| // readBeneathLine reads a one-line file under root without following a |
| @@ -214,27 +297,35 @@ func removeDurable(path string) error { |
| return syncDir(filepath.Dir(path)) |
| } |
| +// holdFreezer runs fn with the rows an operation holds, held still for |
| +// fn's length (registry.WithHoldsFrozen's shape). |
| +type holdFreezer func(fn func(held []registry.Route) error) error |
| + |
| // judgePrev is the one judge of carlos.prev (moves spec §11): it writes the |
| // verdict when carlos.prev is at or above the armed floor and no registry |
| // row is held by an operation, and removes it otherwise. With no floor |
| // armed it touches neither the registry nor carlos.prev, and leaves no |
| // verdict (the units do not read one then). |
| func judgePrev(optDir, dataDir string, reg *registry.Registry) error { |
| - return judgePrevWith(optDir, dataDir, func() ([]registry.Route, error) { |
| + return judgePrevWith(optDir, dataDir, func(fn func([]registry.Route) error) error { |
| if reg == nil { |
| - return nil, errors.New("no registry") |
| + return errors.New("no registry") |
| } |
| - return reg.HoldRows() |
| + return reg.WithHoldsFrozen(fn) |
| }, nil) |
| } |
| -// judgePrevWith is judgePrev with its two inputs as functions: holds lists |
| -// the held rows, and learn (nil to skip) answers carlos.prev's version when |
| -// the sidecar is missing; what it learns is written as the sidecar. |
| +// judgePrevWith is judgePrev with its two inputs as functions: freeze gives |
| +// the held rows, held still while the verdict is written, and learn (nil to |
| +// skip) answers carlos.prev's version when the sidecar is missing; what it |
| +// learns is written as the sidecar. Learning runs carlos.prev, which can |
| +// take seconds, so it happens before any lock is taken. |
| // |
| -// Every refusal removes the verdict before it returns, so an error here |
| -// never leaves an old verdict standing. |
| -func judgePrevWith(optDir, dataDir string, holds func() ([]registry.Route, error), learn func() string) error { |
| +// The judgement itself runs under the floor lock with the holds frozen, so |
| +// the verdict it writes is for the floor and the holds as they stand when |
| +// it lands. Every refusal removes the verdict before it returns, so an |
| +// error here never leaves an old verdict standing. |
| +func judgePrevWith(optDir, dataDir string, freeze holdFreezer, learn func() string) error { |
| deny := func(cause error) error { |
| if err := removeDurable(prevAllowedPath(optDir)); err != nil { |
| return errors.Join(cause, fmt.Errorf("remove the verdict on carlos.prev: %w", err)) |
| @@ -244,36 +335,49 @@ func judgePrevWith(optDir, dataDir string, holds func() ([]registry.Route, error |
| if _, err := os.Stat(systemPrevPath(optDir)); err != nil { |
| return deny(nil) |
| } |
| - floor, err := readLocalFloor(dataDir) |
| - if err != nil { |
| - return deny(fmt.Errorf("moves protocol floor: the floor on this box cannot be read: %v", err)) |
| - } |
| - if floor == nil { |
| - return deny(nil) |
| - } |
| - version, ok := readLine(prevVersionPath(optDir)) |
| - if !ok && learn != nil { |
| - if v := learn(); validSystemVersion(v) { |
| - if err := writePrevVersion(optDir, v); err != nil { |
| - return deny(err) |
| + if _, ok := readLine(prevVersionPath(optDir)); !ok && learn != nil { |
| + if f, _ := readLocalFloor(dataDir); f != nil { |
| + if v := learn(); validSystemVersion(v) { |
| + if err := writePrevVersion(optDir, v); err != nil { |
| + return deny(err) |
| + } |
| } |
| - version, ok = v, true |
| } |
| } |
| - if !ok { |
| - return deny(nil) |
| - } |
| - if at, _ := moves.ReleaseAtOrAbove(version, floor.MinRelease); !at { |
| - return deny(nil) |
| - } |
| - rows, err := holds() |
| - if err != nil { |
| - return deny(fmt.Errorf("held rows: %w", err)) |
| - } |
| - if len(rows) > 0 { |
| - return deny(nil) |
| + ran := false |
| + err := withFloorLock(dataDir, false, func(*movesDir) error { |
| + ran = true |
| + floor, err := readLocalFloor(dataDir) |
| + if err != nil { |
| + return deny(fmt.Errorf("moves protocol floor: the floor on this box cannot be read: %v", err)) |
| + } |
| + if floor == nil { |
| + return deny(nil) |
| + } |
| + version, ok := readLine(prevVersionPath(optDir)) |
| + if !ok { |
| + return deny(nil) |
| + } |
| + if at, _ := moves.ReleaseAtOrAbove(version, floor.MinRelease); !at { |
| + return deny(nil) |
| + } |
| + called := false |
| + err = freeze(func(held []registry.Route) error { |
| + called = true |
| + if len(held) > 0 { |
| + return deny(nil) |
| + } |
| + return writeFileDurable(prevAllowedPath(optDir), []byte(floor.MinRelease+"\n"), 0o644) |
| + }) |
| + if !called { |
| + return deny(fmt.Errorf("held rows: %w", err)) |
| + } |
| + return err |
| + }) |
| + if !ran { |
| + return deny(fmt.Errorf("moves protocol floor: the floor on this box cannot be read: %v", err)) |
| } |
| - return writeFileDurable(prevAllowedPath(optDir), []byte(floor.MinRelease+"\n"), 0o644) |
| + return err |
| } |
| // prevLockFile serialises the two writers of carlos.prev's sidecar and |
| @@ -329,7 +433,7 @@ func rootJudgePrev(reg *registry.Registry) error { |
| } |
| } |
| return withPrevLock(optDir, func() error { |
| - return judgePrevWith(optDir, dd, func() ([]registry.Route, error) { return reg.HoldRows() }, func() string { |
| + return judgePrevWith(optDir, dd, reg.WithHoldsFrozen, func() string { |
| out, err := productionSysOps().run(context.Background(), systemVersionWait, systemPrevPath(optDir), "version") |
| if err != nil { |
| return "" |
| @@ -365,7 +469,12 @@ func movesFloorLoop(ctx context.Context, s release.Store, dataDir string) { |
| // verdict, which the door reads as below the floor). |
| func withMovesFields(f edgesFile, dataDir, optDir string) edgesFile { |
| f.MovesProtocol = moves.ProtocolVersion |
| - f.MovesFloorMinRelease, _ = readLine(filepath.Join(dataDir, moves.FloorMinReleaseFile)) |
| + // From floor.json, the file every Go reader trusts: floor.min-release |
| + // is written first and can be ahead of it (writeFloorFiles). |
| + f.MovesFloorMinRelease = "" |
| + if fl, err := readLocalFloor(dataDir); err == nil && fl != nil { |
| + f.MovesFloorMinRelease = fl.MinRelease |
| + } |
| f.PrevRelease = "" |
| if _, err := os.Stat(systemPrevPath(optDir)); errors.Is(err, os.ErrNotExist) { |
| f.PrevRelease = "none" |
diff --git a/internal/host/movesfloor_linux.go b/internal/host/movesfloor_linux.go| index fb059f382..14fc5cbba 100644 |
| --- a/internal/host/movesfloor_linux.go |
| +++ b/internal/host/movesfloor_linux.go |
| @@ -12,50 +12,117 @@ import ( |
| "path/filepath" |
| "strconv" |
| "syscall" |
| + "time" |
| ) |
| -// writeBeneathDurable writes data to root/dir/name the way writeDurable |
| -// does (a temp file, fsync, rename, fsync of the directory), but every step |
| -// is taken relative to a directory opened with O_NOFOLLOW, so neither dir |
| -// nor name can be a symlink. It is how root writes the local floor files |
| -// under /data, a tree the sandboxed agent owns and could plant a symlink |
| -// in. dir is created (0755) when it is missing; when root creates it, it |
| -// is handed to the service user, because the agent's hourly refresh writes |
| -// the same files. |
| +// movesDir is <DataDir>/.moves, opened with O_NOFOLLOW below a data dir |
| +// root trusts, so nothing root reads or writes through it can be a |
| +// symlink the sandboxed agent planted. Every operation on the floor files |
| +// goes through one: the lock and the writes are all relative to its fd. |
| +type movesDir struct { |
| + fd int |
| + path string |
| + lock int // the flocked floor.lock fd, or -1 |
| +} |
| + |
| +// floorLockFile serialises every writer of the floor files (root's apply, |
| +// the agent's refresh) with every reader whose answer must not change under |
| +// it (root's judgement of carlos.prev and its activation guard). |
| +const floorLockFile = "floor.lock" |
| + |
| +const ( |
| + floorLockWait = 2 * time.Minute |
| + floorLockPoll = 100 * time.Millisecond |
| +) |
| + |
| +// openMovesDir opens <root>/.moves. With create it makes the directory |
| +// when missing; without, a missing directory is os.ErrNotExist. |
| // |
| -// The temp name is random, so root's apply and the agent's refresh never |
| -// write into each other's temp file. |
| -func writeBeneathDurable(root, dir, name string, data []byte, mode os.FileMode) error { |
| +// Crash ordering for a directory created here: mkdir, then fsync of the |
| +// parent (root) so the new entry is on disk, and only then any file inside |
| +// it, each of which is renamed into place and followed by an fsync of |
| +// .moves itself. A power loss can therefore lose a floor only together with |
| +// the write that reported it, never after: a floor write that returned nil |
| +// survives, directory entry and all. A failed parent fsync is the write's |
| +// error, which the callers treat as a floor that could not be written. |
| +func openMovesDir(root string, create bool) (*movesDir, error) { |
| + name := movesDirName |
| rfd, err := syscall.Open(root, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_CLOEXEC, 0) |
| if err != nil { |
| - return fmt.Errorf("open %s: %w", root, err) |
| + return nil, fmt.Errorf("open %s: %w", root, err) |
| } |
| defer syscall.Close(rfd) |
| - dirFlags := syscall.O_RDONLY | syscall.O_DIRECTORY | syscall.O_NOFOLLOW | syscall.O_CLOEXEC |
| - dfd, err := syscall.Openat(rfd, dir, dirFlags, 0) |
| - if errors.Is(err, syscall.ENOENT) { |
| - if err := syscall.Mkdirat(rfd, dir, 0o755); err != nil && !errors.Is(err, syscall.EEXIST) { |
| - return fmt.Errorf("mkdir %s: %w", filepath.Join(root, dir), err) |
| + flags := syscall.O_RDONLY | syscall.O_DIRECTORY | syscall.O_NOFOLLOW | syscall.O_CLOEXEC |
| + fd, err := syscall.Openat(rfd, name, flags, 0) |
| + if errors.Is(err, syscall.ENOENT) && create { |
| + if err := syscall.Mkdirat(rfd, name, 0o755); err != nil && !errors.Is(err, syscall.EEXIST) { |
| + return nil, fmt.Errorf("mkdir %s: %w", filepath.Join(root, name), err) |
| } |
| - if dfd, err = syscall.Openat(rfd, dir, dirFlags, 0); err == nil { |
| - fchownToService(dfd) |
| + if err := syscall.Fsync(rfd); err != nil { |
| + return nil, fmt.Errorf("fsync %s: %w", root, err) |
| } |
| + if fd, err = syscall.Openat(rfd, name, flags, 0); err == nil { |
| + fchownToService(fd) |
| + } |
| + } |
| + if err != nil { |
| + return nil, fmt.Errorf("open %s: %w", filepath.Join(root, name), err) |
| + } |
| + return &movesDir{fd: fd, path: filepath.Join(root, name), lock: -1}, nil |
| +} |
| + |
| +// Close releases the lock (closing its fd) and the directory. |
| +func (d *movesDir) Close() { |
| + if d.lock >= 0 { |
| + syscall.Close(d.lock) |
| + d.lock = -1 |
| } |
| + syscall.Close(d.fd) |
| +} |
| + |
| +// Lock takes floor.lock, waiting up to floorLockWait. Opened read-only (a |
| +// read-only fd can be flocked), so the agent can lock a file root created |
| +// and root a file the agent created; O_NOFOLLOW and O_NONBLOCK so a symlink |
| +// or a FIFO planted in its place cannot redirect or park root. |
| +func (d *movesDir) Lock() error { |
| + fd, err := syscall.Openat(d.fd, floorLockFile, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NOFOLLOW|syscall.O_NONBLOCK|syscall.O_CLOEXEC, 0o644) |
| if err != nil { |
| - return fmt.Errorf("open %s: %w", filepath.Join(root, dir), err) |
| + return fmt.Errorf("open %s: %w", filepath.Join(d.path, floorLockFile), err) |
| + } |
| + var st syscall.Stat_t |
| + if err := syscall.Fstat(fd, &st); err != nil || st.Mode&syscall.S_IFMT != syscall.S_IFREG { |
| + syscall.Close(fd) |
| + return fmt.Errorf("%s is not a regular file", filepath.Join(d.path, floorLockFile)) |
| } |
| - defer syscall.Close(dfd) |
| + deadline := time.Now().Add(floorLockWait) |
| + for { |
| + err := syscall.Flock(fd, syscall.LOCK_EX|syscall.LOCK_NB) |
| + if err == nil { |
| + d.lock = fd |
| + return nil |
| + } |
| + if !errors.Is(err, syscall.EWOULDBLOCK) || time.Now().After(deadline) { |
| + syscall.Close(fd) |
| + return fmt.Errorf("lock %s: %w", filepath.Join(d.path, floorLockFile), err) |
| + } |
| + time.Sleep(floorLockPoll) |
| + } |
| +} |
| +// WriteDurable writes data to .moves/name the way writeDurable does (a |
| +// temp file, fsync, rename, fsync of the directory), all relative to the |
| +// directory's fd. The temp name is random, so two writers never share one. |
| +func (d *movesDir) WriteDurable(name string, data []byte, mode os.FileMode) error { |
| var suffix [6]byte |
| if _, err := rand.Read(suffix[:]); err != nil { |
| return err |
| } |
| tmp := name + ".tmp." + hex.EncodeToString(suffix[:]) |
| - fd, err := syscall.Openat(dfd, tmp, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_EXCL|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, uint32(mode.Perm())) |
| + fd, err := syscall.Openat(d.fd, tmp, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_EXCL|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, uint32(mode.Perm())) |
| if err != nil { |
| - return fmt.Errorf("create %s: %w", filepath.Join(root, dir, tmp), err) |
| + return fmt.Errorf("create %s: %w", filepath.Join(d.path, tmp), err) |
| } |
| - f := os.NewFile(uintptr(fd), filepath.Join(root, dir, tmp)) |
| + f := os.NewFile(uintptr(fd), filepath.Join(d.path, tmp)) |
| werr := func() error { |
| if _, err := f.Write(data); err != nil { |
| return err |
| @@ -69,14 +136,14 @@ func writeBeneathDurable(root, dir, name string, data []byte, mode os.FileMode) |
| werr = cerr |
| } |
| if werr == nil { |
| - werr = syscall.Renameat(dfd, tmp, dfd, name) |
| + werr = syscall.Renameat(d.fd, tmp, d.fd, name) |
| } |
| if werr != nil { |
| - syscall.Unlinkat(dfd, tmp) |
| - return fmt.Errorf("write %s: %w", filepath.Join(root, dir, name), werr) |
| + syscall.Unlinkat(d.fd, tmp) |
| + return fmt.Errorf("write %s: %w", filepath.Join(d.path, name), werr) |
| } |
| - if err := syscall.Fsync(dfd); err != nil { |
| - return fmt.Errorf("fsync %s: %w", filepath.Join(root, dir), err) |
| + if err := syscall.Fsync(d.fd); err != nil { |
| + return fmt.Errorf("fsync %s: %w", d.path, err) |
| } |
| return nil |
| } |
diff --git a/internal/host/movesfloor_other.go b/internal/host/movesfloor_other.go| index 6aca8f058..0390a1d66 100644 |
| --- a/internal/host/movesfloor_other.go |
| +++ b/internal/host/movesfloor_other.go |
| @@ -4,9 +4,18 @@ package host |
| import "os" |
| -// writeBeneathDurable refuses off Linux, like openBeneath: 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). |
| -func writeBeneathDurable(root, dir, name string, data []byte, mode os.FileMode) error { |
| +// 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 |
| +// operation refuses, like openBeneath. |
| + |
| +type movesDir struct{} |
| + |
| +func openMovesDir(root string, create bool) (*movesDir, error) { return nil, errSystemUnsupported } |
| + |
| +func (d *movesDir) Close() {} |
| + |
| +func (d *movesDir) Lock() error { return errSystemUnsupported } |
| + |
| +func (d *movesDir) WriteDurable(name string, data []byte, mode os.FileMode) error { |
| return errSystemUnsupported |
| } |
diff --git a/internal/host/movesfloor_test.go b/internal/host/movesfloor_test.go| index 5d5970cae..45522b9b0 100644 |
| --- a/internal/host/movesfloor_test.go |
| +++ b/internal/host/movesfloor_test.go |
| @@ -8,6 +8,7 @@ import ( |
| "os" |
| "path/filepath" |
| "testing" |
| + "time" |
| "github.com/carlosframework/platform/internal/moves" |
| "github.com/carlosframework/platform/internal/registry" |
| @@ -31,12 +32,15 @@ func armLocalFloor(t *testing.T, dataDir, minRelease string) { |
| } |
| } |
| -func noHolds() ([]registry.Route, error) { return nil, nil } |
| +// The judge's held-row inputs, shaped like registry.WithHoldsFrozen. |
| +func noHolds(fn func([]registry.Route) error) error { return fn(nil) } |
| -func oneHold() ([]registry.Route, error) { |
| - return []registry.Route{{Host: "app.example.com", Status: "moving", HoldOp: "op1"}}, nil |
| +func oneHold(fn func([]registry.Route) error) error { |
| + return fn([]registry.Route{{Host: "app.example.com", Status: "moving", HoldOp: "op1"}}) |
| } |
| +func unreadableHolds(func([]registry.Route) error) error { return errors.New("locked") } |
| + |
| func TestLocalFloorFilesRoundTrip(t *testing.T) { |
| dd := t.TempDir() |
| f, err := readLocalFloor(dd) |
| @@ -166,13 +170,13 @@ func TestJudgePrevWritesTheVerdictOnlyAtOrAboveTheFloorWithoutAHold(t *testing.T |
| }) |
| for _, tc := range []struct { |
| name, floor, prev string |
| - holds func() ([]registry.Route, error) |
| + holds holdFreezer |
| }{ |
| {"below the floor", "v0.30.0", "v0.29.0", noHolds}, |
| {"a prerelease", "v0.30.0", "v0.30.0-rc.1", noHolds}, |
| {"no sidecar", "v0.30.0", "", noHolds}, |
| {"a held row", "v0.30.0", "v0.31.0", oneHold}, |
| - {"the registry unreadable", "v0.30.0", "v0.31.0", func() ([]registry.Route, error) { return nil, errors.New("locked") }}, |
| + {"the registry unreadable", "v0.30.0", "v0.31.0", unreadableHolds}, |
| } { |
| t.Run(tc.name, func(t *testing.T) { |
| opt, dd := setup(t, tc.floor, tc.prev) |
| @@ -216,7 +220,7 @@ func TestJudgePrevIsDormantWithoutAFloor(t *testing.T) { |
| opt, dd := t.TempDir(), t.TempDir() |
| os.WriteFile(systemPrevPath(opt), []byte("#!/bin/sh\n"), 0o755) |
| consulted := false |
| - holds := func() ([]registry.Route, error) { consulted = true; return nil, nil } |
| + holds := func(fn func([]registry.Route) error) error { consulted = true; return fn(nil) } |
| ran := false |
| if err := judgePrevWith(opt, dd, holds, func() string { ran = true; return "v0.29.0" }); err != nil { |
| t.Fatal(err) |
| @@ -251,3 +255,105 @@ func TestMovesFloorRefreshArmsFromTheStore(t *testing.T) { |
| t.Fatal("a failed refresh said nothing") |
| } |
| } |
| + |
| +// The floor never goes down on its own (fix round 1, finding 3). Lowering a |
| +// floor is an operator's manual step on each box: delete the two local files. |
| +func TestSyncLocalFloorRefusesARegression(t *testing.T) { |
| + ctx := context.Background() |
| + dd, s := t.TempDir(), release.NewDirStore(t.TempDir()) |
| + armLocalFloor(t, dd, "v0.31.0") |
| + putFloor(t, s, "v0.30.0") |
| + if f, err := syncLocalFloor(ctx, s, dd); err == nil || f == nil || f.MinRelease != "v0.31.0" { |
| + t.Fatalf("a lower min_release = %+v, %v", f, err) |
| + } |
| + // A lower protocol minimum at the same release is a regression too. |
| + if err := writeLocalFloor(dd, moves.Floor{Min: moves.ProtocolVersion + 1, MinRelease: "v0.31.0"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + s.Delete(ctx, moves.FloorKey) |
| + putFloor(t, s, "v0.31.0") |
| + if f, err := syncLocalFloor(ctx, s, dd); err == nil || f == nil || f.Min != moves.ProtocolVersion+1 { |
| + t.Fatalf("a lower min = %+v, %v", f, err) |
| + } |
| + if b, _ := os.ReadFile(filepath.Join(dd, moves.FloorMinReleaseFile)); string(b) != "v0.31.0\n" { |
| + t.Fatalf("floor.min-release = %q", b) |
| + } |
| +} |
| + |
| +// racingStore answers an old floor, but only after a newer refresh has run |
| +// to completion: an older observation finishing last. |
| +type racingStore struct { |
| + release.Store |
| + newer func() |
| +} |
| + |
| +func (r racingStore) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + r.newer() |
| + return r.Store.Get(ctx, key) |
| +} |
| + |
| +func TestAnOlderRefreshFinishingLastDoesNotLowerTheFloor(t *testing.T) { |
| + ctx := context.Background() |
| + dd := t.TempDir() |
| + old, newer := release.NewDirStore(t.TempDir()), release.NewDirStore(t.TempDir()) |
| + putFloor(t, old, "v0.30.0") |
| + putFloor(t, newer, "v0.31.0") |
| + race := racingStore{Store: old, newer: func() { |
| + if f, err := syncLocalFloor(ctx, newer, dd); err != nil || f == nil || f.MinRelease != "v0.31.0" { |
| + t.Errorf("newer refresh = %+v, %v", f, err) |
| + } |
| + }} |
| + syncLocalFloor(ctx, race, dd) |
| + if f, _ := readLocalFloor(dd); f == nil || f.MinRelease != "v0.31.0" { |
| + t.Fatalf("the older refresh lowered the floor to %+v", f) |
| + } |
| + if b, _ := os.ReadFile(filepath.Join(dd, moves.FloorMinReleaseFile)); string(b) != "v0.31.0\n" { |
| + t.Fatalf("floor.min-release = %q", b) |
| + } |
| +} |
| + |
| +// Root's apply and the agent's refresh write the floor under one lock file |
| +// in .moves, so neither sees the other's half-written pair. |
| +func TestFloorWritersTakeTheFloorLock(t *testing.T) { |
| + ctx := context.Background() |
| + dd, s := t.TempDir(), release.NewDirStore(t.TempDir()) |
| + armLocalFloor(t, dd, "v0.30.0") |
| + putFloor(t, s, "v0.31.0") |
| + held, free := make(chan struct{}), make(chan struct{}) |
| + go withFloorLock(dd, false, func(*movesDir) error { |
| + close(held) |
| + <-free |
| + return nil |
| + }) |
| + <-held |
| + done := make(chan struct{}) |
| + go func() { syncLocalFloor(ctx, s, dd); close(done) }() |
| + select { |
| + case <-done: |
| + t.Fatal("a floor write finished while another writer held the lock") |
| + case <-time.After(300 * time.Millisecond): |
| + } |
| + if f, _ := readLocalFloor(dd); f == nil || f.MinRelease != "v0.30.0" { |
| + t.Fatalf("written under another writer's lock: %+v", f) |
| + } |
| + close(free) |
| + <-done |
| + if f, _ := readLocalFloor(dd); f == nil || f.MinRelease != "v0.31.0" { |
| + t.Fatalf("after the lock = %+v", f) |
| + } |
| +} |
| + |
| +// Readers trust floor.json alone; floor.min-release is written first, so |
| +// after a crash between the two it can only be ahead, and only the units |
| +// read it (where ahead means no verdict matches: the current binary runs). |
| +func TestFloorReadersTrustOnlyTheJSON(t *testing.T) { |
| + dd, opt := t.TempDir(), t.TempDir() |
| + armLocalFloor(t, dd, "v0.30.0") |
| + os.WriteFile(filepath.Join(dd, moves.FloorMinReleaseFile), []byte("v0.31.0\n"), 0o644) |
| + if f, _ := readLocalFloor(dd); f == nil || f.MinRelease != "v0.30.0" { |
| + t.Fatalf("readLocalFloor = %+v", f) |
| + } |
| + if got := withMovesFields(edgesFile{}, dd, opt).MovesFloorMinRelease; got != "v0.30.0" { |
| + t.Fatalf("edges file publishes %q, want the JSON's v0.30.0", got) |
| + } |
| +} |
diff --git a/internal/host/systemapply.go b/internal/host/systemapply.go| index 91c6d606a..88b12494b 100644 |
| --- a/internal/host/systemapply.go |
| +++ b/internal/host/systemapply.go |
| @@ -420,6 +420,9 @@ func (a *systemApplier) install(ctx context.Context, req systemRequest, p *relea |
| } |
| return a.record(req, err) |
| } |
| + // The release on disk, for the activation guard at the rename: read now, |
| + // because reading it runs the binary and the guard holds locks. |
| + running := a.onDiskBuild(ctx) |
| // PromotedAt rides along for the whole transaction: it is half of the |
| // tuple a recovery has to answer for. |
| j := systemJournal{Phase: systemPhaseInstalling, Target: target, PromotedAt: req.PromotedAt, PrevBuild: a.prevBuild(ctx), |
| @@ -436,7 +439,17 @@ func (a *systemApplier) install(ctx context.Context, req systemRequest, p *relea |
| if _, err := a.ops.systemctl(append([]string{"stop"}, systemQuiesceUnits...)...); err != nil { |
| a.ops.logf("system apply: quiesce: %v (continuing)", err) |
| } |
| - if err := os.Rename(staged, systemBinaryPath(a.optDir)); err != nil { |
| + // 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 { |
| + return os.Rename(staged, systemBinaryPath(a.optDir)) |
| + }); err != nil { |
| + var refusal activationRefused |
| + if errors.As(err, &refusal) { |
| + return a.abandonInstall(req, err) |
| + } |
| return fail("install", err) |
| } |
| if err := os.Chown(systemBinaryPath(a.optDir), 0, 0); err != nil && !errors.Is(err, os.ErrPermission) { |
| @@ -471,6 +484,26 @@ func (a *systemApplier) install(ctx context.Context, req systemRequest, p *relea |
| return a.finish(req, target, newPlan) |
| } |
| +// abandonInstall undoes an install that the activation guard refused at the |
| +// rename, when nothing but the quiesce has happened: the binary, the units |
| +// and carlos.prev are all as they were. The journal goes (a recovery from |
| +// it would restore carlos.prev over a binary that never changed), the |
| +// quiesced timers and paths are started again, and the refusal is |
| +// recorded like any other. |
| +func (a *systemApplier) abandonInstall(req systemRequest, err error) error { |
| + a.removeJournal() |
| + var armed []string |
| + for _, u := range systemQuiesceUnits { |
| + if strings.HasSuffix(u, ".timer") || strings.HasSuffix(u, ".path") { |
| + armed = append(armed, u) |
| + } |
| + } |
| + if _, serr := a.ops.systemctl(append([]string{"start"}, armed...)...); serr != nil { |
| + a.ops.logf("system apply: re-arm after a refused install: %v", serr) |
| + } |
| + return a.record(req, fmt.Errorf("%w: %v", errSystemRefused, err)) |
| +} |
| + |
| // finish is steps 11–12, and what recovery runs forward from phase gated. |
| func (a *systemApplier) finish(req systemRequest, target string, plan bootstrapPlan) error { |
| reason := "" |
| @@ -568,7 +601,7 @@ func (a *systemApplier) notePrev(ctx context.Context, replace bool, version stri |
| } |
| } |
| } |
| - return a.floorStep(judgePrevWith(a.optDir, a.dataDir, a.holdRows, func() string { return a.prevBuild(ctx) })) |
| + return a.floorStep(judgePrevWith(a.optDir, a.dataDir, a.freezeHolds, func() string { return a.prevBuild(ctx) })) |
| } |
| err := withPrevLock(a.optDir, steps) |
| if !ran { |
| @@ -644,6 +677,11 @@ func (a *systemApplier) armFloor(ctx context.Context) error { |
| // With no floor armed it reads neither the registry nor any version and |
| // allows, with one log line: main's behaviour. |
| // |
| +// 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, |
| +// which applies the same rule again with its inputs held still. |
| +// |
| // How the hold half relates to systemparticipation.go (region HA H14): that |
| // guard defers any executable change, up or down, while this box takes |
| // part in a claim operation (.hostexec), and only under CARLOS_REGION_HA, |
| @@ -654,53 +692,122 @@ 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) |
| +} |
| + |
| +// 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. |
| +type activationRefused struct{ error } |
| + |
| +func (e activationRefused) Unwrap() error { return e.error } |
| + |
| +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 { |
| + ran := false |
| + err := withFloorLock(a.dataDir, false, func(*movesDir) error { |
| + ran = true |
| + return a.activationRule(version, func() string { return running }, swap) |
| + }) |
| + if !ran { |
| + return refuse("moves protocol floor: the floor on this box cannot be read: %v", err) |
| + } |
| + return 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 } |
| + } |
| floor, err := readLocalFloor(a.dataDir) |
| if err != nil { |
| - return fmt.Errorf("moves protocol floor: the floor on this box cannot be read: %v", err) |
| + 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 nil |
| + return swap() |
| } |
| if version == "" { |
| - return fmt.Errorf("moves protocol floor: carlos.prev records no version, so it is below %s", floor.MinRelease) |
| + 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 fmt.Errorf("moves protocol floor: %s is below %s", version, floor.MinRelease) |
| + return refuse("moves protocol floor: %s is below %s", version, floor.MinRelease) |
| + } |
| + run := running() |
| + if version == run { |
| + return swap() |
| + } |
| + if ok, _ := moves.ReleaseAtOrAbove(version, run); ok { |
| + return swap() // not a downgrade: the held rows do not matter |
| + } |
| + // 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. |
| + 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) |
| + }) |
| + 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) |
| } |
| - rows, err := a.holdRows() |
| + return err |
| +} |
| + |
| +// floorAllowsForward is the floor half alone, for recovery's forward path: |
| +// the gated build is already the one on disk, so only the floor can refuse |
| +// it. Unarmed: allowed. |
| +func (a *systemApplier) floorAllowsForward(version string) error { |
| + floor, err := readLocalFloor(a.dataDir) |
| if err != nil { |
| - return fmt.Errorf("a participant in a held operation does not downgrade, and the registry cannot say whether this box is one: %v", err) |
| - } |
| - if len(rows) == 0 { |
| - return nil |
| + return fmt.Errorf("moves protocol floor: the floor on this box cannot be read: %v", err) |
| } |
| - running := a.onDiskBuild(ctx) |
| - if version == running { |
| + if floor == nil { |
| return nil |
| } |
| - if running == "" { |
| - return errors.New("a participant in a held operation does not downgrade, and the running release is unknown") |
| - } |
| - if ok, _ := moves.ReleaseAtOrAbove(version, running); !ok { |
| - return fmt.Errorf("a participant in a held operation does not downgrade: %s is below the running %s", version, running) |
| + if ok, _ := moves.ReleaseAtOrAbove(version, floor.MinRelease); !ok { |
| + return fmt.Errorf("moves protocol floor: %s is below %s", version, floor.MinRelease) |
| } |
| return nil |
| } |
| -// holdRows opens the registry (openRegistry, as the layout guard does) and |
| -// lists the rows an operation holds. |
| -func (a *systemApplier) holdRows() ([]registry.Route, error) { |
| +// freezeHolds opens the registry (openRegistry, as the layout guard does) |
| +// and runs fn with the held rows frozen (registry.WithHoldsFrozen). |
| +func (a *systemApplier) freezeHolds(fn func(held []registry.Route) error) error { |
| open := a.openRegistry |
| if open == nil { |
| open = func() (*registry.Registry, error) { return openSystemRegistry(a.dataDir) } |
| } |
| reg, err := open() |
| if err != nil { |
| - return nil, err |
| + return err |
| } |
| defer reg.Close() |
| - return reg.HoldRows() |
| + return reg.WithHoldsFrozen(fn) |
| } |
| func (a *systemApplier) prevBuild(ctx context.Context) string { |
| @@ -1118,7 +1225,8 @@ func (a *systemApplier) restoreFiles(ctx context.Context, j *systemJournal) (boo |
| // sidecar is below any armed floor). A refusal keeps the journal: the |
| // box stays on the build it is on, and the caller reports it. |
| prevVersion, _ := readLine(prevVersionPath(a.optDir)) |
| - if err := a.activationAllowed(ctx, prevVersion); err != nil { |
| + running := a.onDiskBuild(ctx) |
| + if err := a.activationRule(prevVersion, func() string { return running }, nil); err != nil { |
| return bootstrapPlan{}, err |
| } |
| // The same quiesce the install did: a root pass that is mid-flight |
| @@ -1126,7 +1234,20 @@ func (a *systemApplier) restoreFiles(ctx context.Context, j *systemJournal) (boo |
| if _, err := a.ops.systemctl(append([]string{"stop"}, systemQuiesceUnits...)...); err != nil { |
| a.ops.logf("system apply: quiesce: %v (continuing)", err) |
| } |
| - if err := copyFileDurable(prev, systemBinaryPath(a.optDir), 0o755); err != nil { |
| + // carlos.prev is copied beside the binary first, outside any lock, and |
| + // renamed over it under the activation guard, so the rule's inputs are |
| + // held still for a rename rather than a whole binary copy. |
| + restore := systemBinaryPath(a.optDir) + ".restore" |
| + if err := copyFileDurable(prev, restore, 0o755); err != nil { |
| + return bootstrapPlan{}, err |
| + } |
| + if err := a.guardActivation(prevVersion, running, func() error { |
| + if err := os.Rename(restore, systemBinaryPath(a.optDir)); err != nil { |
| + return err |
| + } |
| + return syncDir(a.optDir) |
| + }); err != nil { |
| + os.Remove(restore) |
| return bootstrapPlan{}, err |
| } |
| // The unit set to put back is whatever the binary being restored ships, |
| @@ -1242,6 +1363,12 @@ func (a *systemApplier) recoverIfNeeded(ctx context.Context) (bool, error) { |
| if j.Phase == systemPhaseGated { |
| if got := a.onDiskBuild(ctx); got != j.Target { |
| a.ops.logf("system apply: recovery: %s passed its gate but the binary on disk reports %q; rolling back instead", j.Target, got) |
| + } else if err := a.floorAllowsForward(j.Target); err != nil { |
| + // A floor raised since the gate (moves spec §11; fix round 1): the |
| + // build is not finished forward. Back to carlos.prev, which |
| + // restoreFiles judges in turn; if that is refused too, the |
| + // journal stays and the box stays as it is. |
| + a.ops.logf("system apply: recovery: %s passed its gate but %v; rolling back instead", j.Target, err) |
| } else { |
| plan, _, err := a.stagePlan(ctx, systemBinaryPath(a.optDir), filepath.Join(a.stagingDir(), "new")) |
| if err != nil { |
diff --git a/internal/host/systemapply_test.go b/internal/host/systemapply_test.go| index f27a8cf2d..146dbf62c 100644 |
| --- a/internal/host/systemapply_test.go |
| +++ b/internal/host/systemapply_test.go |
| @@ -1852,3 +1852,161 @@ func TestFloorIsDormantWithoutTheObject(t *testing.T) { |
| t.Fatalf("on disk %q", f.onDisk()) |
| } |
| } |
| + |
| +// duringStaging runs change when the apply runs the staged candidate's |
| +// `version`, which is the middle of staging: after the early activation |
| +// check, before the install puts anything in place. |
| +func (f *applyFixture) duringStaging(change func()) { |
| + staged := filepath.Join(f.a.stagingDir(), "carlos") |
| + run := f.a.ops.run |
| + once := false |
| + f.a.ops.run = func(ctx context.Context, d time.Duration, name string, args ...string) (string, error) { |
| + if name == staged && !once { |
| + once = true |
| + change() |
| + } |
| + return run(ctx, d, name, args...) |
| + } |
| +} |
| + |
| +// Fix round 1, finding 1: the early check can go stale while the candidate |
| +// is staged. The install checks again at the rename, under the floor lock |
| +// and with the held rows frozen, and a refusal there leaves the box as it |
| +// was: the binary untouched, no journal, and the quiesced passes re-armed. |
| +func TestActivationIsCheckedAgainAtTheRename(t *testing.T) { |
| + for _, tc := range []struct { |
| + name, running, target, want string |
| + change func(f *applyFixture) |
| + }{ |
| + {"a floor raised during staging", "v0.31.0", "v0.32.0", "moves protocol floor: v0.32.0 is below v0.33.0", |
| + func(f *applyFixture) { armLocalFloor(t, f.a.dataDir, "v0.33.0") }}, |
| + {"a hold set during staging", "v0.32.0", "v0.31.0", "a participant in a held operation does not downgrade", |
| + func(f *applyFixture) { f.holdRow("app.example.com") }}, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + f, _ := floorFixture(t, tc.running, tc.target) |
| + putFloor(t, f.store, "v0.30.0") |
| + f.duringStaging(func() { tc.change(f) }) |
| + if err := f.a.Run(context.Background(), false); err != nil { |
| + t.Fatal(err) |
| + } |
| + r := f.result() |
| + if r == nil || r.Outcome != systemOutcomeRefused || !strings.Contains(r.Reason, tc.want) { |
| + t.Fatalf("result = %+v", r) |
| + } |
| + if f.onDisk() != tc.running { |
| + t.Fatalf("on disk %q, want %s", f.onDisk(), tc.running) |
| + } |
| + if j, _ := readSystemJournal(f.a.optDir); j != nil { |
| + t.Fatalf("journal left open: %+v", j) |
| + } |
| + joined := strings.Join(f.calls, "\n") |
| + if !strings.Contains(joined, "systemctl start carlos-adopt.timer") { |
| + t.Fatalf("the quiesced passes were not re-armed:\n%s", joined) |
| + } |
| + if strings.Contains(joined, "systemctl restart carlos-edge") { |
| + t.Fatalf("a refused install restarted the edge:\n%s", joined) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// Fix round 1, finding 2: a gated journal is finished forward only when its |
| +// target is at or above the floor armed now. Below it, recovery goes back |
| +// to carlos.prev, which restoreFiles judges in turn. |
| +func TestForwardRecoveryChecksTheFloor(t *testing.T) { |
| + for _, tc := range []struct { |
| + name, prev, want string |
| + ok bool |
| + }{ |
| + {"carlos.prev below the floor too", "v0.30.0", "v0.31.0", false}, |
| + {"carlos.prev at the floor", "v0.32.0", "v0.32.0", true}, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + f, _ := floorFixture(t, "v0.30.0", "v0.31.0") |
| + f.brokenBox("v0.31.0", tc.prev, tc.prev) |
| + f.a.journal(systemJournal{Phase: systemPhaseGated, Target: "v0.31.0", PrevBuild: tc.prev, StartedAt: f.now}) |
| + armLocalFloor(t, f.a.dataDir, "v0.32.0") |
| + f.edgeStartedThisBoot = false |
| + err := f.a.Run(context.Background(), true) |
| + if ap, _ := readSystemApplied(f.a.optDir); ap != nil && ap.Version == "v0.31.0" { |
| + t.Fatalf("a below-floor target was finished forward: %+v", ap) |
| + } |
| + j, _ := readSystemJournal(f.a.optDir) |
| + if f.onDisk() != tc.want { |
| + t.Fatalf("on disk %q, want %s", f.onDisk(), tc.want) |
| + } |
| + if tc.ok && (err != nil || j != nil) { |
| + t.Fatalf("err %v, journal %+v", err, j) |
| + } |
| + if !tc.ok && (err == nil || !strings.Contains(err.Error(), "moves protocol floor") || j == nil) { |
| + t.Fatalf("err %v, journal %+v", err, j) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// restoreFiles puts carlos.prev in place under the floor lock: a floor |
| +// writer holding it holds the restore off. |
| +func TestRestoreWaitsForTheFloorLock(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") |
| + held, free := make(chan struct{}), make(chan struct{}) |
| + go withFloorLock(f.a.dataDir, false, func(*movesDir) error { |
| + close(held) |
| + <-free |
| + return nil |
| + }) |
| + <-held |
| + done := make(chan error, 1) |
| + go func() { _, err := f.a.restoreFiles(context.Background(), &j); done <- err }() |
| + time.Sleep(300 * time.Millisecond) |
| + // The bytes, not f.onDisk: the fake ops are the restore's while it runs. |
| + if b, _ := os.ReadFile(systemBinaryPath(f.a.optDir)); !strings.Contains(string(b), "v0.32.0") { |
| + close(free) |
| + t.Fatalf("restored while the floor lock was held: %q", b) |
| + } |
| + close(free) |
| + if err := <-done; err != nil || f.onDisk() != "v0.31.0" { |
| + t.Fatalf("after the lock: err %v, on disk %q", err, f.onDisk()) |
| + } |
| +} |
| + |
| +// The install's rename waits for the floor lock: a floor writer that takes |
| +// it after the journal (at the quiesce) holds the binary where it is. |
| +func TestInstallRenameWaitsForTheFloorLock(t *testing.T) { |
| + f, _ := floorFixture(t, "v0.31.0", "v0.32.0") |
| + putFloor(t, f.store, "v0.30.0") |
| + var duringLock []byte |
| + held := make(chan struct{}) |
| + freed := make(chan struct{}) |
| + systemctl := f.a.ops.systemctl |
| + once := false |
| + f.a.ops.systemctl = func(args ...string) (string, error) { |
| + if args[0] == "stop" && !once { |
| + once = true |
| + go func() { |
| + defer close(freed) |
| + withFloorLock(f.a.dataDir, false, func(*movesDir) error { |
| + close(held) |
| + time.Sleep(time.Second) |
| + duringLock, _ = os.ReadFile(systemBinaryPath(f.a.optDir)) |
| + return nil |
| + }) |
| + }() |
| + <-held |
| + } |
| + return systemctl(args...) |
| + } |
| + if err := f.a.Run(context.Background(), false); err != nil { |
| + t.Fatal(err) |
| + } |
| + <-freed |
| + if !strings.Contains(string(duringLock), "carlos v0.31.0") { |
| + t.Fatalf("the binary was replaced while the floor lock was held: %q", duringLock) |
| + } |
| + if r := f.result(); r == nil || r.Outcome != systemOutcomeApplied || f.onDisk() != "v0.32.0" { |
| + t.Fatalf("result %+v, on disk %q", r, f.onDisk()) |
| + } |
| +} |
diff --git a/internal/registry/hold_test.go b/internal/registry/hold_test.go| index e5b4569d9..ddce56e60 100644 |
| --- a/internal/registry/hold_test.go |
| +++ b/internal/registry/hold_test.go |
| @@ -1,8 +1,10 @@ |
| package registry |
| import ( |
| + "errors" |
| "path/filepath" |
| "testing" |
| + "time" |
| ) |
| const ( |
| @@ -366,3 +368,47 @@ func TestFingerprintsUnchangedForUnheldRows(t *testing.T) { |
| t.Fatalf("SnapshotFingerprint = %s, want %s", got, wantS) |
| } |
| } |
| + |
| +// WithHoldsFrozen is root's activation guard (moves Task 13): while fn runs, |
| +// no other handle can write the registry, so a SetHold from the agent waits |
| +// and the held rows fn saw are still the held rows when it returns. |
| +func TestWithHoldsFrozenHoldsOffASetHold(t *testing.T) { |
| + r := holdFixture(t, "h") |
| + other, err := Open(r.Path()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer other.Close() |
| + done := make(chan error, 1) |
| + err = r.WithHoldsFrozen(func(held []Route) error { |
| + if len(held) != 0 { |
| + t.Errorf("held = %+v", held) |
| + } |
| + go func() { done <- other.SetHold("h", "moving", holdOpA) }() |
| + select { |
| + case err := <-done: |
| + t.Errorf("SetHold finished while the holds were frozen: %v", err) |
| + done <- err // for the read below |
| + case <-time.After(300 * time.Millisecond): |
| + } |
| + return nil |
| + }) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := <-done; err != nil { |
| + t.Fatalf("SetHold after the freeze: %v", err) |
| + } |
| + var seen []Route |
| + if err := r.WithHoldsFrozen(func(held []Route) error { seen = held; return nil }); err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(seen) != 1 || seen[0].Host != "h" || seen[0].HoldOp != holdOpA { |
| + t.Fatalf("held after SetHold = %+v", seen) |
| + } |
| + // fn's error is the call's, and nothing fn did not do is written. |
| + sentinel := errors.New("refused") |
| + if err := r.WithHoldsFrozen(func([]Route) error { return sentinel }); !errors.Is(err, sentinel) { |
| + t.Fatalf("err = %v", err) |
| + } |
| +} |
diff --git a/internal/registry/registry.go b/internal/registry/registry.go| index d14547170..ecdc61823 100644 |
| --- a/internal/registry/registry.go |
| +++ b/internal/registry/registry.go |
| @@ -10,6 +10,7 @@ |
| package registry |
| import ( |
| + "context" |
| "crypto/sha256" |
| "database/sql" |
| "errors" |
| @@ -1132,6 +1133,47 @@ func (r *Registry) HoldRows() ([]Route, error) { |
| return out, rows.Err() |
| } |
| +// WithHoldsFrozen runs fn with the rows an operation holds, while no other |
| +// connection can write the registry: it takes SQLite's write lock first |
| +// (BEGIN IMMEDIATE), so a SetHold or ReleaseHold elsewhere waits on its |
| +// busy_timeout until fn returns, and the rows fn was given are still the |
| +// held rows when it does. It is root's activation guard (moves Task 13): the |
| +// check that no row is held and the step that puts an older executable in |
| +// place happen with the answer held still. fn must be quick (a rename); |
| +// writers give up after busy_timeout. Nothing is written: the transaction is |
| +// rolled back. fn's error is returned. |
| +func (r *Registry) WithHoldsFrozen(fn func(held []Route) error) error { |
| + ctx := context.Background() |
| + conn, err := r.sql.Conn(ctx) |
| + if err != nil { |
| + return err |
| + } |
| + defer conn.Close() |
| + if _, err := conn.ExecContext(ctx, "BEGIN IMMEDIATE"); err != nil { |
| + return err |
| + } |
| + defer conn.ExecContext(ctx, "ROLLBACK") |
| + rows, err := conn.QueryContext(ctx, `SELECT `+routeCols+` FROM routes WHERE hold_op != '' ORDER BY host`) |
| + if err != nil { |
| + return err |
| + } |
| + var held []Route |
| + for rows.Next() { |
| + t, err := scanRoute(rows.Scan) |
| + if err != nil { |
| + rows.Close() |
| + return err |
| + } |
| + held = append(held, t) |
| + } |
| + if err := rows.Err(); err != nil { |
| + rows.Close() |
| + return err |
| + } |
| + rows.Close() |
| + return fn(held) |
| +} |
| + |
| // SetAccount stamps a route's app→account linkage as a narrow UPDATE — the |
| // reconcile pass re-stamps this every tick from the bucket manifests, and a |
| // read-modify-write Upsert racing a deploy's SetVersions (or the activator's |