From f08a8ea3f72d4d45e0d6cca97f3672f38ee9139b Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 23 Sep 2026 23:35:49 +0200 Subject: [PATCH] Refuse every file once the mesh has spoken, plan the cutover as one, and let the kept declaration repair the mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the fix for hq issue 104 found three faults in it. A file applied on an enrolled node — the mesh's own last declaration included — is applied as the bundle is, so its resources are recorded as the machine's own and what the mesh declared reads as undeclared: the plan removed the foundation. `apply FILE` is for a machine the mesh has not spoken to, and is now refused saying so whenever declared.json exists. The plan looked at what is held before what the declaration says is taken, so the one cutover ADR 0100 says must be previewed read as a hold; it now decides in holdOnAdopted's order, models a step run inside a held container, and a test holds the plan's sequence to the apply's outcomes. Genesis wrote the mode on every run, so a re-run after `converge` left the state saying adopted while the kept, signed declaration said converged, and the reconcile loop refused every five minutes with no delivery coming to end it: genesis now writes the mode only when none is recorded, and where the state and the verified kept declaration disagree, the kept declaration wins and the repair is said. Also: a file lock beside the state, taken by the link service, the host's own commands and the installer alike, so a `reconcile` run by hand no longer races the loop's save — chosen over refusing while a named service is active, which would miss a `mesh-host run` started by hand; `--json --dry-run` emits {plan} like an apply emits {plan, report}; the README's duplicate flag line; and the bundle refusal is about the digest, not a claim the carried bytes can never match what genesis applied. --- README.md | 11 ++- cmd/mesh-host/main.go | 124 ++++++++++++++++++---------- cmd/mesh-host/main_test.go | 58 +++++++++++--- internal/apply/plan.go | 44 +++++++++- internal/apply/plan_test.go | 133 +++++++++++++++++++++++++++++++ internal/bootstrap/apply.go | 29 ++++--- internal/bootstrap/apply_test.go | 13 +++ internal/store/lock.go | 58 ++++++++++++++ internal/store/lock_test.go | 45 +++++++++++ 9 files changed, 441 insertions(+), 74 deletions(-) create mode 100644 internal/store/lock.go create mode 100644 internal/store/lock_test.go diff --git a/README.md b/README.md index 79bc636..a197ca9 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,6 @@ mesh-host owned what this host has applied and still owns --json machine-readable --state where this node keeps what it knows --dry-run say what applying would change, and change nothing - --dry-run read and check the declaration, change nothing ``` ``` @@ -119,9 +118,13 @@ make host BUNDLE=path/to/foundation.lock `mesh-host reconcile` then applies it, on a machine the mesh has told nothing yet. That is the first node's path — no mesh present, nothing fetched, nothing else copied onto the machine. Once the mesh has spoken, `reconcile` holds the machine to what it last said and never to the bundle, -which genesis consumed; and any declaration — bundle, file or kept — is refused when it says the -other mode than the node is in, or is not what the mesh last said. Both commands say what they -would change before changing anything, and `--dry-run` is that alone. `copy it and run it` stops being true the moment +which genesis consumed; a bundle or a file is refused when it says the other mode than the node +is in; and `apply FILE` is refused altogether once the mesh has spoken — a file is applied as the +bundle is, its resources recorded as the machine's own, so on an enrolled node it would plan to +remove the foundation. `apply FILE` is for a machine the mesh has not spoken to. (hq's to-be +node lifecycle describes `apply repair.json` as a rescue on an enrolled node; that line is being +amended in hq, and no rescue path exists here yet.) Both commands say what they would change +before changing anything, and `--dry-run` is that alone. `copy it and run it` stops being true the moment a second file has to arrive with it, which is why the bundle is embedded rather than beside it. **A default build carries nothing and refuses to reconcile**, saying so. A host that applied diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 409fe3f..ca47bb6 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -398,10 +398,12 @@ func short(digest string) string { // **A declaration carries no order.** The controller signs a version — the vocabulary — the // node it is for, the mode, and the resources: no sequence, no issued-at. What exists is // identity: the mesh names what it sends by the digest of the bytes, the node keeps the last one -// it was sent, and genesis records the digest of what it consumed. So "older" can be said of -// exactly two things — the bundle genesis consumed, and a bundle other than the one it consumed — -// and of anything else only that it is not what the mesh last said, which is refused too: a -// host that cannot tell older from newer and applies anyway is the fault this issue names. +// it was sent, and genesis records the digest of what it consumed. So the bundle is held to +// genesis's digest, and a file is not applied at all once the mesh has spoken: a file is applied +// as the bundle is, its resources recorded as this machine's own — so the mesh could never remove +// them again — and everything the mesh declared read as no longer declared and removed. Even the +// very declaration the mesh last sent, applied from a file, would plan to remove the foundation. +// `apply FILE` is for a machine the mesh has not spoken to, and is refused saying so. func refuseStale(known store.State, kept store.Declared, keptErr error, digest string, from provenance) error { switch from { case fromDeclared: @@ -410,43 +412,45 @@ func refuseStale(known store.State, kept store.Declared, keptErr error, digest s if known.Genesis == nil || known.Genesis.Digest == digest { return nil } - how := "" - if known.Genesis.Rewritten { - how = ", rewritten for this machine — its foundation ports, root credentials and mode" - } + // By digest. Genesis applies the bundle rewritten for this machine — its foundation + // ports, root credentials, mode — and writes that lock out; a host built from the + // carried template does not match it, and one built from the written lock does. return fmt.Errorf("the bundle this host carries (%s) is not the one genesis applied here on %s "+ - "(%s%s). The bundle was consumed then, and nothing the mesh has said is kept on this node "+ - "yet, so there is nothing to reconcile against: enrol this node, or push to it from the "+ - "controller", short(digest), known.Genesis.At.Format(time.RFC3339), short(known.Genesis.Digest), how) + "(%s), which was the bundle rewritten for this machine. It was consumed then, and nothing "+ + "the mesh has said is kept on this node yet, so there is nothing to reconcile against: "+ + "enrol this node, or push to it from the controller", + short(digest), known.Genesis.At.Format(time.RFC3339), short(known.Genesis.Digest)) } // A file. switch { case errors.Is(keptErr, store.ErrNothingDeclared): - // The mesh has said nothing here. Nothing to be older than. + // The mesh has said nothing here: a machine a file is for. return nil case keptErr != nil: return fmt.Errorf("%w\n\nNothing is applied over what this node cannot read back", keptErr) } last := apply.DigestOf(kept.Declaration) - if digest == last { - return nil + what := fmt.Sprintf("this file (%s) is not it", short(digest)) + switch { + case digest == last: + what = "this file is that declaration, and from a file it would still be applied as a bundle is" + case known.Genesis != nil && digest == known.Genesis.Digest: + what = fmt.Sprintf("this file is the bundle genesis consumed on %s (%s), older than it", + known.Genesis.At.Format(time.RFC3339), short(digest)) } - if known.Genesis != nil && digest == known.Genesis.Digest { - return fmt.Errorf("this file is the bundle genesis consumed on %s (%s), and the mesh has since told "+ - "this node declaration %s, kept as %s. An older declaration is not applied over a newer one: "+ - "`reconcile` applies what the mesh last said", known.Genesis.At.Format(time.RFC3339), short(digest), - short(last), store.DeclaredName) - } - return fmt.Errorf("this file (%s) is not the declaration the mesh last told this node (%s, kept as %s). "+ - "A declaration carries no sequence and no issued-at, so this host cannot tell an older one from a "+ - "newer, and it applies only what the mesh last said: `reconcile` applies that, and `push` from the "+ - "controller changes it", short(digest), short(last), store.DeclaredName) + return fmt.Errorf("`apply FILE` is for a machine the mesh has not spoken to. The mesh has told this "+ + "node declaration %s, kept as %s, and %s. A file is applied as the bundle is — its resources "+ + "recorded as this machine's own, which the mesh could then never remove, and what the mesh "+ + "declared read as no longer declared — so no file is applied here: `reconcile` applies what "+ + "the mesh last said, and `push` from the controller changes it", + short(last), store.DeclaredName, what) } // applied is what an apply says in machine-readable form: what it planned, then what it did. type applied struct { - Plan []apply.Step `json:"plan"` - Report apply.Report `json:"report"` + Plan []apply.Step `json:"plan"` + // Report is absent on a dry run, which is the plan and nothing more. + Report *apply.Report `json:"report,omitempty"` } // runApply makes the machine match a declaration — after refusing one this node must not apply, @@ -465,16 +469,34 @@ func runApply(ctx context.Context, opts options, d *declaration.Declaration, raw if out == nil { out = os.Stdout } + // One apply at a time on this machine, whichever process asks: the link service applies too, + // and two saves of the state interleaved lose what one of them recorded. + unlock, err := store.Lock(opts.state, func() { + fmt.Fprintln(out, "another apply holds this node's state — mesh-host run, or the installer; waiting for it to finish") + }) + if err != nil { + return err + } + defer unlock() known, err := store.Load(opts.state) if err != nil { return err } source, origin, digest := from.name(opts), from.origin(), apply.DigestOf(raw) - // The mode this node is in. Recorded in the state since this check existed; a state written - // before then has it in what the mesh last said, which this node kept. + // The mode this node is in. What the mesh last said is signed and was verified on load; the + // state's note of it is this host's own, and where they disagree the note is what is wrong + // — a genesis re-run over a node the mesh converged since, a state saved when the kept + // declaration could not be — and is repaired, not obeyed. A bundle or a file is held to the + // note, or to the kept declaration when the note predates it. kept, keptErr := store.ReadDeclared(store.DeclaredPath(opts.state)) - if known.Mode == "" && keptErr == nil { + if from == fromDeclared { + if known.Mode != "" && known.Mode != apply.ModeOf(d) { + fmt.Fprintf(out, "this node's record said %s; what the mesh last said, signed, says %s — the record is repaired\n", + known.Mode, apply.ModeOf(d)) + } + known.Mode = apply.ModeOf(d) + } else if known.Mode == "" && keptErr == nil { if last, err := declaration.Parse(kept.Declaration); err == nil { known.Mode = apply.ModeOf(last) } @@ -489,7 +511,7 @@ func runApply(ctx context.Context, opts options, d *declaration.Declaration, raw // Said before anything is done. steps := apply.Plan(d, known, origin) if opts.json && opts.dryRun { - return writeJSONTo(out, steps) + return writeJSONTo(out, applied{Plan: steps}) } mode := known.Mode if mode == "" { @@ -572,7 +594,7 @@ func runApply(ctx context.Context, opts options, d *declaration.Declaration, raw } if opts.json { - return writeJSONTo(out, applied{Plan: steps, Report: report}) + return writeJSONTo(out, applied{Plan: steps, Report: &report}) } if !report.Changed() { fmt.Fprintf(out, "%s: already matches — %d resource(s) checked\n", source, len(report.Outcomes)) @@ -821,7 +843,7 @@ func runLink(ctx context.Context, opts options) error { go sched.Run(ctx) applier := func(ctx context.Context, raw, signature []byte) link.Report { - return applyAndKeep(ctx, opts, raw, &store.Declared{Declaration: raw, Signature: signature}, sched) + return applyAndKeep(ctx, opts, raw, &store.Declared{Declaration: raw, Signature: signature}, sched, say) } // Two things at once, and the second is what makes disconnection ordinary. The link brings @@ -984,7 +1006,7 @@ func holdTheMachine(ctx context.Context, opts options, mine identity.Identity, s continue } - report := applyDeclared(ctx, opts, declared, sched) + report := applyDeclared(ctx, opts, declared, sched, say) // A reconcile is otherwise silent. On an adopted node it speaks when what it holds or // its firewall changed, because that is how a predecessor still writing is caught // (novox/hq ADR 0100); publish decides whether anything did. @@ -1005,25 +1027,33 @@ func holdTheMachine(ctx context.Context, opts options, mine identity.Identity, s // Signature checking happens before this is called, in the link. By the time anything here runs, // the question "is this from the mesh I joined" is settled — which is why this can treat the // bytes as instructions. -func applyDeclared(ctx context.Context, opts options, raw []byte, sched *apply.Scheduler) link.Report { - return applyAndKeep(ctx, opts, raw, nil, sched) +func applyDeclared(ctx context.Context, opts options, raw []byte, sched *apply.Scheduler, say link.Announce) link.Report { + return applyAndKeep(ctx, opts, raw, nil, sched, say) } -// applying serialises applies on this node. +// applying serialises applies within this process. // // **Two things apply here: the link and the reconcile loop**, and each reads the node's state, // acts on the machine, and writes the state back. Run at the same time they interleave, and the // one that saves last writes a state read before the other acted — losing what the first recorded: // a hold, the firewall found here, a resource just applied. The machine would then be one thing // and its record another, which is the fault every read-back in this package exists to prevent. +// Across processes — `reconcile` run by hand beside this service — the lock beside the state does +// the same (store.Lock). var applying sync.Mutex // applyAndKeep applies a declaration and, when it came from the mesh, keeps it so this node can -// go on obeying it while disconnected. One at a time, whoever asks. +// go on obeying it while disconnected. One at a time, whoever asks. Given unsigned, the bytes are +// what this node kept, already verified against the mesh's key on load. func applyAndKeep(ctx context.Context, opts options, raw []byte, signed *store.Declared, - sched *apply.Scheduler) link.Report { + sched *apply.Scheduler, say link.Announce) link.Report { applying.Lock() defer applying.Unlock() + unlock, err := store.Lock(opts.state, nil) + if err != nil { + return link.Report{Refused: err.Error()} + } + defer unlock() declared, err := declaration.Parse(raw) if err != nil { @@ -1034,13 +1064,19 @@ func applyAndKeep(ctx context.Context, opts options, raw []byte, signed *store.D if err != nil { return link.Report{Refused: err.Error()} } - // A declaration the link delivered is the controller's word on the node's mode — the flip - // arrives as exactly that, the first converged declaration after adopted ones — and becomes - // the record. What this node re-applies on its own is held to the record (novox/hq issue 104). - if signed == nil { - if err := apply.CheckMode(known, declared); err != nil { - return link.Report{Refused: err.Error()} + // A declaration from the mesh is the controller's word on the node's mode — the flip arrives + // as exactly that, the first converged declaration after adopted ones — and becomes the + // record. So does what this node kept: it is signed by the mesh and verified on load, where + // the state's note of the mode is this host's own. Where they disagree the note is wrong — a + // genesis re-run over a node the mesh converged since, a state saved when the kept declaration + // could not be — and it is repaired and said, not obeyed: refusing would hold this node off what + // the mesh said until a delivery that comes only when something changes (novox/hq issue 104). + if signed == nil && known.Mode != "" && known.Mode != apply.ModeOf(declared) { + if say != nil { + say(fmt.Sprintf("this node's record said %s; what the mesh last said, signed, says %s — the record is repaired", + known.Mode, apply.ModeOf(declared))) } + known.Mode = apply.ModeOf(declared) } built, err := system.For(builtFor) diff --git a/cmd/mesh-host/main_test.go b/cmd/mesh-host/main_test.go index a61a405..e8621b5 100644 --- a/cmd/mesh-host/main_test.go +++ b/cmd/mesh-host/main_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "crypto/ed25519" + "encoding/json" "errors" "os" "path/filepath" @@ -224,7 +225,7 @@ func TestOnlyOneApplyRunsAtATime(t *testing.T) { // Whatever else is applying — the link, while this is the reconcile — this waits for it. applying.Lock() done := make(chan link.Report, 1) - go func() { done <- applyAndKeep(context.Background(), opts, raw, nil, nil) }() + go func() { done <- applyAndKeep(context.Background(), opts, raw, nil, nil, nil) }() select { case report := <-done: applying.Unlock() @@ -307,16 +308,23 @@ func TestAConvergedDeclarationIsRefusedOnAnAdoptedNode(t *testing.T) { } } -func TestAnAdoptedDeclarationIsRefusedOnAConvergedNode(t *testing.T) { - // Only the mesh can say a node is adopted, so this one arrives the way a disconnected node - // re-applies what it kept: through the reconcile loop, unsigned. +func TestTheKeptDeclarationRepairsARecordThatDisagrees(t *testing.T) { + // What a node kept is signed by the mesh and verified on load; the state's note of the mode is + // the host's own. A genesis re-run after `converge`, or a state saved when the kept declaration + // could not be, leaves them apart — and a loop that refused every five minutes would hold the + // node off what the mesh said until a delivery that comes only when something changes. The + // kept declaration wins, and the repair is said. opts := stateWithMode(t, store.ModeConverged) raw := []byte(`{"declaration":1,"adoption":{"taken":[]},"resources":[{"id":"a","type":"file","path":"` + filepath.Join(filepath.Dir(opts.state), "a.conf") + `","content":"x\n"}]}`) - report := applyAndKeep(context.Background(), opts, raw, nil, nil) - want := "this node is converged; the declaration says adopted" - if !strings.Contains(report.Refused, want) || !strings.Contains(report.Refused, "`adopt`") { - t.Errorf("refused = %q, want it to name both modes and the act that changes it", report.Refused) + var said []string + report := applyAndKeep(context.Background(), opts, raw, nil, nil, func(line string) { said = append(said, line) }) + if strings.Contains(report.Refused, "the declaration says") { + t.Fatalf("what the node kept was refused against its own note: %s", report.Refused) + } + if len(said) != 1 || !strings.Contains(said[0], "record said converged") || + !strings.Contains(said[0], "says adopted") || !strings.Contains(said[0], "repaired") { + t.Errorf("the repair was not said: %q", said) } } @@ -328,15 +336,16 @@ func TestTheMeshItselfMayChangeTheMode(t *testing.T) { opts := stateWithMode(t, store.ModeAdopted) raw := []byte(`{"declaration":1,"resources":[{"id":"a","type":"file","path":"` + filepath.Join(filepath.Dir(opts.state), "a.conf") + `","content":"x\n"}]}`) - report := applyAndKeep(context.Background(), opts, raw, &store.Declared{Declaration: raw}, nil) + report := applyAndKeep(context.Background(), opts, raw, &store.Declared{Declaration: raw}, nil, nil) if strings.Contains(report.Refused, "the declaration says") { t.Errorf("the mesh's own flip was refused for its mode: %s", report.Refused) } } // Defends novox/hq issue 104: a declaration older than what the mesh last said is refused, naming -// both — and a declaration carries no order, so one that is merely not the last is refused too, -// saying what is missing. +// both — and `apply FILE` is refused altogether once the mesh has spoken, the last declaration +// itself included: from a file it is applied as the bundle is, which would record the mesh's +// resources as this machine's own and remove the foundation as undeclared. func TestAnOlderDeclarationIsRefused(t *testing.T) { opts := stateWithMode(t, "") genesis := []byte(`{"declaration":1,"resources":[{"id":"g","type":"file","path":"/tmp/g","content":"genesis\n"}]}`) @@ -372,9 +381,18 @@ func TestAnOlderDeclarationIsRefused(t *testing.T) { if err == nil { t.Fatal("a declaration that is not what the mesh last said was applied") } - if !strings.Contains(err.Error(), "no sequence and no issued-at") || + if !strings.Contains(err.Error(), "for a machine the mesh has not spoken to") || !strings.Contains(err.Error(), short(apply.DigestOf(since))) { - t.Errorf("the refusal does not say what is missing and what was last said: %v", err) + t.Errorf("the refusal does not say what a file is for and what was last said: %v", err) + } + + // The very declaration the mesh last sent, from a file: still a file. + err = runApply(context.Background(), opts, parsed(since), since, fromFile) + if err == nil || !strings.Contains(err.Error(), "applied as the bundle is") { + t.Errorf("the last declaration, from a file, was not refused as a file: %v", err) + } + if known, _ := store.Load(opts.state); len(known.Resources) != 0 { + t.Errorf("a refused file recorded %d resource(s)", len(known.Resources)) } // A bundle other than the one genesis consumed: what genesis applied was rewritten for this @@ -416,6 +434,20 @@ func TestADryRunChangesNothingAndListsTheActions(t *testing.T) { if strings.Contains(out.String(), "applying:") { t.Errorf("a dry run went on to apply:\n%s", out.String()) } + + // Machine-readable, the same shape as an apply's: the plan, and no report. + out.Reset() + opts.json = true + if err := runApply(context.Background(), opts, d, raw, fromFile); err != nil { + t.Fatal(err) + } + var shape struct { + Plan []apply.Step `json:"plan"` + Report *json.RawMessage `json:"report"` + } + if err := json.Unmarshal(out.Bytes(), &shape); err != nil || len(shape.Plan) != 2 || shape.Report != nil { + t.Errorf("a json dry run is not {plan} alone: %v\n%s", err, out.String()) + } } // Defends novox/hq issue 104: once the mesh has told this node anything, `reconcile` holds it to diff --git a/internal/apply/plan.go b/internal/apply/plan.go index aa3c15e..ced96cc 100644 --- a/internal/apply/plan.go +++ b/internal/apply/plan.go @@ -118,18 +118,54 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { } // planned is what one declared resource would come to. +// +// **In the order holdOnAdopted decides it**, because the one cutover ADR 0100 says must be +// previewed is the one a plan gets backwards if it looks at the record first: a resource this +// node holds for a module the declaration now says is taken is not held any longer — it is +// applied, and what was found is replaced. The declaration's word on which modules are untaken +// comes first; the record of what is held only says what that replacement replaces. func planned(r declaration.Resource, d *declaration.Declaration, known store.State) Step { step := Step{Type: string(r.Kind()), ID: r.Identity(), Target: r.Target()} if d.Adoption != nil { - if h, held := known.HeldAt(r.Identity()); held { - step.Verb, step.Why = "hold", "found on this machine and kept as it is until "+h.Module+" is taken" - return step + // Something run inside a held container is held with it, while that container's module + // is untaken; once the module is taken the container is replaced before this runs. + if in := runsIn(r); in != "" { + if container, isHeld := heldContainer(known, in); isHeld { + if _, untaken := d.Adoption.Untaken[container.Module]; untaken { + step.Verb = "hold" + step.Why = "runs in " + in + ", which is held as found; not run until " + container.Module + " is taken" + return step + } + } } - if module, untaken := d.Adoption.UntakenModuleOf(r.Identity()); untaken { + // A file written into replaces nothing that was found, so it is never held (ADR 0102). + into := false + if f, ok := r.(*declaration.File); ok && f.Into != "" { + into = true + } + h, held := known.HeldAt(r.Identity()) + module, untaken := d.Adoption.UntakenModuleOf(r.Identity()) + switch { + case into: + case untaken && held: + step.Verb, step.Why = "hold", "found on this machine and kept as it is until "+module+" is taken" + return step + case untaken: step.Verb = "create" step.Why = "unless it is found on this machine — then held as it is until " + module + " is taken" return step + case held: + // The cutover: the module is taken, and what was held for it is replaced. + step.Verb = "create" + if _, recorded := known.Find(r.Identity()); recorded { + step.Verb = "update" + } + step.Why = h.Module + " is taken: replaces what was found and held" + if h.Kept != "" { + step.Why += "; the original stays at " + h.Kept + } + return step } } diff --git a/internal/apply/plan_test.go b/internal/apply/plan_test.go index 3299723..768230a 100644 --- a/internal/apply/plan_test.go +++ b/internal/apply/plan_test.go @@ -1,6 +1,9 @@ package apply import ( + "context" + "os" + "path/filepath" "strings" "testing" "time" @@ -92,3 +95,133 @@ func TestAPlanHoldsWhatAnAdoptedNodeFound(t *testing.T) { t.Errorf("the plan does not say an untaken module's resource is held if found: %q", steps[1].Why) } } + +// both is a machine with a container runtime and ufw on it at once. +type both struct { + m *machine + u *ufwMachine +} + +func (b *both) run(ctx context.Context, name string, args ...string) (string, error) { + switch name { + case "nft", "ufw", "iptables", "ip6tables", "firewall-cmd": + return b.u.run(ctx, name, args...) + } + return b.m.run(ctx, name, args...) +} + +func ids(steps []Step) []string { + var out []string + for _, s := range steps { + if s.Type == "firewall" { + continue // said, not an outcome + } + out = append(out, s.ID) + } + return out +} + +func outcomeIDs(r Report) []string { + var out []string + for _, o := range r.Outcomes { + out = append(out, o.ID) + } + return out +} + +func write(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +// Defends novox/hq issue 104: the plan is the apply, said first — the same resources in the same +// order, and the one cutover ADR 0100 says must be previewed said as a cutover, not a hold. +func TestThePlanIsTheApplyInOrder(t *testing.T) { + dir := t.TempDir() + guard, page, keep, old := filepath.Join(dir, "guard.nft"), filepath.Join(dir, "index.html"), + filepath.Join(dir, "keep.html"), filepath.Join(dir, "old.conf") + for _, p := range []string{page, keep, old} { + write(t, p, "the predecessor's\n") + } + + // Returning to adopted, with a guard to raise, an orphan, a hold that stays, a hold whose + // module is now taken, and a hold no longer declared. + back := adopted(t, `{"taken":["hello-web"],"untaken":{"keep":["keep.page"]}}`, + `{"id":"adoption.guard.table","type":"file","path":"`+guard+`","content":"table inet mesh-guard {}\n"}, + {"id":"hello-web.page","type":"file","path":"`+page+`","content":"the mesh's page\n"}, + {"id":"keep.page","type":"file","path":"`+keep+`","content":"the mesh's keep\n"}`) + known := store.State{Firewall: &store.FoundFirewall{Kind: "ufw", WasActive: true, DisabledByMesh: true, FoundAt: time.Now()}} + known.Record(store.Applied{ID: "old.conf", Type: "file", Target: old, Origin: store.OriginDeclared}) + for id, module := range map[string]string{"hello-web.page": "hello-web", "keep.page": "keep", "gone.page": "gone"} { + known.RecordHeld(store.Held{ID: id, Module: module, Kind: "file", Target: filepath.Join(dir, id), Since: time.Now()}) + } + fake := &both{m: &machine{containers: map[string]*fakeContainer{}}, u: &ufwMachine{installed: true}} + + plan := Plan(back, known, store.OriginDeclared) + report, state, err := ApplyKeeping(context.Background(), archHost(t), back, known, store.OriginDeclared, + fake.run, nil, nil, KeepIn(dir)) + if err != nil { + t.Fatal(err) + } + if got, want := verbs(plan), "enable ufw, forget gone.page, create adoption.guard.table, remove old.conf, "+ + "create hello-web.page, hold keep.page"; got != want { + t.Errorf("planned %q, want %q", got, want) + } + if got, want := strings.Join(ids(plan), " "), strings.Join(outcomeIDs(report), " "); got != want { + t.Errorf("the plan said %q and the apply did %q", got, want) + } + taken := outcomeOf(report, "hello-web.page") + if !strings.Contains(taken.Detail, "taken") || !strings.Contains(plan[4].Why, "hello-web is taken: replaces what was found") { + t.Errorf("the cutover was applied as %q and planned as %q", taken.Detail, plan[4].Why) + } + if !fake.u.active || state.Firewall.DisabledByMesh { + t.Error("returning to adopted did not enable ufw again") + } + + // Converged, from the mesh: an orphan, a new file, ufw retired, and what protected the node + // removed last. + flip := parse(t, `{"declaration":1,"resources":[{"id":"a","type":"file","path":"`+filepath.Join(dir, "a.conf")+`","content":"a\n"}]}`) + write(t, old, "again\n") + known = store.State{Firewall: &store.FoundFirewall{Kind: "ufw", WasActive: true, FoundAt: time.Now()}} + known.Record(store.Applied{ID: "adoption.guard.table", Type: "file", Target: guard, Origin: store.OriginDeclared}) + known.Record(store.Applied{ID: "old.conf", Type: "file", Target: old, Origin: store.OriginDeclared}) + fake = &both{m: &machine{containers: map[string]*fakeContainer{}}, u: &ufwMachine{installed: true, active: true, + ruleset: "table inet mesh {\n}\n"}} + + plan = Plan(flip, known, store.OriginDeclared) + report, state, err = ApplyKeeping(context.Background(), archHost(t), flip, known, store.OriginDeclared, + fake.run, nil, nil, KeepIn(dir)) + if err != nil { + t.Fatal(err) + } + if got, want := verbs(plan), "remove old.conf, create a, disable ufw, remove adoption.guard.table"; got != want { + t.Errorf("planned %q, want %q", got, want) + } + if got, want := strings.Join(ids(plan), " "), strings.Join(outcomeIDs(report), " "); got != want { + t.Errorf("the plan said %q and the apply did %q", got, want) + } + if fake.u.active || !state.Firewall.DisabledByMesh { + t.Error("converging did not retire ufw") + } +} + +func TestAResourceRunInAHeldContainerIsPlannedAsItIsApplied(t *testing.T) { + // A run-once step sharing a container's namespace runs in it, as an action's `in` does. + img := "example/x@sha256:0000000000000000000000000000000000000000000000000000000000000000" + d := parse(t, `{"declaration":1,"adoption":{"taken":["web"],"untaken":{"db":["db.server"]}},"resources":[ + {"id":"web.server","type":"container","name":"web","image":"`+img+`"}, + {"id":"db.server","type":"container","name":"db","image":"`+img+`"}, + {"id":"db.init","type":"container","name":"db-init","image":"`+img+`","run-once":true,"network":"container:db"}, + {"id":"web.warm","type":"container","name":"web-warm","image":"`+img+`","run-once":true,"network":"container:web"}]}`) + known := store.State{} + known.RecordHeld(store.Held{ID: "db.server", Module: "db", Kind: "container", Target: "db", Container: "predecessor"}) + known.RecordHeld(store.Held{ID: "web.server", Module: "web", Kind: "container", Target: "web", Container: "predecessor"}) + got := verbs(Plan(d, known, store.OriginDeclared)) + // db is untaken, so what runs in it waits; web is taken, so its held container is replaced (the + // cutover) and what runs in it runs. + if got != "create web.server, hold db.server, hold db.init, create web.warm" { + t.Errorf("planned %q", got) + } +} diff --git a/internal/bootstrap/apply.go b/internal/bootstrap/apply.go index 19a5167..54da41a 100644 --- a/internal/bootstrap/apply.go +++ b/internal/bootstrap/apply.go @@ -37,10 +37,11 @@ type Runner = apply.Runner // // What it does record is that the bundle was consumed, and in which mode the operator raised // the machine. `raw` is the exact bytes of what is applied — the bundle as rewritten for this -// machine, not as carried — and its digest is what `mesh-host reconcile` later holds the carried -// bundle against: what genesis applied had its ports, root credentials and adoption rewritten, -// so the carried bytes are never it, and applying them on a raised node recreated the store and -// loaded the converged filter on an adopted one (novox/hq issue 104). +// machine — and its digest is what `mesh-host reconcile` later holds the carried bundle against, +// by digest: a host built from the carried template does not match it, since genesis rewrote the +// ports, root credentials and adoption; a host built from the lock genesis wrote out does. +// Applying the unrewritten template on a raised node recreated the store and loaded the converged +// filter on an adopted one (novox/hq issue 104). func ApplyBundle(ctx context.Context, o Options, sys system.System, d *declaration.Declaration, raw []byte, run Runner, say func(string)) (apply.Report, error) { @@ -50,6 +51,12 @@ func ApplyBundle(ctx context.Context, o Options, sys system.System, d *declarati return apply.Report{}, err } + // One apply at a time on this machine: a host already running here applies too. + unlock, err := store.Lock(o.State, func() { say(" waiting another apply holds this node's state") }) + if err != nil { + return apply.Report{}, err + } + defer unlock() known, err := store.Load(o.State) if err != nil { return apply.Report{}, err @@ -65,12 +72,16 @@ func ApplyBundle(ctx context.Context, o Options, sys system.System, d *declarati // The bundle is consumed, whichever way the apply went: what is on the machine came from these // bytes, and the carried ones must not be applied over it. The mode is the operator's word at - // genesis; the controller records the same and says it in every declaration from then on - // (novox/hq ADR 0100). + // genesis, and only at genesis: the controller records the same and says it in every + // declaration from then on (novox/hq ADR 0100), so a node the mesh has spoken to — this + // installer re-run on it, or finishing a pivot — keeps the mode it has, which may since have + // been flipped. updated.Genesis = &store.Genesis{Digest: apply.DigestOf(raw), At: time.Now().UTC(), Rewritten: true} - updated.Mode = store.ModeConverged - if o.Adopted { - updated.Mode = store.ModeAdopted + if updated.Mode == "" { + updated.Mode = store.ModeConverged + if o.Adopted { + updated.Mode = store.ModeAdopted + } } // Saved whichever way it went, for the reason `mesh-host` gives: what was applied before a diff --git a/internal/bootstrap/apply_test.go b/internal/bootstrap/apply_test.go index e4092b0..780a00c 100644 --- a/internal/bootstrap/apply_test.go +++ b/internal/bootstrap/apply_test.go @@ -166,4 +166,17 @@ func TestGenesisConsumesTheBundleAndRecordsTheMode(t *testing.T) { t.Errorf("adopted=%v: genesis recorded the mode as %q, want %q", adopted, known.Mode, want) } } + + // Re-run on a node the mesh has spoken to since — and converged — genesis leaves the mode + // alone: it is the operator's word at genesis, and the controller's from then on. + o := Options{State: filepath.Join(dir, "state-flipped.json"), Adopted: true} + if err := store.Save(o.State, store.State{Mode: store.ModeConverged}); err != nil { + t.Fatal(err) + } + if _, err := ApplyBundle(context.Background(), o, sys, d, raw, nil, quietly); err != nil { + t.Fatal(err) + } + if known, _ := store.Load(o.State); known.Mode != store.ModeConverged { + t.Errorf("a genesis re-run set the mode back to %q over the mesh's converged", known.Mode) + } } diff --git a/internal/store/lock.go b/internal/store/lock.go new file mode 100644 index 0000000..22245c7 --- /dev/null +++ b/internal/store/lock.go @@ -0,0 +1,58 @@ +package store + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "syscall" +) + +// One apply at a time on a machine, whoever asks. +// +// Three things apply here and each reads the state, acts, and writes the state back: the link +// and the reconcile loop inside `mesh-host run`, the host's own `reconcile` and `apply` run by +// hand, and the installer at genesis. Inside one process a mutex serialises them; across +// processes nothing did, and two applies interleaved leave the last saver writing a state read +// before the other acted — a hold, a firewall record, a resource just applied, lost (novox/hq +// issue 104 review). A lock on a file beside the state is what every one of them can take, however +// it was started: a `reconcile` run against a service, or against a `mesh-host run` somebody +// started by hand, waits the same way. Refusing while a named service is active would have known +// the service's name and missed the hand-started one. +// +// An advisory lock, held for the life of the open file and released by the kernel when the +// process ends, so a host that dies mid-apply leaves no lock behind for the next one to clear. + +// LockName is the file the lock is taken on, beside the state. +const LockName = "state.lock" + +// Lock takes the machine's apply lock and returns what releases it. When another process holds +// it, wait is told once — so a person running a command knows what they are waiting for — and +// Lock blocks until it is free. +func Lock(statePath string, wait func()) (func(), error) { + dir := filepath.Dir(statePath) + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, err + } + f, err := os.OpenFile(filepath.Join(dir, LockName), os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + return nil, fmt.Errorf("cannot take this node's apply lock: %w", err) + } + if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX|syscall.LOCK_NB); err != nil { + if !errors.Is(err, syscall.EWOULDBLOCK) { + f.Close() + return nil, fmt.Errorf("cannot take this node's apply lock: %w", err) + } + if wait != nil { + wait() + } + if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil { + f.Close() + return nil, fmt.Errorf("waiting for this node's apply lock: %w", err) + } + } + return func() { + _ = syscall.Flock(int(f.Fd()), syscall.LOCK_UN) + f.Close() + }, nil +} diff --git a/internal/store/lock_test.go b/internal/store/lock_test.go new file mode 100644 index 0000000..2423434 --- /dev/null +++ b/internal/store/lock_test.go @@ -0,0 +1,45 @@ +package store + +import ( + "path/filepath" + "testing" + "time" +) + +// Defends the node's record across processes: a `reconcile` run by hand beside the link service, +// or the installer beside either, waits for the other's apply rather than saving over it. +func TestASecondApplyWaitsForTheFirst(t *testing.T) { + state := filepath.Join(t.TempDir(), "state.json") + release, err := Lock(state, nil) + if err != nil { + t.Fatal(err) + } + + waited := make(chan struct{}, 1) + got := make(chan struct{}) + go func() { + unlock, err := Lock(state, func() { waited <- struct{}{} }) + if err != nil { + t.Error(err) + } + close(got) + unlock() + }() + + select { + case <-waited: + case <-time.After(5 * time.Second): + t.Fatal("the second apply was not told it is waiting") + } + select { + case <-got: + t.Fatal("the second apply took the lock while the first held it") + case <-time.After(50 * time.Millisecond): + } + release() + select { + case <-got: + case <-time.After(5 * time.Second): + t.Fatal("the second apply never got the lock once the first let go") + } +}