diff --git a/checks/particulars/covered b/checks/particulars/covered new file mode 100644 index 00000000..9ded26c5 --- /dev/null +++ b/checks/particulars/covered @@ -0,0 +1,3 @@ +# The modules the check covers (novox/hq ADR 0507 §9, issue 1838), one a line; issue 1838 widens it. +pacman +kernel diff --git a/checks/particulars/go.mod b/checks/particulars/go.mod new file mode 100644 index 00000000..310d5ebe --- /dev/null +++ b/checks/particulars/go.mod @@ -0,0 +1,3 @@ +module git.novox.be/novox/mesh-catalog/checks/particulars + +go 1.22 diff --git a/checks/particulars/main.go b/checks/particulars/main.go new file mode 100644 index 00000000..5203f1f6 --- /dev/null +++ b/checks/particulars/main.go @@ -0,0 +1,159 @@ +// Command particulars keeps this mesh's node names out of the modules it covers (novox/hq ADR 0507 §9, +// issue 1838): a module is for any mesh, so no manifest, file, code string, identifier, comment, test name or +// fixture of a covered module names a node of this one. +// +// The names are in the file `particulars` at the catalogue's root, one a line, `#` comments — the one place +// they may be written. The modules covered are in `covered` at checks/particulars/covered, one a line. A name is matched +// whole and in any case; a hyphen, an underscore or a CamelCase boundary parts one, so a longer word holding a name is another word. +// The forge is not a node name: its host (git.novox.be) and the organisation path (novox/) are blanked +// before a line is read. +// +// go run . +// +// Exits 1 naming file:line and the word, 2 when a list cannot be read. +package main + +import ( + "bufio" + "bytes" + "fmt" + "io/fs" + "os" + "path/filepath" + "regexp" + "sort" + "strings" +) + +// forge is the forge's host and organisation path, blanked before a line is read; a path segment novox/ elsewhere +// (testdata/novox/x, /srv/novox/data) is not blanked. +var forge = regexp.MustCompile(`(?i)git\.novox\.be(?:/novox/[\w.-]+)?|(?:^|[^/\w.-])novox/[\w.-]+`) + +func main() { + root := "." + if len(os.Args) > 1 { + root = os.Args[1] + } + os.Exit(check(root)) +} + +// check runs the check over a catalogue and returns the exit code: 0 clean, 1 a finding, 2 a list unreadable. +func check(root string) int { + names, err := readList(filepath.Join(root, "particulars")) + if err == nil && len(names) == 0 { + err = fmt.Errorf("particulars lists no name — the check would pass on anything") + } + var covered []string + if err == nil { + covered, err = readList(filepath.Join(root, "checks/particulars/covered")) + if err == nil && len(covered) == 0 { + err = fmt.Errorf("covered lists no module — the check would pass on anything") + } + } + if err != nil { + fmt.Fprintln(os.Stderr, "particulars:", err) + return 2 + } + findings, err := run(root, names, covered) + if err != nil { + fmt.Fprintln(os.Stderr, "particulars:", err) + return 2 + } + for _, f := range findings { + fmt.Println(f) + } + if len(findings) > 0 { + fmt.Printf("particulars: %d node name(s) in a module for any mesh (novox/hq ADR 0507 §9) — say what it shows, not where it was seen\n", len(findings)) + return 1 + } + fmt.Printf("particulars: none of %d node names in the %d covered modules\n", len(names), len(covered)) + return 0 +} + +// readList reads a list: one entry a line; blank lines and lines starting with # are not entries. +func readList(path string) ([]string, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + var out []string + sc := bufio.NewScanner(f) + for sc.Scan() { + line := strings.TrimSpace(sc.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + out = append(out, line) + } + return out, sc.Err() +} + +// run reads every text file of each covered module and returns the findings, sorted. +func run(root string, names, covered []string) ([]string, error) { + var out []string + for _, m := range covered { + dir := filepath.Join(root, "modules", m) + err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + switch d.Name() { + case "node_modules", "dist", "vendor", ".git": + return filepath.SkipDir + } + return nil + } + rel, _ := filepath.Rel(root, path) + // A path is read too: a testdata directory named after a node is a finding. + for _, n := range hits(filepath.ToSlash(rel), names) { + out = append(out, fmt.Sprintf("%s:0: node name %q in the path", filepath.ToSlash(rel), n)) + } + if d.Name() == "go.sum" { + return nil + } + b, err := os.ReadFile(path) + if err != nil { + return err + } + if bytes.IndexByte(b, 0) >= 0 { + return nil + } + out = append(out, scan(filepath.ToSlash(rel), string(b), names)...) + return nil + }) + if err != nil { + return nil, err + } + } + sort.Strings(out) + return out, nil +} + +// scan returns a finding for each node name in src, whole-word and in any case. +func scan(file, src string, names []string) []string { + var out []string + for i, line := range strings.Split(src, "\n") { + for _, n := range hits(line, names) { + out = append(out, fmt.Sprintf("%s:%d: node name %q — %s", file, i+1, n, strings.Join(strings.Fields(line), " "))) + } + } + return out +} + +// hits returns the node names one line holds: as a word (a hyphen or underscore parts it), as a part of a +// CamelCase identifier (TestAce, novoxTree), the forge's host and organisation path blanked first. +func hits(line string, names []string) []string { + line = forge.ReplaceAllStringFunc(line, func(s string) string { return strings.Repeat(" ", len(s)) }) + var out []string + for _, n := range names { + q := regexp.QuoteMeta(n) + camel := regexp.QuoteMeta(strings.ToUpper(n[:1]) + n[1:]) + rx := regexp.MustCompile(`(?i:(?:^|[^A-Za-z0-9])` + q + `(?:$|[^A-Za-z0-9]))|[a-z0-9]` + camel + `(?:$|[^a-z])|(?:^|[^A-Za-z0-9])` + q + `[A-Z0-9]`) + if rx.MatchString(line) { + out = append(out, strings.ToLower(n)) + } + } + return out +} diff --git a/checks/particulars/main_test.go b/checks/particulars/main_test.go new file mode 100644 index 00000000..9522c00f --- /dev/null +++ b/checks/particulars/main_test.go @@ -0,0 +1,116 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +var names = []string{"novox", "ace", "shanks", "g14"} + +func TestANodeNameIsRefusedWhereItIsWritten(t *testing.T) { + cases := map[string]struct{ file, src string }{ + "manifest string": {"modules/kernel/module.json", "{\n \"description\": \"as on Shanks\"\n}\n"}, + "file line": {"modules/kernel/files/boot.sh", "#!/bin/sh\necho run on g14\n"}, + "go identifier": {"modules/kernel/cmd/x/a.go", "package main\n\nfunc novox() {}\n"}, + "test name": {"modules/kernel/cmd/x/a_test.go", "package main\n\nfunc TestAce(t *testing.T) {}\n\nfunc TestOnAce_x(t *testing.T) {}\n"}, + } + for name, c := range cases { + if got := scan(c.file, c.src, names); len(got) == 0 { + t.Errorf("%s: %q not refused", name, c.src) + } + } +} + +func TestTheFindingNamesTheLineAndTheWord(t *testing.T) { + got := scan("f.sh", "a\nb\nrun on ACE now\n", names) + if len(got) != 1 || !strings.HasPrefix(got[0], "f.sh:3:") || !strings.Contains(got[0], "ace") { + t.Errorf("got %v", got) + } +} + +func TestTheForgeIsNotANodeName(t *testing.T) { + for _, s := range []string{ + "see novox/hq issue 5", + "module git.novox.be/novox/mesh-sdk/go v1.2.3", + "https://git.novox.be/novox/mesh-catalog", + "git.novox.be", + "space trace surface grace g14x", + } { + if got := scan("f", s, names); len(got) != 0 { + t.Errorf("%q refused: %v", s, got) + } + } +} + +func TestANodeNameInAPathOrJoinedIsRefused(t *testing.T) { + for _, s := range []string{ + "modules/kernel/testdata/novox/meminfo", + "/srv/novox/data", + "testdata/shanks-meminfo.txt", + "on-shanks", + "shanks-desktop", + "mesh-novox", + "func novoxTree() {}", + "aceFiles := 1", + "g14Meminfo := 2", + "novox_tree := 3", + } { + if got := scan("f", s, names); len(got) == 0 { + t.Errorf("%q not refused", s) + } + } +} + +func TestANodeNamedPathIsRefusedThroughRun(t *testing.T) { + root := t.TempDir() + p := filepath.Join(root, "modules/kernel/testdata/ace/x") + os.MkdirAll(filepath.Dir(p), 0o755) + os.WriteFile(p, []byte("clean\n"), 0o644) + got, err := run(root, names, []string{"kernel"}) + if err != nil || len(got) != 1 || !strings.HasPrefix(got[0], "modules/kernel/testdata/ace/x:0: ") { + t.Errorf("got %v, %v", got, err) + } +} + +func TestCheckReturnsOneOnAFindingAndTwoOnAnUnreadableList(t *testing.T) { + root := t.TempDir() + write := func(p, s string) { + p = filepath.Join(root, p) + os.MkdirAll(filepath.Dir(p), 0o755) + os.WriteFile(p, []byte(s), 0o644) + } + write("particulars", "ace\n") + write("checks/particulars/covered", "kernel\n") + write("modules/kernel/a", "clean\n") + if got := check(root); got != 0 { + t.Errorf("clean = %d, want 0", got) + } + write("modules/kernel/a", "on ace\n") + if got := check(root); got != 1 { + t.Errorf("finding = %d, want 1", got) + } + os.Remove(filepath.Join(root, "particulars")) + if got := check(root); got != 2 { + t.Errorf("no list = %d, want 2", got) + } +} + +func TestOnlyCoveredModulesAreRead(t *testing.T) { + root := t.TempDir() + write := func(p, s string) { + p = filepath.Join(root, p) + os.MkdirAll(filepath.Dir(p), 0o755) + os.WriteFile(p, []byte(s), 0o644) + } + write("modules/kernel/files/a", "on novox\n") + write("modules/other/files/a", "on novox\n") + got, err := run(root, names, []string{"kernel"}) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 || !strings.Contains(got[0], "modules/kernel/files/a:1") { + t.Errorf("got %v", got) + } +} diff --git a/merge-check.sh b/merge-check.sh index fb1d9b8b..3fe55bd1 100755 --- a/merge-check.sh +++ b/merge-check.sh @@ -55,6 +55,9 @@ set -eu (cd checks/words && go test -count=1 . && go run . ../..) +# Node names (novox/hq ADR 0507 §9, issue 1838): no node name of this mesh in a covered module; the names are in +# `particulars`, the covered modules in checks/particulars/covered. +(cd checks/particulars && go test -count=1 . && go run . ../..) # The toolchains (novox/hq ADR 0477 clause 4): the package go only in modules/go, nodejs and npm only in modules/nodejs. (cd checks/toolchains && go test -count=1 . && go run . ../..) # The image builder (novox/hq ADR 0487 §3): only modules/buildah provides rootless-image-build, docker never. diff --git a/modules/mesh-build-go/README.md b/modules/mesh-build-go/README.md index 9874d55c..49d164c3 100644 --- a/modules/mesh-build-go/README.md +++ b/modules/mesh-build-go/README.md @@ -40,6 +40,12 @@ with `GOTOOLCHAIN=local`, `GOFLAGS=-mod=mod` and the module's cache directory. E environment built from scratch (`StepRunner`): `PATH`, `HOME` and what the step passes. Workspaces older than a day are removed. +A pin on a pull request's head (issue 1148): when `go mod download` fails on a `git.novox.be/novox/*` pseudo-version, the +fetch looks for that commit among the forge's `refs/pull/*/head`, places the repository at it under the workspace's +`pinned/beside/` as a plain directory, and hands it to Go (the fetch's commands, and the build with `GOPROXY=off`) through a +temporary `-modfile` in the workspace's `pins/` that replaces the module by that directory. A pin Go can fetch is left as +it is, a pin on no ref fails naming the module and the commit, and a directory that vendors is never resolved. + The account's ssh files (`files/forge`, `files/known_hosts`, `files/gitconfig`) are filled on the node by the provisioner's client from the module's entry of `forge-account` (the manifest says `moved`, ADR 0466 §4, ADR 0400 §5). The forge writes that entry only once the module has run and reported its key (ADR 0400 §2), so until then the client diff --git a/modules/mesh-build-go/cmd/mesh-build-go/build.go b/modules/mesh-build-go/cmd/mesh-build-go/build.go index f6839629..0ed375c3 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/build.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/build.go @@ -172,6 +172,8 @@ func (m *Module) buildEnv(dir string) []string { mod := "-mod=mod" if _, err := os.Stat(filepath.Join(dir, "vendor", "modules.txt")); err == nil { mod = "-mod=vendor" + } else if modfile, ok := m.pinModfile(dir); ok { + mod += " -modfile=" + modfile // a pin on a pull request's head the fetch placed beside the workspace (pins.go) } return []string{ "CGO_ENABLED=0", diff --git a/modules/mesh-build-go/cmd/mesh-build-go/fetch.go b/modules/mesh-build-go/cmd/mesh-build-go/fetch.go index a6f1ef7c..c273c230 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/fetch.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/fetch.go @@ -127,16 +127,47 @@ func (m *Module) fetch(ctx context.Context, args map[string]any) (any, error) { if _, err := os.Stat(filepath.Join(tree, d, "vendor", "modules.txt")); err == nil { continue // a module that vendors resolves nothing } - for _, verb := range []string{"download", "verify"} { - r := m.Run(ctx, m.goEnv(), "go", "-C", filepath.Join(tree, d), "mod", verb) - if r.Status != 0 || r.Err != "" { - return nil, fmt.Errorf("go mod %s in %s: %s", verb, d, causeLine(r.Err+"\n"+r.Stdout+"\n"+r.Stderr)) - } + if err := m.downloadModules(ctx, root, filepath.Join(tree, d), d); err != nil { + return nil, err } } return res, nil } +// downloadModules downloads and verifies the Go modules of the directory dir (d in the tree). A pin of a forge module +// that Go cannot fetch is looked for among the forge's pull request heads and handed to Go by a modfile (pins.go); a +// failure that names no such pin is the first failure. +func (m *Module) downloadModules(ctx context.Context, root, dir, d string) error { + for { + failure := m.downloadAndVerify(ctx, dir, d) + if failure == nil { + return nil + } + progress, err := m.resolvePins(ctx, root, dir, failure.Error()) + if err != nil { + return err + } + if !progress { + return failure + } + } +} + +func (m *Module) downloadAndVerify(ctx context.Context, dir, d string) error { + for _, verb := range []string{"download", "verify"} { + cmd, env := []string{"-C", dir, "mod", verb}, m.goEnvFor(dir) + if modfile, ok := m.pinModfile(dir); ok && verb == "verify" { + // go mod verify takes no -modfile: it is run on the modfile itself, which names the placed repositories. + cmd, env = []string{"-C", filepath.Dir(modfile), "mod", verb}, m.goEnv() + } + r := m.Run(ctx, env, "go", cmd...) + if r.Status != 0 || r.Err != "" { + return fmt.Errorf("go mod %s in %s: %s", verb, d, causeLine(r.Err+"\n"+r.Stdout+"\n"+r.Stderr)) + } + } + return nil +} + // goModDirs are the directories of the tree that hold a go.mod, relative to it, in order. func (m *Module) goModDirs(ctx context.Context, tree string) ([]string, error) { r := m.Run(ctx, nil, "git", "-C", tree, "ls-files", "--", "go.mod", "*/go.mod") @@ -186,6 +217,8 @@ func (m *Module) goEnvFor(dir string) []string { env := m.goEnv() if _, err := os.Stat(filepath.Join(dir, "vendor", "modules.txt")); err == nil { env = append(env, "GOFLAGS=-mod=vendor") // the last value of a name is the one used + } else if modfile, ok := m.pinModfile(dir); ok { + env = append(env, "GOFLAGS=-mod=mod -modfile="+modfile) // a pin on a pull request's head (pins.go) } return env } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/pins.go b/modules/mesh-build-go/cmd/mesh-build-go/pins.go new file mode 100644 index 00000000..4247d447 --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/pins.go @@ -0,0 +1,175 @@ +package main + +// Pins on a pull request's head (novox/hq ADR 0482 §2, ADR 0500 §2; issue 1148): a group's Go members pin the head of +// the group's mesh-sdk pull request, a commit that only refs/pull//head of the forge carries until the merge. Go +// cannot fetch it by its pseudo-version, so a fetch that meets "unknown revision" for such a pin finds the commit among +// the forge's pull request heads, places that repository beside the workspace as a plain directory, and hands it to +// Go through a temporary -modfile, a copy of the directory's go.mod that replaces the module by that directory. The +// fetch's go commands and the build use the same modfile, and the build still runs with GOPROXY=off. A pin Go can +// fetch is not touched, and a directory that vendors is never resolved. + +import ( + "context" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" +) + +const forgePrefix = "git.novox.be/novox/" + +var pseudoVersion = regexp.MustCompile(`[-.][0-9]{14}-([0-9a-f]{12})$`) + +// pin is a requirement of a go.mod on a commit of a repository of the forge, by a pseudo-version. +type pin struct { + module string // git.novox.be/novox/[/] + repo string // + sub string // the module's directory in the repository, "" for its root + short string // the 12 digits of the commit the pseudo-version ends in +} + +// forgePins are the pseudo-version requirements of a go.mod on the forge's own modules. +func forgePins(gomod string) []pin { + var pins []pin + for _, line := range strings.Split(gomod, "\n") { + if i := strings.Index(line, "//"); i >= 0 { + line = line[:i] + } + f := strings.Fields(line) + if len(f) > 0 && f[0] == "require" { + f = f[1:] + } + if len(f) != 2 || !strings.HasPrefix(f[0], forgePrefix) { + continue + } + v := pseudoVersion.FindStringSubmatch(f[1]) + if v == nil { + continue + } + parts := strings.SplitN(strings.TrimPrefix(f[0], forgePrefix), "/", 2) + p := pin{module: f[0], repo: parts[0], short: v[1]} + if len(parts) == 2 { + p.sub = parts[1] + } + pins = append(pins, p) + } + return pins +} + +// pinsDir is where the temporary modfile of a module directory of a workspace lies: the workspace's pins directory, +// by the directory's path in the tree. dir is outside any workspace of the work directory, and the answer empty, when +// it does not lie in //tree. +func (m *Module) pinsDir(dir string) string { + rel, err := filepath.Rel(m.Work, dir) + if err != nil { + return "" + } + parts := strings.Split(filepath.ToSlash(rel), "/") + if len(parts) < 2 || parts[0] == ".." || parts[1] != "tree" { + return "" + } + name := strings.Join(parts[2:], "_") + if name == "" { + name = "root" + } + return filepath.Join(m.Work, parts[0], "pins", name) +} + +// pinModfile is the modfile handed to Go for dir, and whether there is one: only a directory a fetch resolved a pin +// for has it. +func (m *Module) pinModfile(dir string) (string, bool) { + p := m.pinsDir(dir) + if p == "" { + return "", false + } + p = filepath.Join(p, "go.mod") + _, err := os.Stat(p) + return p, err == nil +} + +// resolvePins resolves the pins of dir that a go command failed on: those whose commit the failure names. It answers +// false when there is none, and the failure is then the caller's. +func (m *Module) resolvePins(ctx context.Context, root, dir, failure string) (bool, error) { + raw, err := os.ReadFile(filepath.Join(dir, "go.mod")) + if err != nil { + return false, nil + } + modfile, _ := m.pinModfile(dir) + done := "" + if modfile != "" { + if b, err := os.ReadFile(modfile); err == nil { + done = string(b) + } + } + progress := false + for _, p := range forgePins(string(raw)) { + if !strings.Contains(failure, p.short) || strings.Contains(done, "replace "+p.module+" =>") { + continue + } + where, err := m.placePin(ctx, root, p, failure) + if err != nil { + return false, err + } + if err := m.replaceInModfile(dir, p.module, where); err != nil { + return false, err + } + progress = true + } + return progress, nil +} + +// placePin finds the commit of a pin among the pull request heads of the forge, places the repository at it beside +// the workspace (the way a fetch places a sibling) and answers the directory of the module in it. +func (m *Module) placePin(ctx context.Context, root string, p pin, failure string) (string, error) { + repository := "novox/" + p.repo + r := m.Run(ctx, []string{"GIT_TERMINAL_PROMPT=0"}, "git", "-c", "credential.helper=", "ls-remote", m.remote(repository), "refs/pull/*/head") + if r.Status != 0 || r.Err != "" { + return "", fmt.Errorf("the pin of %s at commit %s: listing the pull requests of %s: %s", p.module, p.short, repository, firstLine(r.Err+" "+r.Stderr)) + } + commit, ref := "", "" + for _, l := range strings.Split(r.Stdout, "\n") { + if sha, name, ok := strings.Cut(strings.TrimSpace(l), "\t"); ok && commitOK.MatchString(sha) && strings.HasPrefix(sha, p.short) && strings.HasSuffix(name, "/head") { + commit, ref = sha, name + break + } + } + if commit == "" { + return "", fmt.Errorf("the pin of %s at commit %s is on no pull request head of %s on the forge; go said: %s", p.module, p.short, repository, causeLine(failure)) + } + // One placement per repository and commit, shared by every go.mod directory of the tree that pins it. + at := filepath.Join(root, "pinned", p.repo+"@"+commit) + where := filepath.Join(at, "beside", p.repo, filepath.FromSlash(p.sub)) + if _, err := os.Stat(filepath.Join(at, "beside", p.repo)); err == nil { + return where, nil + } + if _, err := m.placeSiblings(ctx, at, []siblingArg{{repository: repository, commit: commit, ref: ref}}); err != nil { + return "", fmt.Errorf("the pin of %s at commit %s: %w", p.module, p.short, err) + } + return where, nil +} + +// replaceInModfile adds `replace module => where` to the temporary modfile of dir, made from the directory's go.mod and +// go.sum the first time; Go reads the sums beside the modfile. +func (m *Module) replaceInModfile(dir, module, where string) error { + pins := m.pinsDir(dir) + if pins == "" { + return fmt.Errorf("the directory %s lies outside a workspace", dir) + } + if err := os.MkdirAll(pins, 0o700); err != nil { + return err + } + modfile := filepath.Join(pins, "go.mod") + body, err := os.ReadFile(modfile) + if err != nil { + if body, err = os.ReadFile(filepath.Join(dir, "go.mod")); err != nil { + return err + } + sum, _ := os.ReadFile(filepath.Join(dir, "go.sum")) + if err := os.WriteFile(filepath.Join(pins, "go.sum"), sum, 0o600); err != nil { + return err + } + } + body = append(body, []byte("\nreplace "+module+" => "+where+"\n")...) + return os.WriteFile(modfile, body, 0o600) +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go b/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go new file mode 100644 index 00000000..d32cfe2b --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/pins_integration_test.go @@ -0,0 +1,59 @@ +//go:build integration + +package main + +// A pin on a pull request's head, on real git repositories and the real go (issue 1148); pins_test.go proves the +// commands with a fake runner. + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// 1148/AC2: given a forge with a mesh-sdk-like pull request whose branch is deleted (only refs/pull/7/head carries its +// commit) and a Go member pinning that head by a pseudo-version, when build_go_fetch then build_go_build run on the +// member, then both pass, the build with GOPROXY=off. +func TestRealFetchAndBuildOfAMemberPinningAPullHead(t *testing.T) { + // Not skipped when a tool is missing: a pass of the check must mean this test ran (the check prints no test log). + for _, tool := range []string{"go", "git"} { + if _, err := exec.LookPath(tool); err != nil { + t.Fatalf("no %s where this check runs: the pin on a pull request's head cannot be proven", tool) + } + } + f := newForge(t) + work, fork := f.repo("novox/dep", map[string]string{"README": "dep\n"}) + head := f.commit("novox/dep", work, fork, map[string]string{ + "go/go.mod": "module git.novox.be/novox/dep/go\n\ngo 1.25.0\n", + "go/dep.go": "package dep\n\n// Name is what the member prints.\nconst Name = \"from the pull request\"\n", + }, "refs/pull/7/head") + git(t, work, "checkout", "-q", "main") // no branch of the repository carries the head + member := map[string]string{ + "go.mod": "module fixture/member\n\ngo 1.25.0\n\nrequire git.novox.be/novox/dep/go v0.0.0-20261022120000-" + head[:12] + "\n", + "cmd/prog/main.go": "package main\n\nimport (\n\t\"fmt\"\n\n\tdep \"git.novox.be/novox/dep/go\"\n)\n\n" + + "func main() { fmt.Println(dep.Name) }\n", + "module.json": `{"module":"fixture-member","version":"1","build":{"artifacts":[{"name":"tools","kind":"bundle","language":"go","system":"arch","from":"cmd/prog","binary":"prog-bin","loads":["prog-bin"]}]}}`, + } + _, memberHead := f.repo("novox/member", member) + m, states := f.module(t) + + res, st := fetched(t, m, states, map[string]any{"repository": "novox/member", "commit": memberHead}) + if st.State != delivery.StepPassed { + t.Fatalf("the fetch ended %s: %s", st.State, st.Reason) + } + if _, err := os.Stat(filepath.Join(m.Work, res.Workspace, "pinned", "dep@"+head, "beside", "dep", "go", "dep.go")); err != nil { + t.Errorf("the pull request's repository was not placed beside the workspace: %v", err) + } + built, err := m.build(context.Background(), map[string]any{"workspace": res.Workspace, "module": ".", "artifact": "tools"}) + if err != nil { + t.Fatal(fmt.Errorf("the build: %w", err)) + } + if b := built.(BuildResult); b.Digest == "" || b.Binary != "prog-bin" { + t.Errorf("the build answered %+v", b) + } +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/pins_test.go b/modules/mesh-build-go/cmd/mesh-build-go/pins_test.go new file mode 100644 index 00000000..bbdc0ff7 --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/pins_test.go @@ -0,0 +1,246 @@ +package main + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +// pinShort is the 12 digits of commitSHA a pseudo-version ends in. +const pinShort = "0123456789ab" + +const pinnedGoMod = "module fixture/member\n\ngo 1.26\n\nrequire (\n\tgit.novox.be/novox/mesh-sdk/go v0.1.15-0.20261022120000-" + pinShort + "\n)\n" + +// pinHarness runs a fetch of a tree whose only go.mod is gomod (a vendoring one when vendor is set) against a runner +// that stands for the forge and for go: a pull request's head is listed by ls-remote when pulls says so, and go mod +// download of a pseudo-version that no ref carries fails with "unknown revision" unless the command was handed a +// -modfile. +type pinHarness struct { + t *testing.T + m *Module + f *fakeStep + pulls string // what git ls-remote answers + onTrunk bool // the pin is fetchable as it is: go mod download never says unknown revision + cmds []cmdRecord +} + +func newPinHarness(t *testing.T, gomod string, vendor bool, pulls string) (*pinHarness, *keptStates) { + f := &fakeStep{gomods: "go.mod\n"} + m, states := fetchModule(t, f) + h := &pinHarness{t: t, m: m, f: f, pulls: pulls} + m.Run = func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + line := name + " " + strings.Join(args, " ") + h.cmds = append(h.cmds, cmdRecord{env: env, line: line}) + switch { + case name == "git" && strings.Contains(line, " checkout ") && !strings.Contains(line, "pinned"): + for i, a := range args { + if a == "-C" { + os.MkdirAll(args[i+1], 0o755) + for _, g := range strings.Fields(f.gomods) { + d := filepath.Join(args[i+1], filepath.Dir(g)) + os.MkdirAll(d, 0o755) + os.WriteFile(filepath.Join(d, "go.mod"), []byte(gomod), 0o644) + os.WriteFile(filepath.Join(d, "go.sum"), []byte("sum\n"), 0o644) + } + if vendor { + os.MkdirAll(filepath.Join(args[i+1], "vendor"), 0o755) + os.WriteFile(filepath.Join(args[i+1], "vendor", "modules.txt"), nil, 0o644) + } + } + } + case name == "git" && strings.Contains(line, "ls-remote"): + return forgekey.Ran{Stdout: h.pulls} + case name == "go" && strings.Contains(line, " mod download"): + if !h.onTrunk && !strings.Contains(strings.Join(env, " "), "-modfile=") { + return forgekey.Ran{Status: 1, Stderr: "go: git.novox.be/novox/mesh-sdk/go@v0.1.15-0.20261022120000-" + pinShort + ": invalid version: unknown revision " + pinShort + "\n"} + } + } + return f.run(ctx, env, name, args...) + } + return h, states +} + +func (h *pinHarness) fetch(states *keptStates) (FetchResult, delivery.Step) { + h.t.Helper() + if _, err := fetchTool(h.t, h.m)(fetchArgs); err != nil { + h.t.Fatal(err) + } + st := states.waitEnd(h.t) + var res FetchResult + if st.State == delivery.StepPassed { + json.Unmarshal(st.Result, &res) + } + return res, st +} + +func (h *pinHarness) ran(sub string) bool { + for _, c := range h.cmds { + if strings.Contains(c.line, sub) { + return true + } + } + return false +} + +const pullList = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\trefs/pull/6/head\n" + commitSHA + "\trefs/pull/7/head\n" + +// 1148/AC1: a go.mod requiring mesh-sdk at a pseudo-version only a pull request's head carries is fetched: the +// repository is placed beside the workspace at that commit, go mod download and verify succeed through a temporary +// -modfile replacing the module by that directory, and the build is handed the same -modfile with GOPROXY=off. +func TestAPinOnAPullHeadIsPlacedBesideAndHandedToGoByAModfile(t *testing.T) { + h, states := newPinHarness(t, pinnedGoMod, false, pullList) + res, st := h.fetch(states) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + placed := filepath.Join(h.m.Work, res.Workspace, "pinned", "mesh-sdk@"+commitSHA, "beside", "mesh-sdk") + if !h.ran("-C " + placed + " checkout -q --detach " + commitSHA) { + t.Errorf("mesh-sdk was not placed at %s at the commit:\n%s", placed, strings.Join(h.f.lines(), "\n")) + } + if !h.ran("refs/pull/*/head") { + t.Errorf("the forge's pull request heads were not listed") + } + dir := filepath.Join(h.m.Work, res.Workspace, "tree") + flags := "" + for _, e := range h.m.goEnvFor(dir) { + if strings.HasPrefix(e, "GOFLAGS=") { + flags = e + } + } + _, modfile, ok := strings.Cut(flags, "-modfile=") + if !ok { + t.Fatalf("go is not handed a -modfile: %v", h.m.goEnvFor(dir)) + } + raw, err := os.ReadFile(strings.Fields(modfile)[0]) + if err != nil { + t.Fatal(err) + } + if want := "replace git.novox.be/novox/mesh-sdk/go => " + filepath.Join(placed, "go"); !strings.Contains(string(raw), want) || !strings.Contains(string(raw), "module fixture/member") { + t.Errorf("the modfile lacks %q:\n%s", want, raw) + } + build := strings.Join(h.m.buildEnv(dir), " ") + if !strings.Contains(build, "-modfile="+strings.Fields(modfile)[0]) || !strings.Contains(build, "GOPROXY=off") { + t.Errorf("the build is not handed the modfile with GOPROXY=off: %s", build) + } + if !h.ran(" mod verify") { + t.Errorf("go mod verify did not run") + } +} + +// 1148/AC3: a pin go downloads as it is (on the trunk or a tag) behaves as today: no listing of pull requests, no +// repository placed, no -modfile in the fetch or the build. +func TestAPinGoCanDownloadIsLeftAsToday(t *testing.T) { + h, states := newPinHarness(t, pinnedGoMod, false, pullList) + h.onTrunk = true + res, st := h.fetch(states) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + if h.ran("ls-remote") || h.ran("pinned") { + t.Errorf("a pin on the trunk made the fetch look for a pull request or place a repository:\n%s", strings.Join(h.f.lines(), "\n")) + } + dir := filepath.Join(h.m.Work, res.Workspace, "tree") + for _, e := range append(h.m.goEnvFor(dir), h.m.buildEnv(dir)...) { + if strings.Contains(e, "-modfile") { + t.Errorf("a -modfile is handed to go: %s", e) + } + } + if !h.ran(" mod download") || !h.ran(" mod verify") { + t.Errorf("go mod download and verify did not run as before") + } +} + +// 1148/AC4: a pin that no ref of the forge carries ends the step failed, naming the module and the commit. +func TestAPinOnNoRefFailsNamingTheModuleAndTheCommit(t *testing.T) { + h, states := newPinHarness(t, pinnedGoMod, false, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\trefs/pull/6/head\n") + _, st := h.fetch(states) + if st.State != delivery.StepFailed { + t.Fatalf("the step ended %s, want failed", st.State) + } + if !strings.Contains(st.Reason, "no pull request head") || !strings.Contains(st.Reason, "unknown revision") || + !strings.Contains(st.Reason, "git.novox.be/novox/mesh-sdk/go") || !strings.Contains(st.Reason, pinShort) { + t.Errorf("the reason does not name the module and the commit: %q", st.Reason) + } + if !h.ran("ls-remote") { + t.Errorf("the forge's pull request heads were never looked at") + } +} + +// 1148/AC5: a directory that vendors resolves nothing: no go command, no listing, no repository placed. +func TestAVendoringDirectoryResolvesNothing(t *testing.T) { + h, states := newPinHarness(t, pinnedGoMod, true, pullList) + _, st := h.fetch(states) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + if h.ran("go -C") || h.ran("ls-remote") || h.ran("pinned") { + t.Errorf("a vendoring directory was resolved:\n%s", strings.Join(h.f.lines(), "\n")) + } +} + +// 1148/AC1 (a repository with several go.mod directories, as mesh-catalog has): two directories pinning the same pull +// head share one placement and both are handed a modfile; the fetch does not fail placing it twice. +func TestTwoDirectoriesPinningOneHeadShareOnePlacement(t *testing.T) { + h, states := newPinHarness(t, pinnedGoMod, false, pullList) + h.f.gomods = "go.mod\nsub/go.mod\n" + res, st := h.fetch(states) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + checkouts := 0 + for _, c := range h.cmds { + if strings.Contains(c.line, "pinned") && strings.Contains(c.line, " checkout ") { + checkouts++ + } + } + if checkouts != 1 { + t.Errorf("the repository was placed %d times, want once:\n%s", checkouts, strings.Join(h.f.lines(), "\n")) + } + for _, d := range []string{"tree", "tree/sub"} { + if _, ok := h.m.pinModfile(filepath.Join(h.m.Work, res.Workspace, d)); !ok { + t.Errorf("%s was not handed a modfile", d) + } + } +} + +// 1148/AC1: two directories pinning two different commits of one repository get two placements, each its own. +func TestTwoCommitsOfOneRepositoryGetTwoPlacements(t *testing.T) { + other := "fedcba9876543210fedcba9876543210fedcba98" + h, states := newPinHarness(t, pinnedGoMod, false, commitSHA+"\trefs/pull/7/head\n"+other+"\trefs/pull/8/head\n") + h.f.gomods = "go.mod\nsub/go.mod\n" + base := h.m.Run + h.m.Run = func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + if name == "git" && strings.Contains(strings.Join(args, " "), "pinned/mesh-sdk@"+other+"/") && strings.Contains(strings.Join(args, " "), " rev-parse HEAD") { + return forgekey.Ran{Stdout: other + "\n"} + } + // the second directory pins the other commit + if name == "git" && strings.Contains(strings.Join(args, " "), " checkout ") && !strings.Contains(strings.Join(args, " "), "pinned") { + r := base(ctx, env, name, args...) + for i, a := range args { + if a == "-C" { + os.WriteFile(filepath.Join(args[i+1], "sub", "go.mod"), []byte(strings.ReplaceAll(pinnedGoMod, pinShort, other[:12])), 0o644) + } + } + return r + } + if name == "go" && strings.Contains(strings.Join(args, " "), "/sub mod download") && !strings.Contains(strings.Join(env, " "), "-modfile=") { + return forgekey.Ran{Status: 1, Stderr: "go: git.novox.be/novox/mesh-sdk/go@v0.1.15-0.20261022120000-" + other[:12] + ": invalid version: unknown revision " + other[:12] + "\n"} + } + return base(ctx, env, name, args...) + } + _, st := h.fetch(states) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + for _, c := range []string{commitSHA, other} { + if !h.ran("pinned/mesh-sdk@" + c) { + t.Errorf("no placement of mesh-sdk at %s", c) + } + } +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/preview.go b/modules/mesh-build-go/cmd/mesh-build-go/preview.go index 14ab2b81..32fbd3af 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/preview.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/preview.go @@ -47,7 +47,8 @@ func (m *Module) merge(ctx context.Context, res *FetchResult, tree, remote, base return err } -type siblingArg struct{ repository, commit string } +// ref is the pull request head carrying the commit, when only it does (a pin on a pull request, issue 1148). +type siblingArg struct{ repository, commit, ref string } func siblingsArg(v any) ([]siblingArg, error) { if v == nil { @@ -74,7 +75,7 @@ func siblingsArg(v any) ([]siblingArg, error) { return nil, fmt.Errorf("beside: two repositories are named %s", name) } seen[name] = true - out = append(out, siblingArg{repository, commit}) + out = append(out, siblingArg{repository: repository, commit: commit}) } return out, nil } @@ -94,7 +95,7 @@ func (m *Module) placeSiblings(ctx context.Context, root string, siblings []sibl if err := os.MkdirAll(dir, 0o700); err != nil { return nil, err } - if err := buildstep.FetchCommit(ctx, m.Run, dir, m.remote(s.repository), s.commit, "", m.FetchOptions...); err != nil { + if err := buildstep.FetchCommit(ctx, m.Run, dir, m.remote(s.repository), s.commit, s.ref, m.FetchOptions...); err != nil { return nil, fmt.Errorf("beside %s: %w", s.repository, err) } if link := firstSymlink(dir); link != "" { diff --git a/modules/mesh-delivery-gate/README.md b/modules/mesh-delivery-gate/README.md index 4f8b9de3..5b208ec6 100644 --- a/modules/mesh-delivery-gate/README.md +++ b/modules/mesh-delivery-gate/README.md @@ -20,6 +20,19 @@ of such a delivery asks the controller (`delivery-backups-again`) to take the ba delivering; a record from before the controller cleared the failure does not hold it again. A held delivery already ends as covered or delivered when a later merge covers it. +## When a refused or rejected merge is said (ADR 0505 §4, §5) + +`stalled` is what the controller raises as a condition for the operator. The gate's own list (`show`, `deliveries`) +names every refusal and rejection at once, and the pull request is told at once; only `stalled` waits: + +- a merge refused because its trunk moved, whose check the gate asked again, is stalled only once that check answered and + the merge was refused again, or twice the repository's last check time passed (at least the setting + `refusal-stall-floor-minutes`, 10). Every other refusal is stalled at once. +- a delivery that needs its author's new head (its check failed after its Approve: a conflict, a failed check, a rejected + group) is stalled only once no new head came for the setting `author-new-head-minutes` (120). Its line says + `needs: new-head` and names its author: the branch, and the session a `Pushed-by: ` line in the pull + request's description names. + ## Taking the seat over from today's module The gate claims the seat `mesh-delivery` and took it over from the module `mesh-delivery`, which the catalogue no longer holds. diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/adr505_bus_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/adr505_bus_test.go new file mode 100644 index 00000000..0bde5945 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/adr505_bus_test.go @@ -0,0 +1,88 @@ +//go:build integration + +package main + +import ( + "context" + "encoding/json" + "strconv" + "strings" + "testing" + "time" + + sdk "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// novox/hq ADR 0505 §4 over the real bus, in the bed of bus_test.go: the gate whole, its deliveries kept in its buckets. + +// call asks the bundle one of its verbs, as the runtime does, and answers its result's text. +func (r *runtime) call(t *testing.T, name string, args map[string]any) string { + t.Helper() + r.mu.Lock() + r.next++ + id := "adr505-" + strconv.Itoa(r.next) + ch := make(chan json.RawMessage, 1) + r.wait[id] = ch + r.mu.Unlock() + r.say(map[string]any{"id": id, "method": "tools/call", "params": map[string]any{"name": name, "arguments": args}}) + select { + case raw := <-ch: + return string(raw) + case <-time.After(30 * time.Second): + t.Fatalf("%s was not answered in 30 s", name) + } + return "" +} + +// ADR 0505 §4: an approved head the merge tool refused because its trunk moved is shown by `show` at once but is not in +// `stalled` while its fresh check is unanswered; once that check passed and the merge is refused again, it is. +func TestATrunkMovedRefusalIsStalledOnlyOnceItsFreshCheckDidNotCureItOverARealBus(t *testing.T) { + b := newTestBus(t) + ctx := context.Background() + rt := b.start(t) + evctx, cancel := context.WithCancel(ctx) + defer cancel() + rt.deliverEvents(evctx) + rt.until("the module to subscribe to the events it follows", 20*time.Second, func() bool { + rt.mu.Lock() + defer rt.mu.Unlock() + return len(rt.subs) >= len(followed) + }) + rt.untilHolding() + + const head = "1111111111111111111111111111111111111111" + id := IDOf("novox/app", head) + announced := pr("novox/app", 7, head, "fix/x", "src/a.go") + announced.BaseSHA = "t1t1t1t1t1t1" + b.publish(t, "mesh.mod.gitea.event.pull.updated", announced) + b.publish(t, "mesh.seat."+ControllerSeat+".event.checked", judgedAt("novox/app", 7, head, "pass", "t1t1t1t1t1t1")) + var key string + rt.until("the ask to be written", 10*time.Second, func() bool { key = onlyAsk(b); return key != "" }) + b.answer(t, key, OptionApprove) + rt.until("the merge to be handed to the bridge", 10*time.Second, func() bool { return len(mergeAt(b, "fix/x").Pulls) > 0 }) + + refused := func() { + b.forgeSays(t, sdk.Forge{Kind: sdk.ForgeRefused, Act: "merge", Repository: "novox/app", Number: 7, Head: head, + Words: errAskedAgain.Error()}) + } + refused() + rt.until("show to name the refusal at once", 10*time.Second, func() bool { + return strings.Contains(rt.call(t, Seat+".show", map[string]any{"id": id}), "its verdict was judged against trunk") + }) + if got := rt.call(t, Seat+".stalled", map[string]any{}); strings.Contains(got, id) { + t.Fatalf("said as stalled while its fresh check runs: %s", got) + } + + // The trunk moved; the head is announced again and its fresh check passes; the merge is refused again on it. + b.publish(t, "mesh.mod.gitea.event.pull.merged", merged("novox/app", 9, "9999999999999999999999999999999999999999", "t2t2t2t2t2t2")) + b.publish(t, "mesh.mod.gitea.event.pull.updated", announced) + time.Sleep(time.Second) + b.publish(t, "mesh.seat."+ControllerSeat+".event.checked", judgedAt("novox/app", 7, head, "pass", "t2t2t2t2t2t2")) + rt.until("the fresh verdict to be heard", 10*time.Second, func() bool { + return strings.Contains(rt.call(t, Seat+".show", map[string]any{"id": id}), "t2t2t2t2t2t2") + }) + refused() + rt.until("the refusal on the fresh verdict to be said as stalled", 10*time.Second, func() bool { + return strings.Contains(rt.call(t, Seat+".stalled", map[string]any{}), id) + }) +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/adr505_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/adr505_test.go new file mode 100644 index 00000000..76833d2f --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/adr505_test.go @@ -0,0 +1,294 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + stdio "git.novox.be/novox/mesh-sdk/go" +) + +// The tests of novox/hq ADR 0505 "How it is checked" for the gate: a refusal the gate is already answering is no +// stall yet (§4), and a delivery whose cure is its author's new head is its author's for two hours (§5). + +// ADR 0505 §4, §5: the two bounds are settings with their decided defaults. +func TestTheBoundsAreSettingsWithTheirDefaults(t *testing.T) { + read := func(body string) (Settings, error) { + p := filepath.Join(t.TempDir(), "settings.json") + if err := os.WriteFile(p, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + return readSettings(p) + } + h := &Holder{} + if h.refusalFloor() != 10*time.Minute || h.authorBound() != 2*time.Hour { + t.Fatalf("no settings: floor %s, author %s", h.refusalFloor(), h.authorBound()) + } + s, err := read(`{"refusal-stall-floor-minutes": 5, "author-new-head-minutes": 60}`) + if err != nil { + t.Fatal(err) + } + h.Settings = func() Settings { return s } + if h.refusalFloor() != 5*time.Minute || h.authorBound() != time.Hour { + t.Fatalf("set: floor %s, author %s", h.refusalFloor(), h.authorBound()) + } + s, _ = read(`{}`) + if h.refusalFloor() != 10*time.Minute || h.authorBound() != 2*time.Hour { + t.Fatalf("empty file: floor %s, author %s", h.refusalFloor(), h.authorBound()) + } + if _, err := read(`{"author-new-head-minutes": 0}`); err == nil { + t.Fatalf("a bound of zero minutes was taken") + } +} + +// ADR 0505 §4: a repository's last measured check time is its newest verdict's (`took`, as mesh-controller +// cmd/mesh-builder main.go writes it: a duration rounded to the second); none, and the floor alone holds. +func TestTheLastMeasuredCheckTimeIsTheNewestVerdictsOfItsRepository(t *testing.T) { + h := &Holder{} + h.init() + at := time.Date(2026, 10, 22, 10, 0, 0, 0, time.UTC) + h.deliveries["novox/mesh-catalog@a"] = &Delivery{ID: "novox/mesh-catalog@a", Repository: "novox/mesh-catalog", + Check: &Verdict{At: at, Took: "4m0s"}} + h.deliveries["novox/mesh-catalog@b"] = &Delivery{ID: "novox/mesh-catalog@b", Repository: "novox/mesh-catalog", + Check: &Verdict{At: at.Add(time.Hour), Took: "7m30s"}} + h.deliveries["novox/hq@c"] = &Delivery{ID: "novox/hq@c", Repository: "novox/hq", + Check: &Verdict{At: at.Add(2 * time.Hour), Took: "1m0s"}} + if got := h.lastCheckTime("novox/mesh-catalog"); got != 7*time.Minute+30*time.Second { + t.Fatalf("the newer of two: %s", got) + } + if got := h.answeredRefusalBound("novox/mesh-catalog"); got != 15*time.Minute { + t.Fatalf("twice 7m30s: %s", got) + } + if got := h.answeredRefusalBound("novox/hq"); got != 10*time.Minute { + t.Fatalf("twice one minute is under the floor: %s", got) + } + if got := h.answeredRefusalBound("novox/mesh-sdk"); got != 10*time.Minute { + t.Fatalf("no verdict: %s", got) + } +} + +// refusedForAMovedTrunk is a lone delivery approved and refused by the merge tool because its trunk moved and its check +// was asked again (gitea merge.ts:147, errAskedAgain). +func refusedForAMovedTrunk(t *testing.T) (*world, string) { + t.Helper() + w := announced(t) + id := IDOf("novox/app", readyHead) + hear(t, w, ControllerSeat+".checked", judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1")) + w.forge.refuse = errAskedAgain + approve(w) + if len(w.forge.merges) != 1 || w.delivery(id).Ready.RefusedSince == nil { + t.Fatalf("the merge was not asked and refused: %v %+v", w.forge.merges, w.delivery(id).Ready) + } + return w, id +} + +func linesFor(w *world, id string) []StalledLine { + var out []StalledLine + for _, l := range w.h.Stalled() { + if l.ID == id { + out = append(out, l) + } + } + return out +} + +// ADR 0505 §4: a refusal because the trunk moved, whose check the gate asked again, is in the gate's own list at once +// but is no stalled line while its check is unanswered; once that check passed and the merge is refused again, it is. +func TestATrunkMovedRefusalIsStalledOnlyOnceItsFreshCheckAnsweredAndTheMergeIsRefusedAgain(t *testing.T) { + w, id := refusedForAMovedTrunk(t) + if got := linesFor(w, id); len(got) != 0 { + t.Fatalf("said as stalled while its fresh check runs: %+v", got) + } + shown, err := w.h.Show(id) + if err != nil { + t.Fatal(err) + } + if raw, _ := json.Marshal(shown); !strings.Contains(string(raw), "is not merged: its verdict was judged against trunk") { + t.Fatalf("the gate's own list does not show the refusal at once: %s", raw) + } + w.later(5 * time.Minute) + if got := linesFor(w, id); len(got) != 0 { + t.Fatalf("said as stalled at five minutes, its check unanswered: %+v", got) + } + // As on the live mesh (events_test.go): the trunk moved, the head is announced again, its fresh verdict passes + // against the new trunk, and the merge is asked and refused again, the trunk having moved once more. + w.h.PullMerged(merged("novox/app", 9, "9999999999999999", "t2t2t2t2t2t2")) + w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go")) + w.later(time.Second) + hear(t, w, ControllerSeat+".checked", judgedAt("novox/app", 7, readyHead, "pass", "t2t2t2t2t2t2")) + w.settleAll() + if len(w.forge.merges) != 2 { + t.Fatalf("the fresh verdict did not ask the merge again: %v", w.forge.merges) + } + if got := linesFor(w, id); len(got) != 1 { + t.Fatalf("its check answered and the merge refused again, yet no stalled line: %+v", got) + } +} + +// ADR 0505 §4: with no answer, the refusal is said at twice the repository's last check time, at least ten minutes. +func TestATrunkMovedRefusalIsStalledAtTwiceTheLastCheckTimeAtLeastTheFloor(t *testing.T) { + w, id := refusedForAMovedTrunk(t) + w.later(9 * time.Minute) + if got := linesFor(w, id); len(got) != 0 { + t.Fatalf("said before the floor: %+v", got) + } + w.later(2 * time.Minute) + if got := linesFor(w, id); len(got) != 1 { + t.Fatalf("not said past the floor of ten minutes: %+v", got) + } + + w, id = refusedForAMovedTrunk(t) + w.h.mu.Lock() + w.h.deliveries[id].Check.Took = "7m0s" // its repository's last check ran seven minutes: the bound is fourteen + w.h.mu.Unlock() + w.later(11 * time.Minute) + if got := linesFor(w, id); len(got) != 0 { + t.Fatalf("said at eleven minutes, under twice the last check time: %+v", got) + } + w.later(4 * time.Minute) + if got := linesFor(w, id); len(got) != 1 { + t.Fatalf("not said past twice the last check time: %+v", got) + } +} + +// ADR 0505 §4: any other refusal is said at once, as issue 1115 has it. +func TestAnyOtherRefusalIsStalledAtOnce(t *testing.T) { + w := announced(t) + id := IDOf("novox/app", readyHead) + hear(t, w, ControllerSeat+".checked", judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1")) + w.forge.refuse = refusedWords("novox/app#7") + approve(w) + if got := linesFor(w, id); len(got) != 1 { + t.Fatalf("a missing status refusal is not said at once: %+v", got) + } +} + +// rejectedAfterItsApprove is a delivery approved and then rejected: its check failed after the ask (as in events_test.go), +// its pull request's description naming who pushed it when pushedBy is given. +func rejectedAfterItsApprove(t *testing.T, pushedBy string) (*world, string) { + t.Helper() + w := newWorld(t) + w.op.auto = false + p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go") + if pushedBy != "" { + p.Body += "\nPushed-by: " + pushedBy + } + w.h.PullUpdated(p) + w.settleAll() + id := IDOf("novox/app", readyHead) + hear(t, w, ControllerSeat+".checked", judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1")) + if _, err := w.h.Recheck(id, "a second look", "tester"); err != nil { + t.Fatal(err) + } + hear(t, w, ControllerSeat+".checked", judgedAt("novox/app", 7, readyHead, "fail", "t1t1t1t1t1t1")) + approve(w) + w.h.React() + if w.state(id) != Rejected { + t.Fatalf("not rejected: %s", w.state(id)) + } + return w, id +} + +// ADR 0505 §5: a rejected delivery is told on its pull request at once, and reaches the operator only after two hours +// with no new head, in words naming its author (its branch, and the session that pushed it) and needing a new head — +// never "stop it", never "the operator's". +func TestARejectedDeliveryIsItsAuthorsForTwoHours(t *testing.T) { + w, id := rejectedAfterItsApprove(t, "quiet-deliveries@shanks") + said := false + for _, c := range w.forge.comments { + said = said || strings.Contains(c, "not merged") + } + if !said { + t.Fatalf("its author was not told on the pull request at once: %v", w.forge.comments) + } + w.later(2*time.Hour - time.Minute) + if got := linesFor(w, id); len(got) != 0 { + t.Fatalf("the operator's before two hours: %+v", got) + } + w.later(2 * time.Minute) + got := linesFor(w, id) + if len(got) != 1 { + t.Fatalf("not the operator's after two hours with no new head: %+v", got) + } + l := got[0] + if l.Needs != NeedsNewHead || l.Author != "the author of feat/x (quiet-deliveries on shanks)" || l.Branch != "feat/x" { + t.Fatalf("the line does not name its author and the new head it needs: %+v", l) + } + for _, never := range []string{"operator's", "stop it"} { + if strings.Contains(l.Says+l.H2, never) { + t.Fatalf("the line says %q: %+v", never, l) + } + } + if !strings.Contains(l.Says, "needs a new head from the author of feat/x (quiet-deliveries on shanks)") { + t.Fatalf("the words do not say a new head is needed from its author: %q", l.Says) + } +} + +// ADR 0505 §5: with no Pushed-by line, the branch alone names its author. +func TestARejectedDeliveryWithNoPushedByLineIsNamedByItsBranch(t *testing.T) { + w, id := rejectedAfterItsApprove(t, "") + w.later(2*time.Hour + time.Minute) + if got := linesFor(w, id); len(got) != 1 || got[0].Author != "the author of feat/x" { + t.Fatalf("not named by its branch: %+v", got) + } +} + +// ADR 0505 §5: a new head within the two hours ends it: the rejected head is never the operator's. +func TestANewHeadWithinTwoHoursKeepsTheRejectedHeadFromTheOperator(t *testing.T) { + w, id := rejectedAfterItsApprove(t, "quiet-deliveries@shanks") + w.later(time.Hour) + w.h.PullUpdated(pr("novox/app", 7, pushed, "feat/x", "src/a.go")) + w.settleAll() + w.later(2 * time.Hour) + if got := linesFor(w, id); len(got) != 0 { + t.Fatalf("the superseded head reached the operator: %+v", got) + } +} + +// ADR 0505 §2: a walk's end said again by the controller with its declared machines (`declared_read`, a touch marked +// declared) acts once: no transition, comment or ask of the controller the first saying did not make. +func TestAWalkSaidAgainWithItsDeclaredMachinesActsOnceInTheGate(t *testing.T) { + w, id, plan := aDeliveringWalk(t) + done := w.ctl.walk(plan) + sent := time.Date(2026, 10, 6, 12, 40, 0, 0, time.UTC) + done.Modules["app"] = &WalkModule{State: "built", First: []string{"anchor"}, FirstAt: &sent, SentAt: &sent, Build: "build-1", + Gate: &WalkGate{Machines: []string{"anchor"}, Since: &sent, Passes: 1, Verdict: "passed", JudgedAt: &sent}} + done.State = walkDone + done = w.ctl.put(done) + body := func(again bool) []byte { + raw, err := json.Marshal(done) + if err != nil { + t.Fatal(err) + } + var said map[string]any + _ = json.Unmarshal(raw, &said) + said["walk"], said["phase"], said["outcome"] = plan, "ended", "delivered" + said["touches"] = []map[string]any{{"machine": "anchor", "modules": []string{"app"}}} + if again { + said["touches"] = append(said["touches"].([]map[string]any), + map[string]any{"machine": "shanks", "modules": []string{}, "declared": true}) + said["declared_read"] = true + } + out, _ := json.Marshal(said) + return out + } + if err := take(w.h, stdio.Envelope{Key: ControllerSeat + ".plan-moved", Body: body(false)}); err != nil { + t.Fatal(err) + } + w.settleAll() + if s := w.state(id); s != Delivered { + t.Fatalf("the walk's end left it %s", s) + } + transitions, comments, delivers := len(w.delivery(id).Transitions), len(w.forge.comments), len(w.ctl.delivers) + if err := take(w.h, stdio.Envelope{Key: ControllerSeat + ".plan-moved", Body: body(true)}); err != nil { + t.Fatal(err) + } + w.settleAll() + if len(w.delivery(id).Transitions) != transitions || len(w.forge.comments) != comments || len(w.ctl.delivers) != delivers { + t.Fatalf("said again it acted again: transitions %d→%d, comments %d→%d, asks of the controller %d→%d", + transitions, len(w.delivery(id).Transitions), comments, len(w.forge.comments), delivers, len(w.ctl.delivers)) + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/checktime.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/checktime.go new file mode 100644 index 00000000..91bc0eec --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/checktime.go @@ -0,0 +1,34 @@ +package main + +import "time" + +// lastCheckTime is how long the newest answered check of a repository ran, as its verdict says it (`took`): the +// repository's last measured check time (novox/hq ADR 0505 §4). Zero when no verdict of the repository says it. +// Under the lock. +func (h *Holder) lastCheckTime(repository string) time.Duration { + var newest *Verdict + var took time.Duration + for _, d := range h.deliveries { + v := d.Check + if d.Repository != repository || v == nil || v.Took == "" { + continue + } + t, err := time.ParseDuration(v.Took) + if err != nil || t <= 0 { + continue + } + if newest == nil || v.At.After(newest.At) { + newest, took = v, t + } + } + return took +} + +// answeredRefusalBound is how long a refusal the gate is already answering waits before it is said as stalled: twice +// the repository's last measured check time, at least the floor (ADR 0505 §4). Under the lock. +func (h *Holder) answeredRefusalBound(repository string) time.Duration { + if b := 2 * h.lastCheckTime(repository); b > h.refusalFloor() { + return b + } + return h.refusalFloor() +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events_test.go index 90e2d95c..f5e1f299 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "strings" "testing" + "time" stdio "git.novox.be/novox/mesh-sdk/go" ) @@ -229,7 +230,8 @@ func TestAPushAfterTheAskEndsItAndSaysSoOnceOnThePullRequest(t *testing.T) { } } -// Issue 1022 (ADR 0421 §7): a check that failed after the ask merges nothing and is said once and as a stalled line. +// Issue 1022 (ADR 0421 §7), as novox/hq ADR 0505 §5 changed it: a check that failed after the ask merges nothing, is +// said once on the pull request at once, and is a stalled line only after two hours with no new head. func TestACheckThatFailsAfterTheAskIsNotMergedAndIsSaid(t *testing.T) { w := announced(t) id := IDOf("novox/app", readyHead) @@ -252,6 +254,10 @@ func TestACheckThatFailsAfterTheAskIsNotMergedAndIsSaid(t *testing.T) { if said != 1 { t.Fatalf("the failure is not said once: %v", w.forge.comments) } + if lines := w.h.Stalled(); len(lines) != 0 { + t.Fatalf("the failure is the operator's before its author had two hours: %+v", lines) + } + w.later(2*time.Hour + time.Minute) lines := w.h.Stalled() if len(lines) != 1 || lines[0].ID != id || strings.Contains(lines[0].Says, "marked") { t.Fatalf("the failure is not a stalled line: %+v", lines) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go index c5627690..2e1d3c87 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go @@ -128,6 +128,19 @@ type fakeController struct { planned *DeliveryPlan // forgot are the pull requests whose check builds were let go of (novox/hq ADR 0350 §5), "owner/repo#n". forgot []string + // queue is the build queue as Check fills it: every ask made is held, waiting, until a test takes it out (a lost + // ask, issue 1274); queueDown makes the queue unreadable. + queue []QueuedAsk + queueDown bool +} + +func (c *fakeController) Queue() ([]QueuedAsk, error) { + c.mu.Lock() + defer c.mu.Unlock() + if c.queueDown { + return nil, errors.New("the queue cannot be read") + } + return append([]QueuedAsk(nil), c.queue...), nil } // BackupsAgain records the word, and clears the walk's failed backups as the controller does. @@ -199,6 +212,7 @@ func (c *fakeController) Check(group string, ms []Member) (string, error) { } c.checks = append(c.checks, group+":"+strings.Join(heads, ",")) for _, m := range ms { + c.queue = append(c.queue, QueuedAsk{ID: id, State: "waiting", Kind: "check", Repository: m.Repository, Commit: m.Head}) c.checkReady = append(c.checkReady, m.Ready) c.baseSHAs = append(c.baseSHAs, m.BaseSHA) } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go index ef4b0045..fa731a8e 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -22,6 +22,8 @@ import ( // Holder is the seat's holder. type Holder struct { + // Settings reads the operator's values (settings.go); nil keeps every default. + Settings func() Settings Store Store Controller Controller Forge Forge @@ -69,6 +71,8 @@ type Holder struct { unreached string // askTried is when a check this owner asks was last tried and could not be asked: tried again a minute on. askTried map[string]time.Time + // lostRead is when the build queue was last read for a proposed delivery's asked check (issue 1274). + lostRead map[string]time.Time // walkAsked is when a walk that carried a step still judging was last asked for by its id: once an hour. walkAsked map[string]time.Time refusals []string @@ -163,6 +167,7 @@ func (h *Holder) init() { h.unmatched = map[string]unmatched{} h.waitsWhy = map[string]string{} h.askTried = map[string]time.Time{} + h.lostRead = map[string]time.Time{} h.walkAsked = map[string]time.Time{} h.unchecked = map[string]unchecked{} h.walks = map[string]seenWalk{} @@ -374,6 +379,11 @@ func (h *Holder) pullUpdated(p PullEvent) (isNew bool) { d.ReviewedBy, d.ReviewedAt = by, at h.keep(d) } + // So is the line naming who pushed it (ADR 0505 §5). + if by := PushedIn(p.Body); p.Body != "" && by != d.PushedBy { + d.PushedBy = by + h.keep(d) + } // An `after:` line written into the description later is told about too (novox/hq ADR 0419 §2). if after := AfterIn(p.Body); p.Body != "" && !slices.Equal(after, d.After) { d.After = after @@ -451,6 +461,7 @@ func (h *Holder) fillFromAnnouncement(d *Delivery, p PullEvent) { d.ModuleDirs, d.ModuleDirsSaid = p.ModuleDirs, p.ModuleDirsSaid d.Issues, d.After = IssuesIn(p.Title+"\n"+p.Body), AfterIn(p.Body) d.ReviewedBy, d.ReviewedAt = ReviewIn(p.Body) + d.PushedBy = PushedIn(p.Body) if p.BaseSHA != "" { d.BaseSHA = p.BaseSHA } @@ -504,6 +515,7 @@ func (h *Holder) PullEdited(p PullEvent) { } before, _ := reviewWords(d) d.ReviewedBy, d.ReviewedAt = ReviewIn(p.Body) + d.PushedBy = PushedIn(p.Body) d.After = AfterIn(p.Body) title := p.Title if title == "" { @@ -571,6 +583,7 @@ func (h *Holder) freshAnnouncement(p PullEvent, now time.Time, startedOver strin PathsTruncated: p.PathsTruncated, Removed: p.Removed, ModuleDirs: p.ModuleDirs, ModuleDirsSaid: p.ModuleDirsSaid, After: AfterIn(p.Body), Issues: IssuesIn(p.Title + "\n" + p.Body), Created: now, BaseSHA: p.BaseSHA} d.ReviewedBy, d.ReviewedAt = ReviewIn(p.Body) + d.PushedBy = PushedIn(p.Body) why := fmt.Sprintf("%s#%d's head announced", d.Repository, d.Number) if startedOver != "" { why = fmt.Sprintf("%s; %s", why, startedOver) @@ -1382,6 +1395,10 @@ func (h *Holder) plan() []func() { if a := h.askChecks(now); a != nil { asks = append(asks, a) } + // Proposed, its check asked and held by no entry of the build queue past the bound: asked again (issue 1274). + if a := h.lostChecks(now); a != nil { + asks = append(asks, a) + } // Merged with no walk yet: tied to the newest walk heard that answers it — by name, by its own commit, or by // the forge's word that the walk's commit contains it, asked here (novox/hq issue 362). diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1274_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1274_test.go new file mode 100644 index 00000000..170452c7 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1274_test.go @@ -0,0 +1,153 @@ +package main + +import ( + "strings" + "testing" + "time" +) + +// Issue 1274: a proposed delivery whose asked check the build queue no longer holds is asked again, and a recheck from +// proposed is taken when no check of its head is waiting or in flight. + +// 1274/AC1: the queue holds nothing for the ask, the sweep runs after the bound: the check is asked once more and the +// transition says the earlier ask was lost. +func TestACheckTheQueueLostIsAskedAgainBySweepAfterTheBound(t *testing.T) { + w := newWorld(t) + id := IDOf("novox/app", head) + w.h.PullUpdated(pr("novox/app", 7, head, "feat/x", "src/a.go")) + w.settleAll() + first := w.delivery(id).CheckAsk + if len(w.ctl.checks) != 1 || first == nil { + t.Fatalf("asked %v, ask %+v", w.ctl.checks, first) + } + w.ctl.queue = nil // the queue lost it + w.later(9 * time.Minute) + w.settleAll() + if len(w.ctl.checks) != 1 { + t.Fatalf("asked again inside the bound: %v", w.ctl.checks) + } + w.later(2 * time.Minute) + w.settleAll() + d := w.delivery(id) + last := d.Transitions[len(d.Transitions)-1] + if len(w.ctl.checks) != 2 || d.CheckAsk == nil || d.CheckAsk.ID == first.ID || last.Event != EvLost || + !strings.Contains(last.Why, first.ID) || !strings.Contains(last.Why, "lost") { + t.Fatalf("asked %v; ask %+v; last transition %+v", w.ctl.checks, d.CheckAsk, last) + } + w.later(time.Hour) + w.settleAll() + if len(w.ctl.checks) != 2 { + t.Fatalf("an ask the queue holds was asked again: %v", w.ctl.checks) + } +} + +// 1274/AC1: a verdict that went out of date, its check against the trunk's head asked and lost, is asked again too (the +// case of novox/mesh-controller@28a278c1462a), and the ask's bound runs from the new ask. +func TestACheckAgainstTheTrunkTheQueueLostIsAskedAgain(t *testing.T) { + w := approvedGroup(t) + id := IDOf(gCtl, gHc) + w.h.PullMerged(merged(gCtl, 5, "5555555555555555", gT1)) + w.settleAll() + d := w.delivery(id) + if d.State != Proposed || d.AskedAgain == nil { + t.Fatalf("it is %s, asked again %v", d.State, d.AskedAgain) + } + asked := len(w.ctl.checks) + w.ctl.queue = nil + w.later(11 * time.Minute) + w.settleAll() + d = w.delivery(id) + last := d.Transitions[len(d.Transitions)-1] + if len(w.ctl.checks) != asked+1 || last.Event != EvLost || d.AskedAgain == nil || !d.AskedAgain.After(w.now.Add(-time.Minute)) { + t.Fatalf("asked %v; last transition %+v; asked again %v", w.ctl.checks[asked:], last, d.AskedAgain) + } +} + +// 1274/AC2: the queue still holds the ask, waiting or in flight, or cannot be read: nothing is asked again. +func TestACheckTheQueueStillHoldsIsNotAskedAgain(t *testing.T) { + w := newWorld(t) + id := IDOf("novox/app", head) + w.h.PullUpdated(pr("novox/app", 7, head, "feat/x", "src/a.go")) + w.settleAll() + w.ctl.queue[0].State = "in flight" + w.later(3 * time.Hour) + w.settleAll() + if len(w.ctl.checks) != 1 { + t.Fatalf("an ask in flight was asked again: %v", w.ctl.checks) + } + w.ctl.queue = nil + w.ctl.queueDown = true + w.later(time.Hour) + w.settleAll() + if len(w.ctl.checks) != 1 || w.delivery(id).State != Proposed { + t.Fatalf("an unreadable queue asked again: %v", w.ctl.checks) + } + w.ctl.queueDown = false + w.later(3 * time.Minute) + w.ctl.queue = []QueuedAsk{{ID: "elsewhere", State: "dead", Kind: "check", Repository: "novox/app", Commit: head}} + w.settleAll() + if len(w.ctl.checks) != 2 { + t.Fatalf("a dead ask counted as held: %v", w.ctl.checks) + } +} + +// 1274/AC3: recheck of a proposed delivery with no check waiting or in flight is taken and asks its check; with one +// held it is refused naming it. +func TestARecheckFromProposedIsTakenOnlyWhenNoCheckOfItsHeadIsHeld(t *testing.T) { + w := newWorld(t) + id := IDOf("novox/app", head) + w.h.PullUpdated(pr("novox/app", 7, head, "feat/x", "src/a.go")) + w.settleAll() + held := w.delivery(id).CheckAsk.ID + if _, err := w.h.Recheck(id, "nothing came", "jochen"); err == nil || !strings.Contains(err.Error(), held) { + t.Fatalf("a recheck with a check waiting: %v", err) + } + if _, err := w.h.Recheck(id, "", "jochen"); err == nil { + t.Fatal("a recheck with no why was taken") + } + if len(w.ctl.checks) != 1 { + t.Fatalf("a refused recheck asked: %v", w.ctl.checks) + } + w.ctl.queue = nil + said, err := w.h.Recheck(id, "nothing came", "jochen") + d := w.delivery(id) + last := d.Transitions[len(d.Transitions)-1] + if err != nil || len(w.ctl.checks) != 2 || d.State != Proposed || last.Event != EvLost || last.By != "jochen" || + !strings.Contains(last.Why, "nothing came") || d.CheckAsk.ID != "check-2" || !strings.Contains(said, "check-2") { + t.Fatalf("%q %v; asked %v; last transition %+v", said, err, w.ctl.checks, last) + } + // Taken, it is now held: a second recheck is refused. + if _, err := w.h.Recheck(id, "again", "jochen"); err == nil || !strings.Contains(err.Error(), "check-2") { + t.Fatalf("a second recheck: %v", err) + } +} + +// 1274/AC4: a group member re-asked this way has its group's composed check asked again at the members' heads once its +// own check answers. +func TestAGroupMemberReaskedAfterALostCheckHasTheComposedCheckAskedAgain(t *testing.T) { + w := approvedGroup(t) + id := IDOf(gCat, gHk) + if _, err := w.h.Recheck(id, "its verdict is wanted again", "jochen"); err != nil { + t.Fatal(err) + } + w.settleAll() // its own check is recorded as asked + w.h.mu.Lock() + w.h.groups["feat/g"].Check.OutOfDate = "" // as if the composed check stood: only the lost ask's own act marks it + w.h.mu.Unlock() + w.ctl.queue = nil + w.later(11 * time.Minute) + w.settleAll() + g := w.h.groups["feat/g"] + if g.Check.OutOfDate == "" || !strings.Contains(g.Check.OutOfDate, "lost") { + t.Fatalf("the composed check stands: %+v", g.Check) + } + first := g.Check.Asked + asked := len(w.ctl.checks) + w.later(time.Minute) + w.h.Checked(judged(gCat, 2, gHk, "pass", gTrunk0)) + w.settleAll() + last := w.ctl.checks[len(w.ctl.checks)-1] + if len(w.ctl.checks) <= asked || !strings.HasPrefix(last, "feat/g:") || w.h.groups["feat/g"].Check.Asked == first { + t.Fatalf("the composed check was not asked again: %v", w.ctl.checks[asked:]) + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go index 4530e52c..e26f5b17 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go @@ -249,9 +249,10 @@ func TestAMemberBothRefusedAndHeldForAStatusIsOneStalledLine(t *testing.T) { } } -// 1275/AC6: a member refused because its verdict is out of date (the trunk moved; its fresh check runs) is a warning at -// once, and urgent only when the group is not merged within ten minutes; the pass stops at the refusal all the same. -func TestAnOutOfDateRefusalIsAWarningAtOnceAndUrgentAfterTenMinutes(t *testing.T) { +// 1275/AC6, as novox/hq ADR 0505 §4 changed it: a member refused because its verdict is out of date (the trunk moved; its +// fresh check runs) is no stalled line while the gate answers it, and urgent once ten minutes passed with the group not +// merged; the pass stops at the refusal all the same. +func TestAnOutOfDateRefusalIsNoLineWhileAnsweredAndUrgentAfterTenMinutes(t *testing.T) { w := approvedGroup(t) allGreen(w) w.forge.refuse = errAskedAgain @@ -267,12 +268,12 @@ func TestAnOutOfDateRefusalIsAWarningAtOnceAndUrgentAfterTenMinutes(t *testing.T } return out } - if got := sev(); len(got) != 1 || got[0] != "warning" { - t.Fatalf("an out of date refusal is %v at once; wanted one warning", got) + if got := sev(); len(got) != 0 { + t.Fatalf("an out of date refusal is %v at once; wanted no line while it is answered", got) } w.later(9 * time.Minute) - if got := sev(); len(got) != 1 || got[0] != "warning" { - t.Fatalf("at nine minutes it is %v; wanted still a warning", got) + if got := sev(); len(got) != 0 { + t.Fatalf("at nine minutes it is %v; wanted still no line", got) } w.later(2 * time.Minute) if got := sev(); len(got) != 1 || got[0] != "urgent" { @@ -300,9 +301,9 @@ func severities(w *world) []string { return out } -// 1275/AC6 (b): the same out of date refusal on a LATER member, after the earlier one merged, is a warning at once and -// urgent after ten minutes: the half merged group does not say it urgent at once. -func TestAnOutOfDateRefusalOnALaterMemberIsAWarningThenUrgent(t *testing.T) { +// 1275/AC6 (b), as ADR 0505 §4 changed it: the same out of date refusal on a LATER member, after the earlier one merged, +// is no line at once and urgent after ten minutes: the half merged group does not say it at once. +func TestAnOutOfDateRefusalOnALaterMemberIsNoLineThenUrgent(t *testing.T) { w := approvedGroup(t) allGreen(w) w.forge.refuseRepo = map[string]error{gCtl: errAskedAgain} @@ -310,8 +311,8 @@ func TestAnOutOfDateRefusalOnALaterMemberIsAWarningThenUrgent(t *testing.T) { w.settleAll() w.h.PullMerged(merged(gCat, 2, gHk, gT1)) w.settleAll() - if got := severities(w); len(got) != 1 || got[0] != "warning" { - t.Fatalf("a later member's out of date refusal is %v at once; wanted one warning", got) + if got := severities(w); len(got) != 0 { + t.Fatalf("a later member's out of date refusal is %v at once; wanted no line while it is answered", got) } w.later(11 * time.Minute) if got := severities(w); len(got) != 1 || got[0] != "urgent" { @@ -371,9 +372,10 @@ var mergeToolRefusals = []struct { "Nothing was merged", false}, } -// 1275/AC6 (e): only the merge tool's two out of date refusals (:121, :147) are warnings; the one saying its check could -// not be asked (:144) and every other is urgent at once. -func TestOnlyTheMergeToolsTwoOutOfDateRefusalsAreWarnings(t *testing.T) { +// 1275/AC6 (e), as ADR 0505 §4 changed it: only the merge tool's two out of date refusals (:121, :147) are answered by +// the gate and said as no line at once; the one saying its check could not be asked (:144) and every other is urgent at +// once. +func TestOnlyTheMergeToolsTwoOutOfDateRefusalsWaitWhileAnswered(t *testing.T) { for _, c := range mergeToolRefusals { if got := outOfDateRefusal(c.words); got != c.outOfDated { t.Errorf("%s: outOfDateRefusal = %v, wanted %v", c.line, got, c.outOfDated) @@ -383,12 +385,12 @@ func TestOnlyTheMergeToolsTwoOutOfDateRefusalsAreWarnings(t *testing.T) { w.forge.refuse = errors.New("the forge refused gitea_merge_pull_request: " + c.words) w.op.answer(w.op.open()[0], OptionApprove) w.settleAll() - want := "urgent" - if c.outOfDated { - want = "warning" + got := severities(w) + if c.outOfDated && len(got) != 0 { + t.Errorf("%s: said %v at once, wanted no line while it is answered", c.line, got) } - if got := severities(w); len(got) != 1 || got[0] != want { - t.Errorf("%s: severity %v at once, wanted %s", c.line, got, want) + if !c.outOfDated && (len(got) != 1 || got[0] != "urgent") { + t.Errorf("%s: severity %v at once, wanted urgent", c.line, got) } } } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1349_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1349_test.go index bb282ec2..195bef45 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1349_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1349_test.go @@ -97,7 +97,7 @@ func TestTheGatesCodeCallsNoForgeWriteToolAndNoDeliveryPlan(t *testing.T) { forbidden := []string{"delivery-plan", "gitea_commit_status\"", "gitea_delivery_view", "gitea_note_append", "gitea_add_comment", "gitea_merge_pull_request\"", "gitea_open_pulls_write"} // The controller's verbs the gate calls: ADR 0450 §7. - allowed := []string{"deliver", "delivery-stop", "delivery-backups-again", "delivery-walks", "delivery-check", "forget-checks", "plans"} + allowed := []string{"deliver", "delivery-stop", "delivery-backups-again", "delivery-walks", "delivery-check", "forget-checks", "plans", "queue"} called := map[string]bool{} fset := token.NewFileSet() for _, f := range files { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/lostcheck.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/lostcheck.go new file mode 100644 index 00000000..07725fa8 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/lostcheck.go @@ -0,0 +1,211 @@ +package main + +import ( + "fmt" + "strings" + "time" +) + +// A check the build queue lost (novox/hq issue 1274). +// +// A proposed delivery waits for a check it asked of the controller. When the build queue loses that ask — taken by a +// build agent that was replaced, or dropped with the queue's own state — nothing answers and nothing asks again: the +// delivery stays proposed until a new head is pushed, and a recheck from proposed was refused. The sweep now reads the +// controller's `queue` and asks again a check that no entry holds (neither waiting nor in flight; a dead one is not +// held) for longer than lostCheckBound. A recheck of a proposed delivery is taken on the same fact. Both are the +// table's EvLost, said in the delivery's transitions; neither asks while the queue holds an ask of the head, so no +// ask is made twice. A queue that cannot be read asks nothing: a guess would risk a duplicate. + +// lostReadEvery is how often the queue is read for one proposed delivery's ask: the read is one bus call, and a check +// the queue holds is looked at again no sooner. +const lostReadEvery = 2 * time.Minute + +// lostCheckBound is how long a proposed delivery's asked check may go with no entry in the build queue before it is +// asked again: the repository's last measured check time (ADR 0505 §4, lastCheckTime), at least UncheckedAfter (ten +// minutes, the bound a check never asked is said at). Under the lock. +func (h *Holder) lostCheckBound(repository string) time.Duration { + return max(UncheckedAfter, h.lastCheckTime(repository)) +} + +// awaitedSince is when a proposed delivery's check was asked, when it waits for one this owner asked: no verdict yet, +// or a verdict out of date with its check against the trunk's head asked. False for a delivery waiting for none. +func awaitedSince(d *Delivery) (time.Time, bool) { + if d.State != Proposed || d.MergedAs != "" || d.Number == 0 { + return time.Time{}, false + } + switch { + case d.Check == nil && askedCheck(d): + return d.CheckAsk.At, true + case d.Check != nil && d.outOfDate() != "" && d.AskedAgain != nil: + return *d.AskedAgain, true + } + return time.Time{}, false +} + +// heldBy is the queue's entry that holds a check of the delivery's head: its own ask by id, or any check of its +// repository at its head — a group's composed check included, which a second ask would only repeat. +func heldBy(d *Delivery, asks []QueuedAsk) *QueuedAsk { + for i, a := range asks { + if !a.Holds() { + continue + } + if d.CheckAsk != nil && d.CheckAsk.ID != "" && a.ID == d.CheckAsk.ID { + return &asks[i] + } + if a.Kind == "check" && a.Commit != "" && sameCommit(a.Commit, d.Commit) && + strings.Contains(strings.ToLower(a.Repository), strings.ToLower(d.Repository)) { + return &asks[i] + } + } + return nil +} + +func (a QueuedAsk) words() string { + // The controller's `queue` says the state "in flight", its table's word. + state := strings.ReplaceAll(a.State, "-", " ") + if state == "in flight" && a.Machine != "" { + return a.ID + " is in flight on " + a.Machine + } + return a.ID + " is " + state + " in the build queue" +} + +// lostChecks are the asks again of proposed deliveries whose asked check the build queue holds no entry of, past the +// bound, at each look. Under the lock; the queue is read, and the asks made, outside it. +func (h *Holder) lostChecks(now time.Time) func() { + var ids []string + for id, d := range h.deliveries { + since, ok := awaitedSince(d) + if !ok { + delete(h.lostRead, id) + continue + } + due := since.Add(h.lostCheckBound(d.Repository)) + if now.Before(due) { + h.lookAt(due, "a proposed delivery's asked check, read in the build queue once its bound passed") + continue + } + if read, was := h.lostRead[id]; was && now.Sub(read) < lostReadEvery { + h.lookAt(read.Add(lostReadEvery), "a proposed delivery's asked check, looked for in the build queue again") + continue + } + h.lostRead[id] = now + h.lookAt(now.Add(lostReadEvery), "a proposed delivery's asked check, looked for in the build queue again") + ids = append(ids, id) + } + if len(ids) == 0 { + return nil + } + return func() { + asks, err := h.Controller.Queue() + if err != nil { + h.Logf("[mesh-delivery] the build queue cannot be read, so no check is asked again for a lost ask: %v", err) + return + } + for _, id := range ids { + h.mu.Lock() + d := h.deliveries[id] + since, ok := time.Time{}, false + if d != nil { + since, ok = awaitedSince(d) + } + var held *QueuedAsk + if ok { + held = heldBy(d, asks) + } + var why string + if ok && held == nil { + what := "its check against the trunk's head" + if d.Check == nil { + what = "its check " + d.CheckAsk.ID + } + why = fmt.Sprintf("%s, asked %s ago, is held by no entry of the build queue (neither waiting nor in flight): "+ + "lost, asked again", what, h.Now().Sub(since).Round(time.Second)) + } + h.mu.Unlock() + if why == "" { + continue + } + if _, err := h.askLost(id, byOwner, why, asks); err != nil { + h.Logf("[mesh-delivery] %s: its lost check could not be asked again: %v", id, err) + h.again("a proposed delivery's lost check that could not be asked again") + } + } + } +} + +// recheckProposed is `recheck` of a proposed delivery (issue 1274): taken, with why, when no check of its head is +// waiting or in flight in the build queue, and refused naming the one that is. A queue that cannot be read refuses it. +func (h *Holder) recheckProposed(id, why, by string) (string, error) { + if why == "" { + return "", fmt.Errorf("a recheck says why") + } + asks, err := h.Controller.Queue() + if err != nil { + return "", fmt.Errorf("%s is not rechecked: the build queue cannot be read (%v), so whether its check is still held is not known", id, err) + } + h.mu.Lock() + d := h.deliveries[id] + var held *QueuedAsk + if d != nil { + held = heldBy(d, asks) + } + h.mu.Unlock() + if held != nil { + return "", fmt.Errorf("%s is not rechecked: its check is still held, %s", id, held.words()) + } + return h.askLost(id, by, why, asks) +} + +// askLost asks a proposed delivery's check again, the table's EvLost: tried first on a copy, so an ask the controller +// does not take spends nothing. Its group's composed check goes out of date, and is asked once the member answers. +func (h *Holder) askLost(id, by, why string, asks []QueuedAsk) (string, error) { + h.mu.Lock() + d := h.deliveries[id] + if d == nil { + h.mu.Unlock() + return "", fmt.Errorf("no delivery %q", id) + } + if held := heldBy(d, asks); held != nil { + h.mu.Unlock() + return "", fmt.Errorf("%s is not asked again: its check is still held, %s", id, held.words()) + } + probe := *d + probe.Transitions = nil + if _, err := Apply(&probe, EvLost, true, Facts{Now: h.Now(), Why: why, By: by}); err != nil { + h.mu.Unlock() + return "", err + } + m := MemberOf(d) + if d.Check != nil && d.outOfDate() != "" && d.TrunkNow != "" { + m.BaseSHA = d.TrunkNow + } + h.mu.Unlock() + asked, err := h.Controller.Check("", []Member{m}) + if err != nil { + return "", fmt.Errorf("%s's check could not be asked again yet: %w", id, err) + } + h.mu.Lock() + defer h.mu.Unlock() + d = h.deliveries[id] + if d == nil || d.State != Proposed { + return "", fmt.Errorf("%s moved while its check was asked again", id) + } + now := h.Now() + t, err := Apply(d, EvLost, true, Facts{Now: now, Why: why + " — asked as " + asked, By: by}) + if err != nil { + return "", err + } + d.CheckAsk = &CheckAsk{ID: asked, At: now, By: by, Why: why} + if d.Check != nil { + d.AskedAgain, d.FreshWanted = &now, false + } + delete(h.askTried, id) + if g := h.groups[d.Group]; g != nil && g.Check != nil { + g.Check.OutOfDate = fmt.Sprintf("composed check out of date: %s's check was lost in the build queue and asked again", d.ID) + g.Check.OutOfDateAt = now + g.Updated = now + h.keepGroup(g) + } + h.moved(d, []Transition{t}) + return fmt.Sprintf("%s: its check was held by no entry of the build queue and is asked again as %s (%s)", id, asked, why), nil +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go index 03855f7c..e5ef06db 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go @@ -41,7 +41,24 @@ const forgePattern = "mesh-delivery-forge-bridge.forge.**" func main() { occupancy := &busOccupancy{} + settingsFile := os.Getenv("MESH_DELIVERY_GATE_SETTINGS") + var saidMu sync.Mutex + lastSaid := "" + settings := func() Settings { + s, err := readSettings(settingsFile) + saidMu.Lock() + defer saidMu.Unlock() + if err != nil && err.Error() != lastSaid { + logf("[mesh-delivery-gate] settings cannot be read, the defaults hold: %v", err) + } + lastSaid = "" + if err != nil { + lastSaid = err.Error() + } + return s + } h := &Holder{ + Settings: settings, Store: kvStore{}, Controller: seatController{ask: stdio.Ask}, Forge: toolForge{ask: stdio.Ask}, diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go index b1ca5f0f..4a9c41bc 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/manifest_test.go @@ -92,7 +92,7 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { // It calls exactly the controller's seven verbs (ADR 0450 §7) and reads the forge with three tools; the forge's // writes are the bridge's (ADR 0437 §8). It uses no operator-channel: the teller asks (ADR 0437 §6). want := []string{} - for _, v := range []string{"deliver", "delivery-stop", "delivery-backups-again", "delivery-walks", "delivery-check", "forget-checks", "plans"} { + for _, v := range []string{"deliver", "delivery-stop", "delivery-backups-again", "delivery-walks", "delivery-check", "forget-checks", "queue", "plans"} { want = append(want, "seat:"+ControllerSeat+"."+v) } want = append(want, "gitea.gitea_commit_statuses", "gitea.gitea_contains", "gitea.gitea_open_pulls") diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go index 4070027b..4075efff 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go @@ -284,9 +284,14 @@ func (h *Holder) ForgeRefused(f sdk.Forge) { r, now := d.Ready, h.Now() r.merging, r.refusedNoted = false, false answer := oneLine(f.Words) - if r.RefusedSince == nil { - r.RefusedSince = &now + var on time.Time + if d.Check != nil { + on = d.Check.At } + if r.RefusedSince == nil { + r.RefusedSince, r.FirstRefusedOn = &now, on + } + r.LastRefusedOn = on r.noteRefusal(d, answer) if !outOfDateRefusal(answer) { r.Urgent = true // never a warning again for this head diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go index 9425bfbe..f025d9a9 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go @@ -135,6 +135,9 @@ type Delivery struct { // hold: no Approve is asked until it names the head (novox/hq ADR 0496, issue 1803). ReviewedBy string `json:"reviewed_by,omitempty"` ReviewedAt string `json:"reviewed_at,omitempty"` + // PushedBy is the session that pushed its head, as its pull request's description names it in a line + // `Pushed-by: ` (playbook 07): named with its branch when a new head is needed (novox/hq ADR 0505 §5). + PushedBy string `json:"pushed_by,omitempty"` // FreshWanted is whether a passing verdict that arrived out of date is to have its check asked against the // trunk's head, before the operator is asked (novox/hq ADR 0421 §3, issue 1022). Cleared when it is asked. FreshWanted bool `json:"fresh_wanted,omitempty"` @@ -498,6 +501,31 @@ func ReviewIn(body string) (who, sha string) { return who, sha } +// pushedLine is the line naming the session that pushed a pull request's head: `Pushed-by: ` (ADR 0505 §5). +var pushedLine = regexp.MustCompile(`(?im)^\s*Pushed-by:\s*([A-Za-z0-9_.\-]+(?:@[A-Za-z0-9_.\-]+)?)\s*$`) + +// PushedIn reads it: the last such line, or "" when there is none. +func PushedIn(body string) string { + who := "" + for _, m := range pushedLine.FindAllStringSubmatch(body, -1) { + who = m[1] + } + return who +} + +// authorOf is a delivery's author as the operator reads it: "the author of ", and the session that pushed it +// when its description names one ("who on node": the router refuses an @ in its words). +func authorOf(d *Delivery) string { + who := "the author of " + d.Branch + if d.Branch == "" { + who = "its author" + } + if d.PushedBy != "" { + who += " (" + strings.Replace(d.PushedBy, "@", " on ", 1) + ")" + } + return who +} + // reviewWords is what the delivery's review record says of its head, for the ask: reviewed by whom, or, first, that it // is not reviewed at this head (the ask is held then, ADR 0496). ok is whether it is reviewed at this head. func reviewWords(d *Delivery) (words string, ok bool) { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go index 4cb8767f..c2af77b7 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go @@ -67,8 +67,24 @@ type Controller interface { Walks(walk string) (bool, []Walk, error) // ForgetChecks lets go of the builds a pull request's merge checks made for its merge (novox/hq ADR 0350 §5). ForgetChecks(owner, repo string, number int) error + // Queue is every ask the build queue holds, waiting or in flight (the controller's `queue`, novox/hq issue 455): what + // tells a check still on its way from one the queue lost (issue 1274). A dead ask is not held. + Queue() ([]QueuedAsk, error) } +// QueuedAsk is one ask of the build queue as the controller's `queue` answers it as data. +type QueuedAsk struct { + ID string `json:"id"` + State string `json:"state"` // waiting, in flight or dead + Kind string `json:"kind"` // check or delivery + Repository string `json:"repository"` + Commit string `json:"commit,omitempty"` + Machine string `json:"machine,omitempty"` +} + +// Holds is whether the ask is still the queue's to run: waiting or in flight, never dead. +func (q QueuedAsk) Holds() bool { return q.State != "dead" } + // Forge is what the forge's holder does for a delivery. type Forge interface { // Statuses reads a commit's statuses — a pull request's head's, given its number — and whether the branch @@ -250,6 +266,20 @@ func (c seatController) Check(group string, members []Member) (string, error) { return "", fmt.Errorf("the controller's delivery-check answered no check id (%s): it is not taken as asked", clip(string(raw), 200)) } +func (c seatController) Queue() ([]QueuedAsk, error) { + raw, err := c.verb("queue", map[string]any{}) + if err != nil { + return nil, err + } + var a struct { + Asks []QueuedAsk `json:"asks"` + } + if err := json.Unmarshal(raw, &a); err != nil { + return nil, fmt.Errorf("the controller's queue is not readable: %w", err) + } + return a.Asks, nil +} + func (c seatController) Deliver(walk, why string) error { _, err := c.verb("deliver", map[string]any{"plan": walk, "why": why}) return err diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go index 77737660..b68d1017 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go @@ -35,7 +35,9 @@ import ( // delivery (Ready), said in its log, and the merge's transitions name the Approve as their warrant. // readyRefusedFor is how long the merge tool may refuse an approved head before it is said as stalled: none (issue 1115) — -// a refusal is said at once, and the merge is asked again at each event that can change it, with no timer. +// a refusal is said at once, and the merge is asked again at each event that can change it, with no timer. A refusal the +// gate is already answering (its trunk moved and its check was asked again) is said only once that check answered and +// the merge is still refused, or twice the repository's last check time passed (novox/hq ADR 0505 §4, `answering`). const readyRefusedFor time.Duration = 0 // groupRefusedFor is how long the merge tool may refuse a closed group's member before it is said as stalled: none (issue 1115). @@ -92,6 +94,11 @@ type ReadyRun struct { RepeatTrunk string `json:"repeat_trunk,omitempty"` // RefusedSince is when the merge tool began refusing it without a break. RefusedSince *time.Time `json:"refused_since,omitempty"` + // FirstRefusedOn and LastRefusedOn are when the verdict the merge was asked on was heard, at the first refusal of + // the run of refusals and at the last: a later one than the first is a fresh check answered and refused again + // (novox/hq ADR 0505 §4). + FirstRefusedOn time.Time `json:"first_refused_on,omitzero"` + LastRefusedOn time.Time `json:"last_refused_on,omitzero"` // Merged is when the merge tool merged it. Merged *time.Time `json:"merged,omitempty"` // Failed is why it is not merged and will not be on this head: its check failed. Said once on the pull request. @@ -473,15 +480,25 @@ func (h *Holder) readyLines(now time.Time) []StalledLine { Bound: headHeardBound.String(), H2: "none: a later announcement of the pull request's head (the merge tool announces it again, the forge's announcer restarts, or a new push) or an edit of its description; a recheck does not fill it — the operator's", Says: fmt.Sprintf("%s#%d passed its checks at %s and waits for %s", d.Repository, d.Number, shortOf(r.Head, 12), waitsForHead)}) case r.Failed != "" && d.State == Rejected: - out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(r.Seen).Round(time.Second).String(), - Bound: "0s", H2: "none: a new head, or a recheck that passes — the operator's", - Says: fmt.Sprintf("%s#%d passed its checks at %s and is not merged: %s", d.Repository, d.Number, - shortOf(r.Head, 12), r.Failed)}) + // Its cure is its author's new head: listed here at once (`show`), told on the pull request at once, and the + // operator's only once no new head came within the author's bound (novox/hq ADR 0505 §5). + if now.Sub(d.Since) < h.authorBound() { + continue + } + out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(d.Since).Round(time.Second).String(), + Bound: h.authorBound().String(), H2: "none: a new head from its author, or a recheck that passes", + Needs: NeedsNewHead, Author: authorOf(d), Branch: d.Branch, + Says: fmt.Sprintf("%s#%d passed its checks at %s and is not merged: %s. It needs a new head from %s, "+ + "and none came for %s", d.Repository, d.Number, shortOf(r.Head, 12), r.Failed, authorOf(d), + now.Sub(d.Since).Round(time.Minute))}) case r.Refused != "": out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(r.Seen).Round(time.Second).String(), Bound: "0s", H2: "none: a new head, which asks the operator again — the operator's", Says: fmt.Sprintf("%s#%d at %s is not merged: the operator's answer %s", d.Repository, d.Number, shortOf(r.Head, 12), r.Refused)}) + case h.answering(d, now): + // The trunk moved and its check was asked again: the gate is answering it, so it is no stall yet (ADR 0505 + // §4). `show` lists the refusal meanwhile. case r.RefusedSince != nil && d.Group != "" && h.groupClosed(d.Group) && now.Sub(*r.RefusedSince) >= groupRefusedFor: // A closed group's member (novox/hq ADR 0418 §3, issue 1082): its sibling merged, and its own merge is refused // again after each composed check. Said after the merge window, with the group and the tool's last answer. @@ -542,6 +559,22 @@ func neededOf(d *Delivery) string { return ". Needed: the refusal's cause is met (the merge is asked again at each verdict, status or Approve), or the operator decides." } +// answering is whether an approved head the merge tool refused is one the gate is already answering, and so not said as +// stalled yet (novox/hq ADR 0505 §4): refused only because its trunk moved, its check asked again, and neither has that +// check answered with the merge refused again after it, nor has twice the repository's last check time (at least the +// floor) passed since the refusals began. Any other refusal of this head, and a refusal repeated past its limit, is said +// at once (r.Urgent). Under the lock. +func (h *Holder) answering(d *Delivery, now time.Time) bool { + r := d.Ready + if r == nil || r.RefusedSince == nil || r.Urgent || !outOfDateRefusal(r.Answer) { + return false + } + if r.LastRefusedOn.After(r.FirstRefusedOn) { + return false // its fresh check answered, and the merge was refused again on it + } + return now.Sub(*r.RefusedSince) < h.answeredRefusalBound(d.Repository) +} + // outOfDateFor is how long a refusal for an out of date verdict is a warning before it is urgent (issue 1275, AC6). const outOfDateFor = 10 * time.Minute diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settings.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settings.go new file mode 100644 index 00000000..8bda6e77 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/settings.go @@ -0,0 +1,83 @@ +package main + +// The gate's settings (novox/hq ADR 0505 §4, §5), merged by the mesh into one JSON file the env names. Each has a +// default here, so a gate started without the file, or with a value it cannot read, keeps the decided bound and says +// why once. + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "time" +) + +const ( + // DefaultRefusalFloor is the least a refusal the gate is already answering (the trunk moved and its check was + // asked again) waits before it is said as stalled, when the setting refusal-stall-floor-minutes gives none. + DefaultRefusalFloor = 10 * time.Minute + // DefaultAuthorBound is how long a delivery whose cure is its author's new head waits for one before the operator + // hears of it, when the setting author-new-head-minutes gives none (ADR 0505 §5). + DefaultAuthorBound = 2 * time.Hour +) + +// Settings are the operator's values for this module. +type Settings struct { + RefusalFloorMinutes *int `json:"refusal-stall-floor-minutes"` + AuthorBoundMinutes *int `json:"author-new-head-minutes"` +} + +// readSettings reads the settings file; a value that is not a whole number of minutes, one or more, is refused. +func readSettings(path string) (Settings, error) { + var s Settings + if path == "" { + return s, errors.New("started without a settings file") + } + raw, err := os.ReadFile(path) + if err != nil { + return s, err + } + var loose map[string]any + if err := json.Unmarshal(raw, &loose); err != nil { + return s, fmt.Errorf("the settings file is not JSON: %v", err) + } + for name, into := range map[string]**int{"refusal-stall-floor-minutes": &s.RefusalFloorMinutes, + "author-new-head-minutes": &s.AuthorBoundMinutes} { + switch v := loose[name].(type) { + case nil: + case float64: + n := int(v) + if v < 1 || v != float64(n) { + return Settings{}, fmt.Errorf("%s is not a whole number of minutes, one or more", name) + } + *into = &n + default: + return Settings{}, fmt.Errorf("%s is not a number", name) + } + } + return s, nil +} + +// minutesOr is a setting's minutes, or the default when it gives none. +func minutesOr(n *int, def time.Duration) time.Duration { + if n == nil { + return def + } + return time.Duration(*n) * time.Minute +} + +// refusalFloor is the least an answered refusal waits before it is said as stalled (ADR 0505 §4). +func (h *Holder) refusalFloor() time.Duration { + if h.Settings == nil { + return DefaultRefusalFloor + } + return minutesOr(h.Settings().RefusalFloorMinutes, DefaultRefusalFloor) +} + +// authorBound is how long a delivery that needs its author's new head waits before the operator hears of it (§5). +func (h *Holder) authorBound() time.Duration { + if h.Settings == nil { + return DefaultAuthorBound + } + return minutesOr(h.Settings().AuthorBoundMinutes, DefaultAuthorBound) +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go index b8ed6d08..96826ae5 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go @@ -69,6 +69,7 @@ const ( EvRefused Event = "refused" // the verdict fails, or the check could not run EvRecheck Event = "recheck" // a person asked for it again, or its group changed EvReask Event = "re-ask" // healer H2 asked a check asked and not answered once more + EvLost Event = "check lost" // the build queue holds no check of its head and it was asked: asked again (issue 1274) EvNewHead Event = "new-head" // a newer head of the same pull request EvClosed Event = "closed" // the pull request closed unmerged EvMerged Event = "merged" // it reached the trunk @@ -240,6 +241,24 @@ var Table = []Row{ return nil }}, + // **A check the build queue lost is asked again** (novox/hq issue 1274): a proposed delivery waits for a check, and + // the queue, which the table cannot read, holds none of its head. The act (the sweep's or a person's recheck) reads + // the queue first and says why; the table keeps it to a delivery that waits for a verdict. + {From: []State{Proposed}, Event: EvLost, To: Proposed, Act: true, + Guard: "it waits for a check — no verdict yet, or one out of date — nothing of its head is waiting or in flight in " + + "the build queue, and who asks says why", + Holds: func(d *Delivery, f Facts) error { + switch { + case f.Why == "" || f.By == "": + return errors.New("a lost check's ask says who and why") + case d.MergedAs != "": + return errors.New("it is merged") + case d.Check != nil && d.outOfDate() == "": + return errors.New("its verdict is known") + } + return nil + }}, + {From: []State{Published}, Event: EvSuperseded, To: Superseded, Guard: "its walk was superseded and names no walk that took it over: a walk that took it over answers its merge, and it follows that walk (novox/hq ADR 0276)", Holds: func(d *Delivery, f Facts) error { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go index 573e4935..87f5b5c9 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go @@ -43,7 +43,7 @@ func aDeliveryFor(r Row) (*Delivery, Facts) { d.Check = &Verdict{Gate: "pass"} case EvRefused: d.Check = &Verdict{Gate: "fail"} - case EvRecheck, EvRelease, EvStop: + case EvRecheck, EvRelease, EvStop, EvLost: f.Why, f.By = "a person's reason", "jochen" if r.Event == EvStop && !r.Act { f = Facts{Now: now} @@ -133,7 +133,7 @@ func TestEveryRowOfTheTableIsTakenWhenItsGuardHoldsAndRefusedWhenNot(t *testing. } func TestEveryPairTheTableDoesNotHoldIsRefusedByName(t *testing.T) { - events := []Event{EvAnnounced, EvAppeared, EvAdopted, EvChecked, EvAccepted, EvRefused, EvRecheck, EvReask, EvNewHead, + events := []Event{EvAnnounced, EvAppeared, EvAdopted, EvChecked, EvAccepted, EvRefused, EvRecheck, EvReask, EvLost, EvNewHead, EvClosed, EvMerged, EvMergedUnchecked, EvGo, EvHold, EvRelease, EvDone, EvFailed, EvSuperseded, EvStop, EvHistory, EvRetire, EvCovered, EvLifted, EvRewalk, EvTrunkMoved, EvBackupFailed} for _, from := range append([]State{None}, AllStates...) { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go index 3545cfcd..27e876c5 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/verbs.go @@ -233,8 +233,16 @@ type StalledLine struct { Says string `json:"says"` // Severity is "urgent" for a group that is held or half merged (issue 1275); empty otherwise. Severity string `json:"severity,omitempty"` + // Needs is NeedsNewHead for a delivery whose cure is its author's new head (novox/hq ADR 0505 §5), with Author + // naming that author as the operator reads it and Branch its pull request's branch; empty otherwise. + Needs string `json:"needs,omitempty"` + Author string `json:"author,omitempty"` + Branch string `json:"branch,omitempty"` } +// NeedsNewHead is what a stalled line needs when its cure is its author's new head (ADR 0505 §5). +const NeedsNewHead = "new-head" + // Stalled is the `stalled` verb: what the controller's self-check raises as `delivery..stalled`. func (h *Holder) Stalled() []StalledLine { h.mu.Lock() @@ -295,6 +303,11 @@ func (h *Holder) Recheck(id, why, by string) (string, error) { h.mu.Unlock() return "", fmt.Errorf("no delivery %q", id) } + if d.State == Proposed { + // A proposed delivery's check is asked again when the build queue holds none of its head (issue 1274). + h.mu.Unlock() + return h.recheckProposed(id, why, by) + } // Whether the table takes it, tried on a copy: nothing is said on the forge for a recheck it refuses. probe := *d probe.Transitions = append([]Transition(nil), d.Transitions...) diff --git a/modules/mesh-delivery-gate/files/settings.json b/modules/mesh-delivery-gate/files/settings.json new file mode 100644 index 00000000..848f5158 --- /dev/null +++ b/modules/mesh-delivery-gate/files/settings.json @@ -0,0 +1,4 @@ +{ + "refusal-stall-floor-minutes": 10, + "author-new-head-minutes": 120 +} diff --git a/modules/mesh-delivery-gate/module.json b/modules/mesh-delivery-gate/module.json index a80fad26..b9a06d0e 100644 --- a/modules/mesh-delivery-gate/module.json +++ b/modules/mesh-delivery-gate/module.json @@ -47,6 +47,7 @@ "seat:mesh-controller.delivery-walks", "seat:mesh-controller.delivery-check", "seat:mesh-controller.forget-checks", + "seat:mesh-controller.queue", "seat:mesh-controller.plans", "gitea.gitea_commit_statuses", "gitea.gitea_contains", @@ -84,6 +85,24 @@ "tools": [ "delivery_status" ], + "config": { + "refusal-stall-floor-minutes": { + "type": "number", + "description": "minutes, at least, before a merge refused because its trunk moved, whose check the gate asked again, is said as stalled; twice the repository's last check time when that is longer (ADR 0505 §4).", + "minimum": 1, + "maximum": 1440, + "trusted": false, + "default": 10 + }, + "author-new-head-minutes": { + "type": "number", + "description": "minutes a delivery that needs its author's new head (a conflict, a failed check, a rejected group) waits for one before the operator hears of it (ADR 0505 §5).", + "minimum": 1, + "maximum": 10080, + "trusted": false, + "default": 120 + } + }, "resources": [ { "id": "state", @@ -91,6 +110,18 @@ "mode": "0700", "place": ".", "unknown": "program" + }, + { + "id": "settings", + "type": "file", + "path": "${dir:state}/settings.json", + "mode": "0600", + "takes-config": [ + "refusal-stall-floor-minutes", + "author-new-head-minutes" + ], + "merge": "json", + "template": "files/settings.json" } ], "build": { @@ -104,7 +135,15 @@ "binary": "mesh-delivery-gate", "loads": [ "mesh-delivery-gate" - ] + ], + "env": { + "MESH_DELIVERY_GATE_SETTINGS": "${dir:state}/settings.json" + } + }, + { + "name": "files", + "kind": "archive", + "from": "files" } ] } diff --git a/modules/messenger/README.md b/modules/messenger/README.md index 5a5a26ba..0fbd1a41 100644 --- a/modules/messenger/README.md +++ b/modules/messenger/README.md @@ -190,17 +190,28 @@ the next minute's sweep. history. An event that cannot be read is refused by name, counted, and told to the operator once an hour. -## While a delivery walks (novox/hq ADR 0308) +## While a delivery walks (novox/hq ADR 0308, ADR 0505) The controller's `plan-moved` says each walk's start and end: `walk`, `phase` (`started`, `ended`), `touches` -(`[{machine, modules}]`: what it moves and what a moved tool runner or node-engine carries), `outcome` +(`[{machine, modules, declared}]`: what it moves and what a moved tool runner or node-engine carries, and, marked +`declared`, a machine it sends nothing to whose declaration its merges change), `outcome` (`delivered`, `failed`, `stopped`), `bound_seconds` and `budget_class` (`cmd/messenger/walks.go`). -- **Held:** while a walk is open and within its bound, a message whose subject is a machine it touches, or a +- **Held:** while a walk is open and within its bound and settle, a message whose subject is a machine it touches, or a module it touches (on a machine it touches, when the subject names one), is held: kept on the walk with when and what, and the history says `held`. A message several open walks touch is told when the last ends or passes its bound. At a walk's end, a later open walk that touches a message takes it over only as a first hold would: - still open, not silenced, not said since, and not one that is never held. + still open, not silenced, not said since, and not one that is never held. The controller's own lease, bus and + health conditions (`core.controller.…`, `bus.controller.…`) are about the module `mesh-controller`, wherever a + walk moves it (ADR 0505 §2). +- **The settle (ADR 0505 §3, §6):** a walk that ended delivered holds on for the setting `settle-seconds` + (120 by default): what it touched stays held, a condition raised then on what it touched is held too, and its + end is told when the settle has passed. A walk that failed or stopped is told at once; for its settle after + that, a condition on what it touched is sent at once, never held. +- **The delivery line (ADR 0505 §1):** every problem's message (raised, asked, reopened, escalated, reminded; not + a clearing, a silencing, a plain notice or an ask's own message) ends, after its since-when line, with "During + the delivery of to .", " after the delivery of to , which + succeeded/failed/was stopped." or "No delivery running.", by the walks that touch its subject. - **Never held:** an ask waiting for the operator (and a condition an open ask is about), a secret given or replaced, a subject no open walk touches, and a walk's own failure (a plan, walk, delivery or build). - **The end is one message:** the outcome, the modules and machines reached, how long; each held message still @@ -209,7 +220,7 @@ The controller's `plan-moved` says each walk's start and end: `walk`, `phase` (` only once the message was sent there. Until then each is kept owed in the state (`owed`): a message lost with a restart before it was sent leaves them untold and owed, and the restarted router says each on its own. A record that clears owes nothing more. -- **The bound:** at its start plus `bound_seconds` (900 when not given), what is held and still open is said, +- **The bound:** at its start plus `bound_seconds` (900 when not given) plus its settle (ADR 0505 §7), what is held and still open is said, with "The walk … is still running, since HH:MM", and the cleared counted; the walk holds nothing more. A walk whose end never comes is forgotten a day after it started. A start heard after its end opens nothing: the controller's moments are compared on its own clock, never with one capped at the router's now. @@ -221,6 +232,7 @@ The controller's `plan-moved` says each walk's start and end: `walk`, `phase` (` said in it; not given, the zone of the machine this runs on. **The kinds that say only what needs the operator**, as a setting: `{"only-what-needs-me": [""]}`. A walk that delivered with nothing held and nothing open is not told on them (ADR 0308 §4); empty by default. + **The settle**, as a setting: `{"settle-seconds": 120}`: how long a delivered walk still holds (ADR 0505 §3). 2. **The channels**: assign `telegram` and `desk-channel` (each says in its README what it needs). Linking an account as the operator is done on the channel, never by a setting. diff --git a/modules/messenger/cmd/messenger/adr505_bus_test.go b/modules/messenger/cmd/messenger/adr505_bus_test.go new file mode 100644 index 00000000..440ca55e --- /dev/null +++ b/modules/messenger/cmd/messenger/adr505_bus_test.go @@ -0,0 +1,86 @@ +//go:build integration + +package main + +import ( + "encoding/json" + "strings" + "testing" + "time" +) + +// novox/hq ADR 0505 over the real bus, in the bed of condition_bus_test.go: the router whole, its walks kept in its +// state, the controller's walk and condition events handed to it as the runtime hands them. + +// controllerEvent hands the bundle one of the controller's events, as the runtime does. +func (b *conditionBed) controllerEvent(t *testing.T, event string, body any) { + t.Helper() + raw, err := json.Marshal(body) + if err != nil { + t.Fatal(err) + } + if _, err := b.r.request("mesh/event", map[string]any{"envelope": map[string]any{"key": ControllerSeat + "." + event, + "node": "test", "body": json.RawMessage(raw)}}); err != nil { + t.Fatal(err) + } +} + +// walkEvent is a plan-moved body as the controller says a walk's start or end (mesh-controller walk_said.go), with a +// machine it sends nothing to and whose declaration its merges change (`declared`, ADR 0505 §2b). +func walkEvent(phase, outcome string, started, at time.Time) map[string]any { + b := map[string]any{"id": "plan-bus", "state": "walking", "walk": "plan-bus", "phase": phase, "budget_class": "leaf", + "bound_seconds": 600, "started_at": started.UTC().Format(time.RFC3339Nano), + "touches": []map[string]any{{"machine": "novox", "modules": []string{"gitea"}}, + {"machine": "shanks", "modules": []string{"mesh-build-image"}, "declared": true}}} + if phase == WalkEnded { + b["outcome"], b["ended_at"] = outcome, at.UTC().Format(time.RFC3339Nano) + } + return b +} + +// conditionEvent is a condition as the controller raises it (mesh-controller probes.go D1, signals.go). +func conditionEvent(key, scope, id, machine, headline string) map[string]any { + now := time.Now().UTC().Format(time.RFC3339) + return map[string]any{"key": key, "kind": key[strings.LastIndex(key, ".")+1:], "severity": Urgent, + "subject": map[string]string{"scope": scope, "id": id, "machine": machine}, "summary": headline, + "headline": headline, "explanation": "Needs you: look at it. " + headline + ".", "needs": "look at it.", + "raised": now, "at": now} +} + +func (b *conditionBed) toldWith(words string) *string { + for _, m := range b.told() { + if strings.Contains(m.Title+"\n"+m.Body, words) { + text := m.Title + "\n" + m.Body + return &text + } + } + return nil +} + +// ADR 0505 §1, §2b, §3: during a walk, a condition on a machine whose declaration it changes is held, and one on an +// untouched machine reaches the desk at once ending "No delivery running."; after the walk delivered, the held one +// reaches the desk only once the settle (two seconds here) has passed, inside the walk's end message. +func TestAWalksDeclaredMachineIsHeldUntilItSettledOverTheBus(t *testing.T) { + b := newConditionBedWith(t, `{"settle-seconds": 2}`) + started := time.Now() + b.controllerEvent(t, EventPlanMoved, walkEvent(WalkStarted, "", started, started)) + b.controllerEvent(t, EventRaised, conditionEvent("machine.shanks.uncomposable", "machine", "shanks", "shanks", + "Shanks cannot get new instructions")) + b.controllerEvent(t, EventRaised, conditionEvent("machine.ace.silent", "machine", "ace", "ace", "Ace is silent")) + b.r.until("the untouched machine's condition reaches the desk with its delivery line", 15*time.Second, func() bool { + m := b.toldWith("Ace is silent") + return m != nil && strings.HasSuffix(*m, "\nNo delivery running.") + }) + if b.toldWith("Shanks cannot get new instructions") != nil { + t.Fatalf("the declared machine's condition was told during the walk") + } + b.controllerEvent(t, EventPlanMoved, walkEvent(WalkEnded, WalkDelivered, started, time.Now())) + time.Sleep(time.Second) + if b.toldWith("Shanks cannot get new instructions") != nil || b.toldWith("Delivered") != nil { + t.Fatalf("told before the settle passed: %+v", b.told()) + } + b.r.until("the walk's end tells what it held once it settled", 30*time.Second, func() bool { + m := b.toldWith("Still open after the delivery of gitea to novox:") + return m != nil && strings.Contains(*m, "Shanks cannot get new instructions") && !strings.Contains(*m, "shanks,") + }) +} diff --git a/modules/messenger/cmd/messenger/adr505_test.go b/modules/messenger/cmd/messenger/adr505_test.go new file mode 100644 index 00000000..541b0f4f --- /dev/null +++ b/modules/messenger/cmd/messenger/adr505_test.go @@ -0,0 +1,409 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// The tests of novox/hq ADR 0505 "How it is checked" for the router: what a delivery changes is held until it has +// settled, every problem message says whether a delivery ran, and a failed delivery is told at once. + +// ADR 0505 §3: the settle is a setting, settle-seconds, two minutes when none is given. +func TestTheSettleIsReadFromTheSettingsFileAndIsTwoMinutesByDefault(t *testing.T) { + read := func(body string) Settings { + p := filepath.Join(t.TempDir(), "settings.json") + if err := os.WriteFile(p, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + s, err := readSettings(p) + if err != nil { + t.Fatal(err) + } + return s + } + if s := read(`{"settle-seconds": 60}`); s.SettleSeconds == nil || *s.SettleSeconds != 60 { + t.Fatalf("set: %+v", s) + } + if s := read(`{"time-zone": ""}`); s.SettleSeconds != nil { + t.Fatalf("unset: %+v", s) + } + h := &Holder{} + if got := h.settle(); got != 2*time.Minute { + t.Fatalf("no setting: settle %s, want 2m", got) + } + h.Settle = func() time.Duration { return time.Minute } + if got := h.settle(); got != time.Minute { + t.Fatalf("a setting of 60 s: settle %s, want 1m", got) + } + h.Settle = func() time.Duration { return 0 } + if got := h.settle(); got != 0 { + t.Fatalf("a setting of 0: settle %s, want none", got) + } +} + +// controllerConditions are the controller's own conditions as it raises them: the lease's (mesh-controller +// signals.go watchLease: scope core, id controller.lease, the machine its host names, which need not be a node's +// name) and the bus's (reconnects.go: scope bus, id the bus user). +func controllerConditions(c *clock) []Condition { + lease := cond("core.controller.lease.lost", Urgent, "the controller lost its lease") + lease.ID, lease.Machine, lease.Raised = "controller.lease", "a-host-name", c.now() + bus := cond("bus.controller.bus-reconnects", Urgent, "the controller reconnects to the bus") + bus.Raised = c.now() + health := cond("module.controller.unhealthy", Urgent, "the controller is not healthy") + health.Raised = c.now() + return []Condition{lease, bus, health} +} + +// ADR 0505 §2a: the controller's lease, bus and health conditions are held while a walk moving mesh-controller is +// open, whatever name their keys give the controller and whatever host they name. +func TestTheControllersOwnConditionsAreHeldWhileAWalkMovesMeshController(t *testing.T) { + h, tg, dt, c, _ := newHolder(t) + walk(t, h, "plan-c", WalkStarted, "", c.now(), 1200, novox("mesh-controller")) + for _, k := range controllerConditions(c) { + h.Condition(EventRaised, k) + } + runFor(h, c, time.Minute) + if saidOn(tg, dt) != 0 { + t.Fatalf("the controller's own conditions were said during its delivery: %+v %+v", tg.sends, dt.sends) + } + for _, k := range controllerConditions(c) { + if !has(outcomes(h, k.Key), "held") { + t.Errorf("%s was not held: %v", k.Key, outcomes(h, k.Key)) + } + } +} + +// ADR 0505 §2a: a walk that does not move mesh-controller holds none of them. +func TestTheControllersOwnConditionsAreNotHeldByAWalkOfAnotherModule(t *testing.T) { + h, tg, dt, c, _ := newHolder(t) + walk(t, h, "plan-g", WalkStarted, "", c.now(), 1200, Touch{Machine: "ace", Modules: []string{"gitea"}}) + for _, k := range controllerConditions(c) { + h.Condition(EventRaised, k) + } + runFor(h, c, time.Minute) + for _, k := range controllerConditions(c) { + if has(outcomes(h, k.Key), "held") || !heard(headlineOf(k.Key), tg, dt) { + t.Errorf("%s was held by a walk of gitea, or not said: %v", k.Key, outcomes(h, k.Key)) + } + } +} + +// shanksDeclared is a touch as the controller says a machine whose declaration the walk's merges change and to which +// it sends nothing (mesh-controller walk_said.go walkTouch, `declared`, ADR 0505 §2b). +var shanksDeclared = Touch{Machine: "shanks", Modules: []string{"mesh-build-image"}, Declared: true} + +// ADR 0505 §2b: a machine whose declaration a walk's merges change is held like a machine it sends to, and is not +// named among the machines the walk reached. +func TestAMachineWhoseDeclarationTheWalkChangesIsHeldAndNotNamedAsReached(t *testing.T) { + ev, ok, err := DecodeWalk([]byte(`{"id":"plan-d","walk":"plan-d","phase":"started","budget_class":"leaf",` + + `"bound_seconds":600,"touches":[{"machine":"novox","modules":["gitea"]},` + + `{"machine":"shanks","modules":["mesh-build-image"],"declared":true}]}`)) + if err != nil || !ok || len(ev.Touches) != 2 || !ev.Touches[1].Declared || ev.Touches[0].Declared { + t.Fatalf("a declared touch was not read: %+v %v %v", ev.Touches, ok, err) + } + h, tg, dt, c, _ := newHolder(t) + walk(t, h, "plan-d", WalkStarted, "", c.now(), 600, novox("gitea"), shanksDeclared) + k := cond("machine.shanks.uncomposable", Urgent, "shanks cannot get new instructions") + k.Machine, k.Raised = "shanks", c.now() + h.Condition(EventRaised, k) + runFor(h, c, time.Minute) + if !has(outcomes(h, k.Key), "held") || heard(headlineOf(k.Key), tg, dt) { + t.Fatalf("a condition on the declared machine was not held: %v", outcomes(h, k.Key)) + } + if got := touchWords(mergeTouches([]Touch{novox("gitea"), shanksDeclared}, nil)); got != "gitea on novox" { + t.Fatalf("what the walk reached reads %q, want only novox", got) + } + if got := mergeTouches([]Touch{shanksDeclared}, []Touch{{Machine: "shanks", Modules: []string{"buildah"}}}); got[0].Declared { + t.Fatalf("a machine the walk also sends to stayed declared: %+v", got) + } +} + +// ADR 0505 §3: a condition raised 30 seconds after a delivered walk's end, on what it touched, is held; at the end +// plus the settle the end message tells it under "after the delivery of …" when it is still open, and only counts +// it when it cleared. +func TestAConditionRaisedInTheSettleIsHeldAndToldAtItsEnd(t *testing.T) { + for _, clears := range []bool{false, true} { + h, tg, dt, c, _ := newHolder(t) + walk(t, h, "plan-s", WalkStarted, "", c.now(), 600, novox("gitea")) + runFor(h, c, 3*time.Minute) + walk(t, h, "plan-s", WalkEnded, WalkDelivered, c.now(), 0, novox("gitea")) + c.pass(30 * time.Second) + k := cond("machine.novox.uncomposable", Urgent, "novox cannot get new instructions") + k.Machine, k.Raised = "novox", c.now() + h.Condition(EventRaised, k) + runFor(h, c, time.Minute) + if heard(headlineOf(k.Key), tg, dt) || !has(outcomes(h, k.Key), "held") { + t.Fatalf("raised in the settle and not held: %v", outcomes(h, k.Key)) + } + if heard("Delivered", tg, dt) { + t.Fatalf("the end was told before the settle passed: %+v", dt.sends) + } + if clears { + h.Condition(EventCleared, Condition{Key: k.Key, At: c.now()}) + } + runFor(h, c, time.Minute) + switch { + case !clears && !heard("Still open after the delivery of gitea to novox:\n- Urgent: "+headlineOf(k.Key), tg, dt): + t.Fatalf("still open at the settle's end and not told under the delivery: %+v", dt.sends) + case clears && (heard(headlineOf(k.Key), tg, dt) || !heard("1 problem it raised resolved during it.", dt)): + t.Fatalf("cleared in the settle: told, or not counted: %+v", dt.sends) + } + } +} + +// ADR 0505 §3: a reminder of an older condition that falls on a touched subject while a walk is open is held like +// any other message, and told at the settle's end only when the condition is still open. +func TestAReminderDuringAWalkIsHeld(t *testing.T) { + h, tg, dt, c, _ := newHolder(t) + k := cond("machine.novox.network", Urgent, "novox lost its default route") + k.Raised = c.now() + h.Condition(EventRaised, k) + runFor(h, c, time.Minute) + first := saidOn(tg, dt) + if first == 0 { + t.Fatalf("the condition was not said before the walk") + } + runFor(h, c, RemindUrgent-3*time.Minute) + walk(t, h, "plan-r", WalkStarted, "", c.now(), 1200, novox("node-tools")) + runFor(h, c, 5*time.Minute) + if saidOn(tg, dt) != first { + t.Fatalf("the reminder was said during the walk: %+v", dt.sends) + } + if !has(outcomes(h, k.Key), "held") { + t.Fatalf("the reminder was not held: %v", outcomes(h, k.Key)) + } + walk(t, h, "plan-r", WalkEnded, WalkDelivered, c.now(), 0) + runFor(h, c, DefaultSettle+time.Minute) + if !heard("Still open after the delivery of node-tools to novox:", dt) { + t.Fatalf("the held reminder was not told at the settle's end: %+v", dt.sends) + } +} + +// ADR 0505 §3: a router that starts again while a walk settles holds on and tells its end once the settle passed. +func TestAWalkThatSettlesSurvivesARouterRestart(t *testing.T) { + h, tg, dt, c, st := newHolder(t) + walk(t, h, "plan-x", WalkStarted, "", c.now(), 1200, novox("mesh-tools")) + runFor(h, c, time.Minute) + walk(t, h, "plan-x", WalkEnded, WalkDelivered, c.now(), 0) + c.pass(10 * time.Second) + again := &Holder{Away: tg, Desk: dt, Store: st, Now: c.now, Logf: t.Logf} + if err := again.Load(); err != nil { + t.Fatal(err) + } + again.Start() + k := cond("machine.novox.tools-silent", Urgent, "the tools on novox are silent") + k.Raised = c.now() + again.Condition(EventRaised, k) + runFor(again, c, time.Minute) + if heard(headlineOf(k.Key), tg, dt) { + t.Fatalf("not held by the settling walk read back: %v", outcomes(again, k.Key)) + } + runFor(again, c, DefaultSettle) + if !heard("Still open after the delivery of mesh-tools to novox:\n- Urgent: "+headlineOf(k.Key), tg, dt) { + t.Fatalf("the settled walk's end was not told after the restart: %+v", dt.sends) + } + walk(t, again, "plan-x", WalkStarted, "", c.now().Add(-5*time.Minute), 1200, novox("mesh-tools")) + if len(again.walks) != 0 { + t.Fatalf("its late start opened it again: %+v", again.walks) + } +} + +// ADR 0505 §6: a condition raised on a touched subject 30 seconds after a failed walk is sent at once, even while +// another open walk touches the same machine; the same condition after a delivered walk is held. +func TestAConditionJustAfterAFailedWalkIsSentAtOnceAndAfterADeliveredOneHeld(t *testing.T) { + for _, outcome := range []string{WalkFailed, WalkStopped, WalkDelivered} { + h, tg, dt, c, _ := newHolder(t) + walk(t, h, "plan-f", WalkStarted, "", c.now(), 600, novox("gitea")) + walk(t, h, "plan-o", WalkStarted, "", c.now(), 1200, novox("mesh-issues")) + runFor(h, c, 2*time.Minute) + walk(t, h, "plan-f", WalkEnded, outcome, c.now(), 0, novox("gitea")) + runFor(h, c, 30*time.Second) + k := cond("machine.novox.uncomposable", Urgent, "novox cannot get new instructions") + k.Machine, k.Raised = "novox", c.now() + h.Condition(EventRaised, k) + runFor(h, c, 30*time.Second) + sent := heard(headlineOf(k.Key), tg, dt) + if outcome != WalkDelivered && !sent { + t.Fatalf("after a walk that %s, held: %v", outcome, outcomes(h, k.Key)) + } + if outcome == WalkDelivered && sent { + t.Fatalf("after a delivered walk, sent at once: %+v", dt.sends) + } + } +} + +// ADR 0505 §6: an urgent condition on a machine no open or settling walk touches is sent at once during a walk. +func TestAnUrgentConditionOnAnUntouchedMachineIsSentAtOnceDuringAWalk(t *testing.T) { + h, tg, dt, c, _ := newHolder(t) + walk(t, h, "plan-u", WalkStarted, "", c.now(), 1200, novox("gitea")) + k := cond("machine.ace.silent", Urgent, "ace has not been heard") + k.Machine, k.Raised = "ace", c.now() + h.Condition(EventRaised, k) + if !heard(headlineOf(k.Key), tg, dt) { + t.Fatalf("held, while no walk touches ace: %v", outcomes(h, k.Key)) + } +} + +// bodyOf is a held or open record's message as it would be sent now. +func bodyOf(t *testing.T, h *Holder, key, what string) string { + t.Helper() + h.mu.Lock() + r := h.open[key] + h.mu.Unlock() + if r == nil { + t.Fatalf("%s is not open", key) + } + return h.say(r, what).Body +} + +// ADR 0505 §1: a problem's message ends, after its since-when line, with the delivery line: during a walk, in a +// walk's settle, after a failed walk, and with no walk. +func TestEveryProblemMessageEndsWithWhetherADeliveryRan(t *testing.T) { + h, tg, dt, c, _ := newHolder(t) + k := cond("machine.novox.tools-silent", Urgent, "the tools on novox are silent") + k.Raised = c.now() + h.Condition(EventRaised, k) + if !strings.HasSuffix(tg.sends[0].Body, ".\nNo delivery running.") || !strings.Contains(tg.sends[0].Body, "\nSince ") { + t.Fatalf("no walk: %q", tg.sends[0].Body) + } + walk(t, h, "plan-l", WalkStarted, "", c.now(), 600, novox("gitea", "mesh-issues"), shanksDeclared) + if b := bodyOf(t, h, k.Key, "reminder"); !strings.HasSuffix(b, ".\nDuring the delivery of gitea, mesh-issues to novox.") { + t.Fatalf("during a walk: %q", b) + } + runFor(h, c, time.Minute) + walk(t, h, "plan-l", WalkEnded, WalkDelivered, c.now(), 0) + c.pass(40 * time.Second) + if b := bodyOf(t, h, k.Key, "raised"); !strings.HasSuffix(b, "\n40 s after the delivery of gitea, mesh-issues to novox, which succeeded.") { + t.Fatalf("in the settle of a delivered walk: %q", b) + } + runFor(h, c, 3*time.Minute) + if b := bodyOf(t, h, k.Key, "reminder"); !strings.HasSuffix(b, "\nNo delivery running.") { + t.Fatalf("after the settle: %q", b) + } + walk(t, h, "plan-m", WalkStarted, "", c.now(), 600, novox("gitea")) + walk(t, h, "plan-m", WalkEnded, WalkFailed, c.now(), 0) + c.pass(30 * time.Second) + if b := bodyOf(t, h, k.Key, "reminder"); !strings.HasSuffix(b, "\n30 s after the delivery of gitea to novox, which failed.") { + t.Fatalf("after a failed walk: %q", b) + } + keyless(t, dt.sends...) + // Not on a clearing, a silencing or a plain notice. + for _, what := range []string{"cleared", "silenced", "tell"} { + if carriesDeliveryLine(&Record{Origin: "condition"}, what) { + t.Errorf("a %s message carries the delivery line", what) + } + } + if carriesDeliveryLine(&Record{Origin: "ask"}, "raised") { + t.Errorf("an ask's own message carries the delivery line") + } +} + +// ADR 0505 §1: the line names at most six modules and counts the rest, and says no name the content rule refuses. +func TestTheDeliveryLineNamesAtMostSixModulesAndNothingTheRuleRefuses(t *testing.T) { + many := novox("a1", "a2", "a3", "a4", "a5", "a6", "a7", "a8") + if got := deliveryWords([]Touch{many}); got != "a1, a2, a3, a4, a5, a6 and 2 more to novox" { + t.Fatalf("eight modules: %q", got) + } + h, _, _, c, _ := newHolder(t) + walk(t, h, "plan-p", WalkStarted, "", c.now(), 600, novox("/var/lib/mesh-tools")) + k := cond("machine.novox.tools-silent", Urgent, "the tools on novox are silent") + k.Raised = c.now() + h.Condition(EventRaised, k) + if b := bodyOf(t, h, k.Key, "raised"); !strings.HasSuffix(b, "\nDuring a delivery.") { + t.Fatalf("a refused name in the line: %q", b) + } +} + +// ADR 0505 §7: a condition on a machine whose declaration three back-to-back walks change (a machine that stays +// behind is a declared touch of each) reaches the operator at the first walk's bound and settle, while the third +// still runs: never held walk after walk. +func TestADeclaredMachinesConditionAcrossThreeWalksIsSaidAtTheFirstWalksLimit(t *testing.T) { + h, tg, dt, c, _ := newHolder(t) + t0 := c.now() + walk(t, h, "plan-1", WalkStarted, "", c.now(), 600, novox("gitea"), shanksDeclared) // limit: t0 + 10 + 2 min + k := cond("machine.shanks.uncomposable", Urgent, "shanks cannot get new instructions") + k.Machine, k.Raised = "shanks", c.now() + h.Condition(EventRaised, k) + runFor(h, c, 5*time.Minute) + walk(t, h, "plan-1", WalkEnded, WalkDelivered, c.now(), 0, novox("gitea"), shanksDeclared) + runFor(h, c, time.Minute) + walk(t, h, "plan-2", WalkStarted, "", c.now(), 600, novox("mesh-issues"), shanksDeclared) + runFor(h, c, 3*time.Minute) + walk(t, h, "plan-2", WalkEnded, WalkDelivered, c.now(), 0, novox("mesh-issues"), shanksDeclared) + runFor(h, c, time.Minute) + walk(t, h, "plan-3", WalkStarted, "", c.now(), 1200, novox("gitea"), shanksDeclared) + for c.now().Before(t0.Add(12*time.Minute - 10*time.Second)) { + runFor(h, c, 5*time.Second) + } + if heard(headlineOf(k.Key), tg, dt) { + t.Fatalf("said before the first walk's limit: %+v", dt.sends) + } + runFor(h, c, 30*time.Second) + if !heard(headlineOf(k.Key), tg, dt) { + t.Fatalf("held past the first walk's bound and settle: %v", outcomes(h, k.Key)) + } +} + +// ADR 0505 §2: a walk's start or end said again by the controller with its declared machines (declared_read) acts +// once: one walk with both sayings' touches, and one end message. +func TestAWalkSaidAgainWithItsDeclaredMachinesActsOnce(t *testing.T) { + h, tg, dt, c, _ := newHolder(t) + walk(t, h, "plan-r", WalkStarted, "", c.now(), 600, novox("gitea")) + walk(t, h, "plan-r", WalkStarted, "", c.now(), 600, novox("gitea"), shanksDeclared) + if len(h.walks) != 1 || len(h.walks["plan-r"].Touches) != 2 { + t.Fatalf("said again it is not one walk with both touches: %+v", h.walks) + } + runFor(h, c, time.Minute) + walk(t, h, "plan-r", WalkEnded, WalkFailed, c.now(), 0, novox("gitea")) + walk(t, h, "plan-r", WalkEnded, WalkFailed, c.now(), 0, novox("gitea"), shanksDeclared) + runFor(h, c, 3*time.Minute) + n := 0 + for _, m := range append(append([]Message(nil), dt.sends...), tg.sends...) { + if strings.HasPrefix(m.Title, "Delivery failed") { + n++ + } + } + if n != 1 { + t.Fatalf("its end was told %d times: %+v", n, dt.sends) + } +} + +// ADR 0505 §7: a message two walks first held, the sooner of which ends at its limit, is said exactly once — in that +// walk's end message — and not again on its own for the other walk; once per channel when it is urgent. +func TestAMessageTwoWalksFirstHeldIsSaidOnceAtTheSoonerLimit(t *testing.T) { + for _, sev := range []string{Warning, Urgent} { + h, tg, dt, c, _ := newHolder(t) + walk(t, h, "plan-a", WalkStarted, "", c.now(), 360, novox("gitea")) // its limit: 6 min and the settle + walk(t, h, "plan-b", WalkStarted, "", c.now(), 1200, novox("mesh-issues")) // its limit: 22 min + k := cond("machine.novox.network", sev, "novox lost its default route") + k.Raised = c.now() + h.Condition(EventRaised, k) + runFor(h, c, 7*time.Minute) + walk(t, h, "plan-a", WalkEnded, WalkDelivered, c.now(), 0) // settles until its limit, 8 min after the hold + runFor(h, c, 3*time.Minute) + count := func(ch *fakeChannel) int { + n := 0 + for _, m := range ch.sends { + if strings.Contains(m.Text(), headlineOf(k.Key)) { + n++ + } + } + return n + } + want := map[*fakeChannel]int{dt: 1, tg: 0} + if sev == Urgent { + want[tg] = 1 + } + for ch, n := range want { + if got := count(ch); got != n { + t.Fatalf("%s: said %d times on %s, want %d: %+v", sev, got, ch.name, n, ch.sends) + } + } + if !heard("Still open after the delivery of gitea to novox:", dt) { + t.Fatalf("%s: not told in the sooner walk's end message: %+v", sev, dt.sends) + } + } +} diff --git a/modules/messenger/cmd/messenger/bus_test.go b/modules/messenger/cmd/messenger/bus_test.go index 6e34d5eb..9c67bda6 100644 --- a/modules/messenger/cmd/messenger/bus_test.go +++ b/modules/messenger/cmd/messenger/bus_test.go @@ -56,6 +56,8 @@ type busRuntime struct { subscribed, taking bool buckets map[string]jetstream.KeyValue + // settings is the settings file the bundle starts with; empty is `{}`. + settings string } func newBusRuntime(t *testing.T) *busRuntime { @@ -130,7 +132,11 @@ func (r *busRuntime) request(method string, params any) (json.RawMessage, error) func (r *busRuntime) start(dir string) { r.t.Helper() settings := filepath.Join(dir, "settings.json") - _ = os.WriteFile(settings, []byte(`{}`), 0o600) + body := r.settings + if body == "" { + body = `{}` + } + _ = os.WriteFile(settings, []byte(body), 0o600) cmd := exec.Command(os.Args[0], "-test.run=^$") cmd.Env = append(os.Environ(), bundleVar+"=1", "MESH_MESSENGER_SETTINGS="+settings, "MESH_MESSENGER_STATE="+dir, "MESH_SERVED_MODULE=messenger", "MESH_MODULE=messenger", "MESH_NODE=test") diff --git a/modules/messenger/cmd/messenger/condition_bus_test.go b/modules/messenger/cmd/messenger/condition_bus_test.go index 69dd1739..9057315a 100644 --- a/modules/messenger/cmd/messenger/condition_bus_test.go +++ b/modules/messenger/cmd/messenger/condition_bus_test.go @@ -28,8 +28,15 @@ type conditionBed struct { } func newConditionBed(t *testing.T) *conditionBed { + t.Helper() + return newConditionBedWith(t, "") +} + +// newConditionBedWith is the bed with the bundle started on these settings. +func newConditionBedWith(t *testing.T, settings string) *conditionBed { t.Helper() b := &conditionBed{r: newBusRuntime(t)} + b.r.settings = settings b.r.setKinds(map[string][]string{"desktop": {"choice", "edit"}}) if _, err := b.r.nc.Subscribe("mesh.seat."+asks.ChannelSeat+".accept.*.desktop", func(m *nats.Msg) { var msg asks.Message diff --git a/modules/messenger/cmd/messenger/deliveryline.go b/modules/messenger/cmd/messenger/deliveryline.go new file mode 100644 index 00000000..29be208e --- /dev/null +++ b/modules/messenger/cmd/messenger/deliveryline.go @@ -0,0 +1,78 @@ +package main + +// The delivery line (novox/hq ADR 0505 §1): every message about a problem — a condition or a module's notice, +// raised, asked, reopened, escalated or reminded — ends, after its since-when line, with whether a delivery was +// running on what it is about: +// +// - "During the delivery of to ." when an open walk touches its subject; +// - " after the delivery of to , which ." when a walk that touched it ended within +// its settle (a delivered walk settling, a failed or stopped one in the two minutes after it, §6); +// - "No delivery running." otherwise. +// +// The line is this router's, from the walks it keeps (ADR 0308 §1); no producer adds it. It names modules and +// machines as the operator says them, never an id, and is judged by the content rule with the rest of the body. + +import ( + "sort" + "time" +) + +// NoDeliveryLine is the delivery line of a message no walk touches. +const NoDeliveryLine = "No delivery running." + +// outcomeWords are a walk's outcome as the delivery line says it. +var outcomeWords = map[string]string{WalkDelivered: "succeeded", WalkFailed: "failed", WalkStopped: "was stopped"} + +// carriesDeliveryLine says whether a message of a record says the delivery line: a problem's message, and not its +// clearing, its silencing, a plain notice or an ask's own message. +func carriesDeliveryLine(r *Record, what string) bool { + switch what { + case "cleared", "silenced", "tell": + return false + } + return r.Origin != "ask" +} + +// deliveryLine is the line for a record's message at a moment. +func (h *Holder) deliveryLine(r *Record, now time.Time) string { + s := subjectOf(r) + h.mu.Lock() + defer h.mu.Unlock() + h.init() + var during, after []*Walk + for _, w := range h.walks { + if !w.touches(s) { + continue + } + switch { + case w.settling() && now.Before(w.SettleUntil) && !now.Before(w.Ended): + after = append(after, w) + case !w.settling(): + during = append(during, w) // open, within its bound or past it (ADR 0505 §7) + } + } + if len(during) > 0 { + sort.Slice(during, func(i, j int) bool { return during[i].Started.Before(during[j].Started) }) + return plainly("During the delivery of "+deliveryWords(during[0].Touches)+".", "During a delivery.") + } + if f := h.failedJustNow(s, now); f != nil { + after = append(after, f) + } + if len(after) == 0 { + return NoDeliveryLine + } + sort.Slice(after, func(i, j int) bool { return after[i].Ended.After(after[j].Ended) }) + w := after[0] + ago := roughly(now.Sub(w.Ended)) + return plainly(ago+" after the delivery of "+deliveryWords(w.Touches)+", which "+outcomeWords[w.Outcome]+".", + ago+" after a delivery, which "+outcomeWords[w.Outcome]+".") +} + +// plainly is a line when the content rule lets it leave the mesh, else the same said without the names that it +// refused, so the line never withholds the message it ends. +func plainly(line, without string) string { + if _, ok := Check(line); ok { + return line + } + return without +} diff --git a/modules/messenger/cmd/messenger/holder.go b/modules/messenger/cmd/messenger/holder.go index 076fc270..a2dcfaf3 100644 --- a/modules/messenger/cmd/messenger/holder.go +++ b/modules/messenger/cmd/messenger/holder.go @@ -327,6 +327,9 @@ type Holder struct { // CapPerHour is the most messages one channel gets an hour (the setting messages-per-hour, issue 1476); // nil, or a value below zero, is DefaultCapPerHour; 0 is no cap at all. TellCapPerHour is its own and stays fixed. CapPerHour func() int + // Settle is how long a delivered walk holds what it touched after its end (the setting settle-seconds, novox/hq + // ADR 0505 §3); nil is DefaultSettle. + Settle func() time.Duration work sync.Mutex // one event, call or tick at a time mu sync.Mutex // what the status reads @@ -840,6 +843,7 @@ func (h *Holder) Flush() { now := h.Now() h.mu.Unlock() h.duePasses(now) + h.settledWalks(now) // a settle's end is told at the next flush, not the next minute (ADR 0505 §3) h.flush(now, false) } @@ -1050,11 +1054,21 @@ func (h *Holder) words(r *Record, what string) Message { // rule judges the message as it is sent. func (h *Holder) composeTracked(r *Record, what string, now time.Time, withhold int) (Message, string) { m := compose(r, what, now, h.zone(), withhold) - if !h.tracksAsked() || withhold >= 2 { - return m, "" + line := "" + if h.tracksAsked() && withhold < 2 { + issues, known := h.trackedBy(r.Key) + m, line = withTracking(m, r, what, issues, known, now, h.zone()) } - issues, known := h.trackedBy(r.Key) - return withTracking(m, r, what, issues, known, now, h.zone()) + if withhold < 2 && carriesDeliveryLine(r, what) { + // Last, after the since-when line, the delivery line (ADR 0505 §1): judged with the body by the content rule. + dl := h.deliveryLine(r, now) + if m.Body == "" { + m.Body = dl + } else { + m.Body += "\n" + dl + } + } + return m, line } // allowedWords is the message the content rule leaves of a record: its own words when they may leave the diff --git a/modules/messenger/cmd/messenger/main.go b/modules/messenger/cmd/messenger/main.go index 5fbfa29b..57bccaaa 100644 --- a/modules/messenger/cmd/messenger/main.go +++ b/modules/messenger/cmd/messenger/main.go @@ -35,6 +35,9 @@ type Settings struct { // MessagesPerHour is the most messages one channel gets in an hour (issue 1476); 0, not given, is // DefaultCapPerHour; 0 is no cap at all. MessagesPerHour *int `json:"messages-per-hour"` + // SettleSeconds is how long a delivered walk still holds what it touched after its end (novox/hq ADR 0505 §3): + // nil, not given, is DefaultSettle; 0 holds nothing after the end. + SettleSeconds *int `json:"settle-seconds"` } // zoneOf is the location a time-zone setting names, or this machine's own when it names none or one @@ -90,6 +93,17 @@ func readSettings(path string) (Settings, error) { default: return s, fmt.Errorf("only-what-needs-me is not a list of channel kinds") } + switch v := loose["settle-seconds"].(type) { + case nil: + case float64: + n := int(v) + if v < 0 || v != float64(n) { + return s, fmt.Errorf("settle-seconds is not a whole number of zero or more") + } + s.SettleSeconds = &n + default: + return s, fmt.Errorf("settle-seconds is not a number") + } return s, nil } @@ -351,6 +365,12 @@ func main() { } return DefaultCapPerHour }, + Settle: func() time.Duration { + if n := settings().SettleSeconds; n != nil && *n >= 0 { + return time.Duration(*n) * time.Second + } + return DefaultSettle + }, NeedsOnly: func(kind string) bool { for _, k := range settings().OnlyWhatNeedsMe { if k == kind { diff --git a/modules/messenger/cmd/messenger/outbox.go b/modules/messenger/cmd/messenger/outbox.go index b8b2c880..496e5a7f 100644 --- a/modules/messenger/cmd/messenger/outbox.go +++ b/modules/messenger/cmd/messenger/outbox.go @@ -39,6 +39,11 @@ func (h *Holder) deliver(r *Record, what string) { // An open walk touches it: told when the walk ends or passes its bound (ADR 0308 §2). return } + h.route(r, what) +} + +// route sends a record's message where its severity routes it, held by no walk. +func (h *Holder) route(r *Record, what string) { var to []Channel if kinds := h.askKinds(r); len(kinds) > 0 { // An ask goes to every channel that carries one of its answers (novox/hq ADR 0259): the desk for an diff --git a/modules/messenger/cmd/messenger/walks.go b/modules/messenger/cmd/messenger/walks.go index 365937f7..ada2b575 100644 --- a/modules/messenger/cmd/messenger/walks.go +++ b/modules/messenger/cmd/messenger/walks.go @@ -42,6 +42,13 @@ package main // that the walk is still running and since when, and the cleared ones counted; it holds nothing more after // that. A walk whose end never comes is forgotten a day after it started. // +// **The settle (novox/hq ADR 0505 §3, §7).** A walk that ended delivered holds on, settling, for the setting +// settle-seconds (two minutes): what it touched stays held, a condition raised then on what it touched is held with +// it, and its end is told when the settle has passed. A walk that failed or stopped is told at once. The bound of +// §5 runs with the settle added: an open walk holds at most its bound and its settle. What a walk touches includes +// the machines whose declaration its merges change (`declared` touches, §2) and, for a walk that moves +// mesh-controller, the controller's own lease, bus and health conditions. +// // **Kept in the module's state** (the bucket `walks`, one key per walk), so a router that restarts mid-walk // holds and tells as if it had not. @@ -68,6 +75,9 @@ const ( // DefaultWalkBound is a walk's bound when the controller gives none: the bound of a walk none of whose // deliveries has a budget (ADR 0308 §5). DefaultWalkBound = 15 * time.Minute + // DefaultSettle is how long a delivered walk still holds what it touched after its end when the setting + // settle-seconds gives none: the longest the first-node gate holds a healthy module (novox/hq ADR 0505 §3, ADR 0352). + DefaultSettle = 2 * time.Minute // WalkForget: a walk whose end never came is forgotten this long after it started. WalkForget = 24 * time.Hour // WalkEndWhat is the outbox's name for a walk's end or bound message: it goes alone, never in a digest. @@ -76,10 +86,25 @@ const ( MostNamed = 6 ) +// settle is how long a delivered walk still holds after its end (ADR 0505 §3). +func (h *Holder) settle() time.Duration { + if h.Settle == nil { + return DefaultSettle + } + if d := h.Settle(); d >= 0 { + return d + } + return DefaultSettle +} + // Touch is what a walk moves on one machine. type Touch struct { Machine string `json:"machine"` Modules []string `json:"modules"` + // Declared: the walk sends nothing to this machine, but its merges change the machine's declaration (an + // assignment, a provision or a requirement added or removed, ADR 0505 §2b). Held like any touch; never named + // among the machines the walk reached. + Declared bool `json:"declared,omitempty"` } // WalkEvent is a walk's start or end as the controller says it. @@ -100,8 +125,15 @@ type WalkEvent struct { type HeldMessage struct { At time.Time `json:"at"` What string `json:"what"` + // Until is the first hold's limit: the start, bound and settle of the walk that first held it, or that walk's + // settle's end when it was held settling (ADR 0505 §7). Kept when it is handed on: no walk holds it past this, so a + // fault on a machine walk after walk touches reaches the operator all the same. Zero in a hold kept before it was. + Until time.Time `json:"until,omitzero"` } +// past says whether a held message's first hold's limit has passed. +func (hm HeldMessage) past(now time.Time) bool { return !hm.Until.IsZero() && !now.Before(hm.Until) } + // Walk is one open walk as this router keeps it. type Walk struct { ID string `json:"id"` @@ -117,6 +149,13 @@ type Walk struct { // EndedSaid is the end's moment as the controller said it, by the controller's clock and never capped at // this router's now: a start heard after its end is compared with it on that one clock (issue 538 C). EndedSaid time.Time `json:"ended_said,omitempty"` + // SettleSeconds is the settle taken at its start (the setting settle-seconds, ADR 0505 §3): the hold of an open + // walk lasts its bound and this (§7), and a delivered walk holds this long after its end. + SettleSeconds int `json:"settle_seconds,omitempty"` + // SettleUntil is set on a walk that ended delivered and still holds, settling, until then (ADR 0505 §3); Outcome + // is how it ended. Its end is told when SettleUntil passes. + SettleUntil time.Time `json:"settle_until,omitempty"` + Outcome string `json:"outcome,omitempty"` } func (w *Walk) bound() time.Duration { @@ -126,9 +165,27 @@ func (w *Walk) bound() time.Duration { return time.Duration(w.BoundSeconds) * time.Second } -// holding says whether the walk still holds at a moment: open, and within its bound. +// settleFor is the settle the walk took at its start. +func (w *Walk) settleFor() time.Duration { return time.Duration(w.SettleSeconds) * time.Second } + +// limit is the longest a walk holds anything: its start, its bound and its settle (ADR 0505 §7). +func (w *Walk) limit() time.Time { return w.Started.Add(w.bound() + w.settleFor()) } + +// holdEnd is the last moment the walk holds anything it holds now: its limit, or its settle's end when that is +// sooner. +func (w *Walk) holdEnd() time.Time { + if w.settling() && w.SettleUntil.Before(w.limit()) { + return w.SettleUntil + } + return w.limit() +} + +// settling says whether the walk ended delivered and holds on until its settle has passed (ADR 0505 §3). +func (w *Walk) settling() bool { return !w.SettleUntil.IsZero() } + +// holding says whether the walk still holds at a moment: open or settling, and within its bound and settle. func (w *Walk) holding(now time.Time) bool { - return !w.BoundSaid && now.Before(w.Started.Add(w.bound())) + return !w.BoundSaid && now.Before(w.limit()) && (!w.settling() || now.Before(w.SettleUntil)) } // touches says whether the walk touches a subject: a machine it sends to, or — when the subject names no @@ -141,7 +198,7 @@ func (w *Walk) touches(s subject) bool { } } } - if s.machineNamed { + if s.machineNamed && !s.controller { return false } for _, t := range w.Touches { @@ -215,9 +272,10 @@ func DecodeWalk(body []byte) (WalkEvent, bool, error) { } if v, ok := raw["touches"]; ok && string(v) != "null" { var list []struct { - Machine string `json:"machine"` - Node string `json:"node"` - Modules []string `json:"modules"` + Machine string `json:"machine"` + Node string `json:"node"` + Modules []string `json:"modules"` + Declared bool `json:"declared"` } if json.Unmarshal(v, &list) != nil { return WalkEvent{}, false, fmt.Errorf("walk %s: touches is not a list of {machine, modules}", ev.Walk) @@ -228,7 +286,7 @@ func DecodeWalk(body []byte) (WalkEvent, bool, error) { m = t.Node } if m != "" { - ev.Touches = append(ev.Touches, Touch{Machine: m, Modules: t.Modules}) + ev.Touches = append(ev.Touches, Touch{Machine: m, Modules: t.Modules, Declared: t.Declared}) } } } @@ -252,9 +310,11 @@ func DecodeWalk(body []byte) (WalkEvent, bool, error) { return ev, true, nil } -// mergeTouches is what two sayings of a walk's touches name together. +// mergeTouches is what two sayings of a walk's touches name together. A machine is declared only while no saying +// sends to it (ADR 0505 §2b). func mergeTouches(a, b []Touch) []Touch { by := map[string]map[string]bool{} + sent := map[string]bool{} var order []string for _, list := range [][]Touch{a, b} { for _, t := range list { @@ -265,6 +325,9 @@ func mergeTouches(a, b []Touch) []Touch { for _, m := range t.Modules { by[t.Machine][m] = true } + if !t.Declared { + sent[t.Machine] = true + } } } sort.Strings(order) @@ -275,7 +338,7 @@ func mergeTouches(a, b []Touch) []Touch { mods = append(mods, m) } sort.Strings(mods) - out = append(out, Touch{Machine: machine, Modules: mods}) + out = append(out, Touch{Machine: machine, Modules: mods, Declared: !sent[machine]}) } return out } @@ -294,7 +357,8 @@ func (h *Holder) Walked(ev WalkEvent) { } switch ev.Phase { case WalkStarted: - w := &Walk{ID: ev.Walk, Started: at, BoundSeconds: ev.BoundSeconds, Class: ev.Class} + w := &Walk{ID: ev.Walk, Started: at, BoundSeconds: ev.BoundSeconds, Class: ev.Class, + SettleSeconds: int(h.settle() / time.Second)} h.mu.Lock() end := h.ended[ev.Walk] h.mu.Unlock() @@ -324,6 +388,13 @@ func (h *Holder) Walked(ev WalkEvent) { case WalkEnded: h.mu.Lock() w := h.walks[ev.Walk] + if w != nil && w.settling() { + // Its end said again while it settles: the same end, its touches as both sayings name them. + w.Touches = mergeTouches(w.Touches, ev.Touches) + h.mu.Unlock() + h.keepWalk(w) + return + } if w != nil && !ev.StartedAt.IsZero() && w.Started.After(ev.StartedAt) { // The end of an earlier walk under the same id, heard after the next one started: that one stays open. h.mu.Unlock() @@ -332,18 +403,38 @@ func (h *Holder) Walked(ev WalkEvent) { return } if w != nil { - delete(h.walks, ev.Walk) w.Touches = mergeTouches(w.Touches, ev.Touches) if !ev.StartedAt.IsZero() { w.Started = ev.StartedAt } } - tomb := &Walk{ID: ev.Walk, Started: ev.StartedAt, Ended: at, EndedSaid: ev.At} + tomb := &Walk{ID: ev.Walk, Started: ev.StartedAt, Ended: at, EndedSaid: ev.At, Outcome: ev.Outcome, + Touches: mergeTouches(ev.Touches, nil)} + if w != nil { + tomb.Touches, tomb.SettleSeconds = w.Touches, w.SettleSeconds + } if prev := h.ended[ev.Walk]; prev != nil && prev.Ended.After(at) { tomb = prev } h.ended[ev.Walk] = tomb + settles := w != nil && ev.Outcome == WalkDelivered && !w.BoundSaid && w.settleFor() > 0 + if settles { + // Delivered: it holds on, settling, until its settle has passed and never past its limit (ADR 0505 §3, §7). + w.Ended, w.EndedSaid, w.Outcome = at, ev.At, ev.Outcome + w.SettleUntil = at.Add(w.settleFor()) + if l := w.limit(); w.SettleUntil.After(l) { + w.SettleUntil = l + } + } else if w != nil { + delete(h.walks, ev.Walk) + } h.mu.Unlock() + if settles { + h.keepWalk(w) // settling, in place of the open walk: read back as settling after a restart + h.record(Sent{At: now, Channel: "-", Key: "walk." + ev.Walk, What: "ended " + ev.Outcome, + Outcome: "settling: holding what it touched until " + w.SettleUntil.In(h.zone()).Format("15:04:05")}) + return + } h.keepWalk(tomb) // in place of the open walk: a start heard after this end opens nothing if w == nil { h.Logf("[messenger] walk %s %s, and its start was not heard here: nothing held, nothing said", ev.Walk, ev.Outcome) @@ -408,11 +499,12 @@ func (h *Holder) release(w *Walk, now time.Time) (open []*Record, whats map[stri if o.ID == w.ID || !o.holding(now) { continue } - if _, ok := o.Held[key]; ok { - elsewhere = true + if other, ok := o.Held[key]; ok { + elsewhere = elsewhere || !other.past(now) continue } - if live && o.touches(s) { + if live && o.touches(s) && !hm.past(now) { + // Handed on with its first hold's limit: never held past it (ADR 0505 §7). if o.Held == nil { o.Held = map[string]HeldMessage{} } @@ -456,7 +548,11 @@ func (h *Holder) release(w *Walk, now time.Time) (open []*Record, whats map[stri // endWalk tells a walk's end once (ADR 0308 §4). func (h *Holder) endWalk(w *Walk, outcome string, ended, now time.Time) { open, whats, cleared := h.release(w, now) - if now.Sub(ended) > FreshFor && len(open) == 0 { + due := ended + if w.settling() { + due = w.SettleUntil + } + if now.Sub(due) > FreshFor && len(open) == 0 { h.Logf("[messenger] walk %s %s %s ago, heard late: history, nothing said (%d held cleared meanwhile)", w.ID, outcome, roughly(now.Sub(ended)), cleared) h.record(Sent{At: now, Channel: "-", Key: "walk." + w.ID, What: "ended " + outcome, Outcome: "quiet: heard late"}) @@ -474,17 +570,102 @@ func (h *Holder) endWalk(w *Walk, outcome string, ended, now time.Time) { m = Message{Title: head + ": " + reached + ", in " + roughly(took), Quiet: true} } else { lines := []string{"It took " + roughly(took) + "."} - lines = append(lines, h.heldLines(open, whats, cleared)...) + lines = append(lines, h.heldLines(open, whats, cleared, "Still open after the delivery of "+deliveryWords(w.Touches)+":")...) m = Message{Title: head + ": " + reached, Body: strings.Join(lines, "\n")} } h.tellWalk(w, m, open, whats, oneLine, "ended "+outcome, now) } +// settledWalks tells the end of every delivered walk whose settle has passed (ADR 0505 §3), oldest first. +func (h *Holder) settledWalks(now time.Time) { + h.overdueHolds(now) + h.mu.Lock() + var settled []*Walk + for _, w := range h.walks { + if w.settling() && !now.Before(w.SettleUntil) { + settled = append(settled, w) + } + } + h.mu.Unlock() + sort.Slice(settled, func(i, j int) bool { return settled[i].SettleUntil.Before(settled[j].SettleUntil) }) + for _, w := range settled { + h.endSettled(w, now) + } +} + +// overdueHolds says every held message whose first hold's limit has passed while a walk that took it over still holds, +// on its own, with its delivery line (ADR 0505 §7): no walk holds it past that limit. A walk whose own end or bound is +// due tells what it holds in its message instead. Called at each flush. +func (h *Holder) overdueHolds(now time.Time) { + h.mu.Lock() + // What a walk whose own end or bound is due holds is told in that walk's message, and nowhere else: one place + // says it, so a message two walks hold is never said twice. + tellsItself := map[string]bool{} + for _, w := range h.walks { + if !now.Before(w.holdEnd()) { + for key := range w.Held { + tellsItself[key] = true + } + } + } + due := map[string]HeldMessage{} + var touched []*Walk + for _, w := range h.walks { + changed := false + if !now.Before(w.holdEnd()) { + continue // its own end or bound tells what it holds, in its message + } + for key, hm := range w.Held { + if hm.past(now) { + if prev, ok := due[key]; !tellsItself[key] && (!ok || hm.At.Before(prev.At)) { + due[key] = hm + } + delete(w.Held, key) + changed = true + } + } + if changed { + touched = append(touched, w) + } + } + var recs []*Record + for key, hm := range due { + if r := h.open[key]; r != nil && r.Cleared.IsZero() && !r.silenced(now) && !lastSaid(r).After(hm.At) { + recs = append(recs, r) + } + } + h.mu.Unlock() + for _, w := range touched { + h.keepWalk(w) + } + sort.Slice(recs, func(i, j int) bool { return recs[i].Key < recs[j].Key }) + for _, r := range recs { + what := due[r.Key].What + h.record(Sent{At: now, Channel: "-", Key: r.Key, What: what, Outcome: "released: past its first hold's limit"}) + h.route(r, what) + } +} + +// endSettled tells the end of a delivered walk whose settle has passed (ADR 0505 §3): what it held still open, one +// line each, and the cleared counted; then it is only the tomb that keeps a late start from opening it again. +func (h *Holder) endSettled(w *Walk, now time.Time) { + h.mu.Lock() + delete(h.walks, w.ID) + tomb := h.ended[w.ID] + if tomb == nil { + tomb = &Walk{ID: w.ID, Started: w.Started, Ended: w.Ended, EndedSaid: w.EndedSaid} + h.ended[w.ID] = tomb + } + h.mu.Unlock() + h.keepWalk(tomb) + h.endWalk(w, w.Outcome, w.Ended, now) +} + // heldLines are the lines a walk's end or bound says of what it held: each still open, then the cleared count. -func (h *Holder) heldLines(open []*Record, whats map[string]string, cleared int) []string { +func (h *Holder) heldLines(open []*Record, whats map[string]string, cleared int, heading string) []string { var lines []string if len(open) > 0 { - lines = append(lines, "Still open after it:") + lines = append(lines, heading) for _, r := range open { what := whats[r.Key] if what == "" { @@ -510,7 +691,7 @@ func (h *Holder) boundWalks(now time.Time) { var due, gone []*Walk var stale []string for _, w := range h.walks { - if !w.BoundSaid && !now.Before(w.Started.Add(w.bound())) { + if !w.settling() && !w.BoundSaid && !now.Before(w.limit()) { due = append(due, w) } if now.Sub(w.Started) >= WalkForget { @@ -543,6 +724,7 @@ func (h *Holder) boundWalks(now time.Time) { h.forgetWalk(w.ID) } }() + h.settledWalks(now) sort.Slice(due, func(i, j int) bool { return due[i].Started.Before(due[j].Started) }) for _, w := range due { h.mu.Lock() @@ -562,7 +744,7 @@ func (h *Holder) boundWalks(now time.Time) { if _, ok := Check(lines[0]); !ok { lines[0] = "The walk is still running, since " + since + "." } - lines = append(lines, h.heldLines(open, whats, cleared)...) + lines = append(lines, h.heldLines(open, whats, cleared, "Still open after it:")...) m := Message{Title: "Still running: the delivery to " + touchWords(w.Touches), Body: strings.Join(lines, "\n")} h.tellWalk(w, m, open, whats, false, "past its bound", now) } @@ -670,6 +852,9 @@ func touchWords(touches []Touch) string { mods := map[string]bool{} var machines []string for _, t := range touches { + if t.Declared { + continue // its declaration changed; nothing was sent to it (ADR 0505 §2b) + } machines = append(machines, t.Machine) for _, m := range t.Modules { mods[m] = true @@ -690,6 +875,37 @@ func touchWords(touches []Touch) string { return listed(names) + " on " + listed(machines) } +// deliveryWords names a delivery as the operator says it: its modules, then "to" the machines it sent them to +// (ADR 0505 §1); a machine whose declaration alone changed is not among them. +func deliveryWords(touches []Touch) string { + mods := map[string]bool{} + var machines []string + for _, t := range touches { + if t.Declared { + continue + } + machines = append(machines, t.Machine) + for _, m := range t.Modules { + mods[m] = true + } + } + var names []string + for m := range mods { + names = append(names, m) + } + sort.Strings(names) + sort.Strings(machines) + switch { + case len(names) == 0 && len(machines) == 0: + return "nothing" + case len(machines) == 0: + return listed(names) + case len(names) == 0: + return "its builds to " + listed(machines) + } + return listed(names) + " to " + listed(machines) +} + // listed is at most MostNamed names, then how many more. func listed(names []string) string { if len(names) <= MostNamed { @@ -705,8 +921,18 @@ type subject struct { machineNamed bool // tokens are every name it carries, to tell a message about a walk itself. tokens []string + // controller: the subject is the controller itself — its lease, its bus connection, its health — matched to the + // module mesh-controller wherever a walk moves it, whatever machine or host the condition names (ADR 0505 §2a). + controller bool } +// controllerNames are the names the controller's own conditions give it in their keys (`core.controller.lease.lost`, +// `bus.controller.bus-reconnects`, `module.controller...`): each is the module ControllerModule (ADR 0505 §2a). +var controllerNames = map[string]bool{"controller": true, ControllerModule: true} + +// ControllerModule is the controller's module, as a walk that moves it names it. +const ControllerModule = "mesh-controller" + // subjectOf is a record's subject: a condition's scope and id (`..` when the record kept no // subject of its own), a notice's subject words. func subjectOf(r *Record) subject { @@ -728,6 +954,11 @@ func subjectOf(r *Record) subject { if r.Machine != "" { s.machines, s.machineNamed = append(s.machines, r.Machine), true } + if (scope == "core" || scope == "bus" || scope == "module" || scope == "seat") && len(tokens) > 0 && controllerNames[tokens[0]] { + // The controller's own subject: the module mesh-controller, wherever it runs (ADR 0505 §2a). + s.modules, s.controller = append(s.modules, ControllerModule), true + return s + } switch scope { case "machine", "node", "bus": s.machines, s.machineNamed = append(s.machines, tokens[0]), true @@ -797,6 +1028,33 @@ func (h *Holder) hold(r *Record, what string) bool { now := h.Now() h.mu.Lock() s := subjectOf(r) + if f := h.failedJustNow(s, now); f != nil { + // A failure is the case the operator most needs the cause of: sent at once, with its delivery line (ADR 0505 §6). + h.mu.Unlock() + h.record(Sent{At: now, Channel: "-", Key: r.Key, What: what + ", " + roughly(now.Sub(f.Ended)) + " after walk " + f.ID + " " + f.Outcome, + Outcome: "not held: its walk " + f.Outcome}) + return false + } + // The first hold's limit: kept by whichever walk holds it now, else the soonest end of the walks about to. + var until time.Time + for _, w := range h.walks { + if hm, ok := w.Held[r.Key]; ok && !hm.Until.IsZero() && (until.IsZero() || hm.Until.Before(until)) { + until = hm.Until + } + } + if until.IsZero() { + for _, w := range h.walks { + if w.holding(now) && w.touches(s) && (until.IsZero() || w.holdEnd().Before(until)) { + until = w.holdEnd() + } + } + } + if !until.IsZero() && !now.Before(until) { + // Held to its first hold's limit already: said now, with its delivery line (ADR 0505 §7). + h.mu.Unlock() + h.record(Sent{At: now, Channel: "-", Key: r.Key, What: what, Outcome: "not held: past its first hold's limit"}) + return false + } var by []*Walk var ids []string fresh := false @@ -805,7 +1063,7 @@ func (h *Holder) hold(r *Record, what string) bool { continue } prev, had := w.Held[r.Key] - hm := HeldMessage{At: now, What: what} + hm := HeldMessage{At: now, What: what, Until: until} if had { hm.At = prev.At if !r.told() && prev.What == "raised" { @@ -837,6 +1095,32 @@ func (h *Holder) hold(r *Record, what string) bool { return true } +// failedJustNow is the walk that failed or stopped within its settle and touched a subject (ADR 0505 §6): nothing +// on it is held then. Under the lock. +func (h *Holder) failedJustNow(s subject, now time.Time) *Walk { + var newest *Walk + for _, e := range h.ended { + if e.Outcome != WalkFailed && e.Outcome != WalkStopped { + continue + } + if now.Before(e.Ended) || !now.Before(e.Ended.Add(h.settleOf(e))) || !e.touches(s) { + continue + } + if newest == nil || e.Ended.After(newest.Ended) { + newest = e + } + } + return newest +} + +// settleOf is the settle a walk took at its start, or the setting's for one kept before it was. +func (h *Holder) settleOf(w *Walk) time.Duration { + if w.SettleSeconds > 0 { + return w.settleFor() + } + return h.settle() +} + // heldBy is the walks holding a record now. func (h *Holder) heldBy(key string, now time.Time) []string { var ids []string @@ -886,6 +1170,16 @@ func (h *Holder) loadWalks() error { } for i := range walks { w := walks[i] + if !w.Ended.IsZero() && w.settling() { + // Settling when the router stopped: it holds on until its settle has passed, and its end keeps a late + // start from opening it again (ADR 0505 §3). + if w.Held == nil { + w.Held = map[string]HeldMessage{} + } + h.walks[w.ID] = &w + h.ended[w.ID] = &Walk{ID: w.ID, Started: w.Started, Ended: w.Ended, EndedSaid: w.EndedSaid} + continue + } if !w.Ended.IsZero() { h.ended[w.ID] = &w continue diff --git a/modules/messenger/cmd/messenger/walks_test.go b/modules/messenger/cmd/messenger/walks_test.go index a7b86909..84fd2e33 100644 --- a/modules/messenger/cmd/messenger/walks_test.go +++ b/modules/messenger/cmd/messenger/walks_test.go @@ -131,7 +131,7 @@ func TestAMessageTouchedByTwoOverlappingWalksIsHeldUntilTheSecondEnds(t *testing h.Condition(EventRaised, k) runFor(h, c, time.Minute) walk(t, h, "plan-a", WalkEnded, WalkDelivered, c.now(), 0, novox("mesh-tools")) - runFor(h, c, time.Minute) + runFor(h, c, DefaultSettle+time.Minute) // its end is told once it settled (ADR 0505 §3) if heard(headlineOf(k.Key), tg, dt) { t.Fatalf("said at the first walk's end, while the second still holds it: %+v %+v", tg.sends, dt.sends) } @@ -139,8 +139,8 @@ func TestAMessageTouchedByTwoOverlappingWalksIsHeldUntilTheSecondEnds(t *testing t.Fatalf("the first walk's end was not told: %+v", dt.sends) } walk(t, h, "plan-b", WalkEnded, WalkDelivered, c.now(), 0) - runFor(h, c, time.Minute) - if !heard("Still open after it:\n- Urgent: "+headlineOf(k.Key), tg, dt) { + runFor(h, c, DefaultSettle+time.Minute) + if !heard("Still open after the delivery of node-tools to ace, novox:\n- Urgent: "+headlineOf(k.Key), tg, dt) { t.Fatalf("the second walk's end did not tell it: telegram %+v, desk %+v", tg.sends, dt.sends) } } @@ -222,7 +222,7 @@ func TestAWalksEndIsToldOnceWithWhatIsStillOpen(t *testing.T) { if m.Title != "Delivery failed: mesh-tools on novox" || !strings.Contains(m.Body, "It took 7 min.") { t.Fatalf("the end does not say its outcome, what it reached and how long: %q", m.Text()) } - want := "Still open after it:\n- " + headlineOf(still.Key) + "\n- Urgent: " + headlineOf(other.Key) + + want := "Still open after the delivery of mesh-tools to novox:\n- " + headlineOf(still.Key) + "\n- Urgent: " + headlineOf(other.Key) + "\n1 problem it raised resolved during it." if !strings.Contains(m.Body, want) || strings.Contains(m.Text(), headlineOf(gone.Key)) { t.Fatalf("the end message %q does not say %q", m.Body, want) @@ -241,7 +241,7 @@ func TestADeliveredWalkWithNothingHeldIsOneLineOrNothing(t *testing.T) { walk(t, h, "plan-4", WalkStarted, "", c.now(), 600, novox("mesh-tools"), Touch{Machine: "ace", Modules: []string{"mesh-tools"}}) runFor(h, c, 3*time.Minute) walk(t, h, "plan-4", WalkEnded, WalkDelivered, c.now(), 0) - runFor(h, c, time.Minute) + runFor(h, c, DefaultSettle+time.Minute) if len(dt.sends) != 1 || len(tg.sends) != 0 { t.Fatalf("desk %+v, telegram %+v", dt.sends, tg.sends) } @@ -252,7 +252,7 @@ func TestADeliveredWalkWithNothingHeldIsOneLineOrNothing(t *testing.T) { walk(t, h, "plan-5", WalkStarted, "", c.now(), 600, novox("mesh-tools")) runFor(h, c, 3*time.Minute) walk(t, h, "plan-5", WalkEnded, WalkDelivered, c.now(), 0) - runFor(h, c, time.Minute) + runFor(h, c, DefaultSettle+time.Minute) if len(dt.sends) != 1 || len(tg.sends) != 0 { t.Fatalf("told on a kind that says only what needs the operator: desk %+v, telegram %+v", dt.sends, tg.sends) } @@ -262,8 +262,9 @@ func TestADeliveredWalkWithNothingHeldIsOneLineOrNothing(t *testing.T) { } // §5: the bound is the controller's (bound_seconds: twice the largest budget of the walk's deliveries; none -// given, 15 minutes). At it, what is held and still open is sent with the still-running line, and what -// cleared is counted; before it, nothing; after it, the walk holds nothing more. +// given, 15 minutes), and the settle is added to it (ADR 0505 §7). At it, what is held and still open is sent +// with the still-running line, and what cleared is counted; before it, nothing; after it, the walk holds nothing +// more. func TestAHoldEndsAtTheWalksBound(t *testing.T) { for _, tc := range []struct { name string @@ -287,7 +288,7 @@ func TestAHoldEndsAtTheWalksBound(t *testing.T) { h.Condition(EventRaised, gone) c.pass(30 * time.Second) h.Condition(EventCleared, Condition{Key: gone.Key, At: c.now()}) - runFor(h, c, tc.bound-time.Minute-30*time.Second) + runFor(h, c, tc.bound+DefaultSettle-time.Minute-30*time.Second) if saidOn(tg, dt) != 0 { t.Fatalf("said before the bound: %+v %+v", tg.sends, dt.sends) } @@ -330,7 +331,7 @@ func TestHeldMessagesSurviveARouterRestart(t *testing.T) { again.Start() c.pass(2 * time.Minute) walk(t, again, "plan-8", WalkEnded, WalkDelivered, c.now(), 0) - runFor(again, c, time.Minute) + runFor(again, c, DefaultSettle+time.Minute) if !heard("- Urgent: "+headlineOf(k.Key), dt) || saidOn(tg, dt) != 2 { t.Fatalf("what was held before the restart was not told at the walk's end: desk %+v, telegram %+v", dt.sends, tg.sends) } @@ -462,7 +463,7 @@ func TestWhatAWalkHeldIsSaidOnceAChannelCanSendAgain(t *testing.T) { walk(t, h, "plan-c", WalkEnded, WalkFailed, c.now(), 0) runFor(h, c, time.Minute) } else { - runFor(h, c, 10*time.Minute) + runFor(h, c, 10*time.Minute+DefaultSettle) // the bound and the settle (ADR 0505 §7) } if saidOn(tg, dt) != 0 { t.Fatalf("said on a channel that cannot send: %+v %+v", tg.sends, dt.sends) @@ -479,11 +480,12 @@ func TestWhatAWalkHeldIsSaidOnceAChannelCanSendAgain(t *testing.T) { } } -// Review of pull request 230, 2: a message held by walk A, about a machine walk B also touches, is held on at -// A's end while B — started after it was held — is still open, and told at B's end. +// Review of pull request 230, 2, as novox/hq ADR 0505 §7 bounds it: a message held by walk A, about a machine walk B +// also touches, is held on at A's end while B — started after it was held — is still open, and is said once A's own +// limit (its start, bound and settle) has passed, while B still runs, with B's delivery line. func TestAMessageIsHeldOnByALaterWalkThatTouchesIt(t *testing.T) { h, tg, dt, c, _ := newHolder(t) - walk(t, h, "plan-a", WalkStarted, "", c.now(), 1200, novox("mesh-tools")) + walk(t, h, "plan-a", WalkStarted, "", c.now(), 360, novox("mesh-tools")) // its limit: 6 min and the settle k := cond("machine.novox.network", Urgent, "novox lost its default route") k.Raised = c.now() h.Condition(EventRaised, k) @@ -491,17 +493,16 @@ func TestAMessageIsHeldOnByALaterWalkThatTouchesIt(t *testing.T) { walk(t, h, "plan-b", WalkStarted, "", c.now(), 1200, novox("node-tools")) runFor(h, c, time.Minute) walk(t, h, "plan-a", WalkEnded, WalkDelivered, c.now(), 0) - runFor(h, c, time.Minute) + runFor(h, c, DefaultSettle+time.Minute) if heard(headlineOf(k.Key), tg, dt) { t.Fatalf("told at the first walk's end while the second touches it: %+v %+v", tg.sends, dt.sends) } if got := h.heldBy(k.Key, c.now()); len(got) != 1 || got[0] != "plan-b" { t.Fatalf("the second walk does not hold it on: %v", got) } - walk(t, h, "plan-b", WalkEnded, WalkDelivered, c.now(), 0) - runFor(h, c, time.Minute) - if !heard("Still open after it:\n- Urgent: "+headlineOf(k.Key), tg, dt) { - t.Fatalf("the second walk's end did not tell it: %+v %+v", tg.sends, dt.sends) + runFor(h, c, 4*time.Minute) // past the first walk's limit, 8 minutes after it was held + if !heard("During the delivery of node-tools to novox.", tg, dt) || !heard(headlineOf(k.Key), tg, dt) { + t.Fatalf("held past its first hold's limit: %+v %+v", tg.sends, dt.sends) } } @@ -651,7 +652,7 @@ func TestAHeldMessageIsToldOnlyOnceTheWalksMessageWasSent(t *testing.T) { tg.fail, dt.fail = nil, nil if then == "sent" { runFor(h, c, 2*time.Minute) - if !heard("Still open after it:\n- Urgent: "+headlineOf(k.Key), tg, dt) || saidOn(tg, dt) != 2 { + if !heard("Still open after the delivery of mesh-tools to novox:\n- Urgent: "+headlineOf(k.Key), tg, dt) || saidOn(tg, dt) != 2 { t.Fatalf("not told once in the walk's message: telegram %+v, desk %+v", tg.sends, dt.sends) } if r := h.open[k.Key]; r == nil || !r.told() || r.Owed != "" || r.Sent["telegram"] != "" || r.Sent["desktop"] != "" { @@ -726,7 +727,7 @@ func TestAWalksEndHandsOnOnlyWhatALaterWalkWouldHold(t *testing.T) { } before := map[string]int{quiet.Key: helds(quiet.Key), asked.Key: helds(asked.Key)} walk(t, h, "plan-a", WalkEnded, WalkDelivered, c.now(), 0) - runFor(h, c, time.Minute) + runFor(h, c, DefaultSettle+time.Minute) for _, key := range []string{quiet.Key, asked.Key} { if got := h.heldBy(key, c.now()); len(got) != 0 { t.Errorf("%s was handed on to %v, which a first hold would never hold", key, got) diff --git a/modules/messenger/cmd/messenger/words_test.go b/modules/messenger/cmd/messenger/words_test.go index f646a7cc..5403bdf4 100644 --- a/modules/messenger/cmd/messenger/words_test.go +++ b/modules/messenger/cmd/messenger/words_test.go @@ -58,7 +58,7 @@ var examples = []example{{ resolved: "openrazer delivery no longer waiting", popup: true, title: "Urgent: openrazer delivery waiting to start", body: "Needs you: start it, or stop it. The change to openrazer is merged and built, and mesh-delivery (the module " + - "that decides when a delivery goes out) has not let it start for 5 hours, so mesh-delivery may be stuck.\nNo issue tracks this yet.\nSince 12:40.", + "that decides when a delivery goes out) has not let it start for 5 hours, so mesh-delivery may be stuck.\nNo issue tracks this yet.\nSince 12:40.\nNo delivery running.", offers: "Details", choose: "Details", cleared: "Resolved: openrazer delivery no longer waiting, after 5.0 h", openFor: 5 * time.Hour, @@ -80,7 +80,7 @@ var examples = []example{{ title: "openrazer not working on g14", body: "Needs you: restart its service openrazer-daemon on g14; if it fails again, the details say why. openrazer " + "on g14 is not healthy: its service openrazer-daemon stopped with an error. It clears as soon as it runs " + - "again.\nNo issue tracks this yet.\nSince 12:40.", + "again.\nNo issue tracks this yet.\nSince 12:40.\nNo delivery running.", offers: "Details", choose: "Details", cleared: "Resolved: openrazer works again on g14, after 2.0 h", openFor: 2 * time.Hour, }, { @@ -104,7 +104,7 @@ var examples = []example{{ title: "3 failed services on shanks", body: "Needs you: mend or remove them on shanks, or silence this if they do not matter. On shanks, mnt-recalbox " + "(a mount), storage-media (a mount) and greenclip failed. No module manages them, so the mesh does not repair " + - "them.\nNo issue tracks this yet.\nSince 12:40.", + "them.\nNo issue tracks this yet.\nSince 12:40.\nNo delivery running.", offers: "Details", choose: "Details", cleared: "Resolved: no failed services on shanks any more, after 45 min", openFor: 45 * time.Minute, }, { @@ -135,7 +135,7 @@ var examples = []example{{ explanation: "Needs you: release it, or stop it. A delivery of hq has been held for 36 hours, past its limit.", resolved: "Delivery of hq is no longer held", popup: true, title: "Delivery of hq held for 36 hours", - body: "Needs you: release it, or stop it. A delivery of hq has been held for 36 hours, past its limit.\nNo issue tracks this yet.\nSince 12:40.", + body: "Needs you: release it, or stop it. A delivery of hq has been held for 36 hours, past its limit.\nNo issue tracks this yet.\nSince 12:40.\nNo delivery running.", offers: "Details", choose: "Details", cleared: "Resolved: delivery of hq is no longer held, after 3 min", openFor: 3 * time.Minute, @@ -296,7 +296,7 @@ func TestAConditionWithoutPlainWordsIsSaidFromItsSummary(t *testing.T) { Summary: "ace has not been heard from: `push ace` sends it again", Raised: c.now()} h.Condition(EventRaised, k) m := tg.sends[0] - if m.Title != "Urgent: Silent on ace" || m.Body != "Ace has not been heard from: a command sends it again\nSince 12:00." { + if m.Title != "Urgent: Silent on ace" || m.Body != "Ace has not been heard from: a command sends it again\nSince 12:00.\nNo delivery running." { t.Errorf("%q / %q", m.Title, m.Body) } keyless(t, m) diff --git a/modules/messenger/files/settings.json b/modules/messenger/files/settings.json index 6d22854a..726c29cb 100644 --- a/modules/messenger/files/settings.json +++ b/modules/messenger/files/settings.json @@ -1,5 +1,6 @@ { "time-zone": "", "only-what-needs-me": [], - "messages-per-hour": 100 + "messages-per-hour": 100, + "settle-seconds": 120 } diff --git a/modules/messenger/module.json b/modules/messenger/module.json index 60451958..d7da1d68 100644 --- a/modules/messenger/module.json +++ b/modules/messenger/module.json @@ -140,6 +140,14 @@ "maximum": 1000, "trusted": false, "default": 100 + }, + "settle-seconds": { + "type": "number", + "description": "seconds a delivery that ended well still holds the messages about what it changed, so a problem that passes as the delivery settles is not sent (ADR 0505 §3); 0 holds nothing after the end.", + "minimum": 0, + "maximum": 3600, + "trusted": false, + "default": 120 } }, "resources": [ @@ -163,7 +171,7 @@ "type": "file", "path": "${dir:state}/settings.json", "mode": "0600", - "takes-config": ["time-zone", "desktop-machines", "only-what-needs-me", "messages-per-hour"], + "takes-config": ["time-zone", "desktop-machines", "only-what-needs-me", "messages-per-hour", "settle-seconds"], "merge": "json", "template": "files/settings.json", "owner": "messenger" diff --git a/modules/nats/README.md b/modules/nats/README.md index 3100d5c6..0994ea89 100644 --- a/modules/nats/README.md +++ b/modules/nats/README.md @@ -52,8 +52,11 @@ live one**, which a person swaps in. 1. **Take the night back, beside the live data.** `node-backup.restore` for `nats` on the bus's machine (optionally naming a restore point) restores the snapshots directory as `.restored-/`, holding that night's `bus.tar`. -2. **Check it.** `docker exec -i mesh-broker-nats /opt/mesh/mesh-nats-snapshot verify < /bus.tar` — - every file against the manifest's checksum, every archive read to its end. +2. **Check it.** The tool `/nats.nats_snapshot_verify` checks the newest `bus.tar` of the live snapshots + directory (or one `file` named in it) — every file against the manifest's checksum, every archive read to + its end. It reads nothing outside that directory, so a restored copy (`.restored-/`) is + checked by `docker exec -i mesh-broker-nats /opt/mesh/mesh-nats-snapshot verify < /bus.tar`, and + step 3's `restore` holds the archive to its manifest first and refuses on a fault. 3. **Build the new store**, with the bus's own image and the mounted program (the server that fills it is the same release the bus runs): @@ -89,8 +92,8 @@ merge of a change to this module builds the bus and sends nothing. The steps, in 1. **Nothing else runs.** No delivery plan is open (`mesh-controller.plans`). 2. **Snapshot with the dump the bus runs now**, through the machine's backup holder (`/node-backup.now` for `nats` on the bus's machine), and verify the `bus.tar` it wrote - (`docker exec -i mesh-broker-nats /opt/mesh/mesh-nats-snapshot verify`, or the old path - `mesh-nats-snapshot verify` on the declaration before ADR 0480). Note where it is: that is `snapshot-taken`. + with `/nats.nats_snapshot_verify` (it answers the file, its time and size, each stream's last + sequence, and `passed` or the first fault; run it after the dump and read `taken`). Note where it is: that is `snapshot-taken`. 3. **Ask the operator for the moment** on the proven channel, naming what it raises: bus maintenance is open while the step runs; every bus connection is dropped for seconds and reconnects; conditions of other modules may flash and clear. Go on only on their yes. diff --git a/modules/nats/cmd/mesh-nats-snapshot/image_test.go b/modules/nats/cmd/mesh-nats-snapshot/image_test.go index 17818617..d53fcee3 100644 --- a/modules/nats/cmd/mesh-nats-snapshot/image_test.go +++ b/modules/nats/cmd/mesh-nats-snapshot/image_test.go @@ -20,6 +20,7 @@ import ( "time" "github.com/nats-io/nats.go" + "nats-tools/internal/busarchive" ) // manifestOf is the nats module's own definition, beside this program. @@ -172,7 +173,7 @@ func TestTheDeclaredDumpRunsInTheBussImage(t *testing.T) { if err != nil { t.Fatalf("the dump left no bus.tar: %v", err) } - a, err := ReadArchive(f, "") + a, err := busarchive.ReadArchive(f, "") f.Close() if err != nil { t.Fatal(err) diff --git a/modules/nats/cmd/mesh-nats-snapshot/live_test.go b/modules/nats/cmd/mesh-nats-snapshot/live_test.go index 3c173340..99af6599 100644 --- a/modules/nats/cmd/mesh-nats-snapshot/live_test.go +++ b/modules/nats/cmd/mesh-nats-snapshot/live_test.go @@ -35,6 +35,7 @@ import ( "time" "github.com/nats-io/nats.go" + "nats-tools/internal/busarchive" ) // image is the bus's release (the manifest's server artifact names the same release in its tag). @@ -296,7 +297,7 @@ func fill(t *testing.T, js nats.JetStreamContext) { // sameContent holds every stream the manifest names, in the restored server, to the source: the same // state at the snapshot, every message by sequence (subject, headers, body, and a deleted one still // deleted), and the durable consumers where they were. -func sameContent(t *testing.T, m *Manifest, src, dst nats.JetStreamContext) { +func sameContent(t *testing.T, m *busarchive.Manifest, src, dst nats.JetStreamContext) { t.Helper() for _, s := range m.Streams { info, err := dst.StreamInfo(s.Name) @@ -464,7 +465,7 @@ func TestASnapshotOfALiveBusRestoresToTheSameContent(t *testing.T) { if err := os.WriteFile(tarPath, archive.Bytes(), 0o644); err != nil { t.Fatal(err) } - a, err := ReadArchive(bytes.NewReader(archive.Bytes()), "") + a, err := busarchive.ReadArchive(bytes.NewReader(archive.Bytes()), "") if err != nil { t.Fatal(err) } @@ -517,7 +518,7 @@ func TestASnapshotOfALiveBusRestoresToTheSameContent(t *testing.T) { damaged := append([]byte(nil), archive.Bytes()...) // A byte in the last stream's archive, well past every header. damaged[len(damaged)-2048] ^= 0xff - d, err := ReadArchive(bytes.NewReader(damaged), "") + d, err := busarchive.ReadArchive(bytes.NewReader(damaged), "") if err == nil { defer d.Close() err = d.Verify() diff --git a/modules/nats/cmd/mesh-nats-snapshot/main.go b/modules/nats/cmd/mesh-nats-snapshot/main.go index bbfffb7d..f71e49d8 100644 --- a/modules/nats/cmd/mesh-nats-snapshot/main.go +++ b/modules/nats/cmd/mesh-nats-snapshot/main.go @@ -29,6 +29,7 @@ import ( "flag" "fmt" "io" + "nats-tools/internal/busarchive" "os" "time" ) @@ -114,7 +115,7 @@ func runVerify(args []string) error { return err } defer closer() - a, err := ReadArchive(in, "") + a, err := busarchive.ReadArchive(in, "") if err != nil { return err } @@ -146,7 +147,7 @@ func runRestore(args []string) error { return err } defer closer() - a, err := ReadArchive(in, "") + a, err := busarchive.ReadArchive(in, "") if err != nil { return err } diff --git a/modules/nats/cmd/mesh-nats-snapshot/restore.go b/modules/nats/cmd/mesh-nats-snapshot/restore.go index 069325bc..5d586609 100644 --- a/modules/nats/cmd/mesh-nats-snapshot/restore.go +++ b/modules/nats/cmd/mesh-nats-snapshot/restore.go @@ -17,6 +17,7 @@ import ( "time" "github.com/nats-io/nats.go" + "nats-tools/internal/busarchive" ) // restoreChunk is what is sent at a time; the server answers each before the next goes. @@ -25,7 +26,7 @@ const restoreChunk = 64 * 1024 // RestoreAll puts every stream of the archive into the server nc reaches, one at a time, and holds // each restored stream's state to the manifest. **Nothing is restored over anything**: a server that // already holds any of the streams is refused before the first is sent. -func RestoreAll(ctx context.Context, nc *nats.Conn, a *Archive, log func(string, ...any)) error { +func RestoreAll(ctx context.Context, nc *nats.Conn, a *busarchive.Archive, log func(string, ...any)) error { var present []string for _, s := range a.Manifest.Streams { err := request(ctx, nc, apiStreamInfo+s.Name, nil, nil) @@ -52,8 +53,8 @@ func RestoreAll(ctx context.Context, nc *nats.Conn, a *Archive, log func(string, return nil } -func restoreStream(ctx context.Context, nc *nats.Conn, a *Archive, s StreamEntry) error { - meta, archive, err := a.open(s) +func restoreStream(ctx context.Context, nc *nats.Conn, a *busarchive.Archive, s busarchive.StreamEntry) error { + meta, archive, err := a.Open(s) if err != nil { return err } @@ -123,7 +124,7 @@ func restoreStream(ctx context.Context, nc *nats.Conn, a *Archive, s StreamEntry // the messages in that tail some may be there and some not. Every message up to the manifest's last // sequence is there, exactly; that is the snapshot's promise, and what the test holds it to message // by message. What must never happen is a restored stream that ends before the manifest said it did. -func heldTo(restored streamState, s StreamEntry) error { +func heldTo(restored streamState, s busarchive.StreamEntry) error { if restored.LastSeq < s.LastSeq || (s.Messages > 0 && restored.Messages == 0) { return fmt.Errorf("restored with %d message(s) up to sequence %d; the snapshot held %d up to %d", restored.Messages, restored.LastSeq, s.Messages, s.LastSeq) @@ -140,8 +141,8 @@ func heldTo(restored streamState, s StreamEntry) error { // Why a server of its own rather than the live bus: a stream is restored only where it does not // exist, and on the live bus every stream exists — the controller asserts them on every start. So a // restore beside the live data, swapped in by a person, is the only one that never touches it. -func RestoreOffline(ctx context.Context, a *Archive, into, account string, log func(string, ...any)) (string, error) { - if !validName(account) { +func RestoreOffline(ctx context.Context, a *busarchive.Archive, into, account string, log func(string, ...any)) (string, error) { + if !busarchive.ValidName(account) { return "", fmt.Errorf("%q is not an account name", account) } if entries, err := os.ReadDir(into); err == nil && len(entries) > 0 { diff --git a/modules/nats/cmd/mesh-nats-snapshot/snapshot.go b/modules/nats/cmd/mesh-nats-snapshot/snapshot.go index 9793a671..1a153c62 100644 --- a/modules/nats/cmd/mesh-nats-snapshot/snapshot.go +++ b/modules/nats/cmd/mesh-nats-snapshot/snapshot.go @@ -11,12 +11,11 @@ import ( "io" "os" "sort" - "strings" "sync" "time" - "github.com/klauspost/compress/s2" "github.com/nats-io/nats.go" + "nats-tools/internal/busarchive" ) // The JetStream API's subjects this program speaks. Each is granted to the bus's own module and @@ -45,56 +44,6 @@ type SnapshotOptions struct { Log func(format string, args ...any) } -// Manifest is the first entry of an archive: what was taken, when, how long it took, and the checksum -// of every file beside it. It is what `verify` and `restore` hold the archive to. -type Manifest struct { - Format int `json:"format"` - Server string `json:"server_version"` - Started time.Time `json:"started"` - Finished time.Time `json:"finished"` - Seconds float64 `json:"seconds"` - // Messages and StreamBytes are the streams' own totals at their snapshots; ArchiveBytes what the - // archives take, compressed. - Messages uint64 `json:"messages"` - StreamBytes int64 `json:"stream_bytes"` - ArchiveBytes int64 `json:"archive_bytes"` - Streams []StreamEntry `json:"streams"` - // Skipped are streams that hold nothing across a restart (memory storage), said rather than - // silently left out. - Skipped []Skipped `json:"skipped,omitempty"` -} - -// StreamEntry is one stream as it was snapshotted. -type StreamEntry struct { - Name string `json:"name"` - Subjects []string `json:"subjects,omitempty"` - Retention string `json:"retention,omitempty"` - Messages uint64 `json:"messages"` - Bytes uint64 `json:"bytes"` - FirstSeq uint64 `json:"first_seq"` - LastSeq uint64 `json:"last_seq"` - Deleted int `json:"deleted,omitempty"` - Consumers int `json:"consumers"` - // Meta is the server's own description of the stream (its configuration and state), in the - // shape the restore API and the nats CLI take back: backup.json. - Meta string `json:"meta"` - MetaSHA256 string `json:"meta_sha256"` - // Archive is the server's snapshot of the stream, as the server sent it: stream.tar.s2. - Archive string `json:"archive"` - ArchiveBytes int64 `json:"archive_bytes"` - ArchiveSHA256 string `json:"archive_sha256"` - Seconds float64 `json:"seconds"` -} - -// Skipped is a stream not snapshotted, and why. -type Skipped struct { - Name string `json:"name"` - Why string `json:"why"` -} - -// ManifestName is the archive's first entry. -const ManifestName = "manifest.json" - // apiError is the error every JetStream API answer may carry. type apiError struct { Code int `json:"code"` @@ -187,14 +136,14 @@ func StreamNames(ctx context.Context, nc *nats.Conn) ([]string, error) { // Snapshot writes every stream, one at a time, as a tar: the manifest first, then each stream's // meta and archive under streams//. Any stream failing fails the whole: a partial copy of the // bus written where a whole one is expected is the silent failure this exists to prevent. -func Snapshot(ctx context.Context, nc *nats.Conn, out io.Writer, o SnapshotOptions) (*Manifest, error) { +func Snapshot(ctx context.Context, nc *nats.Conn, out io.Writer, o SnapshotOptions) (*busarchive.Manifest, error) { if o.StreamTimeout <= 0 { o.StreamTimeout = 10 * time.Minute } if o.Log == nil { o.Log = func(string, ...any) {} } - m := &Manifest{Format: 1, Server: nc.ConnectedServerVersion(), Started: time.Now().UTC()} + m := &busarchive.Manifest{Format: 1, Server: nc.ConnectedServerVersion(), Started: time.Now().UTC()} names, err := StreamNames(ctx, nc) if err != nil { return nil, err @@ -206,7 +155,7 @@ func Snapshot(ctx context.Context, nc *nats.Conn, out io.Writer, o SnapshotOptio } type taken struct { - entry StreamEntry + entry busarchive.StreamEntry meta []byte archive *os.File } @@ -233,7 +182,7 @@ func Snapshot(ctx context.Context, nc *nats.Conn, out io.Writer, o SnapshotOptio return nil, err } if info.Config.Storage == "memory" { - m.Skipped = append(m.Skipped, Skipped{Name: name, Why: "memory storage: it holds nothing across a restart, and the server cannot snapshot it"}) + m.Skipped = append(m.Skipped, busarchive.Skipped{Name: name, Why: "memory storage: it holds nothing across a restart, and the server cannot snapshot it"}) o.Log("%s: skipped, memory storage", name) continue } @@ -266,7 +215,7 @@ func Snapshot(ctx context.Context, nc *nats.Conn, out io.Writer, o SnapshotOptio if err != nil { return nil, err } - if err := writeEntry(tw, ManifestName, append(manifest, '\n'), m.Finished); err != nil { + if err := writeEntry(tw, busarchive.ManifestName, append(manifest, '\n'), m.Finished); err != nil { return nil, err } for _, t := range files { @@ -307,7 +256,7 @@ func writeEntry(tw *tar.Writer, name string, body []byte, at time.Time) error { // snapshot, then sends the archive in chunks whose reply subject is the acknowledgement it waits for // past its window — two seconds without one and it gives up with "408 No Flow Response". The end is // an empty message whose status header says 204, or the error that ended it. -func snapshotStream(ctx context.Context, nc *nats.Conn, name string, f *os.File) (StreamEntry, []byte, error) { +func snapshotStream(ctx context.Context, nc *nats.Conn, name string, f *os.File) (busarchive.StreamEntry, []byte, error) { started := time.Now() deliver := nc.NewRespInbox() @@ -350,15 +299,15 @@ func snapshotStream(ctx context.Context, nc *nats.Conn, name string, f *os.File) written += int64(len(msg.Data)) }) if err != nil { - return StreamEntry{}, nil, err + return busarchive.StreamEntry{}, nil, err } defer sub.Unsubscribe() // Every chunk is held until the callback has it; the window bounds how many there can be. if err := sub.SetPendingLimits(-1, -1); err != nil { - return StreamEntry{}, nil, err + return busarchive.StreamEntry{}, nil, err } if err := nc.FlushWithContext(ctx); err != nil { - return StreamEntry{}, nil, err + return busarchive.StreamEntry{}, nil, err } var resp struct { @@ -367,12 +316,12 @@ func snapshotStream(ctx context.Context, nc *nats.Conn, name string, f *os.File) } req := map[string]any{"deliver_subject": deliver, "chunk_size": ChunkSize} if err := request(ctx, nc, apiStreamSnapshot+name, req, &resp); err != nil { - return StreamEntry{}, nil, err + return busarchive.StreamEntry{}, nil, err } select { case <-done: case <-ctx.Done(): - return StreamEntry{}, nil, fmt.Errorf("not finished within its bound: %w", ctx.Err()) + return busarchive.StreamEntry{}, nil, fmt.Errorf("not finished within its bound: %w", ctx.Err()) } // The subscription is drained before the archive is read back, so no callback is still writing. _ = sub.Unsubscribe() @@ -380,29 +329,29 @@ func snapshotStream(ctx context.Context, nc *nats.Conn, name string, f *os.File) err = failed mu.Unlock() if err != nil { - return StreamEntry{}, nil, err + return busarchive.StreamEntry{}, nil, err } if written == 0 { - return StreamEntry{}, nil, errors.New("the server sent an empty archive") + return busarchive.StreamEntry{}, nil, errors.New("the server sent an empty archive") } if err := f.Sync(); err != nil { - return StreamEntry{}, nil, err + return busarchive.StreamEntry{}, nil, err } var cfg streamConfig var state streamState if err := json.Unmarshal(resp.Config, &cfg); err != nil { - return StreamEntry{}, nil, fmt.Errorf("its configuration: %w", err) + return busarchive.StreamEntry{}, nil, fmt.Errorf("its configuration: %w", err) } if err := json.Unmarshal(resp.State, &state); err != nil { - return StreamEntry{}, nil, fmt.Errorf("its state: %w", err) + return busarchive.StreamEntry{}, nil, fmt.Errorf("its state: %w", err) } // The archive reads to its end — s2 frames, then a tar — before it is called a snapshot. if _, err := f.Seek(0, io.SeekStart); err != nil { - return StreamEntry{}, nil, err + return busarchive.StreamEntry{}, nil, err } - if err := readsThrough(f); err != nil { - return StreamEntry{}, nil, fmt.Errorf("the archive the server sent does not read: %w", err) + if err := busarchive.ReadsThrough(f); err != nil { + return busarchive.StreamEntry{}, nil, fmt.Errorf("the archive the server sent does not read: %w", err) } // The meta, in the shape the restore API takes and the nats CLI writes (backup.json): the @@ -410,10 +359,10 @@ func snapshotStream(ctx context.Context, nc *nats.Conn, name string, f *os.File) // is never dropped on the way back. meta, err := json.MarshalIndent(map[string]json.RawMessage{"config": resp.Config, "state": resp.State}, "", " ") if err != nil { - return StreamEntry{}, nil, err + return busarchive.StreamEntry{}, nil, err } metaSum := sha256.Sum256(meta) - return StreamEntry{ + return busarchive.StreamEntry{ Name: name, Subjects: cfg.Subjects, Retention: cfg.Retention, Messages: state.Messages, Bytes: state.Bytes, FirstSeq: state.FirstSeq, LastSeq: state.LastSeq, Deleted: state.NumDelete, Consumers: state.Consumers, @@ -422,25 +371,3 @@ func snapshotStream(ctx context.Context, nc *nats.Conn, name string, f *os.File) ArchiveSHA256: hex.EncodeToString(hash.Sum(nil)), Seconds: time.Since(started).Seconds(), }, meta, nil } - -// readsThrough is whether a stream's archive decompresses and lists to its end. -func readsThrough(r io.Reader) error { - tr := tar.NewReader(s2.NewReader(r)) - for { - _, err := tr.Next() - if err == io.EOF { - return nil - } - if err != nil { - return err - } - if _, err := io.Copy(io.Discard, tr); err != nil { - return err - } - } -} - -// validName is a stream name that is one safe path component. -func validName(name string) bool { - return name != "" && name != "." && name != ".." && !strings.ContainsAny(name, "/\\\x00 *>") -} diff --git a/modules/nats/cmd/nats-tools/main.go b/modules/nats/cmd/nats-tools/main.go index f2e0f6d2..4de769dd 100644 --- a/modules/nats/cmd/nats-tools/main.go +++ b/modules/nats/cmd/nats-tools/main.go @@ -10,6 +10,7 @@ import ( "time" stdio "git.novox.be/novox/mesh-sdk/go" + "nats-tools/internal/busarchive" ) func str(description string) map[string]any { @@ -140,6 +141,12 @@ func tools(monitor Monitor, users func(context.Context) ([]byte, error), written varz, verr := get("/varz") return InForce(answer, at, werr, varz, verr), nil }}, + {Name: "nats_snapshot_verify", + Description: "Verifies the bus's snapshot (novox/hq ADR 0235) before an upgrade or a restore: the newest bus.tar in the module's snapshots directory, or the one named, with every file held to the manifest's checksum and every archive read to its end. Answers the file, its time and size, each stream's name and last sequence, `passed`, and the first fault when it did not. Reads only; nothing outside the snapshots directory.", + Input: map[string]any{"file": str("a snapshot file inside the snapshots directory, e.g. bus.tar (default: the newest .tar there)")}, + Run: func(a map[string]any) (any, error) { + return busarchive.VerifyFile(env("MESH_NATS_SNAPSHOTS_DIR", ""), arg(a, "file")) + }}, } } diff --git a/modules/nats/cmd/mesh-nats-snapshot/archive.go b/modules/nats/internal/busarchive/archive.go similarity index 94% rename from modules/nats/cmd/mesh-nats-snapshot/archive.go rename to modules/nats/internal/busarchive/archive.go index c894aa4a..0d14f545 100644 --- a/modules/nats/cmd/mesh-nats-snapshot/archive.go +++ b/modules/nats/internal/busarchive/archive.go @@ -1,4 +1,4 @@ -package main +package busarchive import ( "archive/tar" @@ -84,7 +84,7 @@ func ReadArchive(r io.Reader, scratch string) (*Archive, error) { return nil, errors.New("the archive is empty") } for _, s := range a.Manifest.Streams { - if !validName(s.Name) || s.Meta != "streams/"+s.Name+"/backup.json" || s.Archive != "streams/"+s.Name+"/stream.tar.s2" { + if !ValidName(s.Name) || s.Meta != "streams/"+s.Name+"/backup.json" || s.Archive != "streams/"+s.Name+"/stream.tar.s2" { a.Close() return nil, fmt.Errorf("the manifest names a stream %q at paths that are not its own", s.Name) } @@ -128,7 +128,7 @@ func (a *Archive) Verify() error { if err != nil { return err } - err = readsThrough(af) + err = ReadsThrough(af) af.Close() if err != nil { return fmt.Errorf("%s does not read: %w", s.Archive, err) @@ -151,8 +151,8 @@ func sha256Of(path string) (string, int64, error) { return hex.EncodeToString(h.Sum(nil)), n, nil } -// open is one stream's meta and archive. -func (a *Archive) open(s StreamEntry) (meta []byte, archive *os.File, err error) { +// Open is one stream's meta and archive. +func (a *Archive) Open(s StreamEntry) (meta []byte, archive *os.File, err error) { meta, err = os.ReadFile(a.files[s.Meta]) if err != nil { return nil, nil, err diff --git a/modules/nats/internal/busarchive/types.go b/modules/nats/internal/busarchive/types.go new file mode 100644 index 00000000..12ca5eba --- /dev/null +++ b/modules/nats/internal/busarchive/types.go @@ -0,0 +1,86 @@ +// Package busarchive is the bus's snapshot archive: its manifest, reading one back and holding it to +// its checksums. The snapshot program (cmd/mesh-nats-snapshot) writes and restores with it and the bus's +// tool nats_snapshot_verify (cmd/nats-tools) checks with it, so the two never differ on what a good +// snapshot is. +package busarchive + +import ( + "archive/tar" + "io" + "strings" + "time" + + "github.com/klauspost/compress/s2" +) + +// Manifest is the first entry of an archive: what was taken, when, how long it took, and the checksum +// of every file beside it. It is what `verify` and `restore` hold the archive to. +type Manifest struct { + Format int `json:"format"` + Server string `json:"server_version"` + Started time.Time `json:"started"` + Finished time.Time `json:"finished"` + Seconds float64 `json:"seconds"` + // Messages and StreamBytes are the streams' own totals at their snapshots; ArchiveBytes what the + // archives take, compressed. + Messages uint64 `json:"messages"` + StreamBytes int64 `json:"stream_bytes"` + ArchiveBytes int64 `json:"archive_bytes"` + Streams []StreamEntry `json:"streams"` + // Skipped are streams that hold nothing across a restart (memory storage), said rather than + // silently left out. + Skipped []Skipped `json:"skipped,omitempty"` +} + +// StreamEntry is one stream as it was snapshotted. +type StreamEntry struct { + Name string `json:"name"` + Subjects []string `json:"subjects,omitempty"` + Retention string `json:"retention,omitempty"` + Messages uint64 `json:"messages"` + Bytes uint64 `json:"bytes"` + FirstSeq uint64 `json:"first_seq"` + LastSeq uint64 `json:"last_seq"` + Deleted int `json:"deleted,omitempty"` + Consumers int `json:"consumers"` + // Meta is the server's own description of the stream (its configuration and state), in the + // shape the restore API and the nats CLI take back: backup.json. + Meta string `json:"meta"` + MetaSHA256 string `json:"meta_sha256"` + // Archive is the server's snapshot of the stream, as the server sent it: stream.tar.s2. + Archive string `json:"archive"` + ArchiveBytes int64 `json:"archive_bytes"` + ArchiveSHA256 string `json:"archive_sha256"` + Seconds float64 `json:"seconds"` +} + +// Skipped is a stream not snapshotted, and why. +type Skipped struct { + Name string `json:"name"` + Why string `json:"why"` +} + +// ManifestName is the archive's first entry. +const ManifestName = "manifest.json" + +// ReadsThrough is whether a stream's archive decompresses and lists to its end. +func ReadsThrough(r io.Reader) error { + tr := tar.NewReader(s2.NewReader(r)) + for { + _, err := tr.Next() + if err == io.EOF { + return nil + } + if err != nil { + return err + } + if _, err := io.Copy(io.Discard, tr); err != nil { + return err + } + } +} + +// ValidName is a stream name that is one safe path component. +func ValidName(name string) bool { + return name != "" && name != "." && name != ".." && !strings.ContainsAny(name, "/\\\x00 *>") +} diff --git a/modules/nats/internal/busarchive/verifyfile.go b/modules/nats/internal/busarchive/verifyfile.go new file mode 100644 index 00000000..fdb4a2fc --- /dev/null +++ b/modules/nats/internal/busarchive/verifyfile.go @@ -0,0 +1,109 @@ +package busarchive + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + "time" +) + +// StreamReport is one stream of a verified snapshot. +type StreamReport struct { + Name string `json:"name"` + LastSeq uint64 `json:"last_seq"` +} + +// Report is the answer of VerifyFile: which file was held to its manifest, and whether it passed. +type Report struct { + File string `json:"file"` + Modified time.Time `json:"modified"` + Size int64 `json:"size"` + Taken *time.Time `json:"taken,omitempty"` + Streams []StreamReport `json:"streams"` + Skipped []Skipped `json:"skipped,omitempty"` + Passed bool `json:"passed"` + // Fault is the first thing wrong, empty when the snapshot passed. + Fault string `json:"fault,omitempty"` +} + +// VerifyFile verifies one snapshot inside dir: the file called name, or the newest *.tar when name is +// empty. Nothing outside dir is read and nothing is written there; a snapshot that fails is an answer +// (Passed false, the first Fault), and only a directory without a snapshot is an error. +func VerifyFile(dir, name string) (*Report, error) { + path, info, err := pick(dir, name) + if err != nil { + return nil, err + } + r := &Report{File: filepath.Base(path), Modified: info.ModTime().UTC(), Size: info.Size(), Streams: []StreamReport{}} + f, err := os.Open(path) + if err != nil { + r.Fault = err.Error() + return r, nil + } + defer f.Close() + a, err := ReadArchive(f, "") + if err != nil { + r.Fault = err.Error() + return r, nil + } + defer a.Close() + taken := a.Manifest.Finished + r.Taken = &taken + r.Skipped = a.Manifest.Skipped + for _, s := range a.Manifest.Streams { + r.Streams = append(r.Streams, StreamReport{Name: s.Name, LastSeq: s.LastSeq}) + } + if err := a.Verify(); err != nil { + r.Fault = err.Error() + return r, nil + } + r.Passed = true + return r, nil +} + +// pick names the regular file to verify, and nothing outside dir: a name is one path component, and a +// symbolic link is not followed out of the directory. +func pick(dir, name string) (string, fs.FileInfo, error) { + if dir == "" { + return "", nil, errors.New("the snapshots directory is not set") + } + if name != "" { + if name != filepath.Base(name) || name == "." || name == ".." || strings.ContainsRune(name, 0) { + return "", nil, fmt.Errorf("%q is not a file name inside the snapshots directory", name) + } + p := filepath.Join(dir, name) + info, err := os.Lstat(p) + if err != nil { + return "", nil, fmt.Errorf("no snapshot %s in the snapshots directory: %w", name, err) + } + if !info.Mode().IsRegular() { + return "", nil, fmt.Errorf("%s is not a regular file", name) + } + return p, info, nil + } + entries, err := os.ReadDir(dir) + if err != nil { + return "", nil, fmt.Errorf("the snapshots directory does not read: %w", err) + } + var best string + var bestInfo fs.FileInfo + for _, e := range entries { + if !strings.HasSuffix(e.Name(), ".tar") { + continue + } + info, err := e.Info() + if err != nil || !info.Mode().IsRegular() { + continue + } + if bestInfo == nil || info.ModTime().After(bestInfo.ModTime()) { + best, bestInfo = e.Name(), info + } + } + if bestInfo == nil { + return "", nil, errors.New("the snapshots directory holds no .tar snapshot") + } + return filepath.Join(dir, best), bestInfo, nil +} diff --git a/modules/nats/internal/busarchive/verifyfile_test.go b/modules/nats/internal/busarchive/verifyfile_test.go new file mode 100644 index 00000000..be62737f --- /dev/null +++ b/modules/nats/internal/busarchive/verifyfile_test.go @@ -0,0 +1,173 @@ +package busarchive + +import ( + "archive/tar" + "bytes" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/klauspost/compress/s2" +) + +func sum(b []byte) string { h := sha256.Sum256(b); return hex.EncodeToString(h[:]) } + +// streamArchive is a stream's archive as the server sends it: a tar of one file, s2-compressed. +func streamArchive(t *testing.T) []byte { + t.Helper() + var raw bytes.Buffer + tw := tar.NewWriter(&raw) + body := []byte(strings.Repeat("a message of the stream\n", 50)) + if err := tw.WriteHeader(&tar.Header{Name: "msgs.blk", Mode: 0o600, Size: int64(len(body))}); err != nil { + t.Fatal(err) + } + tw.Write(body) + tw.Close() + var out bytes.Buffer + w := s2.NewWriter(&out) + w.Write(raw.Bytes()) + w.Close() + return out.Bytes() +} + +// snapshotTar builds a snapshot of two streams; the mutations change what the manifest says or what is +// stored, so each fault is made in the one place it is found. +func snapshotTar(t *testing.T, mutate func(m *Manifest, files map[string][]byte)) []byte { + t.Helper() + files := map[string][]byte{} + m := Manifest{Format: 1, Server: "2.11.17", Finished: time.Date(2026, 10, 22, 3, 0, 0, 0, time.UTC)} + for i, name := range []string{"EVENTS", "KV_conditions"} { + meta := []byte(`{"config":{"name":"` + name + `"}}`) + arc := streamArchive(t) + e := StreamEntry{Name: name, LastSeq: uint64(10 * (i + 1)), + Meta: "streams/" + name + "/backup.json", MetaSHA256: sum(meta), + Archive: "streams/" + name + "/stream.tar.s2", ArchiveBytes: int64(len(arc)), ArchiveSHA256: sum(arc)} + m.Streams = append(m.Streams, e) + files[e.Meta], files[e.Archive] = meta, arc + } + if mutate != nil { + mutate(&m, files) + } + var out bytes.Buffer + tw := tar.NewWriter(&out) + put := func(name string, b []byte) { + if err := tw.WriteHeader(&tar.Header{Name: name, Mode: 0o600, Size: int64(len(b))}); err != nil { + t.Fatal(err) + } + tw.Write(b) + } + raw, _ := json.Marshal(m) + put(ManifestName, raw) + for _, s := range m.Streams { + put(s.Meta, files[s.Meta]) + put(s.Archive, files[s.Archive]) + } + tw.Close() + return out.Bytes() +} + +func put(t *testing.T, dir, name string, b []byte, at time.Time) { + t.Helper() + p := filepath.Join(dir, name) + if err := os.WriteFile(p, b, 0o600); err != nil { + t.Fatal(err) + } + os.Chtimes(p, at, at) +} + +func TestAGoodSnapshotPassesAndSaysItsStreams(t *testing.T) { + dir := t.TempDir() + good := snapshotTar(t, nil) + put(t, dir, "bus.tar", good, time.Now()) + r, err := VerifyFile(dir, "") + if err != nil { + t.Fatal(err) + } + if !r.Passed || r.Fault != "" || r.File != "bus.tar" || r.Size != int64(len(good)) { + t.Fatalf("%+v", r) + } + if r.Taken == nil || !r.Taken.Equal(time.Date(2026, 10, 22, 3, 0, 0, 0, time.UTC)) { + t.Errorf("taken %v", r.Taken) + } + if len(r.Streams) != 2 || r.Streams[0] != (StreamReport{"EVENTS", 10}) || r.Streams[1] != (StreamReport{"KV_conditions", 20}) { + t.Errorf("streams %+v", r.Streams) + } +} + +func TestACorruptedChecksumFailsNamingTheFile(t *testing.T) { + dir := t.TempDir() + put(t, dir, "bus.tar", snapshotTar(t, func(m *Manifest, files map[string][]byte) { + m.Streams[1].ArchiveSHA256 = sum([]byte("something else")) + }), time.Now()) + r, err := VerifyFile(dir, "bus.tar") + if err != nil { + t.Fatal(err) + } + if r.Passed || !strings.Contains(r.Fault, "streams/KV_conditions/stream.tar.s2") || !strings.Contains(r.Fault, "checksum") { + t.Fatalf("%+v", r) + } + if len(r.Streams) != 2 { + t.Errorf("a failed snapshot still says its streams: %+v", r.Streams) + } +} + +func TestATruncatedSnapshotFails(t *testing.T) { + dir := t.TempDir() + good := snapshotTar(t, nil) + put(t, dir, "bus.tar", good[:len(good)*2/3], time.Now()) + r, err := VerifyFile(dir, "") + if err != nil { + t.Fatal(err) + } + if r.Passed || r.Fault == "" { + t.Fatalf("a truncated snapshot passed: %+v", r) + } +} + +func TestAnArchiveThatDoesNotReadToItsEndFailsEvenWithMatchingChecksums(t *testing.T) { + dir := t.TempDir() + put(t, dir, "bus.tar", snapshotTar(t, func(m *Manifest, files map[string][]byte) { + bad := []byte("not an s2 stream at all") + s := &m.Streams[0] + files[s.Archive] = bad + s.ArchiveSHA256, s.ArchiveBytes = sum(bad), int64(len(bad)) + }), time.Now()) + r, _ := VerifyFile(dir, "") + if r.Passed || !strings.Contains(r.Fault, "does not read") { + t.Fatalf("%+v", r) + } +} + +func TestTheNewestTarIsVerifiedAndOnlyInsideTheDirectory(t *testing.T) { + dir := t.TempDir() + now := time.Now() + put(t, dir, "old.tar", []byte("junk"), now.Add(-48*time.Hour)) + put(t, dir, "bus.tar", snapshotTar(t, nil), now) + put(t, dir, "bus.tar.partial", []byte("half"), now.Add(time.Hour)) + r, err := VerifyFile(dir, "") + if err != nil || r.File != "bus.tar" || !r.Passed { + t.Fatalf("%+v %v", r, err) + } + if r, _ := VerifyFile(dir, "old.tar"); r.Passed || r.File != "old.tar" { + t.Errorf("a named file is the one checked: %+v", r) + } + outside := t.TempDir() + put(t, outside, "x.tar", snapshotTar(t, nil), now) + for _, name := range []string{"../" + filepath.Base(outside) + "/x.tar", outside + "/x.tar", ".."} { + if _, err := VerifyFile(dir, name); err == nil { + t.Errorf("%q was read", name) + } + } + os.Symlink(filepath.Join(outside, "x.tar"), filepath.Join(dir, "link.tar")) + if _, err := VerifyFile(dir, "link.tar"); err == nil { + t.Error("a symbolic link out of the directory was followed") + } + if _, err := VerifyFile(t.TempDir(), ""); err == nil { + t.Error("an empty directory is not a snapshot") + } +} diff --git a/modules/nats/module.json b/modules/nats/module.json index fe27a566..106aac92 100644 --- a/modules/nats/module.json +++ b/modules/nats/module.json @@ -44,7 +44,8 @@ "nats_backlog", "nats_buckets", "nats_users", - "nats_user_can" + "nats_user_can", + "nats_snapshot_verify" ], "guards": [ 8222 @@ -192,7 +193,8 @@ "env": { "MESH_NATS_MONITOR": "http://127.0.0.1:8222", "MESH_NATS_CONTAINER": "mesh-broker-nats", - "MESH_NATS_USERS_FILE": "/etc/nats/accounts.conf" + "MESH_NATS_USERS_FILE": "/etc/nats/accounts.conf", + "MESH_NATS_SNAPSHOTS_DIR": "${dir:snapshots}" } }, { diff --git a/modules/nextcloud/client.ts b/modules/nextcloud/client.ts index e2310704..f0461db8 100644 --- a/modules/nextcloud/client.ts +++ b/modules/nextcloud/client.ts @@ -62,6 +62,10 @@ const ONLYOFFICE_KEYS: ReadonlyArray { } // Keeps Nextcloud's onlyoffice settings matched to the document server the mesh bound (novox/hq -// issue 1830). The client never logs; this says what it did. +// issue 1830). The client never logs; this says what it did. A failed connection check is run +// again on every later tick until it passes (issue 1832): every failing tick is logged, and one +// line marks it passing again; nothing is logged while the last check passed and nothing changed. +let onlyOfficeCheckWasFailing = false; async function applyOnlyOffice(client: NextcloudClient): Promise { const result = client.applyOnlyOffice(); - if (result.changed.length === 0) return; - console.log(`[nextcloud] onlyoffice: set ${result.changed.join(", ")}`); - if (result.checkOk === false) console.error("[nextcloud] onlyoffice: connection check failed after applying settings"); + if (result.changed.length > 0) console.log(`[nextcloud] onlyoffice: set ${result.changed.join(", ")}`); + if (result.checkOk === false) { + onlyOfficeCheckWasFailing = true; + console.error("[nextcloud] onlyoffice: connection check failed" + (result.changed.length > 0 ? " after applying settings" : "")); + } else if (result.checkOk === true && onlyOfficeCheckWasFailing) { + onlyOfficeCheckWasFailing = false; + console.log("[nextcloud] onlyoffice: connection check passes again"); + } } if (nextcloud) { diff --git a/modules/nextcloud/test/onlyoffice.test.ts b/modules/nextcloud/test/onlyoffice.test.ts index 09241597..5c90b9d8 100644 --- a/modules/nextcloud/test/onlyoffice.test.ts +++ b/modules/nextcloud/test/onlyoffice.test.ts @@ -33,8 +33,10 @@ function client(): any { /** A stand-in occ: config:app:get answers `current`, or throws like the real occ when unset. */ function standInOcc(current: Record, opts: { checkFails?: boolean } = {}) { const calls: string[][] = []; - const impl = (args: string[]): string => { + const inputs: Array = []; + const impl = (args: string[], occOpts: { input?: string } = {}): string => { calls.push(args); + inputs.push(occOpts.input); if (args[0] === "config:app:get") { const key = args[2]; if (!(key in current)) throw new Error("occ produced no output"); @@ -46,13 +48,18 @@ function standInOcc(current: Record, opts: { checkFails?: boolea current[key] = value; return "System config value onlyoffice => " + key + " set to string " + value; } + if (args[0] === "config:import") { + const parsed = JSON.parse(occOpts.input ?? "{}"); + Object.assign(current, parsed?.apps?.onlyoffice ?? {}); + return ""; + } if (args[0] === "onlyoffice:documentserver") { if (opts.checkFails) throw new Error("occ produced no output"); return ""; } throw new Error("unexpected occ call: " + args.join(" ")); }; - return { calls, impl }; + return { calls, inputs, impl }; } // 1830/AC3a: when the delivered values differ from Nextcloud's settings, it sets exactly the @@ -77,6 +84,72 @@ test("sets exactly the keys that differ, and runs the connection check", () => { assert.equal(current.StorageUrl, FULL_CONFIG.onlyofficeStorageUrl, "the matching key was left alone"); }); +// 1831/AC1: the jwt_secret never reaches a docker/occ argv — it is applied only as stdin of a +// config:import call, never as a config:app:set --value on the command line (novox/hq issue 1831). +test("the jwt_secret is set by stdin of config:import, never in an argv", () => { + const c = client(); + const current: Record = { + DocumentServerUrl: "https://old-office.example/", + DocumentServerInternalUrl: FULL_CONFIG.onlyofficeInternalUrl, + StorageUrl: FULL_CONFIG.onlyofficeStorageUrl, + jwt_secret: "a-stale-secret", + }; + const stand = standInOcc(current); + mock.method(c, "occ", stand.impl); + + const result = c.applyOnlyOffice(configFile(FULL_CONFIG)); + + assert.ok(result.changed.includes("jwt_secret"), "jwt_secret was changed"); + assert.ok(!stand.calls.some((a) => a.join(" ").includes(SECRET)), "the secret is in no argv"); + const importCall = stand.calls.findIndex((a) => a[0] === "config:import"); + assert.notEqual(importCall, -1, "a config:import call was made"); + assert.ok(!stand.calls.some((a) => a[0] === "config:app:set" && a[2] === "jwt_secret"), + "jwt_secret is never set via config:app:set"); + assert.equal(stand.inputs[importCall], JSON.stringify({ apps: { onlyoffice: { jwt_secret: SECRET } } }), + "the secret is passed only as stdin of config:import"); + assert.equal(current.jwt_secret, SECRET, "the secret was actually applied"); +}); + +// 1832/AC1: a failed check keeps running on every later tick, even with nothing changed, reporting +// checkOk false each time, until it passes — then nothing runs again while nothing changes +// (novox/hq issue 1832). +test("a failed check is re-run every tick until it passes, then nothing runs while unchanged", () => { + const c = client(); + const current: Record = {}; // every key unset, so tick 1 changes all four + const checkOpts: { checkFails?: boolean } = { checkFails: true }; + const stand = standInOcc(current, checkOpts); + mock.method(c, "occ", stand.impl); + const checkCalls = () => stand.calls.filter((a) => a[0] === "onlyoffice:documentserver").length; + const file = configFile(FULL_CONFIG); + + // Tick 1: everything changes, the check runs and fails (1832/AC1c: reported on a failing tick). + const tick1 = c.applyOnlyOffice(file); + assert.deepEqual(tick1.changed.sort(), ["DocumentServerInternalUrl", "DocumentServerUrl", "StorageUrl", "jwt_secret"].sort()); + assert.equal(tick1.checkOk, false); + assert.equal(checkCalls(), 1); + + // Tick 2: nothing changed, but the last check failed — it is run again (1832/AC1a), and fails + // again, reported again (1832/AC1c). + const tick2 = c.applyOnlyOffice(file); + assert.deepEqual(tick2.changed, []); + assert.equal(tick2.checkOk, false); + assert.equal(checkCalls(), 2, "the check ran again though nothing changed"); + + // Tick 3: the document server now answers; still nothing changed, but the last check had + // failed, so it is run again — and this time it passes. + checkOpts.checkFails = false; + const tick3 = c.applyOnlyOffice(file); + assert.deepEqual(tick3.changed, []); + assert.equal(tick3.checkOk, true); + assert.equal(checkCalls(), 3); + + // Tick 4: nothing changed, and the last check passed — no check runs (1832/AC1b). + const tick4 = c.applyOnlyOffice(file); + assert.deepEqual(tick4.changed, []); + assert.equal(tick4.checkOk, undefined); + assert.equal(checkCalls(), 3, "no further check ran once the last one passed"); +}); + // 1830/AC3b: when they match, it changes nothing and runs no check. test("changes nothing and runs no check when every value already matches", () => { const c = client(); diff --git a/modules/only-office/README.md b/modules/only-office/README.md index 3129fc4b..8b1a5291 100644 --- a/modules/only-office/README.md +++ b/modules/only-office/README.md @@ -7,10 +7,24 @@ The `only-office` module of the mesh catalogue: runs the container image `onlyof Each directory or archive this module declares says how it treats a file it did not write (ADR 0422, ADR 0416). - `${dir:state}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). +- `${dir:box}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:logs}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:data}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). +- `${dir:server-lib}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:lib}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:db}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:rabbitmq}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:redis}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:fonts}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). + +## Where its directories live + +Everything the container mounts sits in `${dir:box}` (`/var/lib/only-office/box`), root-owned and 0700 (issue 1843). +The image opens what is below each mount to every account on every start (`chmod -R 755`), so the barrier is a +parent the container never sees. The module root stays root-owned 0711: the converter, which runs on the host +as `only-office-converter`, passes through it to its two secrets and nothing else. + +`${dir:lib}` (`/var/lib/only-office/lib`) is the copy the container used before, no longer mounted, kept by the +mesh because it is not empty and closed to 0700. It keeps its id so mesh-host sets that mode: a new id at a path +the image owns would be used as found (issue 339). Its contents, and those of the old `logs`, `data`, `db`, +`rabbitmq`, `redis` and `fonts` beside it, are removed on the operator's word, and then `lib` leaves the manifest. diff --git a/modules/only-office/files/server.env b/modules/only-office/files/server.env index 19fab119..a92e1c8f 100644 --- a/modules/only-office/files/server.env +++ b/modules/only-office/files/server.env @@ -2,4 +2,5 @@ JWT_ENABLED=true JWT_SECRET=${secret:jwt} JWT_HEADER=Authorization JWT_IN_BODY=true +SECURE_LINK_SECRET=${secret:secure-link} ALLOW_PRIVATE_IP_ADDRESS=true diff --git a/modules/only-office/manifest_test.go b/modules/only-office/manifest_test.go index cbaec2b4..2e44316d 100644 --- a/modules/only-office/manifest_test.go +++ b/modules/only-office/manifest_test.go @@ -1,7 +1,9 @@ package office import ( + "encoding/json" "os" + "strings" "testing" "git.novox.be/novox/mesh-sdk/go/catalogue" @@ -61,3 +63,234 @@ func TestManifestOffersDocumentServer(t *testing.T) { t.Errorf("serves.document-server.public-root = %q, want https://${bound:route:name}/", root) } } + +// 1837/AC1 (unit: manifest check) +// Given the catalogue, when only-office's manifest and its server.env template are read, then the +// document server's secure-link secret is an own secret taken at start, and server.env hands it to +// the container as SECURE_LINK_SECRET, so its document service and its nginx sign and check +// Editor.bin links with one value across every restart. +func TestSecureLinkSecretIsPinned(t *testing.T) { + raw, err := os.ReadFile("module.json") + if err != nil { + t.Fatalf("reading module.json: %v", err) + } + m, err := catalogue.ParseManifest(raw) + if err != nil { + t.Fatalf("parsing module.json: %v", err) + } + own, ok := m.OwnSecrets["secure-link"] + if !ok { + t.Fatal("manifest declares no own-secret secure-link") + } + if own.Taken != catalogue.TakenAtStart { + t.Errorf("own-secrets.secure-link.taken = %q, want %q", own.Taken, catalogue.TakenAtStart) + } + + env, err := os.ReadFile("files/server.env") + if err != nil { + t.Fatalf("reading files/server.env: %v", err) + } + if !strings.Contains(string(env), "\nSECURE_LINK_SECRET=${secret:secure-link}\n") { + t.Errorf("files/server.env does not set SECURE_LINK_SECRET=${secret:secure-link}:\n%s", env) + } +} + +// 1837/AC3 (unit: manifest check) +// Given the catalogue, when only-office's manifest is read, then the directory mounted at +// /var/lib/onlyoffice (server-lib, where the document service writes the converted files nginx +// serves) lets other users pass through it (mode 0711): the image makes it the document service's +// own, and nginx runs as another user, so a mode of 0700 makes nginx answer 403 "Permission denied" +// on Editor.bin. Host accounts are kept out by the box above it (1843/AC1). +func TestLibDirectoryLetsNginxThrough(t *testing.T) { + m := readLayout(t) + src := m.volumeSource(t, "/var/lib/onlyoffice") + if src != "${dir:server-lib}" { + t.Fatalf("/var/lib/onlyoffice is mounted from %q, want ${dir:server-lib}", src) + } + d := m.dir(t, "server-lib") + if d.Mode != "0711" { + t.Errorf("server-lib directory mode = %q, want 0711 so nginx can reach the document service's cache", d.Mode) + } +} + +// 1843/AC1 (unit: manifest check) +// Given the catalogue, when only-office's manifest is read, then it declares a directory box with no +// place, no path and no owner (root, /var/lib/only-office/box) and mode 0700, and every host source of +// the server container's volumes is a directory whose path is ${dir:box}/, so no account but +// root can pass to anything the container writes: the image opens what is below each mount to every +// account on every start. +func TestContainerDirectoriesSitInARootOnlyBox(t *testing.T) { + m := readLayout(t) + box := m.dir(t, "box") + if box.Place != "" || box.Path != "" || box.Owner != "" { + t.Errorf("box = place %q, path %q, owner %q, want none of them (root, at /only-office/box)", box.Place, box.Path, box.Owner) + } + if box.Mode != "0700" { + t.Errorf("box mode = %q, want 0700", box.Mode) + } + c := m.container(t, "server") + if len(c.Volumes) == 0 { + t.Fatal("the server container mounts nothing") + } + for _, v := range c.Volumes { + src, target := splitVolume(v) + id, ok := strings.CutPrefix(src, "${dir:") + if !ok || !strings.HasSuffix(id, "}") { + t.Errorf("volume %q has a host source that is not a directory of the module", v) + continue + } + d := m.dir(t, strings.TrimSuffix(id, "}")) + if !strings.HasPrefix(d.Path, "${dir:box}/") || strings.Count(d.Path, "/") != 1 { + t.Errorf("%s (mounted at %s) has path %q, want ${dir:box}/", d.ID, target, d.Path) + } + if d.Owner != "" { + t.Errorf("%s is owned by %q, want root", d.ID, d.Owner) + } + } +} + +// 1843/AC2 (unit: manifest check) +// Given the catalogue, when only-office's manifest is read, then the module root (state) is still +// root-owned, mode 0711, place ".", and the converter's secrets and environment still name +// ${dir:state}/jwt.secret and ${dir:state}/broker-converter.secret: the converter passes the root and +// nothing below it, and no account but root owns a directory on any path. +func TestConverterPassesOnlyTheRoot(t *testing.T) { + m := readLayout(t) + state := m.dir(t, "state") + if state.Place != "." || state.Owner != "" || state.Mode != "0711" { + t.Errorf("state = place %q, owner %q, mode %q, want place ., root, 0711", state.Place, state.Owner, state.Mode) + } + for _, r := range m.Resources { + if r.Type == "directory" && r.Owner != "" { + t.Errorf("directory %s is owned by %q, want root", r.ID, r.Owner) + } + } + for name, want := range map[string]string{"jwt": "${dir:state}/jwt.secret", "broker-converter": "${dir:state}/broker-converter.secret"} { + if got := m.OwnSecrets[name].Path; got != want { + t.Errorf("own-secrets.%s.path = %q, want %q", name, got, want) + } + } + var conv *layoutResource + for i := range m.Resources { + if m.Resources[i].ID == "converter" { + conv = &m.Resources[i] + } + } + if conv == nil { + t.Fatal("manifest declares no converter") + } + if conv.Env["ONLY_OFFICE_JWT"] != "${dir:state}/jwt.secret" || conv.Env["ONLY_OFFICE_BROKER"] != "${dir:state}/broker-converter.secret" { + t.Errorf("converter env = %v, want its secrets in ${dir:state}", conv.Env) + } +} + +// 1843/AC3 (unit: manifest check) +// Given the catalogue, when only-office's manifest is read, then the directory lib is declared with no +// path (its old place, /var/lib/only-office/lib) and mode 0700, so the next apply closes the copy the +// image opened, which mesh-host keeps because it is not empty; no container volume names ${dir:lib}; +// and every data entry points at a directory that is mounted now. +func TestOldLibIsClosedAndUnmounted(t *testing.T) { + m := readLayout(t) + lib := m.dir(t, "lib") + if lib.Path != "" || lib.Place != "" { + t.Errorf("lib = path %q, place %q, want neither: it stays where the old copy is", lib.Path, lib.Place) + } + if lib.Mode != "0700" { + t.Errorf("lib mode = %q, want 0700", lib.Mode) + } + mounted := map[string]bool{} + for _, v := range m.container(t, "server").Volumes { + src, _ := splitVolume(v) + mounted[src] = true + } + if mounted["${dir:lib}"] { + t.Error("the server container still mounts ${dir:lib}") + } + for _, d := range m.Data.Own { + if !mounted[d.Path] { + t.Errorf("data entry %s points at %q, which the server container does not mount", d.ID, d.Path) + } + } +} + +type layoutResource struct { + ID string `json:"id"` + Type string `json:"type"` + Mode string `json:"mode"` + Place string `json:"place"` + Path string `json:"path"` + Owner string `json:"owner"` + Volumes []string `json:"volumes"` + Env map[string]string `json:"env"` +} + +type layout struct { + Resources []layoutResource `json:"resources"` + OwnSecrets map[string]struct { + Path string `json:"path"` + } `json:"own-secrets"` + Data struct { + Own []struct { + ID string `json:"id"` + Path string `json:"path"` + } `json:"own"` + } `json:"data"` +} + +func readLayout(t *testing.T) layout { + t.Helper() + raw, err := os.ReadFile("module.json") + if err != nil { + t.Fatalf("reading module.json: %v", err) + } + if _, err := catalogue.ParseManifest(raw); err != nil { + t.Fatalf("parsing module.json: %v", err) + } + var m layout + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatalf("parsing module.json: %v", err) + } + return m +} + +func (m layout) dir(t *testing.T, id string) layoutResource { + t.Helper() + for _, r := range m.Resources { + if r.ID == id && r.Type == "directory" { + return r + } + } + t.Fatalf("manifest declares no directory %s", id) + return layoutResource{} +} + +func (m layout) container(t *testing.T, id string) layoutResource { + t.Helper() + for _, r := range m.Resources { + if r.ID == id && r.Type == "container" { + return r + } + } + t.Fatalf("manifest declares no container %s", id) + return layoutResource{} +} + +func (m layout) volumeSource(t *testing.T, target string) string { + t.Helper() + for _, v := range m.container(t, "server").Volumes { + if src, to := splitVolume(v); to == target { + return src + } + } + t.Fatalf("the server container mounts nothing at %s", target) + return "" +} + +// splitVolume is a volume's host source and container target; the source's own ${dir:…} holds a colon. +func splitVolume(v string) (src, target string) { + i := strings.LastIndex(v, ":") + if i < 0 { + return v, "" + } + return v[:i], v[i+1:] +} diff --git a/modules/only-office/module.json b/modules/only-office/module.json index 157ca954..b4c1f5a1 100644 --- a/modules/only-office/module.json +++ b/modules/only-office/module.json @@ -76,6 +76,10 @@ "owner": "only-office-converter", "taken": "at-start" }, + "secure-link": { + "path": "${dir:state}/secure-link.secret", + "taken": "at-start" + }, "broker-converter": { "path": "${dir:state}/broker-converter.secret", "owner": "only-office-converter" @@ -100,7 +104,7 @@ }, { "id": "lib", - "path": "${dir:lib}", + "path": "${dir:server-lib}", "class": "rebuildable", "why": "the document server's working files" }, @@ -157,18 +161,33 @@ "mode": "0600", "template": "files/server.env" }, + { + "id": "box", + "type": "directory", + "mode": "0700", + "unknown": "program" + }, { "id": "logs", "type": "directory", + "path": "${dir:box}/logs", "mode": "0700", "unknown": "program" }, { "id": "data", "type": "directory", + "path": "${dir:box}/data", "mode": "0700", "unknown": "program" }, + { + "id": "server-lib", + "type": "directory", + "path": "${dir:box}/lib", + "mode": "0711", + "unknown": "program" + }, { "id": "lib", "type": "directory", @@ -178,24 +197,28 @@ { "id": "db", "type": "directory", + "path": "${dir:box}/db", "mode": "0700", "unknown": "program" }, { "id": "rabbitmq", "type": "directory", + "path": "${dir:box}/rabbitmq", "mode": "0700", "unknown": "program" }, { "id": "redis", "type": "directory", + "path": "${dir:box}/redis", "mode": "0700", "unknown": "program" }, { "id": "fonts", "type": "directory", + "path": "${dir:box}/fonts", "mode": "0700", "unknown": "program" }, @@ -219,13 +242,13 @@ "volumes": [ "${dir:logs}:/var/log/onlyoffice", "${dir:data}:/var/www/onlyoffice/Data", - "${dir:lib}:/var/lib/onlyoffice", + "${dir:server-lib}:/var/lib/onlyoffice", "${dir:db}:/var/lib/postgresql", "${dir:rabbitmq}:/var/lib/rabbitmq", "${dir:redis}:/var/lib/redis", "${dir:fonts}:/usr/share/fonts/truetype/custom" ], - "secrets-in-environment": "run-document-server.sh regenerates JWT_SECRET from the environment on every start and overwrites local.json; not convertible" + "secrets-in-environment": "run-document-server.sh regenerates JWT_SECRET and SECURE_LINK_SECRET from the environment on every start and overwrites local.json and nginx's ds.conf; not convertible" }, { "id": "converter", diff --git a/particulars b/particulars new file mode 100644 index 00000000..b2ab98b1 --- /dev/null +++ b/particulars @@ -0,0 +1,5 @@ +# This mesh's node names (novox/hq ADR 0507 §9, issue 1838): the one place they may be written. +novox +ace +shanks +g14