From 472744bbaaff7541284c46fe370af9ce3093ba72 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 18 Oct 2026 05:32:14 +0200 Subject: [PATCH] Migration 0105 drops the tables of the chain behind a guard; a source scan proves no code names them (issue 1096) The migration raises first unless chain_moved names builds, rollouts and assignments for every kind that holds rows, and unless pending_assignment holds no open or unanswered row; then it drops build_request, build, batched_merge, release_plan, pending_assignment, assignment, raise_assignment_generation(), node.sent_builds and chain_moved. build_gate stays. --- cmd/mesh-controller/chain_integration_test.go | 3 - internal/inventory/chain_sent_test.go | 4 - internal/inventory/chain_source_test.go | 144 ++++++++++++++++++ .../0105-the-chain-leaves-the-store.sql | 52 +++++++ 4 files changed, 196 insertions(+), 7 deletions(-) create mode 100644 internal/inventory/chain_source_test.go create mode 100644 internal/inventory/migrations/0105-the-chain-leaves-the-store.sql diff --git a/cmd/mesh-controller/chain_integration_test.go b/cmd/mesh-controller/chain_integration_test.go index e5c3610a..81866f2b 100644 --- a/cmd/mesh-controller/chain_integration_test.go +++ b/cmd/mesh-controller/chain_integration_test.go @@ -242,7 +242,4 @@ func TestAnAssignmentWaitingForItsBuildIsOneKeyOfTheAssignmentsBucket(t *testing if !slices.Contains(assignedTo(t, open, "laptop"), "sensors") { t.Fatal("the build registered sensors and it was not assigned") } - if rows := inv.QueryForTest(t, `select count(*) from pending_assignment`); rows[0][0] != "0" { - t.Fatalf("the table of pending assignments took %s rows", rows[0][0]) - } } diff --git a/internal/inventory/chain_sent_test.go b/internal/inventory/chain_sent_test.go index d65614f4..a62ea017 100644 --- a/internal/inventory/chain_sent_test.go +++ b/internal/inventory/chain_sent_test.go @@ -88,10 +88,6 @@ func TestASendReplacesTheWholeMapOfTheNodeAndAHandSendMakesItUnknown(t *testing. if again, _, _ := inv.AssignmentValue(ctx, node, "web"); !again.SentUnknown.IsZero() { t.Fatalf("web still says unknown: %+v", again) } - // The column is not written any more. - if rows := inv.QueryForTest(t, `select sent_builds is null from node where name = $1`, node); rows[0][0] != "true" { - t.Fatalf("node.sent_builds was written: %v", rows) - } } // failingToWrite is an assignments bucket that fails every write. diff --git a/internal/inventory/chain_source_test.go b/internal/inventory/chain_source_test.go new file mode 100644 index 00000000..99f4c76c --- /dev/null +++ b/internal/inventory/chain_source_test.go @@ -0,0 +1,144 @@ +package inventory + +import ( + "go/ast" + "go/parser" + "go/token" + "io/fs" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + "testing" +) + +// The tables of the chain, and the column, that the controller's store no longer holds (novox/hq ADR 0408 §8, issue 1096). +var chainTables = []string{"build", "build_request", "release_plan", "batched_merge", "assignment", "pending_assignment"} + +// sqlStatement is what makes a literal a statement of the store: it opens with a verb of SQL. +var sqlStatement = regexp.MustCompile(`(?is)^\s*(select|insert|update|delete|alter|drop|create|with|truncate|from|join|where|and|set|values)\b|\b(select\s.+\sfrom|insert\s+into|delete\s+from)\b`) + +// **No code names a table of the chain** (acceptance 12, issue 1096), found by reading the sources: a string literal of any +// non-test file of internal/ or cmd/ that names one after from, into, update, join, delete from, references or table — +// build_gate and port_assignment are other words — or the column sent_builds. This is the static half of the proof; the +// integration test reads the catalog. +func TestNoCodeNamesATableOfTheChain(t *testing.T) { + names := regexp.MustCompile(`(?i)\b(from|into|update|join|delete\s+from|references|table)\s+(` + strings.Join(chainTables, "|") + `)\b|\bsent_builds\b`) + var found []string + for _, root := range []string{"../../internal", "../../cmd"} { + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() && d.Name() == "vendor" { + return filepath.SkipDir + } + if d.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + file, err := parser.ParseFile(token.NewFileSet(), path, nil, 0) + if err != nil { + return err + } + ast.Inspect(file, func(n ast.Node) bool { + lit, ok := n.(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + return true + } + text, err := strconv.Unquote(lit.Value) + if err != nil { + return true + } + if !sqlStatement.MatchString(text) { + return true // prose that happens to say "from assignment" is no statement + } + if m := names.FindString(text); m != "" { + found = append(found, filepath.ToSlash(path)+": "+strings.Join(strings.Fields(m), " ")) + } + return true + }) + return nil + }) + if err != nil { + t.Fatal(err) + } + } + if len(found) > 0 { + t.Fatalf("code still names a table of the chain, which the store no longer holds:\n %s", strings.Join(found, "\n ")) + } +} + +// The scan sees what it says it sees: planted, a string naming a table is found, and a comment or a word beside is not. +func TestTheSourceScanFindsATablePlanted(t *testing.T) { + names := regexp.MustCompile(`(?i)\b(from|into|update|join|delete\s+from|references|table)\s+(` + strings.Join(chainTables, "|") + `)\b|\bsent_builds\b`) + for text, want := range map[string]bool{ + "select id from build where id = $1": true, + "insert into assignment (node, module)": true, + "update node set sent_builds = $1": true, + "select 1 from build_gate": false, + "delete from port_assignment": false, + "select a from build_request a join build b": true, + "an assignment from build to build": true, + "the assignment generation": false, + "select count(*) from pending_assignment": true, + } { + if got := names.MatchString(text); got != want { + t.Errorf("%q: %v, want %v", text, got, want) + } + } +} + +// **The last migration drops the tables of the chain, behind its guard** (issue 1096): it names each table, the column and +// the function, drops no build_gate, and raises before it drops anything. +func TestTheLastMigrationDropsTheTablesOfTheChain(t *testing.T) { + migrations, err := Migrations() + if err != nil { + t.Fatal(err) + } + last := migrations[len(migrations)-1] + if last.Number < 105 { + t.Fatalf("the last migration is %d", last.Number) + } + raw, err := os.ReadFile(filepath.Join("migrations", migrationFile(t, last.Number))) + if err != nil { + t.Fatal(err) + } + var code []string + for _, line := range strings.Split(string(raw), "\n") { + if !strings.HasPrefix(strings.TrimSpace(line), "--") { + code = append(code, line) + } + } + sql := strings.ToLower(strings.Join(code, "\n")) + for _, table := range append(append([]string{}, chainTables...), "chain_moved") { + if !strings.Contains(sql, "drop table "+table+";") { + t.Errorf("migration %d does not drop %s", last.Number, table) + } + } + for _, want := range []string{"drop function raise_assignment_generation()", "alter table node drop column sent_builds"} { + if !strings.Contains(sql, want) { + t.Errorf("migration %d lacks %q", last.Number, want) + } + } + if strings.Contains(sql, "build_gate") || strings.Contains(sql, "assignment_generation;") { + t.Errorf("migration %d touches a table that stays", last.Number) + } + if raise, drop := strings.Index(sql, "raise exception"), strings.Index(sql, "drop table"); raise < 0 || drop < 0 || raise > drop { + t.Errorf("migration %d does not raise before it drops (%d, %d)", last.Number, raise, drop) + } + for _, kind := range []string{"'builds'", "'rollouts'", "'assignments'", "pending_assignment", "chain_moved"} { + if !strings.Contains(sql[:strings.Index(sql, "drop table")], kind) { + t.Errorf("the guard of migration %d does not look at %s", last.Number, kind) + } + } +} + +func migrationFile(t *testing.T, number int) string { + t.Helper() + files, err := filepath.Glob(filepath.Join("migrations", strconv.Itoa(number/1000%10)+strconv.Itoa(number/100%10)+strconv.Itoa(number/10%10)+strconv.Itoa(number%10)+"-*.sql")) + if err != nil || len(files) != 1 { + t.Fatalf("the file of migration %d: %v %v", number, files, err) + } + return filepath.Base(files[0]) +} diff --git a/internal/inventory/migrations/0105-the-chain-leaves-the-store.sql b/internal/inventory/migrations/0105-the-chain-leaves-the-store.sql new file mode 100644 index 00000000..e867f97d --- /dev/null +++ b/internal/inventory/migrations/0105-the-chain-leaves-the-store.sql @@ -0,0 +1,52 @@ +-- The tables of builds, rollouts and assignments leave the store (novox/hq ADR 0408 §8, design 47 milestone 5, issue 1096). +-- +-- Delivery 1 wrote their rows into the bus's buckets at the controller's start and recorded each kind it had counted in +-- chain_moved; from then on nothing in the controller reads or writes the tables. This drops them. **It is one-way**: a +-- controller older than this schema is refused by the put-back gate, and the rows are in the buckets only. +-- +-- **The guard comes first, in the same transaction as the drops**, because a migration runs when the store is opened, before +-- the controller of this delivery serves and before any connection to the bus: it can prove the move from the store alone. +-- A kind that holds rows and is not named in chain_moved was never moved, so dropping its tables would lose it; a pending +-- assignment still waiting, being made, or ended unmade with its condition unanswered has no mover (it is a value of the +-- assignments bucket from this delivery on), so the table must hold none. A store that never held a row of a kind — a new +-- mesh — passes. A migration that raises leaves nothing applied, `prepare` fails with the reason, and the running controller +-- keeps its tables. +do $$ +declare + kind text; + holds boolean; + missing text[] := '{}'; +begin + foreach kind in array array['builds', 'rollouts', 'assignments'] loop + holds := case kind + when 'builds' then exists (select 1 from build) or exists (select 1 from build_request) + when 'rollouts' then exists (select 1 from release_plan) or exists (select 1 from batched_merge) + when 'assignments' then exists (select 1 from assignment) + end; + if holds and not coalesce((select counts ? kind from chain_moved), false) then + missing := missing || kind; + end if; + end loop; + if array_length(missing, 1) > 0 then + raise exception 'the controller''s store still holds rows of % that chain_moved does not name as moved to the bus; ' + 'run the controller of the previous delivery and read chain_moved back first', array_to_string(missing, ', '); + end if; + if exists (select 1 from pending_assignment + where state in ('waiting', 'applying') or (raised_at is not null and cleared_at is null)) then + raise exception 'a pending assignment is still in the table (waiting, being made, or ended with its condition ' + 'unanswered); let it settle, then deliver again'; + end if; +end +$$; + +drop table build_request; +drop table build; +drop table batched_merge; +drop table release_plan; +drop table pending_assignment; +-- Its triggers go with it; the function they called goes here. The counter table stays: the controller raises it. +drop table assignment; +drop function raise_assignment_generation(); +alter table node drop column sent_builds; +-- No reader of it remains. build_gate stays (issue 1097). +drop table chain_moved;