From 6e44a27a637ac9b6222ba1118a15ee9a88f258f9 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 14:28:55 +0200 Subject: [PATCH 1/2] Fail only the packages when the package manager does not answer, and apply the rest, so a later declaration can restore what an earlier one removed (issue 233) --- cmd/mesh-host/main.go | 21 +++++- cmd/mesh-host/probe.go | 73 +++++++++++++++++++++ cmd/mesh-host/probe_test.go | 58 +++++++++++++++++ cmd/mesh-host/replay233_test.go | 109 ++++++++++++++++++++++++++++++++ 4 files changed, 258 insertions(+), 3 deletions(-) create mode 100644 cmd/mesh-host/probe.go create mode 100644 cmd/mesh-host/probe_test.go create mode 100644 cmd/mesh-host/replay233_test.go diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 71e835d..b3964f2 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -636,10 +636,15 @@ func runApply(ctx context.Context, opts options, d *declaration.Declaration, raw return err } // And prove this is the machine the host was built for. Installing the arch host on Alpine - // must say so once, at the start, rather than failing later inside pacman. - if err := sys.Confirm(ctx, apply.ExecRunner); err != nil { + // must say so once, at the start, rather than failing later inside pacman. A package manager that + // does not answer fails only what needs it (novox/hq issue 233, probe.go). + probe, err := probePackageManager(ctx, sys) + if err != nil { return err } + if probe != nil { + fmt.Fprintf(out, "\n%s\n", probe.said()) + } fmt.Fprintf(out, "\napplying:\n") // Minding the windows the daemon's scheduler has open: this runs as its own process, and a @@ -671,6 +676,9 @@ func runApply(ctx context.Context, opts options, d *declaration.Declaration, raw if applyErr != nil { return applyErr } + if probe != nil { + return probe.partial() + } // Only now, and only after a clean apply: this version got as far as a completed // reconcile, which is the whole of what "known good" claims (novox/hq ADR 0005). Not @@ -1943,9 +1951,15 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto return link.Report{Refused: err.Error()} } - if err := built.Confirm(ctx, apply.ExecRunner); err != nil { + // A package manager that does not answer fails only what needs it, and the report says so (novox/hq + // issue 233, probe.go). + probe, err := probePackageManager(ctx, built) + if err != nil { return link.Report{Refused: err.Error()} } + if probe != nil { + announceOr(say)(probe.said()) + } // Declared, not carried. A declaration from the mesh removes only what the mesh previously // declared — never what this machine raised for itself from its bundle (04-ISSUES/010). @@ -2117,6 +2131,7 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto if applyErr != nil { report.Failed = map[string]string{"apply": applyErr.Error()} } + report.Failed = withUnanswered(report.Failed, probe) return report } diff --git a/cmd/mesh-host/probe.go b/cmd/mesh-host/probe.go new file mode 100644 index 0000000..9b3709d --- /dev/null +++ b/cmd/mesh-host/probe.go @@ -0,0 +1,73 @@ +package main + +import ( + "context" + "errors" + + "github.com/novox/mesh-host/internal/apply" + "github.com/novox/mesh-host/internal/system" +) + +// packageManagers are the tools a system's probe asks, for a system whose probe is its package manager. +var packageManagers = map[string]string{"arch": "pacman", "alpine": "apk"} + +// unanswered is a probe of the package manager that failed before an apply: which tool, and its words. +type unanswered struct { + tool string + words string +} + +// failedKey is how a report names the probe among what failed: one key, the same on every report, so the +// mesh counts a package manager that keeps not answering as one failure that goes on. +func (u *unanswered) failedKey() string { return "the package manager (" + u.tool + ")" } + +// said is the line the apply says before it begins. +func (u *unanswered) said() string { + return u.tool + " does not answer here, so every package in this declaration fails and the rest of it is " + + "applied (novox/hq issue 233): " + u.words +} + +// partial is the failure of an apply that applied everything else, when no package was declared to fail. +func (u *unanswered) partial() error { + return errors.New("applied only in part: " + u.words + "\n\nevery resource that does not need " + u.tool + + " was attempted; none here needed it, and " + u.tool + " must answer before this machine applies whole " + + "(novox/hq issue 233)") +} + +// probePackageManager proves the machine is the system the host was built for, before anything is applied. +// +// **A package manager that does not answer fails only what needs it** (novox/hq issue 233). The probe was a +// refusal of the whole declaration — right for a host installed on a machine it was not built for, and a +// wedge when what the probe reads is the mesh's own: a declaration that left the package manager's module +// out removed its configuration file, and from then on every declaration was refused, the one that would +// write the file back and the node-engine's own upgrade included. A person on the machine was the only way +// back. Now the rest applies — files, units, accounts, the node-engine itself — and each package fails with +// the probe's words (the system's PackageInstalled asks the same probe first), so a later declaration can +// 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 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) { + err := sys.Confirm(ctx, apply.ExecRunner) + if err == nil { + return nil, nil + } + tool, ok := packageManagers[sys.Name()] + if !ok { + return nil, err + } + return &unanswered{tool: tool, words: err.Error()}, nil +} + +// withUnanswered adds the probe to what a report says failed. +func withUnanswered(failed map[string]string, u *unanswered) map[string]string { + if u == nil { + return failed + } + if failed == nil { + failed = map[string]string{} + } + failed[u.failedKey()] = u.said() + return failed +} diff --git a/cmd/mesh-host/probe_test.go b/cmd/mesh-host/probe_test.go new file mode 100644 index 0000000..294a861 --- /dev/null +++ b/cmd/mesh-host/probe_test.go @@ -0,0 +1,58 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/system" +) + +// Defends novox/hq issue 233: a package manager that does not answer is said even when nothing declared +// needs it — the apply is partial, never "applied" — and the report names it under one key, so the mesh +// counts it as one failure that goes on. +func TestAPackageManagerThatDoesNotAnswerIsSaidWithNoPackageDeclared(t *testing.T) { + unansweringPacman(t) + dir := t.TempDir() + opts := options{state: filepath.Join(dir, "state.json"), out: &strings.Builder{}} + conf := filepath.Join(dir, "pacman.conf") + raw := []byte(`{"declaration":1,"resources":[{"id":"pacman.conf","type":"file","path":"` + conf + + `","content":"[options]\n"}]}`) + d, err := declaration.ParseFileTrusted(raw) + if err != nil { + t.Fatal(err) + } + err = runApply(context.Background(), opts, d, raw, fromFile) + if err == nil || !strings.Contains(err.Error(), "applied only in part") || !strings.Contains(err.Error(), "could not be read") { + t.Fatalf("an apply whose package manager did not answer was not said to be partial: %v", err) + } + if _, statErr := os.Stat(conf); statErr != nil { + t.Fatalf("the file was not written: %v", statErr) + } + + report := applyAndKeep(context.Background(), opts, raw, nil, nil, nil) + if report.Refused != "" { + t.Fatalf("refused whole: %s", report.Refused) + } + why, ok := report.Failed["the package manager (pacman)"] + if !ok || !strings.Contains(why, "could not be read") { + t.Errorf("the report does not name the package manager with the probe's words: %v", report.Failed) + } +} + +// A system whose probe asks the system itself, not a tool a resource needs, still refuses whole: nothing +// the mesh declares can remove it, and nothing else stands between that host and the wrong machine. +func TestASystemWithNoPackageManagerStillRefusesWhole(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + sys, err := system.For("android") + if err != nil { + t.Fatal(err) + } + probe, err := probePackageManager(context.Background(), sys) + if err == nil || probe != nil { + t.Fatalf("the android host's failed probe did not refuse: %v, %+v", err, probe) + } +} diff --git a/cmd/mesh-host/replay233_test.go b/cmd/mesh-host/replay233_test.go new file mode 100644 index 0000000..c46cf04 --- /dev/null +++ b/cmd/mesh-host/replay233_test.go @@ -0,0 +1,109 @@ +package main + +import ( + "bytes" + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/store" +) + +// unansweringPacman puts a pacman on PATH that answers as one whose configuration file is gone, as the +// workstation's did on 2026-10-04 (novox/hq issue 233), and makes this an arch host. +func unansweringPacman(t *testing.T) { + t.Helper() + bin := t.TempDir() + script := "#!/bin/sh\necho 'error: config file /etc/pacman.conf could not be read: No such file or directory' >&2\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 }) +} + +// TestReplay233 replays novox/hq issue 233. The package manager's module wrote pacman's configuration +// file; a declaration that left the module out removed it; from then on pacman did not answer, and the +// node-engine refused every declaration whole — the one that named the module again and would have +// written the file back included. A person restored the file by hand an hour and a half later. +// +// Held to: a declaration that restores the file is applied while pacman does not answer. Only what needs +// pacman fails, with the probe's words; the file is written and recorded. Written only with what the +// node-engine had before the fix, so it lays over the commit before it, where it fails. +func TestReplay233(t *testing.T) { + unansweringPacman(t) + dir := t.TempDir() + opts := options{state: filepath.Join(dir, "state.json")} + conf := filepath.Join(dir, "pacman.conf") + raw := []byte(`{"declaration":1,"resources":[` + + `{"id":"pacman.conf","type":"file","path":"` + conf + `","content":"[options]\n"},` + + `{"id":"pacman.base","type":"package","package":"base"}]}`) + + report := applyAndKeep(context.Background(), opts, raw, nil, nil, nil) + + if report.Refused != "" { + t.Fatalf("the declaration that restores the package manager's configuration was refused whole: %s", report.Refused) + } + if got, err := os.ReadFile(conf); err != nil || string(got) != "[options]\n" { + t.Fatalf("the file that restores the package manager was not written: %q, %v", got, err) + } + known, err := store.Load(opts.state) + if err != nil { + t.Fatal(err) + } + if _, ok := known.Find("pacman.conf"); !ok { + t.Error("the file written was not recorded, so the next apply would not know it is the mesh's") + } + words := strings.Join(failedWords(report.Failed), "\n") + if !strings.Contains(words, "pacman does not answer") || !strings.Contains(words, "could not be read") { + t.Errorf("the report does not say the package manager did not answer, in the probe's words: %q", words) + } + if !strings.Contains(words, "pacman.base") { + t.Errorf("the report does not name the package that was not applied: %q", words) + } +} + +func failedWords(failed map[string]string) []string { + var out []string + for id, why := range failed { + out = append(out, id+": "+why) + } + return out +} + +// Defends novox/hq issue 233 at the machine: `mesh-host apply` applies what does not need the package +// manager, and says the apply was partial with the probe's words — a non-zero exit, never "applied". +func TestApplyByHandAppliesWhatDoesNotNeedAPackageManagerThatDoesNotAnswer(t *testing.T) { + unansweringPacman(t) + dir := t.TempDir() + out := &bytes.Buffer{} + opts := options{state: filepath.Join(dir, "state.json"), out: out} + conf := filepath.Join(dir, "pacman.conf") + raw := []byte(`{"declaration":1,"resources":[` + + `{"id":"pacman.conf","type":"file","path":"` + conf + `","content":"[options]\n"},` + + `{"id":"pacman.base","type":"package","package":"base"}]}`) + d, err := declaration.ParseFileTrusted(raw) + if err != nil { + t.Fatal(err) + } + + err = runApply(context.Background(), opts, d, raw, fromFile) + + if _, statErr := os.Stat(conf); statErr != nil { + t.Fatalf("the file was not written: %v (the apply said %v)", statErr, err) + } + if err == nil { + t.Fatal("an apply whose package manager did not answer said it applied") + } + if !strings.Contains(err.Error(), "pacman does not answer") || !strings.Contains(err.Error(), "pacman.base") { + t.Errorf("the failure does not name the package or the probe's words: %v", err) + } + if !strings.Contains(out.String(), "pacman does not answer") { + t.Errorf("what was said before applying does not say the package manager does not answer:\n%s", out.String()) + } +} From 74ed7cbe0ab4dd8ad444b5954492eedd377639fa Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 15:39:07 +0200 Subject: [PATCH 2/2] Say a re-apply whose result changed, refuse whole when the package manager is not there, and install the packages in the apply that made it answer (issue 233 review) --- cmd/mesh-host/main.go | 82 ++++++++++++++++---- cmd/mesh-host/probe.go | 83 +++++++++++++++++++- cmd/mesh-host/probe_test.go | 151 ++++++++++++++++++++++++++++++++++++ internal/link/messages.go | 5 ++ 4 files changed, 307 insertions(+), 14 deletions(-) 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,