amadan

carlos / platform Public

hostbox: tests pin the source hold's last three guards: a row another op holds between the read and the write is refused, and the hold-only check refuses an incomplete inventory or one without exactly one route entry

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude d150fdd86ed6479f807a4d28663b7dc589f74f96 parent 0eec5d7
2 files changed, +126 −0
  • internal/hostbox/reconcile_relocation_source_census_test.go +70 −0
  • internal/hostbox/reconcile_relocation_source_test.go +56 −0
diff --git a/internal/hostbox/reconcile_relocation_source_census_test.go b/internal/hostbox/reconcile_relocation_source_census_test.go
index 017d46595..f4168ac81 100644
--- a/internal/hostbox/reconcile_relocation_source_census_test.go
+++ b/internal/hostbox/reconcile_relocation_source_census_test.go
@@ -249,3 +249,73 @@ func TestCensusCheckHeldRefusesAContradictingOrMissingSourceInventory(t *testing
t.Fatalf("a contradicting inventory: the row is %+v (was %+v)", row, minted)
}
}
+
+// CheckHeld accepts the source's installation only from a complete
+// inventory with exactly one route entry for the host. The inventory as
+// recorded is accepted from a fresh inventory database; the same
+// inventory marked incomplete, with its route entry removed, or with that
+// entry recorded twice is ErrTypedInventory.
+func TestCensusCheckHeldRefusesAnIncompleteInventoryOrOneWithoutExactlyOneRoute(t *testing.T) {
+ ctx := t.Context()
+ s := newSourceReconcile(t)
+ minted := *s.row(t)
+ o := s.releasingThenInstalling(t)
+ selected := RecordSelection{Host: s.f.host, Incarnation: o.Claim.Holder.Incarnation}
+ recorded, err := s.inv.ReadInstallation(ctx, s.f.host, minted.ClaimIncarnation, minted.ClaimStamp)
+ if err != nil {
+ t.Fatal(err)
+ }
+ isRoute := func(e StopInventoryEntry) bool { return e.Kind == "route" && e.RelativePath == s.f.host }
+ routes := 0
+ for _, e := range recorded.Entries {
+ if isRoute(e) {
+ routes++
+ }
+ }
+ if !recorded.Complete || routes != 1 {
+ t.Fatalf("the recorded inventory: complete %v, %d route entries", recorded.Complete, routes)
+ }
+ for _, tc := range []struct {
+ name string
+ change func(*StopInventory)
+ want error
+ }{
+ {"as recorded", func(*StopInventory) {}, nil},
+ {"incomplete", func(in *StopInventory) { in.Complete = false }, ErrTypedInventory},
+ {"no route entry", func(in *StopInventory) {
+ kept := []StopInventoryEntry{}
+ for _, e := range in.Entries {
+ if !isRoute(e) {
+ kept = append(kept, e)
+ }
+ }
+ in.Entries = kept
+ }, ErrTypedInventory},
+ {"two route entries", func(in *StopInventory) {
+ for _, e := range in.Entries {
+ if isRoute(e) {
+ e.ArtifactID, e.Inode = e.ArtifactID+"-2", e.Inode+1
+ in.Entries = append(in.Entries, e)
+ break
+ }
+ }
+ }, ErrTypedInventory},
+ } {
+ in := recorded
+ in.Entries = append([]StopInventoryEntry(nil), recorded.Entries...)
+ tc.change(&in)
+ db, err := OpenStopInventoryDB(filepath.Join(t.TempDir(), "inventory.db"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { db.Close() })
+ if err := db.WriteStopInventory(ctx, in); err != nil {
+ t.Fatalf("%s: writing the inventory: %v", tc.name, err)
+ }
+ c := *s.census
+ c.Inventory = db
+ if err := c.CheckHeld(ctx, o, selected); !errors.Is(err, tc.want) || (tc.want == nil && err != nil) {
+ t.Fatalf("%s: %v, want %v", tc.name, err, tc.want)
+ }
+ }
+}
diff --git a/internal/hostbox/reconcile_relocation_source_test.go b/internal/hostbox/reconcile_relocation_source_test.go
index d6bd84468..4a1a4ecdd 100644
--- a/internal/hostbox/reconcile_relocation_source_test.go
+++ b/internal/hostbox/reconcile_relocation_source_test.go
@@ -4,6 +4,7 @@ package hostbox
import (
"context"
+ "database/sql"
"errors"
"os"
"path/filepath"
@@ -198,3 +199,58 @@ func TestReconcileSourceHoldNeverTakesAnotherOpsRow(t *testing.T) {
t.Fatalf("an unreadable row: %v (want the read's error)", err)
}
}
+
+// RegistryHoldRow reads the row unheld, but another op holds it before
+// the conditional write lands: SetHoldIf matches nothing and answers
+// false. That is never a hold: the write is ErrChanged, the other op's
+// hold is left as it is, and the next attempt, judging afresh, refuses
+// the other op's row too. The concurrent writer is a trigger on the
+// registry's own database that holds the row for "mv-9" as the hold
+// write starts and abandons that write (RAISE(IGNORE)), so the write
+// touches nothing and counts no change.
+func TestRegistryHoldRowRefusesARowHeldBetweenTheReadAndTheWrite(t *testing.T) {
+ ctx := context.Background()
+ path := filepath.Join(t.TempDir(), "registry.db")
+ reg, err := registry.Open(path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { reg.Close() })
+ owner := registry.ClaimOwnership{ClaimIncarnation: "inc-1", ClaimStamp: 7, ClaimOwner: "owner-1", ClaimRowID: "row-1"}
+ if err := reg.Upsert(registry.Route{Host: "shop.example", Status: "ready", Socket: "/run/shop.sock", ClaimOwnership: owner}); err != nil {
+ t.Fatal(err)
+ }
+ before := *rowOf(t, reg, "shop.example")
+ db, err := sql.Open("sqlite", path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { db.Close() })
+ if _, err := db.Exec(`CREATE TRIGGER concurrent_hold BEFORE UPDATE OF status, hold_op ON routes
+ WHEN NEW.status = 'relocating' AND OLD.hold_op = ''
+ BEGIN
+ UPDATE routes SET status = 'moving', hold_op = 'mv-9' WHERE host = OLD.host;
+ SELECT RAISE(IGNORE);
+ END`); err != nil {
+ t.Fatal(err)
+ }
+ hold := RegistryHoldRow(reg)
+ if err := hold(ctx, "shop.example", "rel-1"); !errors.Is(err, ErrChanged) {
+ t.Fatalf("a row held between the read and the write: %v (want ErrChanged)", err)
+ }
+ got := rowOf(t, reg, "shop.example")
+ other := before
+ other.Status, other.HoldOp = "moving", "mv-9"
+ if got == nil || *got != other {
+ t.Fatalf("after the refused write: %+v (want the other op's hold %+v)", got, other)
+ }
+ if _, err := db.Exec(`DROP TRIGGER concurrent_hold`); err != nil {
+ t.Fatal(err)
+ }
+ if err := hold(ctx, "shop.example", "rel-1"); !errors.Is(err, ErrChanged) {
+ t.Fatalf("the next attempt over the other op's row: %v (want ErrChanged)", err)
+ }
+ if got := rowOf(t, reg, "shop.example"); got == nil || *got != other {
+ t.Fatalf("the next attempt took the other op's row: %+v", got)
+ }
+}