diff --git a/internal/inventory/nodes.go b/internal/inventory/nodes.go index a1da8cf..e35990a 100644 --- a/internal/inventory/nodes.go +++ b/internal/inventory/nodes.go @@ -851,9 +851,9 @@ func (i *Inventory) DoingOf(ctx context.Context, name string) (Doing, bool, erro var d Doing var failed []byte err = i.store.Pool().QueryRow(ctx, - `select outcome, refused, failed, applied, at, failing_since, failures + `select outcome, refused, failed, applied, at, failing_since, failures, coalesce(declared, '') from node_report where node = $1`, node.ID). - Scan(&d.Outcome, &d.Refused, &failed, &d.Applied, &d.At, &d.Since, &d.Times) + Scan(&d.Outcome, &d.Refused, &failed, &d.Applied, &d.At, &d.Since, &d.Times, &d.Declared) if errors.Is(err, pgx.ErrNoRows) { return Doing{}, false, nil } diff --git a/internal/link/enrolment.go b/internal/link/enrolment.go index d84714c..0f59178 100644 --- a/internal/link/enrolment.go +++ b/internal/link/enrolment.go @@ -410,6 +410,25 @@ func (e Enrolment) Heard(ctx context.Context, report Report) (news bool, err err if err := e.Inventory.RecordCarried(ctx, report.Node, report.Carried); err != nil { return false, err } + // **An account of a declaration the mesh has moved past never replaces the account it keeps** + // (novox/hq issue 267). The machine-facts half of such a report is kept above, whenever it + // arrives; this half is the machine's word on what it did with what it was sent, and the release + // plan reads it as such. A reconcile that held a machine to the older declaration a moment before + // the newer one arrived reported *after* the newer apply's report, and stored last, it read as the + // machine never having applied what it was sent — the plan waited until somebody pushed by hand. + // One row per node means the last write wins, so the order of arrival must not decide it. + if report.Declared != "" { + sent, err := e.Inventory.Outstanding(ctx, report.Node) + if err != nil { + return false, err + } + if Superseded(report.Declared, sent) { + log.Printf("kept what %s says about the machine, and not its account of declaration %s: "+ + "the mesh has sent it %s since", report.Node, short(report.Declared), short(sent)) + return false, e.Inventory.Seen(ctx, node.ID) + } + } + // **Whether this is news** is the store's answer: it holds the previous report, and a machine // that reconciles every minute says the same thing until something changes (novox/hq ADR 0134). news, err = e.Inventory.RecordDoing(ctx, node.ID, doing) diff --git a/internal/link/heard_test.go b/internal/link/heard_test.go index c59e9dd..22d9777 100644 --- a/internal/link/heard_test.go +++ b/internal/link/heard_test.go @@ -260,3 +260,67 @@ func TestWhatFiltersAMachineIsKeptFromItsReport(t *testing.T) { t.Fatalf("the next report did not replace what filters the machine: %+v", again) } } + +// Measured on the home server (novox/hq issue 267): the mesh sent d2; the machine applied it and +// reported, and a reconcile's report about d1 — made just before d2 arrived — reached the mesh after +// it. Stored last, it read as the machine never having applied d2, and the release plan waited on a +// report it had already been given. +func TestAnAccountOfAnOlderDeclarationDoesNotReplaceTheNewer(t *testing.T) { + inv := inventory.ForTest(t) + ctx := context.Background() + node, err := inv.AddNode(ctx, "home-server") + if err != nil { + t.Fatal(err) + } + if err := inv.RecordSent(ctx, node.ID, "d2", nil); err != nil { + t.Fatal(err) + } + heard := link.Enrolment{Inventory: inv} + if _, err := heard.Heard(ctx, link.Report{Node: "home-server", Declared: "d2", + Applied: []string{"a", "b"}, Outward: []string{"eth0"}}); err != nil { + t.Fatal(err) + } + if _, err := heard.Heard(ctx, link.Report{Node: "home-server", Declared: "d1", + Applied: []string{"a"}, Outward: []string{"eth1"}}); err != nil { + t.Fatal(err) + } + doing, said, err := inv.DoingOf(ctx, "home-server") + if err != nil || !said { + t.Fatalf("no account kept: %v", err) + } + if doing.Declared != "d2" || doing.Applied != 2 { + t.Fatalf("the older report replaced the account of what was sent: %+v", doing) + } + // What it says about the machine is kept whenever it arrives, as before. + if links, err := inv.OutwardLinksOf(ctx, "home-server"); err != nil || len(links) != 1 || links[0] != "eth1" { + t.Fatalf("what the older report said about the machine was not kept: %v %v", links, err) + } +} + +// The account of the declaration that is outstanding replaces whatever was kept, and so does one +// from a machine nothing was ever recorded as sent to. +func TestAnAccountOfTheSentDeclarationReplacesTheKeptOne(t *testing.T) { + inv := inventory.ForTest(t) + ctx := context.Background() + node, err := inv.AddNode(ctx, "home-server") + if err != nil { + t.Fatal(err) + } + heard := link.Enrolment{Inventory: inv} + if _, err := heard.Heard(ctx, link.Report{Node: "home-server", Declared: "d1", Applied: []string{"a"}}); err != nil { + t.Fatal(err) + } + if err := inv.RecordSent(ctx, node.ID, "d2", nil); err != nil { + t.Fatal(err) + } + if _, err := heard.Heard(ctx, link.Report{Node: "home-server", Declared: "d2", Applied: []string{"a", "b"}}); err != nil { + t.Fatal(err) + } + doing, _, err := inv.DoingOf(ctx, "home-server") + if err != nil { + t.Fatal(err) + } + if doing.Declared != "d2" || doing.Applied != 2 { + t.Fatalf("the account of what was sent was not kept: %+v", doing) + } +}