From 6c7af5c63f884cc56c3173e5bfeb620cecd09496 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 7 Oct 2026 02:13:17 +0200 Subject: [PATCH] Run a verb from the controller's own running image, and refuse what it cannot run as a handover (hq issue 289) The witness moves a running build aside into a directory the controller's user cannot enter, then deletes it; a verb exec'd from os.Executable() in that window failed with permission denied. /proc/self/exe stays valid while the process lives. A verb that still cannot start, or arrives while the controller stops, is refused with link.ErrHandingOver and marked retry: handing-over. --- cmd/mesh-controller/push.go | 4 + cmd/mesh-controller/seatverbs.go | 20 ++- cmd/mesh-controller/selfexec.go | 46 +++++++ cmd/mesh-controller/selfexec_test.go | 177 +++++++++++++++++++++++++++ internal/link/calls.go | 14 ++- internal/link/calls_test.go | 22 ++++ 6 files changed, 278 insertions(+), 5 deletions(-) create mode 100644 cmd/mesh-controller/selfexec.go create mode 100644 cmd/mesh-controller/selfexec_test.go diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index e6e78e9..e2ba9a5 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -128,6 +128,10 @@ func serve(ctx context.Context) (err error) { stopActing() case <-ctx.Done(): } + // Stopping, whichever way: a verb arriving from here is refused as a handover, so its caller + // asks the controller after this one rather than have a command started and killed with this + // process (novox/hq issue 289). + handingOver.Store(true) }() defer func() { select { diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index b65dff4..de0acbb 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -740,12 +740,18 @@ func isWhyFlag(word string) bool { } // runVerb runs this binary with the given command line and gathers what it said. +// +// **This binary is the image this process runs, never the file at the path it started from** +// (novox/hq issue 289): the node-engine's witness moves a running build aside when it places the next +// one, into a directory only it may enter, and deletes it once the next is proved — while this process +// may still be serving. A verb run from the path then failed with "permission denied" for the seconds +// the old controller still answered. selfCommand runs what this process is running, wherever its file +// went; a verb it still cannot start is refused as a handover, which the caller asks again. func runVerb(ctx context.Context, argv []string) (verbAnswer, error) { - self, err := os.Executable() - if err != nil { - return verbAnswer{}, err + if handingOver.Load() { + return verbAnswer{}, fmt.Errorf("%w: this controller is stopping and runs no new command", link.ErrHandingOver) } - cmd := exec.CommandContext(ctx, self, argv...) + cmd := selfCommand(ctx, argv) // The same environment: the stores' credentials, the bus, the broker — everything a command run // from a shell in this container would have, because it is that. cmd.Env = os.Environ() @@ -773,6 +779,12 @@ func runVerb(ctx context.Context, argv []string) (verbAnswer, error) { var exit *exec.ExitError if runErr != nil && !errors.As(runErr, &exit) { // Not the command refusing — the command not running at all, which is this process's fault. + if errors.Is(runErr, os.ErrPermission) || errors.Is(runErr, os.ErrNotExist) { + // Its own image unreachable: a build replaced under a process that has not yet stopped. + // Refused as a handover, so the caller asks the controller that follows. + return answer, fmt.Errorf("%w: could not run %s from this controller's own build: %v", + link.ErrHandingOver, strings.Join(argv, " "), runErr) + } return answer, fmt.Errorf("could not run %s: %w", strings.Join(argv, " "), runErr) } return answer, nil diff --git a/cmd/mesh-controller/selfexec.go b/cmd/mesh-controller/selfexec.go new file mode 100644 index 0000000..3a83abd --- /dev/null +++ b/cmd/mesh-controller/selfexec.go @@ -0,0 +1,46 @@ +package main + +import ( + "context" + "os" + "os/exec" + "runtime" + "sync/atomic" +) + +// startedFrom is the path this process's executable had when it started: what a verb's command line +// is named in a process listing, and what is run where the image cannot be named otherwise. +var startedFrom, _ = os.Executable() + +// ownImage is how a process names the executable it is running, as long as it runs, wherever the file +// has gone since. On Linux the kernel keeps it: /proc/self/exe is the running image itself, not a path, +// so it is valid after the file is renamed into a directory this process may not enter, or deleted — +// which is what the node-engine's witness does to a build it replaces (novox/hq issue 289). Read in the +// child, it names the child's image, which until the exec is this process's. +// +// os.Executable reads the same link and returns the path it points at *now*: correct at start and +// wrong the moment the file moves, which is the fault. A variable so a test can name another. +var ownImage = func() string { + if runtime.GOOS == "linux" { + if _, err := os.Stat("/proc/self/exe"); err == nil { + return "/proc/self/exe" + } + } + return startedFrom +} + +// selfCommand is this binary run with a command line: the image this process runs, named in a process +// listing as the path it started from. +func selfCommand(ctx context.Context, argv []string) *exec.Cmd { + cmd := exec.CommandContext(ctx, ownImage(), argv...) + if startedFrom != "" { + cmd.Args[0] = startedFrom + } + return cmd +} + +// handingOver is set when the serving controller begins to stop — a signal from its supervisor, a lease +// lost. From then a verb that would run a command is refused as a handover rather than started and +// killed with this process: the caller asks again, and the controller after this one answers +// (novox/hq issue 289). +var handingOver atomic.Bool diff --git a/cmd/mesh-controller/selfexec_test.go b/cmd/mesh-controller/selfexec_test.go new file mode 100644 index 0000000..05d8102 --- /dev/null +++ b/cmd/mesh-controller/selfexec_test.go @@ -0,0 +1,177 @@ +package main + +import ( + "bufio" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/link" +) + +// The roles a copy of this test binary plays in TestAVerbRunsWhileItsBuildIsMovedOrDeleted. +const selfExecRole = "MESH_CONTROLLER_SELFEXEC_ROLE" + +// TestSelfExecServer is a controller standing in, when run as one: it waits until its build has been +// moved, then runs a verb as the seat runs one, and prints what came of it as JSON. +func TestSelfExecServer(t *testing.T) { + if os.Getenv(selfExecRole) != "server" { + t.Skip("run by TestAVerbRunsWhileItsBuildIsMovedOrDeleted") + } + line, _ := bufio.NewReader(os.Stdin).ReadString('\n') + if strings.TrimSpace(line) != "go" { + t.Fatalf("told %q", line) + } + if err := os.Setenv(selfExecRole, "verb"); err != nil { + t.Fatal(err) + } + answer, err := runVerb(t.Context(), []string{"-test.run", "^TestSelfExecVerb$", "-test.v"}) + said := map[string]any{"ok": answer.OK, "output": answer.Output} + if err != nil { + said["error"] = err.Error() + } + body, _ := json.Marshal(said) + fmt.Println("ANSWER " + string(body)) +} + +// TestSelfExecVerb is the verb, when run as one. +func TestSelfExecVerb(t *testing.T) { + if os.Getenv(selfExecRole) != "verb" { + t.Skip("run by TestSelfExecServer") + } + fmt.Println("the verb ran") +} + +// **A verb runs while the build it was started from is moved where its user may not go, or deleted** +// (novox/hq issue 289). The node-engine's witness moves a running controller's build into a directory +// only it may enter as it places the next, and deletes it once the next is proved; the controller +// still serving in between ran its verbs from the path and answered "permission denied". +// +// A copy of this test binary is the controller: started from one place, moved into a directory closed +// to everyone (or deleted), and only then asked to run a verb. +func TestAVerbRunsWhileItsBuildIsMovedOrDeleted(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("the image is named through /proc on Linux only") + } + self, err := os.Executable() + if err != nil { + t.Fatal(err) + } + for _, how := range []string{"moved into a closed directory", "deleted"} { + t.Run(how, func(t *testing.T) { + root := t.TempDir() + placed := filepath.Join(root, "mesh-controller", "mesh-controller") + if err := os.MkdirAll(filepath.Dir(placed), 0o755); err != nil { + t.Fatal(err) + } + copyFile(t, self, placed) + + server := exec.Command(placed, "-test.run", "^TestSelfExecServer$", "-test.v") + server.Env = append(os.Environ(), selfExecRole+"=server") + stdin, err := server.StdinPipe() + if err != nil { + t.Fatal(err) + } + stdout, err := server.StdoutPipe() + if err != nil { + t.Fatal(err) + } + server.Stderr = os.Stderr + if err := server.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = server.Process.Kill(); _ = server.Wait() }) + + // What the witness does to a running build, once the process runs. + switch how { + case "deleted": + if err := os.RemoveAll(filepath.Dir(placed)); err != nil { + t.Fatal(err) + } + default: + kept := filepath.Join(root, ".witness", "mesh-controller", "previous") + if err := os.MkdirAll(filepath.Dir(kept), 0o700); err != nil { + t.Fatal(err) + } + if err := os.Rename(filepath.Dir(placed), kept); err != nil { + t.Fatal(err) + } + if err := os.Chmod(filepath.Join(root, ".witness"), 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(filepath.Join(root, ".witness"), 0o700) }) + } + if _, err := io.WriteString(stdin, "go\n"); err != nil { + t.Fatal(err) + } + out, _ := io.ReadAll(stdout) + _ = server.Wait() + var said struct { + OK bool `json:"ok"` + Output string `json:"output"` + Error string `json:"error"` + } + found := false + for _, line := range strings.Split(string(out), "\n") { + if rest, ok := strings.CutPrefix(line, "ANSWER "); ok { + found = json.Unmarshal([]byte(rest), &said) == nil + } + } + if !found { + t.Fatalf("the controller standing in answered nothing:\n%s", out) + } + if said.Error != "" || !said.OK || !strings.Contains(said.Output, "the verb ran") { + t.Fatalf("the verb did not run from the controller's own image after its build was %s: %+v", how, said) + } + }) + } +} + +// A verb the controller cannot start from its own build is refused as a handover — marked so its +// caller asks again — never a permission error; and so is one arriving once the controller is stopping. +func TestAVerbThatCannotRunIsRefusedAsAHandover(t *testing.T) { + closed := filepath.Join(t.TempDir(), "closed") + if err := os.MkdirAll(closed, 0o700); err != nil { + t.Fatal(err) + } + was := ownImage + ownImage = func() string { return filepath.Join(closed, "gone", "mesh-controller") } + t.Cleanup(func() { ownImage = was }) + _, err := runVerb(t.Context(), []string{"status"}) + if !errors.Is(err, link.ErrHandingOver) { + t.Fatalf("a build that is not there was answered %v, not a handover", err) + } + + ownImage = was + handingOver.Store(true) + t.Cleanup(func() { handingOver.Store(false) }) + if _, err := runVerb(t.Context(), []string{"-test.run", "^$"}); !errors.Is(err, link.ErrHandingOver) { + t.Fatalf("a controller stopping ran a verb: %v", err) + } +} + +func copyFile(t *testing.T, from, to string) { + t.Helper() + in, err := os.Open(from) + if err != nil { + t.Fatal(err) + } + defer in.Close() + out, err := os.OpenFile(to, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o755) + if err != nil { + t.Fatal(err) + } + if _, err := io.Copy(out, in); err != nil { + t.Fatal(err) + } + if err := out.Close(); err != nil { + t.Fatal(err) + } +} diff --git a/internal/link/calls.go b/internal/link/calls.go index a122c70..588b9e7 100644 --- a/internal/link/calls.go +++ b/internal/link/calls.go @@ -29,6 +29,15 @@ import ( // then and with "still running as call " when it does not; and every call is kept here, with // what came of it, including an answer the bus refused, so `calls` can say it. +// ErrHandingOver is a call refused because the controller answering it is being replaced: stopping, or +// unable to run its own build because the node-engine has moved it aside (novox/hq issue 289). Nothing +// was done, and the controller after it answers the same call — so the answer carries +// `"retry": RetryHandingOver`, and a caller asks once more. +var ErrHandingOver = errors.New("the controller is handing over to the next one; nothing was done, ask again") + +// RetryHandingOver is the `retry` mark of an answer refused by ErrHandingOver. +const RetryHandingOver = "handing-over" + // AnswerWithin is how long a call runs before its caller is answered that it is still running. Well // inside the shortest wait of a caller the mesh ships (the console's thirty seconds) and the bus's // own window for an answer (broker.ResponseTTL), so the one answer a call has is never late for @@ -524,7 +533,10 @@ func (l *CallLog) serveCall(seat, verb string, args json.RawMessage, reply strin var body []byte result, err := handle(ctx, args) failed := err != nil - if err != nil { + if errors.Is(err, ErrHandingOver) { + // Marked as well as said, so a caller asks again without reading the words (issue 289). + body, _ = json.Marshal(map[string]any{"error": err.Error(), "retry": RetryHandingOver}) + } else if err != nil { body, _ = json.Marshal(map[string]any{"error": err.Error()}) } else if body, err = json.Marshal(map[string]any{"result": result}); err != nil { failed = true diff --git a/internal/link/calls_test.go b/internal/link/calls_test.go index 1291dee..faa5d58 100644 --- a/internal/link/calls_test.go +++ b/internal/link/calls_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "log" "strings" "sync" @@ -177,3 +178,24 @@ func recentOf(l *CallLog) []Call { out, _ := l.Recent() return out } + +// A call refused because its controller is handing over says so in a mark as well as in words, so the +// caller asks once more without parsing a sentence (novox/hq issue 289). +func TestAHandoverRefusalIsMarkedRetryable(t *testing.T) { + l, a := NewCallLog(), newAnswers(t) + l.serveCall("mesh-controller", "rotate", nil, "_INBOX.x.9", func(context.Context, json.RawMessage) (any, error) { + return nil, fmt.Errorf("%w: stopping", ErrHandingOver) + }, a.respond, nil) + got := a.only() + if got["retry"] != RetryHandingOver || !strings.Contains(fmt.Sprint(got["error"]), "handing over") { + t.Fatalf("answered %v", got) + } + + l, a = NewCallLog(), newAnswers(t) + l.serveCall("mesh-controller", "rotate", nil, "_INBOX.x.10", func(context.Context, json.RawMessage) (any, error) { + return nil, errors.New("refused for its own reason") + }, a.respond, nil) + if _, marked := a.only()["retry"]; marked { + t.Fatal("an ordinary refusal was marked to be asked again") + } +}