From 41f7c510328da9342629020a92d3a2a8ca03da41 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 1 Sep 2026 18:32:38 +0200 Subject: [PATCH] Assign around what a machine already holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The other half of ADR 0038, and what 04-ISSUES/028 was actually about. A module can now avoid colliding with another module; until this it could not avoid colliding with the mesh itself. The substrate is not a module. A node raises it from the bundle it carries before any mesh exists, so the control plane had never heard of the store, the broker, or its own container — and handed a database module 5432, which the store already had. So the machine says. The host records what each resource binds, distinguishing what it carried from what the mesh sent — a distinction that already existed so the two never remove each other — and reports the carried ones. The node states and this context writes, which is the shape of every message between them. What the declaration binds, not what is open. A machine's open ports are a moving target, and assigning around them would mean a port that was free when it was asked for and taken when it was used. Replaced whole each time rather than merged: a machine that gave a port back must be believed about that too, and a set that only grows keeps a port reserved for something no longer there. Tested against a real database, and the tests bite — removing the check hands the module 20000, which the machine had said it holds. --- .../0017-what-a-machine-already-holds.sql | 12 ++++ internal/inventory/ports.go | 41 ++++++++++++ internal/inventory/ports_test.go | 62 +++++++++++++++++++ internal/link/enrolment.go | 7 +++ internal/link/protocol.go | 9 +++ 5 files changed, 131 insertions(+) create mode 100644 internal/inventory/migrations/0017-what-a-machine-already-holds.sql diff --git a/internal/inventory/migrations/0017-what-a-machine-already-holds.sql b/internal/inventory/migrations/0017-what-a-machine-already-holds.sql new file mode 100644 index 0000000..14de2cf --- /dev/null +++ b/internal/inventory/migrations/0017-what-a-machine-already-holds.sql @@ -0,0 +1,12 @@ +-- Ports a machine holds that the mesh did not assign. +-- +-- novox/hq ADR 0038. The substrate is not a module: a node raises it from the bundle it carries +-- before any mesh exists, so the control plane has never heard of the store or the broker. Told +-- what they hold, it can put a module somewhere else; not told, it hands out a port one of them +-- has and finds out from a container runtime three layers down. +-- +-- On the node rather than in the assignment table, because these are not assignments -- nothing +-- here chose them, and nothing here can move them. They are a fact about the machine, replaced +-- whole each time the machine states it. + +alter table node add column carried_ports integer[] not null default '{}'; diff --git a/internal/inventory/ports.go b/internal/inventory/ports.go index 52d8b23..b765b87 100644 --- a/internal/inventory/ports.go +++ b/internal/inventory/ports.go @@ -76,6 +76,35 @@ func (i *Inventory) PortFor( return i.assignPort(ctx, record.ID, node, module, wanted, fixed) } +// RecordCarried keeps what a machine says it already holds, replacing whatever it said before. +// +// **Replaced whole, not merged.** A machine that gave a port back must be believed about that too, +// and a set the mesh only ever adds to would keep a port reserved for something that is no longer +// there. +func (i *Inventory) RecordCarried(ctx context.Context, node string, ports []int) error { + record, err := i.NodeByName(ctx, node) + if err != nil { + return err + } + if ports == nil { + ports = []int{} + } + _, err = i.store.Pool().Exec(ctx, + `update node set carried_ports = $2 where id = $1`, record.ID, ports) + return err +} + +// carriedOn is what a machine said it already holds. +func (i *Inventory) carriedOn(ctx context.Context, nodeID any) ([]int, error) { + var ports []int + err := i.store.Pool().QueryRow(ctx, + `select carried_ports from node where id = $1`, nodeID).Scan(&ports) + if err != nil { + return nil, err + } + return ports, nil +} + func (i *Inventory) freePort(ctx context.Context, node any, module string, wanted int) error { _, err := i.store.Pool().Exec(ctx, `delete from port_assignment where node = $1 and module = $2 and wanted = $3`, @@ -90,6 +119,18 @@ func (i *Inventory) assignPort( if err != nil { return Assigned{}, err } + // And what the machine itself says it already holds — the substrate it raised before there + // was a mesh to ask (novox/hq ADR 0038). Not assignments: nothing here chose them, and + // nothing here can move them. + carried, err := i.carriedOn(ctx, nodeID) + if err != nil { + return Assigned{}, err + } + for _, port := range carried { + if _, mine := taken[port]; !mine { + taken[port] = "something this machine already runs" + } + } machine := wanted if !fixed { diff --git a/internal/inventory/ports_test.go b/internal/inventory/ports_test.go index dc057c5..0a2a835 100644 --- a/internal/inventory/ports_test.go +++ b/internal/inventory/ports_test.go @@ -149,3 +149,65 @@ func contains(s, what string) bool { return false })() } + +// **The bug this whole thing is for** (novox/hq 04-ISSUES/028). The mesh keeps its own store on a +// machine, from the bundle, before there is any mesh to ask. A database module assigned there was +// handed 5432 — the port the store already had — and found out from a container runtime. +func TestAModuleIsNotGivenAPortTheMachineAlreadyHolds(t *testing.T) { + inv, node := aNodeWithModules(t, "some-service") + ctx := t.Context() + + // What the machine says it raised for itself: the store, the broker, the control plane. + if err := inv.RecordCarried(ctx, node, []int{5432, 5671, 8080, 20000, 20001}); err != nil { + t.Fatal(err) + } + + got, err := inv.PortFor(ctx, node, "some-service", 5432, false) + if err != nil { + t.Fatal(err) + } + for _, held := range []int{5432, 5671, 8080, 20000, 20001} { + if got.Machine == held { + t.Fatalf("it was given %d, which the machine already holds", held) + } + } + if got.Machine != 20002 { + t.Errorf("expected the lowest free one, 20002, and got %d", got.Machine) + } +} + +// A port the protocol fixes, already held by something the mesh did not put there, is refused — +// and refused here rather than by a container runtime on the machine. +func TestAFixedPortTheMachineAlreadyHoldsIsRefused(t *testing.T) { + inv, node := aNodeWithModules(t, "mailu") + ctx := t.Context() + if err := inv.RecordCarried(ctx, node, []int{25}); err != nil { + t.Fatal(err) + } + _, err := inv.PortFor(ctx, node, "mailu", 25, true) + if err == nil { + t.Fatal("a module was given a port something on the machine already holds") + } + if !contains(err.Error(), "already runs") { + t.Errorf("the refusal does not say the machine itself has it: %v", err) + } +} + +// A machine that gave a port back is believed about that too. +func TestWhatAMachineNoLongerHoldsIsAvailableAgain(t *testing.T) { + inv, node := aNodeWithModules(t, "a-service") + ctx := t.Context() + if err := inv.RecordCarried(ctx, node, []int{20000}); err != nil { + t.Fatal(err) + } + if err := inv.RecordCarried(ctx, node, nil); err != nil { + t.Fatal(err) + } + got, err := inv.PortFor(ctx, node, "a-service", 1234, false) + if err != nil { + t.Fatal(err) + } + if got.Machine != 20000 { + t.Errorf("a port the machine gave back was still reserved: got %d", got.Machine) + } +} diff --git a/internal/link/enrolment.go b/internal/link/enrolment.go index 6c962db..2f2893c 100644 --- a/internal/link/enrolment.go +++ b/internal/link/enrolment.go @@ -170,6 +170,13 @@ func (e Enrolment) Heard(ctx context.Context, report Report) error { } // Ordered, so two readings of one failure are the same reading. sort.Slice(doing.Failed, func(i, j int) bool { return doing.Failed[i].ID < doing.Failed[j].ID }) + + // And what that machine says it already holds, so a port is assigned around it rather than + // on top of it (novox/hq ADR 0038). Kept even when the declaration was refused: what the + // machine carries is true regardless of what it thought of the last thing it was sent. + if err := e.Inventory.RecordCarried(ctx, report.Node, report.Carried); err != nil { + return err + } if err := e.Inventory.RecordDoing(ctx, node.ID, doing); err != nil { return err } diff --git a/internal/link/protocol.go b/internal/link/protocol.go index 6823e1d..40d6cf8 100644 --- a/internal/link/protocol.go +++ b/internal/link/protocol.go @@ -85,6 +85,15 @@ type Report struct { Applied []string `json:"applied,omitempty"` Failed map[string]string `json:"failed,omitempty"` Refused string `json:"refused,omitempty"` + + // Carried are the machine's ports held by what that host raised from its own bundle. + // + // **The half the mesh cannot know** (novox/hq ADR 0038). The substrate is not a module — a + // node raises it before any mesh exists — so without being told, the mesh assigns a module a + // port the store or the broker already holds, and hears about it from a container runtime. + // + // The node states and this context writes, which is the shape of every message here. + Carried []int `json:"carried,omitempty"` } // EnrolReply is what the mesh says back.