amadan
provision: the swap refuses rows the edge does not serve from their addr (Plan 7 Task 3 fix round 1)
I1: a fleet-placed row (region fleet/<name>, or fleet_box set), a hibernating instance (served through the activator's own child) and a claim-owned row are refused SD-N96 with the column's name. The resume also needs the journal's record key; the assignment is read only when it can decide; the held and bad-host tests now bite on their own guard.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+97
−8
internal/provision/seedswap.go+27 −7internal/provision/seedswap_test.go+70 −1
CI passed — run details
diff --git a/internal/provision/seedswap.go b/internal/provision/seedswap.go| index 6019780fd..c197b5866 100644 |
| --- a/internal/provision/seedswap.go |
| +++ b/internal/provision/seedswap.go |
| @@ -16,7 +16,15 @@ import ( |
| // importShape is the legacy row's shape check (spec §5.4, codex 10, R1-10): |
| // "" when the row is an addr-backed instance or service this import may |
| -// take over, else what makes it not one. |
| +// take over, else what makes it not one (the column's name). |
| +// |
| +// Addr-backed means the edge serves the row from its addr or addrs and |
| +// from nothing else. The edge's dispatch (edge.Reload) takes a tunnel, |
| +// then a fleet placement, then a socket, before it reaches the addrs, and |
| +// an instance flagged hibernate goes through the activator, which starts |
| +// a process of its own. So each of those is refused here, beside the |
| +// columns shapeMismatch refuses adoption over and a claim's ownership, |
| +// which the import journal cannot put back (registry.SwapForImport). |
| func importShape(rt registry.Route) string { |
| switch { |
| case rt.Kind != "instance" && rt.Kind != "service": |
| @@ -37,6 +45,15 @@ func importShape(rt registry.Route) string { |
| return "platform" |
| case rt.Canary: |
| return "canary" |
| + case rt.FleetBox != "": |
| + return "fleet_box" |
| + case rt.Kind == "instance" && rt.Hibernate: |
| + return "hibernate" |
| + case rt.ClaimOwnership != (registry.ClaimOwnership{}): |
| + return "claim" |
| + } |
| + if _, fleet := registry.FleetPlacement(rt.Region); fleet { |
| + return "region" |
| } |
| return "" |
| } |
| @@ -72,10 +89,10 @@ func importRefusal(rec Record, existing *registry.Route, cfg ReconcileConfig, ow |
| if existing.App != rec.App { |
| return fmt.Sprintf("the route belongs to app %s", existing.App) |
| } |
| - switch release.RouteAccount(*existing) { |
| - case rec.Account: |
| + switch { |
| + case release.RouteAccount(*existing) == rec.Account: |
| return "" |
| - case release.OpsAccount: |
| + case operatorFiled(rec, existing): |
| if o, ok := owners.Lookup(existing.App); ok && o.Account == rec.Account && o.App == rec.App { |
| return "" |
| } |
| @@ -86,7 +103,8 @@ func importRefusal(rec Record, existing *registry.Route, cfg ReconcileConfig, ow |
| // operatorFiled reports a row the swap may take only through the operator |
| // assignment: filed under the operator account, for a record that is not |
| -// the operator's own. |
| +// the operator's own. importRefusal's account rule and swapImport's |
| +// decision to read the assignment both ask this one question. |
| func operatorFiled(rec Record, rt *registry.Route) bool { |
| if rt == nil { |
| return false |
| @@ -151,7 +169,7 @@ func swapImport(ctx context.Context, s release.Store, reg *registry.Registry, cf |
| return |
| } |
| if j != nil { |
| - if j.SeedID != rec.Seed.ID || j.State != registry.JournalSwapped { |
| + if j.SeedID != rec.Seed.ID || j.State != registry.JournalSwapped || j.RecordKey != RecordKey(rec.Account, rec.App, rec.Host) { |
| log.Printf("provision: %s: import %s left alone: the import journal holds seed %s at %s", rec.Host, rec.Seed.ID, j.SeedID, j.State) |
| return |
| } |
| @@ -186,8 +204,10 @@ func swapImport(ctx context.Context, s release.Store, reg *registry.Registry, cf |
| log.Printf("provision: %s: import deferred: the route is held by operation %s", rec.Host, existing.HoldOp) |
| return |
| } |
| + // The assignment is read only when it can decide: claims off (SD-N98 |
| + // comes first) and the box nested (SD-N97 next). |
| var owners *seeds.LegacyOwners |
| - if operatorFiled(rec, existing) { |
| + if !cfg.ClaimEnabled && isNested && operatorFiled(rec, existing) { |
| if cfg.Seeds.LegacyOwners == nil { |
| log.Printf("provision: %s: import deferred: this box cannot read the legacy route assignments", rec.Host) |
| return |
diff --git a/internal/provision/seedswap_test.go b/internal/provision/seedswap_test.go| index c8dc81177..6dd803c58 100644 |
| --- a/internal/provision/seedswap_test.go |
| +++ b/internal/provision/seedswap_test.go |
| @@ -201,6 +201,21 @@ func TestSwapRefusesEachCondition(t *testing.T) { |
| {"caps", func(r Record, a string) *registry.Route { x := addrRow(r, a); x.Caps = "host-role"; return &x }, nil, nil, "(caps)"}, |
| {"platform", func(r Record, a string) *registry.Route { x := addrRow(r, a); x.Platform = true; return &x }, nil, nil, "(platform)"}, |
| {"canary", func(r Record, a string) *registry.Route { x := addrRow(r, a); x.Canary = true; return &x }, nil, nil, "(canary)"}, |
| + {"sidecar", func(r Record, a string) *registry.Route { x := addrRow(r, a); x.Kind = "sidecar"; return &x }, nil, nil, "(kind sidecar)"}, |
| + // The edge serves these from somewhere other than the addr: a |
| + // fleet box over the reverse channel, or the activator's own child. |
| + {"fleet placed", func(r Record, a string) *registry.Route { x := addrRow(r, a); x.Region = "fleet/edge1"; return &x }, nil, nil, "(region)"}, |
| + {"fleet box", func(r Record, a string) *registry.Route { x := addrRow(r, a); x.FleetBox = "box1"; return &x }, nil, nil, "(fleet_box)"}, |
| + {"hibernating instance", func(r Record, a string) *registry.Route { |
| + x := addrRow(r, a) |
| + x.Kind, x.Hibernate = "instance", true |
| + return &x |
| + }, nil, nil, "(hibernate)"}, |
| + {"claim owned", func(r Record, a string) *registry.Route { |
| + x := addrRow(r, a) |
| + x.ClaimOwnership = registry.ClaimOwnership{ClaimIncarnation: "inc", ClaimStamp: 1, ClaimOwner: "owner", ClaimRowID: "row"} |
| + return &x |
| + }, nil, nil, "(claim)"}, |
| {"flat", func(r Record, a string) *registry.Route { x := addrRow(r, a); return &x }, nil, func(c *ReconcileConfig) { c.Layout = nil }, |
| "this box does not run instances nested"}, |
| {"flat intent", func(r Record, a string) *registry.Route { x := addrRow(r, a); return &x }, nil, func(c *ReconcileConfig) { |
| @@ -332,6 +347,34 @@ func TestSwapSkipsARecordItCannotGate(t *testing.T) { |
| } |
| } |
| +// Each skip is importUngateable's own (not a later guard's), and a sound |
| +// import record passes it. |
| +func TestImportUngateable(t *testing.T) { |
| + good := newImport("abc") |
| + if why := importUngateable(good, importRow(good, "/data"), "/data"); why != "" { |
| + t.Fatalf("a sound record: %q", why) |
| + } |
| + for name, edit := range map[string]func(*Record){ |
| + "empty verify hash": func(r *Record) { r.Seed.VerifyHash = "" }, |
| + "closed gate": func(r *Record) { r.Seed.VerifyHash = "closed" }, |
| + "bad seed id": func(r *Record) { r.Seed.ID = "sd_../../x" }, |
| + "bad host": func(r *Record) { r.Host = "A.shop.example.com/.." }, |
| + "does not hibernate": func(r *Record) { |
| + off := false |
| + r.Hibernate = &off |
| + }, |
| + "not an instance": func(r *Record) { r.Kind = "service" }, |
| + } { |
| + rec := newImport("abc") |
| + seed := *rec.Seed |
| + rec.Seed = &seed |
| + edit(&rec) |
| + if why := importUngateable(rec, importRow(rec, "/data"), "/data"); why == "" { |
| + t.Errorf("%s: not skipped", name) |
| + } |
| + } |
| +} |
| + |
| // The same rule through Reconcile, for a record shape CreateRecord stores. |
| func TestReconcileDoesNotSwapAnImportThatWouldNotHibernate(t *testing.T) { |
| s, sqid := shopSetup(t) |
| @@ -418,7 +461,13 @@ func TestSwapDefersWhatTheWriteCannotTakeYet(t *testing.T) { |
| t.Run(c.name, func(t *testing.T) { |
| s, sqid := shopSetup(t) |
| rec := importRecord(t, s, sqid) |
| - reg := regWith(t, addrRow(rec, sqid)) |
| + r := addrRow(rec, sqid) |
| + if c.held { |
| + // A wrong address too: without the hold's deferral this row |
| + // would be refused SD-N92, not left for the next pass. |
| + r.Addr = "127.0.0.1:9" |
| + } |
| + reg := regWith(t, r) |
| if c.held { |
| if err := reg.SetHold(rec.Host, "moving", "op_1"); err != nil { |
| t.Fatal(err) |
| @@ -472,6 +521,26 @@ func TestSwapResumesTheRecordCASAfterACrash(t *testing.T) { |
| } |
| } |
| +// A journal row naming this seed ID under another record key is not this |
| +// record's swap: left alone, no CAS. |
| +func TestSwapResumeNeedsTheRecordKey(t *testing.T) { |
| + s, sqid := shopSetup(t) |
| + rec := importRecord(t, s, sqid) |
| + reg := regWith(t, addrRow(rec, sqid)) |
| + original := mustGet(t, reg, rec.Host) |
| + next := importRow(rec, "/data") |
| + if err := reg.SwapForImport(registry.ImportJournal{Host: rec.Host, SeedID: rec.Seed.ID, Original: *original, |
| + RecordKey: RecordKey(sqid, "other", rec.Host), At: 1}, next); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := Reconcile(ctx, s, reg, importCfg(t, nil)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if st := seedState(t, s, rec); st.State != seeds.StateReady || st.Reason != "" { |
| + t.Fatalf("record = %s %q, want ready", st.State, st.Reason) |
| + } |
| +} |
| + |
| // A journal row this record's seed does not own (another seed's, or one |
| // already rolling back) leaves the record alone: no refusal, no swap. |
| func TestSwapLeavesAnotherSeedsJournalAlone(t *testing.T) { |