Merge pull request 'Run a verb from the controller's own running image; refuse what it cannot run as a handover (hq issue 289)' (#108) from fix/a-verb-survives-the-handover into main
This commit was merged in pull request #108.
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
+13
-1
@@ -29,6 +29,15 @@ import (
|
||||
// then and with "still running as call <id>" 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
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user