diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index b3964f2..feb1da7 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -650,12 +650,17 @@ func runApply(ctx context.Context, opts options, d *declaration.Declaration, raw // Minding the windows the daemon's scheduler has open: this runs as its own process, and a // `reconcile` by hand at 03:31 is exactly the apply that must not restart a server a collector // is holding still (novox/hq issue 224). - report, updated, applyErr := apply.ApplyMindingWindows(ctx, sys, d, known, origin, - apply.ExecRunner, func(line string) { - if !opts.json { - fmt.Fprintln(out, line) - } - }, sealOpener(opts.state), apply.KeepIn(filepath.Dir(opts.state)), apply.WindowsIn(filepath.Dir(opts.state))) + // Once more when the package manager did not answer and this apply wrote something: what it wrote may be + // what makes it answer, and then the packages go in now rather than in five minutes (issue 233, probe.go). + tell := func(line string) { + if !opts.json { + fmt.Fprintln(out, line) + } + } + report, updated, applyErr, probe, _ := applyMindingProbe(ctx, sys, probe, known, func(known store.State) (apply.Report, store.State, error) { + return apply.ApplyMindingWindows(ctx, sys, d, known, origin, apply.ExecRunner, tell, sealOpener(opts.state), + apply.KeepIn(filepath.Dir(opts.state)), apply.WindowsIn(filepath.Dir(opts.state))) + }, tell) // What this apply settles about the node, whichever way it went. The mode is what the // declaration said and the check above agreed with; the bundle, once applied, is consumed. @@ -1204,7 +1209,7 @@ func runLink(ctx context.Context, opts options) error { Membership: membership, Apply: applier, Reconcile: reconcileKept(opts, mine.Membership.Signer, sched, say), - News: func(r link.Report) bool { return worthSaying(r) && watch.differs(r) }, + News: watch.news, // Counted as said only once the broker has taken it: queued and lost — the link down, the // publish refused — the change would never be said again (novox/hq ADR 0100). // And the first account under this build is what proves it to the launcher (to-be 45 §8). @@ -1345,6 +1350,8 @@ func (n numbersFile) Save(reportSequence, refusedOlder int64) error { type adoptionWatch struct { mu sync.Mutex last string + // faulty is whether the last report the mesh heard said an apply refused or failed (novox/hq issue 233). + faulty bool } // fingerprint is what a report says about adoption: each hold and whether it changed, the @@ -1371,6 +1378,12 @@ func adoptionFingerprint(r link.Report) string { for _, v := range r.Rollbacks { parts = append(parts, fmt.Sprintf("rollback %s %s %s %s", v.Component, v.From, v.To, v.Outcome)) } + // **And how the apply went** (novox/hq issue 233): refused, clean, or which resources it did not apply. A + // reconcile is the only apply a machine makes between deliveries, so a machine whose package manager stopped + // answering, or answers again, says so at the next reconcile — otherwise the mesh's record of what it did + // stays whatever the last delivery left, and a fault that began or ended between deliveries is never seen. + // Not the words: an error that carries a time would read as new on every reconcile. + parts = append(parts, applyState(r)) if r.FoundFirewall != nil { parts = append(parts, fmt.Sprintf("found-firewall %s active=%v retired-by=%s", r.FoundFirewall.Kind, r.FoundFirewall.Active, r.FoundFirewall.RetiredBy)) @@ -1383,6 +1396,40 @@ func adoptionFingerprint(r link.Report) string { return strings.Join(parts, "\n") } +// applyState is how an apply went, as the fingerprint keeps it: refused, clean, or failed with what it did not +// apply and which keys its failure is said under. +func applyState(r link.Report) string { + switch { + case r.Refused != "": + return "apply refused" + case len(r.Failed) == 0: + return "apply clean" + } + keys := make([]string, 0, len(r.Failed)) + for k := range r.Failed { + keys = append(keys, k) + } + sort.Strings(keys) + return "apply failed " + strings.Join(keys, ",") + " unapplied " + strings.Join(r.Unapplied, ",") +} + +// faulty is whether a report says an apply refused or failed. +func faulty(r link.Report) bool { return r.Refused != "" || len(r.Failed) > 0 } + +// news is whether a reconcile's report is worth saying unasked: it says something the last one the mesh heard +// did not, and it is about the machine (worthSaying) — or about how the apply went, refused or failed now, or +// clean after the last one the mesh heard was not (novox/hq issue 233). A refusal says nothing about the +// machine, but that the machine refuses is what the mesh raises a condition from, and a clean apply after it is +// what clears one. +func (w *adoptionWatch) news(r link.Report) bool { + w.mu.Lock() + defer w.mu.Unlock() + if adoptionFingerprint(r) == w.last { + return false + } + return worthSaying(r) || faulty(r) || w.faulty +} + // differs says whether a report says anything the last one that went out did not. It records // nothing: what was said is what reached the mesh, not what was written down to send. func (w *adoptionWatch) differs(r link.Report) bool { @@ -1396,6 +1443,7 @@ func (w *adoptionWatch) said(r link.Report) { w.mu.Lock() defer w.mu.Unlock() w.last = adoptionFingerprint(r) + w.faulty = faulty(r) } // changed is differs and said together, for a report published as it is made. @@ -1786,8 +1834,10 @@ func holdTheMachine(ctx context.Context, queue *link.Queue) { // node when it can say which links face outside (ADR 0140) — a converged node holds nothing and found // no firewall, so without that it could speak only in reply to a declaration, while the mesh composes // no declaration for a node that has not said which links face outside. Whether a report is news is -// the queue's News (worthSaying and the watch), so an unchanged answer costs one comparison every -// reconcile and nothing on the bus. +// the queue's News (the watch's news), so an unchanged answer costs one comparison every reconcile and +// nothing on the bus. And on any node when how the apply went changed — refused, clean, or which resources it +// did not apply — since that is what the mesh raises a machine that does not apply from, and clears it by +// (novox/hq issue 233). func reconcileKept(opts options, signer ed25519.PublicKey, sched *apply.Scheduler, say link.Announce) link.Reconcile { return func(ctx context.Context) (link.Report, bool) { @@ -1973,9 +2023,13 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto // // `say` already reaches stdout, and the launcher's unit sends that to the journal, so this needs // no new mechanism — only for the argument to be passed. - outcome, updated, applyErr := apply.ApplyMindingWindows(ctx, built, declared, known, store.OriginDeclared, - apply.ExecRunner, announceOr(say), sealOpener(opts.state), apply.KeepIn(filepath.Dir(opts.state)), - apply.WindowsIn(filepath.Dir(opts.state))) + // Once more when the package manager did not answer and this apply wrote something (issue 233, probe.go). + outcome, updated, applyErr, probe, firstPass := applyMindingProbe(ctx, built, probe, known, + func(known store.State) (apply.Report, store.State, error) { + return apply.ApplyMindingWindows(ctx, built, declared, known, store.OriginDeclared, + apply.ExecRunner, announceOr(say), sealOpener(opts.state), apply.KeepIn(filepath.Dir(opts.state)), + apply.WindowsIn(filepath.Dir(opts.state))) + }, announceOr(say)) // The mode the mesh said, recorded whichever way the apply went: the declaration is kept // either way, and the node is held to it from the next reconcile (novox/hq ADR 0100). @@ -1986,7 +2040,7 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto // Root acted on what an agent account's judgement reads — an account, a package, an action, sudo's // rules: the search for setuid programs runs again (novox/hq issue 361). A failed apply counts what it // did before it stopped. - moved := outcome.Outcomes + moved := append(append([]apply.Outcome(nil), firstPass...), outcome.Outcomes...) var partial *apply.Error if errors.As(applyErr, &partial) { moved = append(append([]apply.Outcome(nil), moved...), partial.Done.Outcomes...) @@ -2132,6 +2186,8 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto report.Failed = map[string]string{"apply": applyErr.Error()} } report.Failed = withUnanswered(report.Failed, probe) + // Which resources did not apply, for whether a reconcile's account is news (issue 233): not said on the wire. + report.Unapplied = unapplied(declared, outcome, applyErr) return report } diff --git a/cmd/mesh-host/probe.go b/cmd/mesh-host/probe.go index 9b3709d..64934f4 100644 --- a/cmd/mesh-host/probe.go +++ b/cmd/mesh-host/probe.go @@ -3,8 +3,12 @@ package main import ( "context" "errors" + "os/exec" + "sort" "github.com/novox/mesh-host/internal/apply" + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/store" "github.com/novox/mesh-host/internal/system" ) @@ -46,6 +50,11 @@ func (u *unanswered) partial() error { // undo what an earlier one did. Before the rest is applied, a declaration composed from an older view of the // assignments than this machine applied has already been refused (issue 234's generation). // +// **A package manager that is not there at all still refuses whole.** That is the host installed on a +// machine it was not built for — the arch host on Alpine — which is what the probe exists to catch, and no +// declaration the mesh sends removes a package manager's program. Only one that is there and errors (a +// configuration file gone, a database locked) is the mesh's own fault and applies in part. +// // A system with no package manager (android) still refuses whole: what its probe asks is the system itself, // which no resource the mesh declares can remove, and nothing else stands between it and the wrong machine. func probePackageManager(ctx context.Context, sys system.System) (*unanswered, error) { @@ -54,12 +63,84 @@ func probePackageManager(ctx context.Context, sys system.System) (*unanswered, e return nil, nil } tool, ok := packageManagers[sys.Name()] - if !ok { + if !ok || errors.Is(err, exec.ErrNotFound) { return nil, err } return &unanswered{tool: tool, words: err.Error()}, nil } +// applyPass is one apply of the declaration over what the node's state says. +type applyPass func(known store.State) (apply.Report, store.State, error) + +// applyMindingProbe applies, and **once more when the package manager did not answer and this apply wrote +// something** (novox/hq issue 233). What it wrote may be what the package manager needed — the declaration +// that names its module again writes its configuration file back — and then the packages go in now rather +// than at the next reconcile. Asked again only then: an apply that wrote nothing changed nothing the package +// manager reads. The second pass is the same declaration over the state the first left, so what the first +// applied reads unchanged and only what needs the package manager moves. +// +// It returns the last pass's account, the probe as it stands after it (nil once the package manager +// answers), and the first pass's outcomes when a second ran, for whoever counts what an apply moved. +func applyMindingProbe(ctx context.Context, sys system.System, probe *unanswered, known store.State, + pass applyPass, say func(string)) (apply.Report, store.State, error, *unanswered, []apply.Outcome) { + report, updated, err := pass(known) + if probe == nil || !wrote(report, err) { + return report, updated, err, probe, nil + } + again, refused := probePackageManager(ctx, sys) + if refused != nil || again != nil { + return report, updated, err, probe, nil + } + say(probe.tool + " answers now that this apply wrote what it reads; applying again, so its packages are " + + "installed in this apply (novox/hq issue 233)") + first := append([]apply.Outcome(nil), report.Outcomes...) + var partial *apply.Error + if errors.As(err, &partial) { + first = append(first, partial.Done.Outcomes...) + } + report, updated, err = pass(updated) + return report, updated, err, nil, first +} + +// wrote is whether an apply moved anything on the machine, failed or not. +func wrote(report apply.Report, err error) bool { + if report.Changed() { + return true + } + var partial *apply.Error + return errors.As(err, &partial) && partial.Done.Changed() +} + +// unapplied is which declared resources an apply did not apply, failed or not attempted, sorted; nil for a +// clean one. +func unapplied(d *declaration.Declaration, report apply.Report, err error) []string { + if err == nil { + return nil + } + done := map[string]bool{} + for _, o := range report.Outcomes { + done[o.ID] = o.Action != "skipped" + } + var partial *apply.Error + if errors.As(err, &partial) { + for _, o := range partial.Done.Outcomes { + done[o.ID] = done[o.ID] || o.Action != "skipped" + } + } + var out []string + for _, r := range d.Resources { + if !done[r.Identity()] { + out = append(out, r.Identity()) + } + } + sort.Strings(out) + if len(out) == 0 { + // Failed, and every resource has an account: what failed is no resource (the firewall found, a removal). + out = []string{"(apply)"} + } + return out +} + // withUnanswered adds the probe to what a report says failed. func withUnanswered(failed map[string]string, u *unanswered) map[string]string { if u == nil { diff --git a/cmd/mesh-host/probe_test.go b/cmd/mesh-host/probe_test.go index 294a861..d26e156 100644 --- a/cmd/mesh-host/probe_test.go +++ b/cmd/mesh-host/probe_test.go @@ -4,10 +4,12 @@ import ( "context" "os" "path/filepath" + "slices" "strings" "testing" "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/link" "github.com/novox/mesh-host/internal/system" ) @@ -56,3 +58,152 @@ func TestASystemWithNoPackageManagerStillRefusesWhole(t *testing.T) { t.Fatalf("the android host's failed probe did not refuse: %v, %+v", err, probe) } } + +// pacmanAnsweringOnce is a pacman that does not answer until conf exists, as the workstation's did until its +// configuration file was written back, and installs into a directory of its own once it does. +func pacmanAnsweringOnce(t *testing.T, conf string) { + t.Helper() + bin, installed := t.TempDir(), t.TempDir() + script := "#!/bin/sh\n" + + "[ -f '" + conf + "' ] || { echo \"error: config file " + conf + " could not be read: No such file or directory\" >&2; exit 1; }\n" + + "for a; do last=$a; done\n" + + "case \"$1\" in\n" + + "-Q) [ \"$2\" = pacman ] && exit 0; [ -f '" + installed + "'/\"$2\" ] && exit 0; exit 1;;\n" + + "-S) touch '" + installed + "'/\"$last\"; exit 0;;\n" + + "esac\nexit 1\n" + if err := os.WriteFile(filepath.Join(bin, "pacman"), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + was := builtFor + builtFor = "arch" + t.Cleanup(func() { builtFor = was }) +} + +// Defends the review of novox/hq issue 233: **the apply that writes the package manager's configuration back +// installs its packages too**. The probe is asked again once the partial apply wrote something, and when it +// answers the declaration is applied again over what the first pass left — the packages in this apply, not five +// minutes later, and the report clean. +func TestThePackagesGoInWhenTheApplyThatRestoresThePackageManagerWroteIt(t *testing.T) { + dir := t.TempDir() + conf := filepath.Join(dir, "pacman.conf") + pacmanAnsweringOnce(t, conf) + opts := options{state: filepath.Join(dir, "state.json")} + // The package first: within one pass it is asked about before the file is written, which is the order that + // leaves it to a second pass. + raw := []byte(`{"declaration":1,"resources":[` + + `{"id":"pacman.base","type":"package","package":"base"},` + + `{"id":"pacman.conf","type":"file","path":"` + conf + `","content":"[options]\n"}]}`) + + report := applyAndKeep(context.Background(), opts, raw, nil, nil, nil) + + if report.Refused != "" || len(report.Failed) != 0 { + t.Fatalf("the apply that made the package manager answer did not install its packages: refused %q, failed %v", + report.Refused, report.Failed) + } + if !slices.Contains(report.Applied, "pacman.base") || !slices.Contains(report.Applied, "pacman.conf") { + t.Errorf("the report does not account for both: %v", report.Applied) + } +} + +// Defends decision (1) of the review of novox/hq issue 233: **a package manager that is not there at all still +// refuses whole**. That is the host installed on the wrong machine, which the probe exists to catch. +func TestAPackageManagerThatIsNotThereRefusesWhole(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + was := builtFor + builtFor = "arch" + t.Cleanup(func() { builtFor = was }) + dir := t.TempDir() + conf := filepath.Join(dir, "pacman.conf") + raw := []byte(`{"declaration":1,"resources":[{"id":"pacman.conf","type":"file","path":"` + conf + + `","content":"[options]\n"}]}`) + + report := applyAndKeep(context.Background(), options{state: filepath.Join(dir, "state.json")}, raw, nil, nil, nil) + + if !strings.Contains(report.Refused, "pacman does not answer") { + t.Fatalf("a machine with no pacman at all was not refused whole: %+v", report) + } + if _, err := os.Stat(conf); err == nil { + t.Error("the refused declaration touched the machine") + } +} + +// Defends the review of novox/hq issue 233: **a re-apply whose result changed is said unasked**. A machine +// reconciles every five minutes and speaks only when its account is news; how the apply went was not part of +// it, so a package manager that stopped answering between deliveries was never said, and one that answered +// again never cleared. Now a failure is said once, the same failure again is not, and a clean re-apply after it +// is — which is what clears the mesh's condition. +func TestAReapplyWhoseResultChangedIsSaidUnasked(t *testing.T) { + dir := t.TempDir() + conf := filepath.Join(dir, "elsewhere", "pacman.conf") // not declared: the apply cannot make it answer + pacmanAnsweringOnce(t, conf) + opts := options{state: filepath.Join(dir, "state.json")} + raw := []byte(`{"declaration":1,"resources":[` + + `{"id":"motd","type":"file","path":"` + filepath.Join(dir, "motd") + `","content":"hello\n"},` + + `{"id":"pacman.base","type":"package","package":"base"}]}`) + w := &adoptionWatch{} + + // Clean once, as it was before: base installed while pacman answered. + if err := os.MkdirAll(filepath.Dir(conf), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(conf, []byte("[options]\n"), 0o644); err != nil { + t.Fatal(err) + } + clean := applyAndKeep(context.Background(), opts, raw, nil, nil, nil) + if faulty(clean) { + t.Fatalf("the first apply was not clean: %+v", clean) + } + w.said(clean) + if w.news(applyAndKeep(context.Background(), opts, raw, nil, nil, nil)) { + t.Error("a clean re-apply after a clean one was said, so every machine speaks every five minutes") + } + + // The configuration goes: the re-apply fails, and that is news once. + if err := os.Remove(conf); err != nil { + t.Fatal(err) + } + failed := applyAndKeep(context.Background(), opts, raw, nil, nil, nil) + if !faulty(failed) { + t.Fatalf("the re-apply did not fail: %+v", failed) + } + if !w.news(failed) { + t.Fatal("a re-apply that failed after a clean one was not said, so the mesh never hears the machine fails") + } + w.said(failed) + if w.news(applyAndKeep(context.Background(), opts, raw, nil, nil, nil)) { + t.Error("the same failure was said again on the next re-apply") + } + + // It comes back: the clean re-apply is news, and is what clears the mesh's condition. + if err := os.WriteFile(conf, []byte("[options]\n"), 0o644); err != nil { + t.Fatal(err) + } + again := applyAndKeep(context.Background(), opts, raw, nil, nil, nil) + if faulty(again) { + t.Fatalf("the re-apply did not come back clean: %+v", again) + } + if !w.news(again) { + t.Fatal("a clean re-apply after a failed one was not said, so the mesh's condition never clears") + } +} + +// A refusal is news too, once, and a reconcile with nothing to say after a clean one stays silent. +func TestARefusalIsSaidOnceAndNothingIsNotSaid(t *testing.T) { + w := &adoptionWatch{} + w.said(link.Report{}) + if w.news(link.Report{}) { + t.Error("nothing was said again") + } + refused := link.Report{Refused: "pacman does not answer here"} + if !w.news(refused) { + t.Fatal("a refusal was not said") + } + w.said(refused) + if w.news(link.Report{Refused: "pacman does not answer here, at another time"}) { + t.Error("the same refusal in other words was said again") + } + if !w.news(link.Report{}) { + t.Error("a clean apply after a refusal was not said") + } +} diff --git a/internal/link/messages.go b/internal/link/messages.go index d34c744..3f84232 100644 --- a/internal/link/messages.go +++ b/internal/link/messages.go @@ -229,6 +229,11 @@ type Report struct { // as "not known", never as healthy and never as a reason to raise anything. Fresh on every report // this engine makes, and said between reports on HealthSubject when it changes. Health *Health `json:"health,omitempty"` + + // Unapplied is which declared resources this apply did not apply, by id — failed or not attempted. Not + // said on the wire (Failed says why, in words): it is what makes a reconcile whose result changed news to + // the node-engine, a failure that moved to another resource included (novox/hq issue 233). + Unapplied []string `json:"-"` } // LivenessContract is the version of the health statement this host keeps (ADR 0240 Phase A: liveness,