Assign around what a machine already holds

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.

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 '{}';

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 {

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)
+	}
+}

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
 	}

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.