diff --git a/internal/inventory/chain_links.go b/internal/inventory/chain_links.go index 56ec03b4..37f8f3b7 100644 --- a/internal/inventory/chain_links.go +++ b/internal/inventory/chain_links.go @@ -309,8 +309,8 @@ func (i *Inventory) CopySentBuilds(ctx context.Context) (int, error) { } // copySentBuildsOf is CopySentBuilds for one node, over a chain: for each module the node was sent, a standing key that -// names no commit gets it (and the build, when one is known; no rollout), and where no key stands a standing pulled-in key is -// made. A key with a commit, a hand send's unknown and a wait for a build are left as they are. +// names no commit gets it (and the build, when one is known; no rollout), and where no key exists a standing pulled-in key is +// made. A key with a commit, an ended key, a hand send's unknown and a wait for a build are left as they are. func (i *Inventory) copySentBuildsOf(ctx context.Context, c *chain.Chain, node string, sent map[string]string, at time.Time) error { for _, module := range slices.Sorted(maps.Keys(sent)) { commit := sent[module] @@ -330,11 +330,11 @@ func (i *Inventory) copySentBuildsOf(ctx context.Context, c *chain.Chain, node s if err != nil { return err } - if found && (cur.Value.State != "" || cur.Value.Commit != "" || !cur.Value.SentUnknown.IsZero()) { - break + if found && (cur.Value.State != "" || !cur.Value.Ended.IsZero() || cur.Value.Commit != "" || !cur.Value.SentUnknown.IsZero()) { + break // a wait, an ended key (never revived), one that names a commit, or a hand send's unknown } next := cur.Value - if !found || !cur.Value.Stands() { + if !found { next = chain.Assignment{Node: node, Module: module, PulledIn: true, Since: at} } next.Commit, next.Build = commit, build diff --git a/internal/inventory/chain_sent_copy_test.go b/internal/inventory/chain_sent_copy_test.go index 49bf9474..8e797a3c 100644 --- a/internal/inventory/chain_sent_copy_test.go +++ b/internal/inventory/chain_sent_copy_test.go @@ -59,8 +59,8 @@ func TestWhatTheColumnRecordedIsCopiedOntoTheKeysAndOverwritesNothing(t *testing if a := get("mesh-wireguard"); a.Commit != "c-wg" || a.Build != "b-wg" || !a.PulledIn || !a.Stands() || a.Rollout != "" { t.Errorf("the pulled-in module has no standing key of its own: %+v", a) } - if a := get("ended"); a.Commit != "c-ended" || !a.Stands() || !a.PulledIn { - t.Errorf("an ended key is not made to stand again as pulled in: %+v", a) + if a := get("ended"); a.Commit != "" || a.Stands() { + t.Errorf("an ended key was revived or given a commit: %+v", a) } } } @@ -88,3 +88,24 @@ func TestTheColumnOfASentMachineIsCopiedOntoItsKeysAndEmptied(t *testing.T) { t.Fatalf("a second copy did %d nodes (%v)", n, err) } } + +// A machine delivery 2 has sent has its column emptied by the send itself, so a copy after it leaves a key the send cleared +// without a commit. +func TestASendEmptiesTheColumnSoALaterCopyLeavesAClearedKeyBare(t *testing.T) { + inv := ForTest(t) + ctx := t.Context() + record, err := inv.AddNode(ctx, "laptop") + if err != nil { + t.Fatal(err) + } + inv.ExecForTest(t, `update node set sent_builds = '{"notes": "c-old"}'::jsonb where name = 'laptop'`) + if err := inv.RecordSentUnder(ctx, record.ID, "d1", map[string]string{}, 1); err != nil { + t.Fatal(err) + } + if rows := inv.QueryForTest(t, `select count(*) from node where sent_builds is not null`); rows[0][0] != "0" { + t.Fatalf("a send left the column: %v", rows) + } + if n, err := inv.CopySentBuilds(ctx); err != nil || n != 0 { + t.Fatalf("the copy did %d nodes (%v)", n, err) + } +} diff --git a/internal/inventory/nodes.go b/internal/inventory/nodes.go index bf98a1a7..26f57483 100644 --- a/internal/inventory/nodes.go +++ b/internal/inventory/nodes.go @@ -1263,7 +1263,7 @@ func (i *Inventory) RecordSentUnder(ctx context.Context, node, digest string, bu sentEpoch = &e } if _, err := i.store.Pool().Exec(ctx, - `update node set sent = $2, sent_at = now(), sent_epoch = $3 where id = $1`, node, digest, sentEpoch); err != nil { + `update node set sent = $2, sent_at = now(), sent_builds = null, sent_epoch = $3 where id = $1`, node, digest, sentEpoch); err != nil { return err } // What it was sent of each module is the assignments' (SentBuilds); a write of it that fails fails this record. diff --git a/internal/inventory/sends.go b/internal/inventory/sends.go index 4b28e33e..db8b7095 100644 --- a/internal/inventory/sends.go +++ b/internal/inventory/sends.go @@ -155,7 +155,7 @@ func (i *Inventory) RecordSentWith(ctx context.Context, node, digest string, bui } defer func() { _ = tx.Rollback(ctx) }() if _, err := tx.Exec(ctx, - `update node set sent = $2, sent_at = now(), sent_epoch = $3, sent_generation = $4, + `update node set sent = $2, sent_at = now(), sent_builds = null, sent_epoch = $3, sent_generation = $4, sent_provisioning = $5::jsonb where id = $1`, node, digest, sentEpoch, nullIfZero(generation), nullIfEmpty(s.Provisioning)); err != nil { return err diff --git a/testdata/store-tests b/testdata/store-tests index 03d58c9b..3d2db5be 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -160,3 +160,4 @@ ./internal/inventory TestMigration0105PassesAStoreThatNeverHeldARow ./cmd/mesh-controller TestNoTableOfTheControllersStoreHoldsBuildsRolloutsOrAssignments ./internal/inventory TestTheColumnOfASentMachineIsCopiedOntoItsKeysAndEmptied +./internal/inventory TestASendEmptiesTheColumnSoALaterCopyLeavesAClearedKeyBare