From ea6fe09e019a3d6a1b6dfcbc530f7288782fdb6a Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 5 Oct 2026 21:43:47 +0200 Subject: [PATCH] A reconcile applies what was kept when its turn comes, not when its timer fired (hq issues 257, 261) The five-minute reconcile read the kept declaration and then waited for the link's apply; the link keeps a declaration only once its apply ends, so the reconcile applied the older one over it. A module assigned a moment earlier was given back, and the report named a declaration the mesh no longer recorded as sent, which held a plan at its first machine. --- cmd/mesh-host/main.go | 39 +++++++++++++++++-------- cmd/mesh-host/main_test.go | 60 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 12 deletions(-) diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 3fa89e5..5056c69 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -8,6 +8,7 @@ package main import ( "context" + "crypto/ed25519" "crypto/sha256" "encoding/base64" "encoding/hex" @@ -1254,7 +1255,7 @@ func holdTheMachine(ctx context.Context, opts options, mine identity.Identity, s case <-ticker.C: } - declared, err := store.LoadDeclared(store.DeclaredPath(opts.state), mine.Membership.Signer) + report, err := reapplyKept(ctx, opts, mine.Membership.Signer, sched, say) if errors.Is(err, store.ErrNothingDeclared) { // Nothing to hold this machine to yet. Ordinary on a node that has enrolled and not // been assigned anything. @@ -1264,8 +1265,6 @@ func holdTheMachine(ctx context.Context, opts options, mine identity.Identity, s say("cannot re-apply what this node was told: " + err.Error()) continue } - - 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. @@ -1309,15 +1308,6 @@ func worthSaying(report link.Report) bool { len(report.Filters) > 0 || report.FoundFirewall != nil || len(report.Windows) > 0 } -// applyDeclared applies a declaration that has already been proved to come from the mesh. -// -// 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, say link.Announce) link.Report { - return applyAndKeep(ctx, opts, raw, nil, sched, say) -} - // announceOr is what the apply writes its detail with, given what the caller has to say things with. // // **Never nil.** This argument was nil on the serving path, and nil is silence: everything the apply @@ -1351,6 +1341,31 @@ func applyAndKeep(ctx context.Context, opts options, raw []byte, signed *store.D sched *apply.Scheduler, say link.Announce) link.Report { applying.Lock() defer applying.Unlock() + return applyAndKeepHeld(ctx, opts, raw, signed, sched, say) +} + +// reapplyKept re-applies what this node was last told, **read once it is this apply's turn**. +// +// Read before waiting, it was whatever had been kept when the timer fired — and a declaration +// the link was applying at that moment is kept only once its apply ends. The reconcile then waited +// for that apply and applied the older declaration over it: a module assigned a second earlier was +// given back on the spot, and the report it sent named a declaration the mesh no longer recorded +// as sent, so a plan waiting on this machine waited for a report that could not come (novox/hq +// issues 257, 261). Read under the lock, it is always the latest the mesh said. +func reapplyKept(ctx context.Context, opts options, signer ed25519.PublicKey, + sched *apply.Scheduler, say link.Announce) (link.Report, error) { + applying.Lock() + defer applying.Unlock() + declared, err := store.LoadDeclared(store.DeclaredPath(opts.state), signer) + if err != nil { + return link.Report{}, err + } + return applyAndKeepHeld(ctx, opts, declared, nil, sched, say), nil +} + +// applyAndKeepHeld is applyAndKeep for a caller already holding applying. +func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *store.Declared, + sched *apply.Scheduler, say link.Announce) link.Report { unlock, err := store.Lock(opts.state, nil) if err != nil { return link.Report{Refused: err.Error()} diff --git a/cmd/mesh-host/main_test.go b/cmd/mesh-host/main_test.go index 77b3bb8..86bc6e5 100644 --- a/cmd/mesh-host/main_test.go +++ b/cmd/mesh-host/main_test.go @@ -609,3 +609,63 @@ func TestTheApplysLogIsNeverNil(t *testing.T) { t.Fatalf("the apply's detail did not reach the caller's announce: %v", said) } } + +// Defends novox/hq issues 257 and 261: a reconcile due while the link applies a newer declaration +// applies that newer one once its turn comes — never the one kept when its timer fired. Read before +// waiting, it gave back a module assigned a second earlier, and reported a declaration the mesh no +// longer recorded as sent. +func TestAReconcileAppliesWhatWasKeptWhenItsTurnComes(t *testing.T) { + built, err := system.For("arch") + if err != nil || built.Confirm(context.Background(), apply.ExecRunner) != nil { + t.Skip("this machine is not one these tests can apply on") + } + was := builtFor + builtFor = "arch" + t.Cleanup(func() { builtFor = was }) + dir := t.TempDir() + opts := options{state: filepath.Join(dir, "state.json")} + public, private, err := ed25519.GenerateKey(nil) + if err != nil { + t.Fatal(err) + } + target := filepath.Join(dir, "a.conf") + kept := func(content string) store.Declared { + body := []byte(`{"declaration":1,"resources":[{"id":"a","type":"file","path":"` + target + + `","content":"` + content + `\n"}]}`) + return store.Declared{Declaration: body, Signature: ed25519.Sign(private, body)} + } + if err := store.SaveDeclared(store.DeclaredPath(opts.state), kept("older")); err != nil { + t.Fatal(err) + } + + // The link is applying: the reconcile's timer fires and it waits its turn. + applying.Lock() + done := make(chan error, 1) + go func() { + _, err := reapplyKept(context.Background(), opts, public, nil, nil) + done <- err + }() + time.Sleep(50 * time.Millisecond) + // The link's apply ends by keeping the newer declaration, and lets go. + if err := store.SaveDeclared(store.DeclaredPath(opts.state), kept("newer")); err != nil { + applying.Unlock() + t.Fatal(err) + } + applying.Unlock() + + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(10 * time.Second): + t.Fatal("the reconcile never ran once the node was free") + } + got, err := os.ReadFile(target) + if err != nil { + t.Fatal(err) + } + if string(got) != "newer\n" { + t.Errorf("the reconcile applied %q, what was kept before the newer declaration", got) + } +}