From 65cd7b47873aec492e9b7cf29549f627a5bcc400 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 15:01:55 +0100 Subject: [PATCH] Retire a daemon's running build instead of deleting it before the restart (issue 734) A process no witness judges had its directory removed before the new bundle was unpacked and the unit restarted, so the old process ran on in a deleted working directory: mesh-build-go's `go version` probe failed with "getwd: no such file or directory" and refused three checks on ace on 2026-10-26, each in the second a new build of it arrived. Every process is now replaced the way a judged one is: its build is renamed under .witness//retired/ and deleted only once nothing runs from it. "Runs from it" now counts a process's working directory as well as its image, so the children a daemon starts keep the directory too. Forget keeps a retired build that still drains, and the witness's sweep covers every process it keeps a directory for, judged or not. Two unit-file tests are folded into one, so the unit count goes down by one. --- internal/apply/process.go | 7 ++- internal/apply/process_test.go | 29 +++------- internal/apply/process_witness_test.go | 78 ++++++++++++++++++++++++++ internal/witness/draining.go | 55 +++++++++++++----- internal/witness/kept.go | 22 +++++++- 5 files changed, 155 insertions(+), 36 deletions(-) diff --git a/internal/apply/process.go b/internal/apply/process.go index cbf4e06..78e204d 100644 --- a/internal/apply/process.go +++ b/internal/apply/process.go @@ -121,7 +121,12 @@ func applyProcess(ctx context.Context, r *declaration.Process, run Runner, } // Replaced rather than merged: the bundle is the whole of what it runs, and files left from a // previous version would be loaded by a runtime that walks a directory. - if err := os.RemoveAll(at); err != nil { + // **Retired, never deleted under its process** (novox/hq issue 734): the old process runs until + // the restart below, from this directory and in it. Deleted here, every command it started + // before the restart failed with "getwd: no such file or directory" — a builder's toolchain probe + // refused three checks on 2026-10-26. Renamed aside, its process keeps a directory that exists, + // and the build is deleted once nothing runs from it, as a judged build's is. + if err := witness.Retire(daemonRoot, r.Name, at, time.Now()); err != nil { return out, err } } else { diff --git a/internal/apply/process_test.go b/internal/apply/process_test.go index 20b1cec..6c33d10 100644 --- a/internal/apply/process_test.go +++ b/internal/apply/process_test.go @@ -21,10 +21,16 @@ func aProcess() *declaration.Process { } } -// The unit the mesh writes says what it runs, where, and that it comes back. +// The unit the mesh writes says what it runs, where, and that it comes back — and that it is the +// mesh's: **generated whole and saying so**, because an edit that survives until the next declaration +// and then vanishes is worse than one that is refused. The long-running mode stays up and is not a step. func TestTheUnitRunsWhatTheDaemonSaid(t *testing.T) { unit := unitFor(aProcess()) + if !strings.HasPrefix(unit, "#") { + t.Fatalf("the unit does not open saying it is generated:\n%s", unit) + } for _, want := range []string{ + "Do not edit", "ExecStart=node index.js", "WorkingDirectory=/var/lib/mesh/daemons/greeter", "Restart=always", @@ -34,14 +40,8 @@ func TestTheUnitRunsWhatTheDaemonSaid(t *testing.T) { t.Fatalf("the unit does not say %q:\n%s", want, unit) } } -} - -// **Generated whole and saying so.** Every managed file on a machine carries this, because an edit -// that survives until the next declaration and then vanishes is worse than one that is refused. -func TestTheUnitSaysItIsTheMeshs(t *testing.T) { - unit := unitFor(aProcess()) - if !strings.HasPrefix(unit, "#") || !strings.Contains(unit, "Do not edit") { - t.Fatalf("the unit does not say it is generated:\n%s", unit) + if strings.Contains(unit, "Type=oneshot") { + t.Fatalf("a process that should stay up is declared a step:\n%s", unit) } } @@ -124,17 +124,6 @@ func TestACronBecomesATimersCalendar(t *testing.T) { } } -// And the long-running mode is unchanged by any of it: it stays up and comes back. -func TestAProcessThatStaysUpIsStillRestartedWhenItExits(t *testing.T) { - unit := unitFor(aProcess()) - if !strings.Contains(unit, "Restart=always") { - t.Fatalf("a process that should stay up is not restarted when it exits:\n%s", unit) - } - if strings.Contains(unit, "Type=oneshot") { - t.Fatalf("a process that should stay up is declared a step:\n%s", unit) - } -} - // **A unit file reinterprets a value in three ways nothing else does**, and a module's environment // routinely contains all three — a generated password is arbitrary bytes. func TestAnEnvironmentValueMeansWhatTheDeclarationSaid(t *testing.T) { diff --git a/internal/apply/process_witness_test.go b/internal/apply/process_witness_test.go index 356a39f..fa51ff7 100644 --- a/internal/apply/process_witness_test.go +++ b/internal/apply/process_witness_test.go @@ -3,7 +3,9 @@ package apply import ( "context" "os" + "os/exec" "path/filepath" + "strconv" "strings" "testing" "time" @@ -103,3 +105,79 @@ func TestARolledBackBuildIsNotPlacedAgain(t *testing.T) { t.Fatalf("the apply does not say why it kept the build before: %+v", o) } } + +// **A process nobody judges keeps the directory it works in until its restart** (novox/hq issue 734). +// On 2026-10-26 three checks on one builder were refused in the second a new build of it arrived: the +// applier deleted the running build's directory before the restart, and the `go version` the old +// process started in it failed with "getwd: no such file or directory". Here a process works in the +// build's directory without running from it, as that `go version` did: the apply of a new build leaves +// its directory in place, and the retired build goes once nothing works in it. +func TestAProcessNobodyJudgesKeepsItsDirectoryUntilItsRestart(t *testing.T) { + if _, err := os.Stat("/proc/self/cwd"); err != nil { + t.Skip("which process works in a build is read from /proc") + } + units, bundles := t.TempDir(), t.TempDir() + wasUnits, wasBundles := unitDir, daemonRoot + unitDir, daemonRoot = units, bundles + t.Cleanup(func() { unitDir, daemonRoot = wasUnits, wasBundles }) + run := func(ctx context.Context, name string, args ...string) (string, error) { return "", nil } + + oldBody, oldDigest := anArchive(t, map[string]string{"mesh-build-go": "old\n"}) + newBody, newDigest := anArchive(t, map[string]string{"mesh-build-go": "new\n"}) + builder := func(body []byte, digest string) string { + return `{"id":"mesh-build-go.builder","type":"process","name":"mesh-build-go","source":"` + serving(t, body) + + `","digest":"` + digest + `","run":["./mesh-build-go"]}` + } + _, state, err := Apply(context.Background(), archHost(t), declare(t, builder(oldBody, oldDigest)), store.State{}, + store.OriginDeclared, run, nil, nil) + if err != nil { + t.Fatal(err) + } + + sleep, err := exec.LookPath("sleep") + if err != nil { + t.Skip("no sleep to run") + } + working := exec.Command(sleep, "60") + working.Dir = filepath.Join(bundles, "mesh-build-go") + if err := working.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = working.Process.Kill(); _ = working.Wait() }) + cwd := filepath.Join("/proc", strconv.Itoa(working.Process.Pid), "cwd") + for deadline := time.Now().Add(5 * time.Second); ; { + if at, err := os.Readlink(cwd); err == nil && at == working.Dir { + break + } + if time.Now().After(deadline) { + t.Fatalf("PID %d does not work in %s within 5s", working.Process.Pid, working.Dir) + } + time.Sleep(10 * time.Millisecond) + } + + if _, _, err := Apply(context.Background(), archHost(t), declare(t, builder(newBody, newDigest)), state, + store.OriginDeclared, run, nil, nil); err != nil { + t.Fatal(err) + } + if got, _ := os.ReadFile(filepath.Join(bundles, "mesh-build-go", "mesh-build-go")); string(got) != "new\n" { + t.Fatalf("the new build is not where the unit runs it from: %q", got) + } + at, err := os.Readlink(cwd) + if err != nil || strings.HasSuffix(at, " (deleted)") { + t.Fatalf("the directory a running process works in was deleted before its restart: %q (%v)", at, err) + } + if got, _ := os.ReadFile(filepath.Join(at, "mesh-build-go")); string(got) != "old\n" { + t.Fatalf("the process's directory no longer holds its own build: %q", got) + } + if len(witness.Trials(bundles)) != 0 { + t.Fatal("a process nobody judges went on trial") + } + + // Its process gone, the next look deletes the retired build and keeps nothing about the process. + _ = working.Process.Kill() + _ = working.Wait() + witness.SweepAll(bundles) + if _, err := os.Stat(witness.Dir(bundles, "mesh-build-go")); !os.IsNotExist(err) { + t.Fatalf("a retired build nothing works in was kept: %v", err) + } +} diff --git a/internal/witness/draining.go b/internal/witness/draining.go index 744b6c5..38b8d5a 100644 --- a/internal/witness/draining.go +++ b/internal/witness/draining.go @@ -49,6 +49,13 @@ func keep(root, name string) error { return nil } +// Retire takes a process's build out of the way before another is unpacked where it was: renamed under +// retired/, and deleted at once if nothing runs from it, else at a later sweep. A process nobody judges +// is replaced through it as a judged one is (novox/hq issue 734): deleting the running build before the +// restart deleted the working directory of the process still running from it, and every command it +// started until the restart failed with "getwd: no such file or directory". +func Retire(root, name, dir string, now time.Time) error { return retire(root, name, dir, now) } + // retire takes a build that has no reader any more out of the way: renamed under retired/, and deleted // at once if nothing runs from it, else at a later sweep. A path that is not there is nothing to do. func retire(root, name, dir string, now time.Time) error { @@ -98,8 +105,11 @@ func Sweep(root, name string) ([]string, error) { return draining, nil } -// RunningFrom is every process whose executable is a file under dir — renamed there, or deleted from -// there since it started. A variable so a test can name the machine's processes. +// RunningFrom is every process whose executable is a file under dir, or whose working directory is in +// it — renamed there, or deleted from there since it started. A working directory counts as much as an +// image: a process started by the one that runs from a build (a `go version` a builder asks) works in +// the build's directory and runs nothing from it (novox/hq issue 734). A variable so a test can name the +// machine's processes. var RunningFrom = func(dir string) []int { return runningFromProc("/proc", dir) } @@ -116,14 +126,17 @@ func runningFromProc(proc, dir string) []int { if err != nil { continue } - exe, err := os.Readlink(filepath.Join(proc, e.Name(), "exe")) - if err != nil { - // Gone between the listing and the read, a kernel thread, or another user's process this - // engine may not read — it runs as root, so in practice the first two. - continue - } - if strings.HasPrefix(strings.TrimSuffix(exe, " (deleted)"), dir) { - pids = append(pids, pid) + for _, link := range []string{"exe", "cwd"} { + at, err := os.Readlink(filepath.Join(proc, e.Name(), link)) + if err != nil { + // Gone between the listing and the read, a kernel thread, or another user's process this + // engine may not read — it runs as root, so in practice the first two. + continue + } + if strings.HasPrefix(strings.TrimSuffix(at, " (deleted)")+string(filepath.Separator), dir) { + pids = append(pids, pid) + break + } } } sort.Ints(pids) @@ -140,14 +153,30 @@ func joinPIDs(pids []int) string { // SweepAll is Sweep for every process the engine keeps anything about: what the witness does on every // look, so a build retired while its process drained goes once the process has. +// +// Every process the engine keeps a directory for, not only those it judges: a process nobody judges keeps +// nothing but the builds it retired, and those drain the same way (novox/hq issue 734). func SweepAll(root string) []string { + entries, err := os.ReadDir(filepath.Join(root, ".witness")) + if err != nil { + return nil + } var draining []string - for _, s := range all(root) { - kept, err := Sweep(root, s.Process) + for _, e := range entries { + if !e.IsDir() { + continue + } + name := e.Name() + kept, err := Sweep(root, name) if err != nil { - draining = append(draining, fmt.Sprintf("%s: %v", s.Process, err)) + draining = append(draining, fmt.Sprintf("%s: %v", name, err)) } draining = append(draining, kept...) + // A process nobody judges whose last retired build has drained leaves nothing to keep. + if s, err := Load(root, name); err == nil && s.Process == "" && len(kept) == 0 { + _ = os.Remove(retiredDir(root, name)) + _ = os.Remove(Dir(root, name)) + } } return draining } diff --git a/internal/witness/kept.go b/internal/witness/kept.go index 58db6a3..5e3dcda 100644 --- a/internal/witness/kept.go +++ b/internal/witness/kept.go @@ -126,8 +126,26 @@ func Save(root string, s State) error { return os.Rename(tmp.Name(), statePath(root, s.Process)) } -// Forget is a witnessed process no longer declared: everything kept about it goes with it. -func Forget(root, name string) error { return os.RemoveAll(Dir(root, name)) } +// Forget is a process no longer witnessed, or no longer declared: everything kept about it goes with it, +// but for a build a process still runs from. That one is retired and deleted once nothing runs from it +// (draining.go): a process whose witness was taken away, or which never had one, is still running from +// whatever was retired for it until its unit restarts (novox/hq issue 734). +func Forget(root, name string) error { + dir := Dir(root, name) + if _, err := os.Lstat(dir); os.IsNotExist(err) { + return nil + } + if err := retire(root, name, previousDir(root, name), time.Now()); err != nil { + return err + } + if err := os.Remove(statePath(root, name)); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + if draining, err := Sweep(root, name); err != nil || len(draining) > 0 { + return err + } + return os.RemoveAll(dir) +} // Placing is what the applier is to do with a declared build of a witnessed process. type Placing struct { -- 2.54.0