diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index bff6c87..1f056e0 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -936,6 +936,11 @@ func runLink(ctx context.Context, opts options) error { // which the launcher already reads as "run whatever is on disk now". aside, standAside := context.WithCancel(ctx) defer standAside() + // **And it ends between applies too** (novox/hq issue 1585): whichever way the link ends, the apply in flight ends and + // saves before the process does. Deferred after standAside, so it runs before it: nothing is cancelled under it. + defer func() { _ = stopBetweenApplies(say, stopBound) }() + // What an earlier stop cut off before its rename is removed, and said, before anything saves. + removeLeftoverSaves(opts.state, say) stoodAside := false membership := link.Membership{ Node: mine.Node, @@ -2019,7 +2024,7 @@ func adoptDeliveredMembership(identityPath string, mine *identity.Identity, say return } say(fmt.Sprintf("moving to the %s bus at %s — restarting to dial it", next.Transport, next.Broker)) - os.Exit(0) + exitBetweenApplies(say) } // profileAsReported is the profile as the mesh reads it — the same bytes enrolment sends, so a diff --git a/cmd/mesh-host/main_test.go b/cmd/mesh-host/main_test.go index dbe477b..cd4df78 100644 --- a/cmd/mesh-host/main_test.go +++ b/cmd/mesh-host/main_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" "time" @@ -132,6 +133,57 @@ func TestAFlagAfterAPositionalIsRead(t *testing.T) { // state, acts, and writes it back — so they must not run at the same time, or the last save loses // what the other recorded. func TestOnlyOneApplyRunsAtATime(t *testing.T) { + // Issue 1585: the engine's stop waits for the apply in flight to end and save, and then lets no save start; before, + // the process ended under it and a save cut before its rename left its temporary file and the state before it. + t.Run("the stop waits for the apply in flight and its save", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + applying.Lock() + var said []string + var mu sync.Mutex + waiting := make(chan struct{}) + say := func(line string) { + mu.Lock() + said = append(said, line) + mu.Unlock() + if strings.Contains(line, "waiting up to") { + close(waiting) + } + } + stopped := make(chan func(), 1) + go func() { stopped <- stopBetweenApplies(say, time.Hour) }() + // Said before it blocks, and it cannot end before the apply lets go: so it has not ended now. + <-waiting + select { + case release := <-stopped: + release() + applying.Unlock() + t.Fatal("the stop did not wait for the apply in flight") + default: + } + if err := store.Save(path, store.State{Resources: []store.Applied{{ID: "nats.program-snapshot", Type: "archive"}}}); err != nil { + t.Fatal(err) + } + applying.Unlock() + release := <-stopped + saved := make(chan error, 1) + go func() { saved <- store.Save(path, store.State{}) }() + // A save after the stop blocks; nothing says it blocks, so this one wait is a bounded look. + select { + case <-saved: + release() + t.Fatal("a save started after the stop") + case <-time.After(50 * time.Millisecond): + } + release() + if err := <-saved; err != nil { + t.Fatal(err) + } + mu.Lock() + defer mu.Unlock() + if len(said) != 2 || !strings.Contains(said[0], "waiting up to 1h0m0s") || !strings.Contains(said[1], "its state is saved") { + t.Errorf("said %q", said) + } + }) // A host is built for one system at link time, and a test binary has no link time: this asks // the machine it runs on, and stands aside where the answer is no. built, err := system.For("arch") diff --git a/cmd/mesh-host/stopping.go b/cmd/mesh-host/stopping.go new file mode 100644 index 0000000..e9b4e0c --- /dev/null +++ b/cmd/mesh-host/stopping.go @@ -0,0 +1,72 @@ +package main + +import ( + "fmt" + "os" + "strings" + "time" + + "github.com/novox/mesh-host/internal/store" +) + +// stopBound is how long a stopping engine waits for the apply in flight to end and save what it did: under the service +// manager's own stop bound (90 s by default, then every process of the unit is killed), so the wait ends with a line in +// the journal rather than with a kill. +const stopBound = 60 * time.Second + +// stopBetweenApplies is how this engine ends, whichever way (a stop of the service, standing aside for a successor, a +// move to another bus): it waits, up to bound, for the apply in flight to end, then lets no save start and waits for +// the one writing (novox/hq issue 1585). An apply runs detached from the engine's own stop (applyContext), so without +// this the process ended under it: what it unpacked was never recorded, and a save cut between its write and its +// rename left a temporary file and the state before it. A stopping engine never gives back what it takes (the process +// ends); the release it returns is for a test, and gives back the apply only when this took it. +func stopBetweenApplies(say func(string), bound time.Duration) (release func()) { + took := true + if !applying.TryLock() { + say(fmt.Sprintf("stopping: waiting up to %s for the apply in flight to end and save what it did", bound)) + got := make(chan struct{}) + go func() { + applying.Lock() + close(got) + }() + select { + case <-got: + say("stopping: the apply in flight ended and its state is saved") + case <-time.After(bound): + took = false + say(fmt.Sprintf("stopping while an apply still runs after %s: what it did since its last save is not "+ + "recorded, and the next start applies its module again", bound)) + } + } + saves := store.Quiesce() + return func() { + saves() + if took { + applying.Unlock() + } + } +} + +// removeLeftoverSaves removes, at the engine's start, the temporary files of saves of the node's state that never +// reached their rename, and says how many (issue 1585: about 260 on novox, ~80 MB). +func removeLeftoverSaves(statePath string, say func(string)) { + removed, err := store.RemoveLeftovers(statePath) + if len(removed) > 0 { + shown := removed + if len(shown) > 5 { + shown = append(append([]string(nil), shown[:5]...), "…") + } + say(fmt.Sprintf("removed %d leftover save(s) of this node's state, cut off before their rename by an earlier "+ + "stop: %s", len(removed), strings.Join(shown, ", "))) + } + if err != nil { + say(err.Error()) + } +} + +// exitBetweenApplies ends the process cleanly once no apply runs and no save writes: an exit that is not runLink's +// return (a move to another bus). A variable, so a test sees it called rather than the test process ending. +var exitBetweenApplies = func(say func(string)) { + _ = stopBetweenApplies(say, stopBound) + os.Exit(0) +} diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 6fc5f71..9f63130 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -1375,6 +1375,19 @@ func (a *Agent) baseline(ctx context.Context, module string, w wantNow, comp com a.mu.Unlock() return false } + if h.Phase == string(delivery.PhaseHeld) && (w.Want.Backup == delivery.BackupAgain || w.Want.Backup == delivery.BackupSkip) { + // Held for a failed backup, and the want already carries the operator's answer, written before this engine + // started (nats on novox, 2026-11-04: "Back up again" answered, then the engine restarted and took the + // answer for the hold's own revision, so it was never acted on; issue 1585). The answer is acted on now: + // again backs it up and applies (a.prior makes the apply back it up first), skip applies without one. + // An answer the engine already acted on before its stop, and whose backup failed once more, is so acted + // on once more at this start: one backup more, never one skipped. + a.mu.Lock() + a.sayOnce("answered:"+module, fmt.Sprintf("%s is held for a failed backup, and its want carries the operator's "+ + "answer %q: acted on at this start", module, w.Want.Backup)) + a.mu.Unlock() + return false + } if h.Phase == string(delivery.PhaseHeld) { // Held for a failed backup, and the operator's answer is still to come: the hold is kept, nothing is // backed up again or applied, and the next revision of the want is the answer (ADR 0568 §4). diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index 4efc91e..e2fed75 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -595,7 +595,8 @@ func TestADataModuleIsBackedUpFirstAndHeldWhenItFails(t *testing.T) { } // At a start: a backup the node agent's own stop interrupted (postgres on novox, 2026-10-31) is no failure, so it is - // backed up again and applied while the want asks a backup; a backup that failed stays held for the operator. + // backed up again and applied while the want asks a backup; a backup that failed stays held for the operator's + // answer, and an answer the want already carries is acted on. for _, row := range []struct { detail, backup string backups, applied int @@ -605,7 +606,11 @@ func TestADataModuleIsBackedUpFirstAndHeldWhenItFails(t *testing.T) { {BackupInterrupted, delivery.BackupAgain, 1, 1, delivery.PhaseRunning, false}, // novox's postgres: its held have, written before mesh-host #189, names nothing it runs. {BackupInterrupted, "", 1, 1, delivery.PhaseRunning, true}, - {"restic: the repository is locked", delivery.BackupAgain, 0, 0, delivery.PhaseHeld, false}, + // A backup that failed, and the want carries the operator's answer, written before this start (nats on novox, + // 2026-11-04, issue 1585): again backs it up and applies, skip applies without one; no answer keeps the hold. + {"restic: the repository is locked", delivery.BackupAgain, 1, 1, delivery.PhaseRunning, false}, + {"restic: the repository is locked", delivery.BackupSkip, 0, 1, delivery.PhaseRunning, false}, + {"restic: the repository is locked", "", 0, 0, delivery.PhaseHeld, false}, } { r := newBed(t) r.comps.before(ck1, craw1) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index fd1432e..5fe2f2a 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -610,7 +610,7 @@ func applyScoped( err = fmt.Errorf("%w: %s", errRemovalWaits, said) } default: - action, detail, err = remove(ctx, sys, orphan, run, made) + action, detail, err = remove(ctx, sys, orphan, run, made, journalDir(windows)) } if errors.Is(err, errNoRemoval) && store.IsFormer(orphan.ID) { // **A former target of a kind the host cannot remove is left in place and forgotten, @@ -1572,7 +1572,7 @@ func applyOne(ctx context.Context, sys system.System, r declaration.Resource, ru case *declaration.User: return applyUser(ctx, sys, res, run, previous, in.groups, in.handed) case *declaration.Archive: - return applyArchive(ctx, res, previous, in.away) + return applyArchive(ctx, res, previous, in.away, journalDir(in.windows)) case *declaration.Process: return applyProcess(ctx, res, run, changed, previous, in.away) case *declaration.Action: @@ -2483,7 +2483,7 @@ func reflectOnly(ctx context.Context, sys system.System, r *declaration.Service, // // made is the units whose unit file this host wrote where there was none (meshMadeUnits). func remove(ctx context.Context, sys system.System, a store.Applied, run Runner, - made map[string]bool) (string, string, error) { + made map[string]bool, journals string) (string, string, error) { switch declaration.Type(a.Type) { case declaration.TypeDirectory, declaration.TypeFile, declaration.TypeArchive: // Removal below a home follows no link an account put there either (novox/hq ADR 0266). @@ -2609,7 +2609,7 @@ func remove(ctx context.Context, sys system.System, a store.Applied, run Runner, case declaration.TypeArchive: // Exactly what it unpacked, and the directories the host made for it once they are empty // (novox/hq issue 162). - return removeArchive(a) + return removeArchive(a, journals) default: return "", "", fmt.Errorf("%w: a %q", errNoRemoval, a.Type) diff --git a/internal/apply/archive.go b/internal/apply/archive.go index fef2f12..aeb8cd5 100644 --- a/internal/apply/archive.go +++ b/internal/apply/archive.go @@ -35,9 +35,14 @@ import ( // enough for a desktop theme and small enough to notice. const maxArchive = 512 << 20 -func applyArchive(ctx context.Context, r *declaration.Archive, previous store.Applied, away *storeAway) (Outcome, error) { +func applyArchive(ctx context.Context, r *declaration.Archive, previous store.Applied, away *storeAway, journals string) (Outcome, error) { out := begin(r) out.Action = "unchanged" + recorded := previous.Wrote != "" + previous, err := withJournal(journals, previous, r.Path) + if err != nil { + return out, err + } body, err := fetchMinding(ctx, r.Source, away) if err != nil { @@ -78,8 +83,12 @@ func applyArchive(ctx context.Context, r *declaration.Archive, previous store.Ap return out, err } out.unpacked = &unpacked + // At once, before the rest of the apply can fail or the engine stop (novox/hq issue 1585). + if err := keepJournal(journals, r.Path, got, &unpacked); err != nil { + return out, fmt.Errorf("%s was unpacked, and what it unpacked could not be written down: %w", r.Path, err) + } out.Action = "updated" - if previous.Wrote == "" { + if !recorded { out.Action = "created" } out.Detail = fmt.Sprintf("%d file(s)", written) diff --git a/internal/apply/archive_journal.go b/internal/apply/archive_journal.go new file mode 100644 index 0000000..385135b --- /dev/null +++ b/internal/apply/archive_journal.go @@ -0,0 +1,133 @@ +package apply + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/novox/mesh-host/internal/store" +) + +// What an archive unpacked is written down the moment it is unpacked (novox/hq issue 1585). +// +// The node's state is saved at the end of a module's apply, so an engine that stopped, was replaced or lost its +// state between the unpack and that save held files on the machine it had no record of. The next apply then found +// its own files and refused them as ones "the mesh did not put there" (issue 162), and a module could not move to +// its new build until a person removed them. So, at once and atomically, the host keeps the digest it unpacked and +// the files and directories it placed; an apply that holds no record of the archive reads this instead of treating +// everything there as somebody's. The record in the state stays the authority: the journal is read only when there +// is none. +// +// **In the node-engine's own state directory, readable and writable by root alone, never beside the archive.** The +// archive's directory may be writable by the module's account (a home, a data directory): a journal there could be +// forged, and a root engine would then remove, or take as its own, any path the forger named. And an entry is read +// only when it names a path inside the archive's directory (filepath.IsLocal), and a parent only when it is above it. + +// JournalDirName is the directory under the node's state directory that keeps the journals. +const JournalDirName = "unpacked" + +type unpackJournal struct { + Target string `json:"target"` + Wrote string `json:"wrote"` + Unpacked *store.Unpacked `json:"unpacked"` +} + +// journalDir is where the journals of this apply are kept: under the state directory every applier on the machine +// shares (the one its windows are kept in). Empty for a caller with no state directory (a test), which keeps none. +func journalDir(w *Windows) string { + if w == nil || w.dir == "" { + return "" + } + return filepath.Join(filepath.Dir(w.dir), JournalDirName) +} + +// journalPath names the journal of the archive at target: by the digest of its path, so no path names a file +// outside the journals' directory. +func journalPath(dir, target string) string { + sum := sha256.Sum256([]byte(filepath.Clean(target))) + return filepath.Join(dir, hex.EncodeToString(sum[:])+".json") +} + +// keepJournal writes down what an archive unpacked at path. +func keepJournal(dir, path, wrote string, u *store.Unpacked) error { + if dir == "" { + return nil + } + if err := os.MkdirAll(dir, 0o700); err != nil { + return err + } + raw, err := json.Marshal(unpackJournal{Target: filepath.Clean(path), Wrote: wrote, Unpacked: u}) + if err != nil { + return err + } + return writeAtomicallyByPath(journalPath(dir, path), raw, 0o600) +} + +// journalOf is what the journal says of path: false when there is none, and an error when there is one that cannot +// be read or names a path outside the archive's directory, which is never believed. +func journalOf(dir, path string) (unpackJournal, bool, error) { + if dir == "" { + return unpackJournal{}, false, nil + } + raw, err := os.ReadFile(journalPath(dir, path)) + if err != nil { + return unpackJournal{}, false, nil + } + var j unpackJournal + if err := json.Unmarshal(raw, &j); err != nil { + return unpackJournal{}, false, fmt.Errorf("the journal of what was unpacked at %s is unreadable: %w", path, err) + } + if j.Wrote == "" || j.Unpacked == nil { + return unpackJournal{}, false, fmt.Errorf("the journal of what was unpacked at %s names no digest or files", path) + } + if err := checkJournal(j, filepath.Clean(path)); err != nil { + return unpackJournal{}, false, err + } + return j, true, nil +} + +// checkJournal refuses a journal of another target, an entry that is not a path inside the archive's directory, and +// a parent that is not above it. +func checkJournal(j unpackJournal, target string) error { + if j.Target != target { + return fmt.Errorf("the journal kept for %s names %q", target, j.Target) + } + for _, rel := range append(append([]string(nil), j.Unpacked.Files...), j.Unpacked.Dirs...) { + if !filepath.IsLocal(filepath.FromSlash(rel)) { + return fmt.Errorf("the journal of what was unpacked at %s names %q, which is not inside it: not believed", target, rel) + } + } + for _, p := range j.Unpacked.Parents { + if p = filepath.Clean(p); !filepath.IsAbs(p) || p == target || !strings.HasPrefix(target, p+string(filepath.Separator)) { + return fmt.Errorf("the journal of what was unpacked at %s names the parent %q, which is not above it: not believed", target, p) + } + } + return nil +} + +// withJournal is previous, or, when the state holds no record of the archive, what the journal says the host +// unpacked at path. A journal not believed is an error: the apply stops and says so rather than guess. +func withJournal(dir string, previous store.Applied, path string) (store.Applied, error) { + if previous.Wrote != "" { + return previous, nil + } + j, ok, err := journalOf(dir, path) + if err != nil || !ok { + return previous, err + } + previous.Wrote, previous.Target, previous.Unpacked = j.Wrote, path, j.Unpacked + return previous, nil +} + +// dropJournal removes the journal of the archive at target, once the archive is taken off; whether the state's +// record of it still said what it unpacked or not. +func dropJournal(dir, target string) { + if dir == "" || target == "" { + return + } + _ = os.Remove(journalPath(dir, target)) +} diff --git a/internal/apply/archive_removal_test.go b/internal/apply/archive_removal_test.go index b9f7b67..19475dd 100644 --- a/internal/apply/archive_removal_test.go +++ b/internal/apply/archive_removal_test.go @@ -2,6 +2,7 @@ package apply import ( "context" + "encoding/json" "os" "path/filepath" "strings" @@ -152,6 +153,98 @@ func TestAnArchiveDoesNotWriteOverAFileItFound(t *testing.T) { if entries, _ := os.ReadDir(dir); len(entries) != 1 { t.Fatalf("%d entries beside the directory", len(entries)) } + + // Issue 1585: what the host unpacked is written down at once, in its own state directory, so an archive whose + // record in the state was lost is still the host's: the next version replaces its files, drops the ones it no + // longer has and leaves a file somebody added. A file the host never unpacked still refuses it, and so does a + // journal naming a path outside the archive's directory. The journal goes with the archive, record lost or not. + for _, row := range []struct { + name string + journaled bool // the first version was unpacked by the host + foreign string + forged func(j *unpackJournal) + refused string + }{ + {"unpacked by the host, record lost", true, "", nil, ""}, + {"unpacked by the host, somebody's file beside", true, "mine.txt", nil, ""}, + {"never unpacked by the host", false, "", nil, "did not put there"}, + {"a journal naming a path outside", true, "", func(j *unpackJournal) { + j.Unpacked.Files = append(j.Unpacked.Files, "../../etc/passwd") + }, "not inside it"}, + {"a journal naming a parent not above", true, "", func(j *unpackJournal) { + j.Unpacked.Parents = []string{"/etc"} + }, "not above it"}, + } { + t.Run(row.name, func(t *testing.T) { + dir, state := t.TempDir(), t.TempDir() + windows, journals := WindowsIn(state), filepath.Join(state, JournalDirName) + applyIt := func(decl string, known store.State) (store.State, error) { + _, after, err := ApplyMindingWindows(context.Background(), archHost(t), declare(t, decl), known, + store.OriginDeclared, noServices, nil, nil, nil, windows) + return after, err + } + target := filepath.Join(dir, "tools") + first, _ := archiveDecl(t, "m.tools", target, map[string]string{"bin": "one", "old": "o"}, "") + if row.journaled { + if _, err := applyIt(first, store.State{}); err != nil { + t.Fatal(err) + } + if entries, _ := os.ReadDir(dir); len(entries) != 1 { + t.Fatalf("%d entries beside the archive: the journal is the state directory's", len(entries)) + } + info, err := os.Stat(journalPath(journals, target)) + if err != nil || info.Mode().Perm() != 0o600 { + t.Fatalf("no journal of root's alone after the unpack: %v %v", info, err) + } + } else { + os.MkdirAll(target, 0o755) + os.WriteFile(filepath.Join(target, "bin"), []byte("one"), 0o644) + } + if row.forged != nil { + j, _, err := journalOf(journals, target) + if err != nil { + t.Fatal(err) + } + row.forged(&j) + raw, _ := json.Marshal(j) + os.WriteFile(journalPath(journals, target), raw, 0o600) + } + if row.foreign != "" { + os.WriteFile(filepath.Join(target, row.foreign), []byte("x"), 0o644) + } + second, _ := archiveDecl(t, "m.tools", target, map[string]string{"bin": "two"}, "") + // The state is empty: the record of the first unpack was lost. + after, err := applyIt(second, store.State{}) + if row.refused != "" { + if err == nil || !strings.Contains(err.Error(), row.refused) { + t.Fatalf("refused? %v", err) + } + return + } + if err != nil { + t.Fatal(err) + } + if got, _ := os.ReadFile(filepath.Join(target, "bin")); string(got) != "two" { + t.Fatalf("bin is %q", got) + } + if exists(filepath.Join(target, "old")) { + t.Fatal("a file the new version no longer has is still there") + } + if row.foreign != "" && !exists(filepath.Join(target, row.foreign)) { + t.Fatal("somebody's file went") + } + // Taken off, with a record that no longer says what it unpacked: the journal goes too. + for i := range after.Resources { + after.Resources[i].Unpacked = nil + } + if _, err := applyIt(`{"id":"other","type":"file","path":"`+dir+`/other","content":"o"}`, after); err != nil { + t.Fatal(err) + } + if exists(journalPath(journals, target)) { + t.Fatal("the journal outlived the archive") + } + }) + } } func TestADirectoryTheHostMadeGoesOnlyWhenEmpty(t *testing.T) { diff --git a/internal/apply/archive_unpacked.go b/internal/apply/archive_unpacked.go index cc032cd..6cbce37 100644 --- a/internal/apply/archive_unpacked.go +++ b/internal/apply/archive_unpacked.go @@ -364,7 +364,16 @@ func mergeInto(fresh, path, owner string, files, dirs []string, o ours) (store.U // module's tools from its bundle — sees the whole tree or none of it, never half, and a file it has // open stays readable until it closes it. The runtime is told the module went by its own membership, // not by the files disappearing. -func removeArchive(a store.Applied) (string, string, error) { +func removeArchive(a store.Applied, journals string) (string, string, error) { + action, detail, err := removeArchiveFiles(a) + // And its journal (issue 1585), once the record goes: whether the record said what it unpacked or not. + if err == nil { + dropJournal(journals, a.Target) + } + return action, detail, err +} + +func removeArchiveFiles(a store.Applied) (string, string, error) { if store.IsFormer(a.ID) { // The version before is what a rollback starts (ADR 0141) and what a reader may still have // open; the launcher retires the host's own versions, not the apply (issue 194). diff --git a/internal/apply/home_links_test.go b/internal/apply/home_links_test.go index 97a47e3..aad8d10 100644 --- a/internal/apply/home_links_test.go +++ b/internal/apply/home_links_test.go @@ -159,7 +159,7 @@ func TestRemovalThroughALinkUnderAHomeIsRefused(t *testing.T) { home, outside := aLinkedHome(t) link(t, outside, filepath.Join(home, ".claude")) _, _, err := remove(context.Background(), archHost(t), store.Applied{ID: "m.f", Type: "file", - Target: filepath.Join(home, ".claude", "shadow")}, nil, nil) + Target: filepath.Join(home, ".claude", "shadow")}, nil, nil, "") mustBeLinkRefusal(t, err) if _, err := os.Stat(filepath.Join(outside, "shadow")); err != nil { t.Fatal("removed through the link") diff --git a/internal/store/save.go b/internal/store/save.go new file mode 100644 index 0000000..36bec2d --- /dev/null +++ b/internal/store/save.go @@ -0,0 +1,176 @@ +package store + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "sync" + "time" +) + +// What a node knows is written by one writer at a time, completely or not at all, and never loses a record without +// saying so (novox/hq issue 1585). On novox, 2026-11-04, the node-engine's state had lost the records of two archives +// (nats's program snapshot and its tools bundle) that a save of 2026-10-24 had written into its temporary file and +// never renamed into place, and about 260 such temporary files (~80 MB) lay beside it: the process ended between +// writing a save and renaming it, again and again, because the engine stopped without waiting for the apply in flight +// (cmd/mesh-host, stopBetweenApplies). The temporary files and the lost records are the same fault. + +// tempPattern is the name a save writes before it renames the file into place. +const tempPattern = ".state-*.json" + +// saving is the one writer of this process: a save holds it from its read of what was saved before to its rename and +// the directory's flush, and Quiesce holds it for good. +var saving sync.Mutex + +// Said is where a save says loudly what must not pass in silence: a record the state held before and this save would +// drop although nothing forgot it. The journal when the engine runs as a service. +var Said = func(line string) { fmt.Fprintln(os.Stderr, line) } + +// Save writes the state, atomically and durably: the whole file to a temporary one beside it, flushed, renamed over +// the old one, and the directory flushed so the rename itself survives a power cut. +// +// Atomic because the alternative has a failure mode with no floor: a host interrupted while +// writing loses the record of everything it owns, and then owns nothing it can clean up. +// +// Before it writes, it reads what was saved before and says, through Said, each resource or hold that state recorded +// and this one would drop although this copy never forgot it (Forget, Release): a save from a copy read before +// another writer saved, or a copy that lost a record on the way, is said at once rather than found weeks later when a +// file the host put there reads as foreign. The save still goes ahead: it is what the apply found on the machine, and +// refusing it would lose that too. +func Save(path string, s State) error { + saving.Lock() + defer saving.Unlock() + return save(path, s) +} + +func save(path string, s State) error { + s.UpdatedAt = time.Now().UTC() + + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("making room for the node's state: %w", err) + } + + if before, err := Load(path); err == nil { + for _, lost := range Dropped(before, s) { + Said(fmt.Sprintf("the node's state is saved without the record of %s, which the state saved before held "+ + "and nothing forgot: what it put on this machine is no longer known as the mesh's (issue 1585)", lost)) + } + } + + raw, err := json.MarshalIndent(s, "", " ") + if err != nil { + return fmt.Errorf("encoding the node's state: %w", err) + } + raw = append(raw, '\n') + + tmp, err := os.CreateTemp(dir, tempPattern) + if err != nil { + return fmt.Errorf("writing the node's state: %w", err) + } + defer os.Remove(tmp.Name()) + + if _, err := tmp.Write(raw); err != nil { + tmp.Close() + return fmt.Errorf("writing the node's state: %w", err) + } + // Flushed before the rename: a rename is atomic, and a rename of a file whose contents are + // still in the page cache is atomically the wrong thing. + if err := tmp.Sync(); err != nil { + tmp.Close() + return fmt.Errorf("flushing the node's state: %w", err) + } + if err := tmp.Close(); err != nil { + return fmt.Errorf("closing the node's state: %w", err) + } + if err := os.Chmod(tmp.Name(), 0o600); err != nil { + return fmt.Errorf("securing the node's state: %w", err) + } + if err := os.Rename(tmp.Name(), path); err != nil { + return fmt.Errorf("replacing the node's state: %w", err) + } + // And the directory, so the new name is on disk and not only the new contents. + if d, err := os.Open(dir); err == nil { + syncErr := d.Sync() + d.Close() + if syncErr != nil { + return fmt.Errorf("flushing the directory of the node's state: %w", syncErr) + } + } + return nil +} + +// Dropped is each resource and hold before recorded that after does not, and that after never forgot: "resource " +// or "hold ", sorted. +func Dropped(before, after State) []string { + kept := map[string]bool{} + for _, r := range after.Resources { + kept["resource "+r.ID] = true + } + for _, h := range after.Held { + kept["hold "+h.ID] = true + } + var out []string + for _, r := range before.Resources { + if k := "resource " + r.ID; !kept[k] && !after.forgotten[k] { + out = append(out, k) + } + } + for _, h := range before.Held { + if k := "hold " + h.ID; !kept[k] && !after.forgotten[k] { + out = append(out, k) + } + } + sort.Strings(out) + return out +} + +// forget notes that this copy dropped a record on purpose. +func (s *State) forget(what string) { + if s.forgotten == nil { + s.forgotten = map[string]bool{} + } + s.forgotten[what] = true +} + +// Quiesce waits for a save in flight to end and lets no other start in this process, for the engine's stop: a process +// that ends while a save writes leaves a temporary file and the state before it (issue 1585). It returns what lets saves +// start again, for a test; a stopping engine never calls it. +func Quiesce() (release func()) { + saving.Lock() + return saving.Unlock +} + +// RemoveLeftovers removes the temporary files of saves that never reached their rename, beside the state at path, and +// names each it removed. Under the machine's apply lock, so no save of another process is in flight: every writer of +// the state takes it (Lock). +func RemoveLeftovers(path string) ([]string, error) { + unlock, err := Lock(path, nil) + if err != nil { + return nil, err + } + defer unlock() + saving.Lock() + defer saving.Unlock() + found, err := filepath.Glob(filepath.Join(filepath.Dir(path), tempPattern)) + if err != nil { + return nil, err + } + var removed []string + var failed []string + for _, f := range found { + if err := os.Remove(f); err != nil { + failed = append(failed, fmt.Sprintf("%s: %v", filepath.Base(f), err)) + continue + } + removed = append(removed, filepath.Base(f)) + } + sort.Strings(removed) + if len(failed) > 0 { + return removed, fmt.Errorf("leftover saves of the node's state could not be removed: %s", strings.Join(failed, "; ")) + } + return removed, nil +} diff --git a/internal/store/store.go b/internal/store/store.go index 5755b81..892f5c0 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -16,7 +16,6 @@ import ( "fmt" "io/fs" "os" - "path/filepath" "sort" "strings" "time" @@ -263,6 +262,10 @@ type State struct { // Members is every library group the engine put a tool runner's account in that it was not in before (ADR 0328 // §11, as ADR 0252 records an account's groups): taken back when no longer declared. Members []Membership `json:"members,omitempty"` + + // forgotten is what this copy dropped on purpose since it was loaded (Forget, Release): never written, read by Save + // to tell a record given up from one lost (issue 1585). + forgotten map[string]bool } // MadeGroup is one library group the engine made. @@ -484,7 +487,7 @@ func (s *State) RecordHeld(h Held) { // Release drops a hold, once its module is taken and the host has converged what was held. func (s *State) Release(id string) { - kept := s.Held[:0] + kept := make([]Held, 0, len(s.Held)) for _, h := range s.Held { if h.ID != id { kept = append(kept, h) @@ -494,6 +497,7 @@ func (s *State) Release(id string) { if len(s.Held) == 0 { s.Held = nil } + s.forget("hold " + id) } // RetiredAt returns the retirement of the found configuration at a path, if the mesh retired one. @@ -563,51 +567,6 @@ func Load(path string) (State, error) { return s, nil } -// Save writes the state, atomically. -// -// Atomic because the alternative has a failure mode with no floor: a host interrupted while -// writing loses the record of everything it owns, and then owns nothing it can clean up. -func Save(path string, s State) error { - s.UpdatedAt = time.Now().UTC() - - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { - return fmt.Errorf("making room for the node's state: %w", err) - } - - raw, err := json.MarshalIndent(s, "", " ") - if err != nil { - return fmt.Errorf("encoding the node's state: %w", err) - } - raw = append(raw, '\n') - - tmp, err := os.CreateTemp(filepath.Dir(path), ".state-*.json") - if err != nil { - return fmt.Errorf("writing the node's state: %w", err) - } - defer os.Remove(tmp.Name()) - - if _, err := tmp.Write(raw); err != nil { - tmp.Close() - return fmt.Errorf("writing the node's state: %w", err) - } - // Flushed before the rename: a rename is atomic, and a rename of a file whose contents are - // still in the page cache is atomically the wrong thing. - if err := tmp.Sync(); err != nil { - tmp.Close() - return fmt.Errorf("flushing the node's state: %w", err) - } - if err := tmp.Close(); err != nil { - return fmt.Errorf("closing the node's state: %w", err) - } - if err := os.Chmod(tmp.Name(), 0o600); err != nil { - return fmt.Errorf("securing the node's state: %w", err) - } - if err := os.Rename(tmp.Name(), path); err != nil { - return fmt.Errorf("replacing the node's state: %w", err) - } - return nil -} - // Record adds or replaces what is known about one resource, preserving order. func (s *State) Record(a Applied) { for i, existing := range s.Resources { @@ -640,15 +599,18 @@ func FormerID(id, target string) string { return id + "@former:" + target } // IsFormer says whether a record names a former target. func IsFormer(id string) bool { return strings.Contains(id, "@former:") } -// Forget drops a resource from what the node owns. +// Forget drops a resource from what the node owns, and notes that it was dropped on purpose, so the save that follows +// does not say it lost the record (Save). A new slice, never the old one's array: a copy of the state taken before keeps +// what it held. func (s *State) Forget(id string) { - kept := s.Resources[:0] + kept := make([]Applied, 0, len(s.Resources)) for _, r := range s.Resources { if r.ID != id { kept = append(kept, r) } } s.Resources = kept + s.forget("resource " + id) } // Orphans returns what the host applied and the declaration no longer names, newest first. diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 712c3bd..d650171 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -93,6 +93,66 @@ func TestSavingLeavesNoDebrisBehind(t *testing.T) { } t.Errorf("expected only the state file, found: %v", names) } + + // Issue 1585: a save cut off between its write and its rename (the engine ended under it) left its temporary file; + // the engine's start removes them under the apply lock and names each, and leaves the state and every other file. + t.Run("leftovers of cut-off saves are removed", func(t *testing.T) { + for _, name := range []string{".state-3758449435.json", ".state-1403853615.json", ".declared-1.json"} { + if err := os.WriteFile(filepath.Join(dir, name), []byte("{}"), 0o600); err != nil { + t.Fatal(err) + } + } + removed, err := RemoveLeftovers(path) + if err != nil { + t.Fatal(err) + } + if strings.Join(removed, ",") != ".state-1403853615.json,.state-3758449435.json" { + t.Errorf("removed %v", removed) + } + for _, keep := range []string{"state.json", ".declared-1.json"} { + if _, err := os.Stat(filepath.Join(dir, keep)); err != nil { + t.Errorf("%s was not left: %v", keep, err) + } + } + }) + + // Issue 1585: a save that drops a record the state saved before held, which nothing forgot, says so loudly; one + // forgotten or released on purpose is not said. Forget never writes into the array a copy taken before still reads. + t.Run("a dropped record is said, a forgotten one is not", func(t *testing.T) { + var said []string + was := Said + Said = func(line string) { said = append(said, line) } + defer func() { Said = was }() + before := State{Resources: []Applied{{ID: "c", Type: "file", Target: "/c"}, + {ID: "nats.program-snapshot", Type: "archive", Target: "/s"}, {ID: "nats.bundle-tools", Type: "archive", Target: "/t"}}, + Held: []Held{{ID: "h"}}} + if err := Save(path, before); err != nil { + t.Fatal(err) + } + if len(said) != 1 || !strings.Contains(said[0], "resource a,") { + t.Errorf("the record of a, saved before and dropped, was not said: %v", said) + } + said = nil + copied := before + copied.Forget("c") + copied.Release("h") + if before.Resources[0].ID != "c" { + t.Errorf("Forget wrote into the copy taken before: %+v", before.Resources) + } + if err := Save(path, copied); err != nil { + t.Fatal(err) + } + if len(said) != 0 { + t.Errorf("a record forgotten on purpose was said lost: %v", said) + } + stale := State{Resources: []Applied{{ID: "nats.bundle-tools", Type: "archive", Target: "/t"}}} + if err := Save(path, stale); err != nil { + t.Fatal(err) + } + if len(said) != 1 || !strings.Contains(said[0], "resource nats.program-snapshot") { + t.Errorf("a lost record was not said, or said wrong: %v", said) + } + }) } func TestRecordReplacesRatherThanDuplicating(t *testing.T) { diff --git a/module.json b/module.json index 57270a3..b71573b 100644 --- a/module.json +++ b/module.json @@ -49,7 +49,7 @@ "type": "file", "path": "/usr/lib/nox-mesh-host/launch", "mode": "0755", - "content": "#!/bin/sh\n# Supervise the host: start it, watch it, and decide what to do when it stops.\n#\n# novox/hq ADR 0005. The init is asked for ONE thing — run this at boot — and everything else\n# lives here, in a script that can be tested. Whether to restart, how long to wait, when to give\n# up, when to roll back: all of it is policy, and policy in a unit file can only be read and\n# hoped for.\n#\n# **It is the witness of the host's own successor** (novox/hq to-be 45 §8, ADR 0227 rule 8). A\n# delivered host that is not the known-good one runs on trial: it must report its declaration\n# under its own build — which it marks by writing itself into known-good — within a bound. One\n# that crashes repeatedly, exits without standing aside for anything, or does not report in\n# bound is stopped, recorded in `rolled-back` with why, and the known-good one is run. The host\n# says every record on its reports, so the mesh raises it as a condition. One rollback per\n# version: a version in `rolled-back` is never started again; a newer delivery is.\n#\n# It does NOT exec the host. Exec would replace this process, and then only the init could\n# restart anything — which is the arrangement this exists to remove. The cost of staying is\n# signal handling, below.\n#\n# Everything a rollback does is one of: read a file, look at a directory, write a file. It shares\n# no code with the host and calls none of it: a binary that cannot start cannot be its own\n# recovery.\n#\n# POSIX sh. `set -e` is deliberately absent: this script's whole job is to inspect exit codes,\n# and -e would make it exit on the first one it is meant to handle.\nset -u\n\nSTATE_DIR=\"${MESH_HOST_STATE_DIR:-/var/lib/mesh-host}\"\nLIBEXEC=\"${MESH_HOST_LIBEXEC:-/usr/lib/nox-mesh-host}\"\n# The host that was placed by hand, used only when nothing has been delivered. The first host on a\n# machine always arrives this way; every one after it is delivered (novox/hq ADR 0141).\nFALLBACK=\"${MESH_HOST_BIN:-/usr/bin/nox-mesh-host}\"\nVERSIONS=\"$LIBEXEC/versions\"\nBINARY=\"nox-mesh-host\"\nLIMIT=\"${MESH_HOST_START_LIMIT:-3}\"\nBACKOFF=\"${MESH_HOST_BACKOFF:-5}\"\n# How long a host on trial has to report, in seconds: to-be 45 §8's ten minutes from the apply.\nBOUND=\"${MESH_HOST_TRIAL_BOUND:-600}\"\nTICK=\"${MESH_HOST_TRIAL_TICK:-5}\"\nGRACE=\"${MESH_HOST_STOP_GRACE:-20}\"\nONCE=\"${MESH_HOST_RUN_ONCE:-}\" # tests run one iteration; nothing else sets this\n\nATTEMPTS=\"$STATE_DIR/start-attempts\"\nHALTED=\"$STATE_DIR/halted\"\nPINNED=\"$STATE_DIR/rollback-pinned\"\nKNOWN_GOOD=\"$STATE_DIR/known-good\"\n# One line per verdict: from, to, when (seconds since the epoch), outcome, why — tab-separated, read\n# by the host (internal/upgrade) and said on its reports.\nROLLED=\"$STATE_DIR/rolled-back\"\nTRIAL=\"$STATE_DIR/trial\"\nEXPIRED=\"$STATE_DIR/trial-expired\"\n\nsay() { echo \"nox-mesh-host-launch: $*\" >&2; }\n\nnow() { date +%s; }\n\nknown_good() { tr -d '[:space:]' < \"$KNOWN_GOOD\" 2>/dev/null || true; }\n\ndelivered() { [ -n \"$1\" ] && [ -x \"$VERSIONS/$1/$BINARY\" ]; }\n\nrolled_back() { [ -s \"$ROLLED\" ] && cut -f1 \"$ROLLED\" 2>/dev/null | grep -qxF \"$1\"; }\n\n# The version a host path is: the directory it was delivered in, or nothing for the host placed by hand.\nversion_of() {\n\tcase \"$1\" in\n\t\t\"$VERSIONS\"/*/\"$BINARY\") v=\"${1#\"$VERSIONS\"/}\"; echo \"${v%/\"$BINARY\"}\" ;;\n\t\t*) echo \"\" ;;\n\tesac\n}\n\nrecord() { # from to outcome why\n\tprintf '%s\\t%s\\t%s\\t%s\\t%s\\n' \"$1\" \"$2\" \"$(now)\" \"$3\" \"$4\" >> \"$ROLLED\"\n}\n\n# Which host to run: the newest delivered one that was never rolled back — or, while a rollback's pin\n# stands, the version it pinned — or the one placed by hand when nothing has been delivered.\n#\n# **Asked every time round the loop, not once.** Standing aside for a successor is a clean exit, and\n# the next turn has to run what is on disk NOW — resolving this once would restart the same binary\n# for ever and the upgrade would never take.\n#\n# Newest by when it arrived, never by how its name sorts: a version string is whatever the source was\n# described as, and those do not sort — \"1.10\" orders before \"1.9\". Ordering by name would start an\n# older host and call it an upgrade.\n#\n# **A pin stands until something newer arrives.** A version delivered after the pin was written, and\n# never rolled back, is a new build the mesh asks for: the pin goes and it runs, on trial.\npick_host() {\n\tnewest=\n\t# A directory with no executable in it is not a version: an interrupted delivery leaves one, and\n\t# running \"the newest\" would then mean running nothing.\n\tfor candidate in $(ls -1t \"$VERSIONS\" 2>/dev/null || true); do\n\t\tdelivered \"$candidate\" || continue\n\t\trolled_back \"$candidate\" && continue\n\t\tnewest=\"$candidate\"\n\t\tbreak\n\tdone\n\tif [ -s \"$PINNED\" ]; then\n\t\tpinned=\"$(tr -d '[:space:]' < \"$PINNED\" 2>/dev/null || true)\"\n\t\tif [ -n \"$newest\" ] && [ \"$newest\" != \"$pinned\" ] && [ \"$VERSIONS/$newest\" -nt \"$PINNED\" ]; then\n\t\t\tsay \"host $newest arrived after the rollback to $pinned; running it\"\n\t\t\trm -f \"$PINNED\"\n\t\telif delivered \"$pinned\"; then\n\t\t\techo \"$VERSIONS/$pinned/$BINARY\"\n\t\t\treturn 0\n\t\telse\n\t\t\tsay \"the pinned version '$pinned' is not delivered; ignoring the pin\"\n\t\tfi\n\tfi\n\tif [ -n \"$newest\" ]; then\n\t\techo \"$VERSIONS/$newest/$BINARY\"\n\t\treturn 0\n\tfi\n\techo \"$FALLBACK\"\n}\n\n# Go back from `from` to the known-good version, once, and say so. False when there is nowhere to go.\nroll_back() { # from why\n\tkg=\"$(known_good)\"\n\tif [ -z \"$1\" ] || [ \"$1\" = \"$kg\" ] || ! delivered \"$kg\"; then\n\t\treturn 1\n\tfi\n\trecord \"$1\" \"$kg\" rolled-back \"$2\"\n\t# The pin is what stops the launcher starting a version newer than known-good that is not the one\n\t# rolled back; the record is what stops it starting this one again.\n\tprintf '%s\\n' \"$kg\" > \"$PINNED\"\n\trm -f \"$TRIAL\"\n\tsay \"rolled back from host $1 to $kg: $2\"\n\treturn 0\n}\n\n# Watch a host on trial: done when it writes itself into known-good, stopped when the bound passes.\n# A subshell beside the host, so the launcher's own wait is untouched.\ntrial_watch() { # pid version deadline\n\twhile kill -0 \"$1\" 2>/dev/null; do\n\t\t[ \"$(known_good)\" = \"$2\" ] && return 0\n\t\tif [ \"$(now)\" -ge \"$3\" ]; then\n\t\t\tprintf '%s\\n' \"$2\" > \"$EXPIRED\"\n\t\t\tsay \"host $2 did not report within ${BOUND}s of starting; stopping it\"\n\t\t\tkill -TERM \"$1\" 2>/dev/null\n\t\t\twaited=0\n\t\t\twhile kill -0 \"$1\" 2>/dev/null && [ \"$waited\" -lt \"$GRACE\" ]; do\n\t\t\t\tsleep 1\n\t\t\t\twaited=$((waited + 1))\n\t\t\tdone\n\t\t\tkill -KILL \"$1\" 2>/dev/null\n\t\t\treturn 0\n\t\tfi\n\t\tsleep \"$TICK\"\n\tdone\n}\n\nchild=\nwatcher=\nstopping=\n\n# The machine is shutting down. Pass it on and wait for the host to finish — a supervisor that\n# exits while its child is still running leaves the host to be killed rather than to stop, and\n# an apply interrupted that way is exactly the half-configured machine this project is about.\non_term() {\n\tstopping=yes\n\tif [ -n \"$child\" ]; then\n\t\tsay \"stopping: passing the signal to the host\"\n\t\tkill -TERM \"$child\" 2>/dev/null\n\tfi\n}\ntrap on_term TERM INT\n\nmkdir -p \"$STATE_DIR\"\n# What this launcher is, so a delivered successor of it is run at the next clean exit rather than at\n# the next boot.\nSELF=\"$(cksum < \"$0\" 2>/dev/null || true)\"\n\nwhile :; do\n\tif [ -n \"$stopping\" ]; then\n\t\texit 0\n\tfi\n\n\tif [ -e \"$HALTED\" ]; then\n\t\tsay \"halted: $(cat \"$HALTED\" 2>/dev/null || echo 'reason not recorded')\"\n\t\tsay \"not starting the host. this node needs a person.\"\n\t\texit 0\n\tfi\n\n\t# Consecutive failed starts, not starts. Cleared by the host itself when it reports, which is\n\t# the only evidence either this or known-good has.\n\t#\n\t# Read the FIRST FIELD, then insist it is a plain integer.\n\t#\n\t# Stripping whitespace instead concatenates, and that is not hypothetical: a counter\n\t# holding \"1 2\" became \"12\", past the limit, so a healthy node rolled itself back. An\n\t# unreadable counter must fail towards \"start normally\", never towards \"give up\".\n\tcount=0\n\tif [ -s \"$ATTEMPTS\" ]; then\n\t\tread -r count _ < \"$ATTEMPTS\" 2>/dev/null || count=0\n\tfi\n\tcase \"${count:-}\" in\n\t\t'' | *[!0-9]*) count=0 ;;\n\tesac\n\n\tHOST=\"$(pick_host)\"\n\tversion=\"$(version_of \"$HOST\")\"\n\n\tif [ \"$count\" -ge \"$LIMIT\" ]; then\n\t\tif roll_back \"$version\" \"it failed $count times in a row\"; then\n\t\t\t# Fresh count for the version now run: it deserves its own attempts, and without this\n\t\t\t# it inherits a count already over the limit and halts at once. The variable too, not\n\t\t\t# only the file: resetting one and not the other made the next failure count from the\n\t\t\t# OLD value — so the rolled-back version got one attempt instead of three.\n\t\t\tcount=0\n\t\t\tprintf '%s\\n' \"$count\" > \"$ATTEMPTS\"\n\t\t\tcontinue\n\t\tfi\n\t\tkg=\"$(known_good)\"\n\t\tif [ -n \"$kg\" ] && { [ \"$version\" = \"$kg\" ] || [ -z \"$version\" ]; } && [ -s \"$ROLLED\" ]; then\n\t\t\t# Already gone back, and what it went back to fails as well: this is the machine and\n\t\t\t# not a binary. Rolling back again would flap between two versions for ever.\n\t\t\tsay \"the host failed $count times after a rollback. the version it went back to does\"\n\t\t\tsay \"not start either, so this is the machine and not the binary.\"\n\t\t\trecord \"${version:-placed-by-hand}\" \"\" halted \"the version rolled back to failed $count times as well\"\n\t\t\tprintf 'rolled back and still failing\\n' > \"$HALTED\"\n\t\t\texit 0\n\t\tfi\n\t\t# Nothing to go back to: no host has ever reported here, or the one that did was placed by\n\t\t# hand and is not delivered. An installation failure rather than an upgrade's — said, and\n\t\t# tried again slowly, never guessed at.\n\t\tsay \"the host failed $count times and there is no delivered known-good version to go back to.\"\n\t\tcount=0\n\t\tprintf '%s\\n' \"$count\" > \"$ATTEMPTS\"\n\tfi\n\n\tif [ ! -x \"$HOST\" ]; then\n\t\tsay \"no host to run: nothing delivered under $VERSIONS and $FALLBACK is not executable.\"\n\t\tprintf 'no host binary\\n' > \"$HALTED\"\n\t\texit 0\n\tfi\n\n\t# **On trial**: a delivered version that is not the known-good one, while a known-good one is\n\t# delivered to go back to. The trial starts when this version was first started, and survives\n\t# this launcher restarting.\n\ttrial=\n\tdeadline=\n\tkg=\"$(known_good)\"\n\tif [ -n \"$version\" ] && [ \"$version\" != \"$kg\" ] && delivered \"$kg\"; then\n\t\ttrial=yes\n\t\tstarted=\n\t\tif [ -s \"$TRIAL\" ]; then\n\t\t\tread -r on since _ < \"$TRIAL\" 2>/dev/null || on=\n\t\t\t[ \"${on:-}\" = \"$version\" ] && started=\"${since:-}\"\n\t\tfi\n\t\tcase \"$started\" in\n\t\t\t'' | *[!0-9]*) started=\"$(now)\"; printf '%s %s\\n' \"$version\" \"$started\" > \"$TRIAL\" ;;\n\t\tesac\n\t\tdeadline=$((started + BOUND))\n\t\tif [ \"$(now)\" -ge \"$deadline\" ]; then\n\t\t\troll_back \"$version\" \"it did not report within ${BOUND}s of starting\" && continue\n\t\tfi\n\telse\n\t\trm -f \"$TRIAL\"\n\tfi\n\n\trm -f \"$EXPIRED\"\n\tsay \"running $HOST${trial:+ (on trial until it reports)}\"\n\t\"$HOST\" run &\n\tchild=$!\n\twatcher=\n\tif [ -n \"$trial\" ]; then\n\t\ttrial_watch \"$child\" \"$version\" \"$deadline\" &\n\t\twatcher=$!\n\tfi\n\tstatus=0\n\twait \"$child\" || status=$?\n\tif [ -n \"$stopping\" ]; then\n\t\t# The signal interrupted the wait, not the host: it was told, and is finishing what it was\n\t\t# doing. Waited for, so the service manager does not kill it half way through an apply.\n\t\twait \"$child\" 2>/dev/null\n\tfi\n\tchild=\n\tif [ -n \"$watcher\" ]; then\n\t\tkill \"$watcher\" 2>/dev/null\n\t\twait \"$watcher\" 2>/dev/null\n\t\twatcher=\n\tfi\n\n\tif [ -n \"$stopping\" ]; then\n\t\texit 0\n\tfi\n\n\tif [ -n \"$trial\" ] && [ -s \"$EXPIRED\" ] && [ \"$(cat \"$EXPIRED\" 2>/dev/null)\" = \"$version\" ]; then\n\t\trm -f \"$EXPIRED\"\n\t\troll_back \"$version\" \"it did not report within ${BOUND}s of starting\"\n\t\tcount=0\n\t\tprintf '%s\\n' \"$count\" > \"$ATTEMPTS\"\n\t\tcontinue\n\tfi\n\n\t# A signal the host did not survive, and we are not shutting down: treat it as a crash.\n\tcase \"$status\" in\n\t\t0)\n\t\t\t# Exited cleanly. That is how the host stands aside for a new binary after an\n\t\t\t# upgrade (novox/hq ADR 0005) — so loop and run whatever is now on disk.\n\t\t\t#\n\t\t\t# Deliberately NOT counted, and this is the whole reason the counter is\n\t\t\t# incremented here rather than before the start: counting attempts meant a host\n\t\t\t# that upgraded itself three times rolled itself back, having worked perfectly\n\t\t\t# every time.\n\t\t\t#\n\t\t\t# **Unless it stood aside for nothing.** A host on trial that exits cleanly while the\n\t\t\t# same host is still the one to run has not stood aside for a successor: it has\n\t\t\t# stopped, and a host that stops at once would otherwise loop here for ever unseen.\n\t\t\tif [ -n \"$trial\" ] && [ \"$(pick_host)\" = \"$HOST\" ] && [ \"$(known_good)\" != \"$version\" ]; then\n\t\t\t\tsay \"host $version exited cleanly on trial with nothing newer to stand aside for\"\n\t\t\telse\n\t\t\t\t# A delivered successor of this launcher runs from here on, not from the next boot.\n\t\t\t\tif [ -n \"$SELF\" ] && [ \"$(cksum < \"$0\" 2>/dev/null || true)\" != \"$SELF\" ]; then\n\t\t\t\t\tsay \"the launcher was replaced; running the new one\"\n\t\t\t\t\texec \"$0\"\n\t\t\t\tfi\n\t\t\t\tsay \"the host exited cleanly; starting it again\"\n\t\t\t\tcontinue\n\t\t\tfi\n\t\t\t;;\n\tesac\n\n\tcount=$((count + 1))\n\tprintf '%s\\n' \"$count\" > \"$ATTEMPTS\"\n\n\tsay \"the host exited $status ($count consecutive); restarting in ${BACKOFF}s\"\n\t[ -n \"$ONCE\" ] && exit \"$status\"\n\tsleep \"$BACKOFF\"\ndone\n" + "content": "#!/bin/sh\n# Supervise the host: start it, watch it, and decide what to do when it stops.\n#\n# novox/hq ADR 0005. The init is asked for ONE thing — run this at boot — and everything else\n# lives here, in a script that can be tested. Whether to restart, how long to wait, when to give\n# up, when to roll back: all of it is policy, and policy in a unit file can only be read and\n# hoped for.\n#\n# **It is the witness of the host's own successor** (novox/hq to-be 45 §8, ADR 0227 rule 8). A\n# delivered host that is not the known-good one runs on trial: it must report its declaration\n# under its own build — which it marks by writing itself into known-good — within a bound. One\n# that crashes repeatedly, exits without standing aside for anything, or does not report in\n# bound is stopped, recorded in `rolled-back` with why, and the known-good one is run. The host\n# says every record on its reports, so the mesh raises it as a condition. One rollback per\n# version: a version in `rolled-back` is never started again; a newer delivery is.\n#\n# It does NOT exec the host. Exec would replace this process, and then only the init could\n# restart anything — which is the arrangement this exists to remove. The cost of staying is\n# signal handling, below.\n#\n# Everything a rollback does is one of: read a file, look at a directory, write a file. It shares\n# no code with the host and calls none of it: a binary that cannot start cannot be its own\n# recovery.\n#\n# POSIX sh. `set -e` is deliberately absent: this script's whole job is to inspect exit codes,\n# and -e would make it exit on the first one it is meant to handle.\nset -u\n\nSTATE_DIR=\"${MESH_HOST_STATE_DIR:-/var/lib/mesh-host}\"\nLIBEXEC=\"${MESH_HOST_LIBEXEC:-/usr/lib/nox-mesh-host}\"\n# The host that was placed by hand, used only when nothing has been delivered. The first host on a\n# machine always arrives this way; every one after it is delivered (novox/hq ADR 0141).\nFALLBACK=\"${MESH_HOST_BIN:-/usr/bin/nox-mesh-host}\"\nVERSIONS=\"$LIBEXEC/versions\"\nBINARY=\"nox-mesh-host\"\nLIMIT=\"${MESH_HOST_START_LIMIT:-3}\"\nBACKOFF=\"${MESH_HOST_BACKOFF:-5}\"\n# How long a host on trial has to report, in seconds: to-be 45 §8's ten minutes from the apply.\nBOUND=\"${MESH_HOST_TRIAL_BOUND:-600}\"\nTICK=\"${MESH_HOST_TRIAL_TICK:-5}\"\n# How long a host told to stop is waited for before it is killed, in seconds: past the 60 s the host itself waits for\n# the apply in flight to end and save (novox/hq issue 1585), so a stop never cuts a save off.\nGRACE=\"${MESH_HOST_STOP_GRACE:-75}\"\nONCE=\"${MESH_HOST_RUN_ONCE:-}\" # tests run one iteration; nothing else sets this\n\nATTEMPTS=\"$STATE_DIR/start-attempts\"\nHALTED=\"$STATE_DIR/halted\"\nPINNED=\"$STATE_DIR/rollback-pinned\"\nKNOWN_GOOD=\"$STATE_DIR/known-good\"\n# One line per verdict: from, to, when (seconds since the epoch), outcome, why — tab-separated, read\n# by the host (internal/upgrade) and said on its reports.\nROLLED=\"$STATE_DIR/rolled-back\"\nTRIAL=\"$STATE_DIR/trial\"\nEXPIRED=\"$STATE_DIR/trial-expired\"\n\nsay() { echo \"nox-mesh-host-launch: $*\" >&2; }\n\nnow() { date +%s; }\n\nknown_good() { tr -d '[:space:]' < \"$KNOWN_GOOD\" 2>/dev/null || true; }\n\ndelivered() { [ -n \"$1\" ] && [ -x \"$VERSIONS/$1/$BINARY\" ]; }\n\nrolled_back() { [ -s \"$ROLLED\" ] && cut -f1 \"$ROLLED\" 2>/dev/null | grep -qxF \"$1\"; }\n\n# The version a host path is: the directory it was delivered in, or nothing for the host placed by hand.\nversion_of() {\n\tcase \"$1\" in\n\t\t\"$VERSIONS\"/*/\"$BINARY\") v=\"${1#\"$VERSIONS\"/}\"; echo \"${v%/\"$BINARY\"}\" ;;\n\t\t*) echo \"\" ;;\n\tesac\n}\n\nrecord() { # from to outcome why\n\tprintf '%s\\t%s\\t%s\\t%s\\t%s\\n' \"$1\" \"$2\" \"$(now)\" \"$3\" \"$4\" >> \"$ROLLED\"\n}\n\n# Which host to run: the newest delivered one that was never rolled back — or, while a rollback's pin\n# stands, the version it pinned — or the one placed by hand when nothing has been delivered.\n#\n# **Asked every time round the loop, not once.** Standing aside for a successor is a clean exit, and\n# the next turn has to run what is on disk NOW — resolving this once would restart the same binary\n# for ever and the upgrade would never take.\n#\n# Newest by when it arrived, never by how its name sorts: a version string is whatever the source was\n# described as, and those do not sort — \"1.10\" orders before \"1.9\". Ordering by name would start an\n# older host and call it an upgrade.\n#\n# **A pin stands until something newer arrives.** A version delivered after the pin was written, and\n# never rolled back, is a new build the mesh asks for: the pin goes and it runs, on trial.\npick_host() {\n\tnewest=\n\t# A directory with no executable in it is not a version: an interrupted delivery leaves one, and\n\t# running \"the newest\" would then mean running nothing.\n\tfor candidate in $(ls -1t \"$VERSIONS\" 2>/dev/null || true); do\n\t\tdelivered \"$candidate\" || continue\n\t\trolled_back \"$candidate\" && continue\n\t\tnewest=\"$candidate\"\n\t\tbreak\n\tdone\n\tif [ -s \"$PINNED\" ]; then\n\t\tpinned=\"$(tr -d '[:space:]' < \"$PINNED\" 2>/dev/null || true)\"\n\t\tif [ -n \"$newest\" ] && [ \"$newest\" != \"$pinned\" ] && [ \"$VERSIONS/$newest\" -nt \"$PINNED\" ]; then\n\t\t\tsay \"host $newest arrived after the rollback to $pinned; running it\"\n\t\t\trm -f \"$PINNED\"\n\t\telif delivered \"$pinned\"; then\n\t\t\techo \"$VERSIONS/$pinned/$BINARY\"\n\t\t\treturn 0\n\t\telse\n\t\t\tsay \"the pinned version '$pinned' is not delivered; ignoring the pin\"\n\t\tfi\n\tfi\n\tif [ -n \"$newest\" ]; then\n\t\techo \"$VERSIONS/$newest/$BINARY\"\n\t\treturn 0\n\tfi\n\techo \"$FALLBACK\"\n}\n\n# Go back from `from` to the known-good version, once, and say so. False when there is nowhere to go.\nroll_back() { # from why\n\tkg=\"$(known_good)\"\n\tif [ -z \"$1\" ] || [ \"$1\" = \"$kg\" ] || ! delivered \"$kg\"; then\n\t\treturn 1\n\tfi\n\trecord \"$1\" \"$kg\" rolled-back \"$2\"\n\t# The pin is what stops the launcher starting a version newer than known-good that is not the one\n\t# rolled back; the record is what stops it starting this one again.\n\tprintf '%s\\n' \"$kg\" > \"$PINNED\"\n\trm -f \"$TRIAL\"\n\tsay \"rolled back from host $1 to $kg: $2\"\n\treturn 0\n}\n\n# Watch a host on trial: done when it writes itself into known-good, stopped when the bound passes.\n# A subshell beside the host, so the launcher's own wait is untouched.\ntrial_watch() { # pid version deadline\n\twhile kill -0 \"$1\" 2>/dev/null; do\n\t\t[ \"$(known_good)\" = \"$2\" ] && return 0\n\t\tif [ \"$(now)\" -ge \"$3\" ]; then\n\t\t\tprintf '%s\\n' \"$2\" > \"$EXPIRED\"\n\t\t\tsay \"host $2 did not report within ${BOUND}s of starting; stopping it\"\n\t\t\tkill -TERM \"$1\" 2>/dev/null\n\t\t\twaited=0\n\t\t\twhile kill -0 \"$1\" 2>/dev/null && [ \"$waited\" -lt \"$GRACE\" ]; do\n\t\t\t\tsleep 1\n\t\t\t\twaited=$((waited + 1))\n\t\t\tdone\n\t\t\tkill -KILL \"$1\" 2>/dev/null\n\t\t\treturn 0\n\t\tfi\n\t\tsleep \"$TICK\"\n\tdone\n}\n\nchild=\nwatcher=\nstopping=\n\n# The machine is shutting down. Pass it on and wait for the host to finish — a supervisor that\n# exits while its child is still running leaves the host to be killed rather than to stop, and\n# an apply interrupted that way is exactly the half-configured machine this project is about.\non_term() {\n\tstopping=yes\n\tif [ -n \"$child\" ]; then\n\t\tsay \"stopping: passing the signal to the host\"\n\t\tkill -TERM \"$child\" 2>/dev/null\n\tfi\n}\ntrap on_term TERM INT\n\nmkdir -p \"$STATE_DIR\"\n# What this launcher is, so a delivered successor of it is run at the next clean exit rather than at\n# the next boot.\nSELF=\"$(cksum < \"$0\" 2>/dev/null || true)\"\n\nwhile :; do\n\tif [ -n \"$stopping\" ]; then\n\t\texit 0\n\tfi\n\n\tif [ -e \"$HALTED\" ]; then\n\t\tsay \"halted: $(cat \"$HALTED\" 2>/dev/null || echo 'reason not recorded')\"\n\t\tsay \"not starting the host. this node needs a person.\"\n\t\texit 0\n\tfi\n\n\t# Consecutive failed starts, not starts. Cleared by the host itself when it reports, which is\n\t# the only evidence either this or known-good has.\n\t#\n\t# Read the FIRST FIELD, then insist it is a plain integer.\n\t#\n\t# Stripping whitespace instead concatenates, and that is not hypothetical: a counter\n\t# holding \"1 2\" became \"12\", past the limit, so a healthy node rolled itself back. An\n\t# unreadable counter must fail towards \"start normally\", never towards \"give up\".\n\tcount=0\n\tif [ -s \"$ATTEMPTS\" ]; then\n\t\tread -r count _ < \"$ATTEMPTS\" 2>/dev/null || count=0\n\tfi\n\tcase \"${count:-}\" in\n\t\t'' | *[!0-9]*) count=0 ;;\n\tesac\n\n\tHOST=\"$(pick_host)\"\n\tversion=\"$(version_of \"$HOST\")\"\n\n\tif [ \"$count\" -ge \"$LIMIT\" ]; then\n\t\tif roll_back \"$version\" \"it failed $count times in a row\"; then\n\t\t\t# Fresh count for the version now run: it deserves its own attempts, and without this\n\t\t\t# it inherits a count already over the limit and halts at once. The variable too, not\n\t\t\t# only the file: resetting one and not the other made the next failure count from the\n\t\t\t# OLD value — so the rolled-back version got one attempt instead of three.\n\t\t\tcount=0\n\t\t\tprintf '%s\\n' \"$count\" > \"$ATTEMPTS\"\n\t\t\tcontinue\n\t\tfi\n\t\tkg=\"$(known_good)\"\n\t\tif [ -n \"$kg\" ] && { [ \"$version\" = \"$kg\" ] || [ -z \"$version\" ]; } && [ -s \"$ROLLED\" ]; then\n\t\t\t# Already gone back, and what it went back to fails as well: this is the machine and\n\t\t\t# not a binary. Rolling back again would flap between two versions for ever.\n\t\t\tsay \"the host failed $count times after a rollback. the version it went back to does\"\n\t\t\tsay \"not start either, so this is the machine and not the binary.\"\n\t\t\trecord \"${version:-placed-by-hand}\" \"\" halted \"the version rolled back to failed $count times as well\"\n\t\t\tprintf 'rolled back and still failing\\n' > \"$HALTED\"\n\t\t\texit 0\n\t\tfi\n\t\t# Nothing to go back to: no host has ever reported here, or the one that did was placed by\n\t\t# hand and is not delivered. An installation failure rather than an upgrade's — said, and\n\t\t# tried again slowly, never guessed at.\n\t\tsay \"the host failed $count times and there is no delivered known-good version to go back to.\"\n\t\tcount=0\n\t\tprintf '%s\\n' \"$count\" > \"$ATTEMPTS\"\n\tfi\n\n\tif [ ! -x \"$HOST\" ]; then\n\t\tsay \"no host to run: nothing delivered under $VERSIONS and $FALLBACK is not executable.\"\n\t\tprintf 'no host binary\\n' > \"$HALTED\"\n\t\texit 0\n\tfi\n\n\t# **On trial**: a delivered version that is not the known-good one, while a known-good one is\n\t# delivered to go back to. The trial starts when this version was first started, and survives\n\t# this launcher restarting.\n\ttrial=\n\tdeadline=\n\tkg=\"$(known_good)\"\n\tif [ -n \"$version\" ] && [ \"$version\" != \"$kg\" ] && delivered \"$kg\"; then\n\t\ttrial=yes\n\t\tstarted=\n\t\tif [ -s \"$TRIAL\" ]; then\n\t\t\tread -r on since _ < \"$TRIAL\" 2>/dev/null || on=\n\t\t\t[ \"${on:-}\" = \"$version\" ] && started=\"${since:-}\"\n\t\tfi\n\t\tcase \"$started\" in\n\t\t\t'' | *[!0-9]*) started=\"$(now)\"; printf '%s %s\\n' \"$version\" \"$started\" > \"$TRIAL\" ;;\n\t\tesac\n\t\tdeadline=$((started + BOUND))\n\t\tif [ \"$(now)\" -ge \"$deadline\" ]; then\n\t\t\troll_back \"$version\" \"it did not report within ${BOUND}s of starting\" && continue\n\t\tfi\n\telse\n\t\trm -f \"$TRIAL\"\n\tfi\n\n\trm -f \"$EXPIRED\"\n\tsay \"running $HOST${trial:+ (on trial until it reports)}\"\n\t\"$HOST\" run &\n\tchild=$!\n\twatcher=\n\tif [ -n \"$trial\" ]; then\n\t\ttrial_watch \"$child\" \"$version\" \"$deadline\" &\n\t\twatcher=$!\n\tfi\n\tstatus=0\n\twait \"$child\" || status=$?\n\tif [ -n \"$stopping\" ]; then\n\t\t# The signal interrupted the wait, not the host: it was told, and is finishing what it was\n\t\t# doing. Waited for, so the service manager does not kill it half way through an apply.\n\t\twait \"$child\" 2>/dev/null\n\tfi\n\tchild=\n\tif [ -n \"$watcher\" ]; then\n\t\tkill \"$watcher\" 2>/dev/null\n\t\twait \"$watcher\" 2>/dev/null\n\t\twatcher=\n\tfi\n\n\tif [ -n \"$stopping\" ]; then\n\t\texit 0\n\tfi\n\n\tif [ -n \"$trial\" ] && [ -s \"$EXPIRED\" ] && [ \"$(cat \"$EXPIRED\" 2>/dev/null)\" = \"$version\" ]; then\n\t\trm -f \"$EXPIRED\"\n\t\troll_back \"$version\" \"it did not report within ${BOUND}s of starting\"\n\t\tcount=0\n\t\tprintf '%s\\n' \"$count\" > \"$ATTEMPTS\"\n\t\tcontinue\n\tfi\n\n\t# A signal the host did not survive, and we are not shutting down: treat it as a crash.\n\tcase \"$status\" in\n\t\t0)\n\t\t\t# Exited cleanly. That is how the host stands aside for a new binary after an\n\t\t\t# upgrade (novox/hq ADR 0005) — so loop and run whatever is now on disk.\n\t\t\t#\n\t\t\t# Deliberately NOT counted, and this is the whole reason the counter is\n\t\t\t# incremented here rather than before the start: counting attempts meant a host\n\t\t\t# that upgraded itself three times rolled itself back, having worked perfectly\n\t\t\t# every time.\n\t\t\t#\n\t\t\t# **Unless it stood aside for nothing.** A host on trial that exits cleanly while the\n\t\t\t# same host is still the one to run has not stood aside for a successor: it has\n\t\t\t# stopped, and a host that stops at once would otherwise loop here for ever unseen.\n\t\t\tif [ -n \"$trial\" ] && [ \"$(pick_host)\" = \"$HOST\" ] && [ \"$(known_good)\" != \"$version\" ]; then\n\t\t\t\tsay \"host $version exited cleanly on trial with nothing newer to stand aside for\"\n\t\t\telse\n\t\t\t\t# A delivered successor of this launcher runs from here on, not from the next boot.\n\t\t\t\tif [ -n \"$SELF\" ] && [ \"$(cksum < \"$0\" 2>/dev/null || true)\" != \"$SELF\" ]; then\n\t\t\t\t\tsay \"the launcher was replaced; running the new one\"\n\t\t\t\t\texec \"$0\"\n\t\t\t\tfi\n\t\t\t\tsay \"the host exited cleanly; starting it again\"\n\t\t\t\tcontinue\n\t\t\tfi\n\t\t\t;;\n\tesac\n\n\tcount=$((count + 1))\n\tprintf '%s\\n' \"$count\" > \"$ATTEMPTS\"\n\n\tsay \"the host exited $status ($count consecutive); restarting in ${BACKOFF}s\"\n\t[ -n \"$ONCE\" ] && exit \"$status\"\n\tsleep \"$BACKOFF\"\ndone\n" }, { "id": "next", diff --git a/packaging/nox-mesh-host-launch b/packaging/nox-mesh-host-launch index e75d59a..cf946a2 100755 --- a/packaging/nox-mesh-host-launch +++ b/packaging/nox-mesh-host-launch @@ -38,7 +38,9 @@ BACKOFF="${MESH_HOST_BACKOFF:-5}" # How long a host on trial has to report, in seconds: to-be 45 §8's ten minutes from the apply. BOUND="${MESH_HOST_TRIAL_BOUND:-600}" TICK="${MESH_HOST_TRIAL_TICK:-5}" -GRACE="${MESH_HOST_STOP_GRACE:-20}" +# How long a host told to stop is waited for before it is killed, in seconds: past the 60 s the host itself waits for +# the apply in flight to end and save (novox/hq issue 1585), so a stop never cuts a save off. +GRACE="${MESH_HOST_STOP_GRACE:-75}" ONCE="${MESH_HOST_RUN_ONCE:-}" # tests run one iteration; nothing else sets this ATTEMPTS="$STATE_DIR/start-attempts" diff --git a/packaging/nox-mesh-host.openrc b/packaging/nox-mesh-host.openrc index cc740f0..7e96b88 100644 --- a/packaging/nox-mesh-host.openrc +++ b/packaging/nox-mesh-host.openrc @@ -5,4 +5,6 @@ name="nox-mesh-host" command="/usr/lib/nox-mesh-host/launch" supervisor="supervise-daemon" +# The same stop bound as the systemd unit's TimeoutStopSec (issue 1585): TERM, 120 s, then KILL. +retry="TERM/120/KILL/5" depend() { need net; } diff --git a/packaging/nox-mesh-host.service b/packaging/nox-mesh-host.service index c9c6df1..05e97dc 100644 --- a/packaging/nox-mesh-host.service +++ b/packaging/nox-mesh-host.service @@ -12,6 +12,9 @@ Restart=always RestartSec=5s StateDirectory=mesh-host KillMode=mixed +# Past the launcher's own stop grace (75 s), which is past the 60 s the host waits for the apply in +# flight to end and save (novox/hq issue 1585): the service manager's kill is the last resort. +TimeoutStopSec=120 [Install] WantedBy=multi-user.target