[drudge]

What changed

Blake's two-node run on real port-67 namespaces passes at 34a707a with F56-F59 fixed (C169), and the suite and CI are green (C170).

Red, then green

• Primary and replica serve their pinned ranges, a joining node holds, the primary refuses unpinned/stranger/older-member settings, peers aren't other servers, and two partitioned nodes never lease an address twice.: export PATH=/usr/local/go/bin:$PATH GOEXPERIMENT=jsonv2; go test -count=1 -run 'TestDHCPServesOnEveryClusterNode|TestDHCPClusterNodeLeavesUnpinnedRangesAlone|TestDHCPClusterCheck|TestDHCPProbeTreatsClusterPeersAsThisNode|TestDHCPServiceRefusesUnpinnedRangeOnCluster|TestDHCPNamesOnlyWhereDHCPIsServed|TestDHCPClusterRole' ./internal/app/ && go test -count=1 -run 'TestClusterPartitionNeverLeasesAnAddressTwice' ./internal/dhcp/
  • fails on the base (b7fd3ca): exit 1
  • passes on the branch (dd062a1): exit 0

Checks

• The app, dhcp, config and docs packages pass, and the two DHCP cluster integration tests (real nodes, replica joining a DHCP-enabled primary) still pass. (Whiteface): export PATH=/usr/local/go/bin:$PATH GOEXPERIMENT=jsonv2; go test -count=1 ./internal/app/ ./internal/dhcp/ ./internal/config/ ./docs/ && go test -count=1 -tags integration -run 'TestClusterReplica.*DHCP' ./internal/app/ && test -z "$(gofmt -l internal docs)", exit 0
• Read the diff against b7fd3ca (role/joining handling, unpinned-range exclusion on cluster nodes, older-member check, peer-aware probe tagging) and reran the DHCP and cluster tests, which pass. I found no defect. Untested: two real nodes binding port 67, and the console text that still says cluster nodes can't serve. (Prover): export PATH=/usr/local/go/bin:$PATH GOEXPERIMENT=jsonv2; cd /home/peak/work/dhcp-server-for-sable/sable-8b && go test -count=1 -run 'TestDHCPServesOnEveryClusterNode|TestDHCPClusterNodeLeavesUnpinnedRangesAlone|TestDHCPClusterCheck|TestDHCPProbeTreatsClusterPeersAsThisNode|TestDHCPServiceRefusesUnpinnedRangeOnCluster|TestDHCPNamesOnlyWhereDHCPIsServed|TestDHCPClusterRole' ./internal/app/ && go test -count=1 -run 'TestClusterPartitionNeverLeasesAnAddressTwice' ./internal/dhcp/, exit 0
• On two real nodes on port 67 with real clients, dhcp: serve pinned ranges on every cluster node (slice 8b) #638's head leases each address from one node only, mirrors every lease to both nodes within seconds with the primary naming them all, never answers for a dead node's addresses, answers a reservation from both nodes, drops peers' probes, and keeps serving through a promote. The six checks filed as F54 to F59 are printed, not counted. (Blake): ./cluster-prove.sh, exit 0
• At PR head 57b917e in my own worktree, F56-F59 tests and the cluster serve/partition tests pass, as do the app, dhcp, web, config and cluster packages; I found no defect. Untested: two real nodes binding port 67, and the console's stale "Cluster nodes can't serve DHCP yet" text (cosmetic). (Prover): export PATH=/usr/local/go/bin:$PATH GOEXPERIMENT=jsonv2; cd /tmp/p638 && go test -count=1 ./internal/app/ ./internal/dhcp/ ./internal/web/ ./internal/config/ ./internal/cluster/, exit 0
• At dhcp: serve pinned ranges on every cluster node (slice 8b) #638's new head 57b917e the two-node harness passes with nothing filed: a member removed with a range pinned to it, a primary updated first, a replica's lease removed on the replica, a client moving between nodes, 250 maximum-size leases renewed while the primary is down, and a primary rebuilt from its backup all behave on real nodes on port 67. (Blake): FILED= ./cluster-prove.sh, exit 0
• At 34a707a, git diff vs 57b917e is five pure renames (zz_→review_, 0 line changes), and the app, dhcp, web, config, cluster and docs packages pass with gofmt clean. Untested: two real nodes on port 67 at this head, and the stale console "can't serve" text. (Prover): export PATH=/usr/local/go/bin:$PATH GOEXPERIMENT=jsonv2; cd /tmp/pv638 && test "$(git diff -M --numstat 57b917e 34a707a | awk '{a+=$1+$2} END{print a+0}')" = 0 && go test -count=1 ./internal/app/ ./internal/dhcp/ ./internal/web/ ./internal/config/ ./internal/cluster/ ./docs/ && test -z "$(gofmt -l internal docs)", exit 0
• At 34a707a the app, dhcp, web, config, cluster and docs packages pass with gofmt clean and dhcp passes under -race, including the F56-F59 review tests; I found no defect. Untested: two real nodes on port 67 (relied on Blake's C165) and the stale console "can't serve" text. (Prover): export PATH=/usr/local/go/bin:$PATH GOEXPERIMENT=jsonv2; cd /tmp/pv638 && go test -count=1 ./internal/app/ ./internal/dhcp/ ./internal/web/ ./internal/config/ ./internal/cluster/ ./docs/ && test -z "$(gofmt -l internal docs)", exit 0
• At dhcp: serve pinned ranges on every cluster node (slice 8b) #638's head 34a707a (the review tests renamed, code unchanged) the two-node harness on real nodes on port 67 passes with nothing filed, including the member removed with a range pinned to it, the primary updated first, and a replica's lease removed on the replica. (Blake): FILED= ./cluster-prove.sh, exit 0
• At dhcp: serve pinned ranges on every cluster node (slice 8b) #638's head gofmt, vet and the docs check are clean and the race tests for cluster, store, dhcp, config, app and web pass, with the five review tests in them. (Blake): ./run-suite.sh 638 /home/peak/work/dhcp-server-for-sable/sable-638, exit 0
• At PR head 34a707a (still current) the app, dhcp, web, config, cluster and docs packages pass, including F56-F59 review tests, and I found no defect. Untested: two real nodes on port 67 (relied on Blake's C169) and the stale console "can't serve" text. (Prover): export PATH=/usr/local/go/bin:$PATH GOEXPERIMENT=jsonv2; cd /tmp/pv638 && go test -count=1 ./internal/app/ ./internal/dhcp/ ./internal/web/ ./internal/config/ ./internal/cluster/ ./docs/, exit 0
• At 34a707a I reran the app, dhcp, web, config, cluster and docs packages (pass, gofmt clean), which include the F56-F59 review tests, and found no defect. Untested: two real nodes on port 67 (relied on Blake's C169) and the stale console "can't serve" text. (Prover): export PATH=/usr/local/go/bin:$PATH GOEXPERIMENT=jsonv2; cd /tmp/pv638 && go test -count=1 ./internal/app/ ./internal/dhcp/ ./internal/web/ ./internal/config/ ./internal/cluster/ ./docs/ && test -z "$(gofmt -l internal docs)", exit 0
• At 34a707a I reran the app, dhcp, web, config, cluster and docs packages (all pass, including the F56-F59 review tests) and found no defect. Untested: two real nodes on port 67 (relied on Blake's C169) and the stale console "can't serve" text (cosmetic). (Prover): export PATH=/usr/local/go/bin:$PATH GOEXPERIMENT=jsonv2; cd /tmp/pv638 && go test -count=1 ./internal/app/ ./internal/dhcp/ ./internal/web/ ./internal/config/ ./internal/cluster/ ./docs/, exit 0

Found and fixed

• F56 bug: Remove a dead member from the cluster while a range is still pinned to it and the last node keeps serving until its next restart, then logs "DHCP server not started ... is not a cluster member" and answers nothing. On two real nodes that left the whole network without DHCP after a routine restart. (fixed)
• F57 bug: Update the primary before the replica and the primary stops serving DHCP at its restart ("update node-b (9.9.0) to Sable 9.9.1 or later"), though the replica already holds the same settings. On two real nodes it stayed off after the replica was updated too, until the primary was restarted again. (fixed)
• F58 bug: On a cluster a lease that a replica granted can't be removed anywhere: the primary answers 404 "has no lease on this node" for a lease its own list shows, and the replica answers 409 "Make control-plane changes on the cluster primary". Seen on two real nodes through the API; the console and MCP go through the same service. (fixed)
• F59 bug: A node rewrites a lease row another node owns: when a client holding a lease on the replica takes one from the primary, the primary ends the replica's row and saves it under a counter of its own. On two real nodes the primary held the row as released under sequence 1 while the replica held it active under 5, which the design rules out (only the owner writes a lease). (fixed)

Risks

Not run: three nodes, Postgres cluster, a relay. Full ./... takes ~20 min on this box; run internal/dhcp, internal/cluster, internal/web with GOFLAGS=-p=1 GOEXPERIMENT=jsonv2 to finish. Known unrelated: internal/outbound TestHTTPClientResolvesThroughSable fails here (proxy), and clusterfault TestRecoveryPromotionKeepsSOASerialsMonotonic is a pre-existing race on main (D66).

Prover

Passed (Sonnet): At 34a707a I reran the app, dhcp, web, config, cluster and docs packages (all pass, including the F56-F59 review tests) and found no defect. Untested: two real nodes on port 67 (relied on Blake's C169) and the stale console "can't serve" text (cosmetic).

Every command above was run by Fortysix's Basecamp itself, on the branch named.

[drudge]

Review of #638 (slice 8b, serve on a cluster) at dd062a1: block. One finding takes DHCP off the whole network; three more need fixing with it.

Run on two real nodes, each in its own network namespace on one bridge, port 67, real clients, including a node killed, a promote, a rolling update and a member removed: zz-review-638-cluster.sh. What holds:

• The primary refuses a range without a node (422), a range naming a non-member (422), and the replica refuses the edit (409); nothing binds port 67 after a refusal. With ranges pinned, both nodes bind within 5 s.
• Each node leases only from its own range; every DISCOVER draws one offer from each node, each from its own range. A reboot is ACKed by the owner alone. With the owner dead the other node stays silent for its addresses (REBIND and INIT-REBOOT, no NAK) and serves new clients from its own range.
• A reservation is answered by both nodes with the same address.
• A peer's probe (option 60 sable-probe/<cluster id>) is answered by neither node; another cluster's by both. Neither node reported its peer as another DHCP server.
• After a promote the new primary keeps serving and names new leases.

Findings:

1. High: remove a dead member with a range still pinned to it, and the last node serves nothing after its next restart. The removal is accepted and DHCP keeps running, so nothing looks wrong. At the next start Apply runs Check, which finds a range naming a non-member, and the node logs DHCP server not started ... is not a cluster member and doesn't bind port 67. In a two-node cluster that is the whole network without DHCP after a routine restart. Test: TestReview638StartServesAfterAMemberIsRemoved.

2. Medium: update the primary before the replica and the primary stops serving DHCP, and stays off. Both on 9.9.0 serving; primary restarted on 9.9.1: update node-b (9.9.0) to Sable 9.9.1 or later before serving DHCP on this cluster, port 67 unbound. After the replica was on 9.9.1 too the primary still didn't serve, until it was restarted again. The replica already held that [dhcp], so the refusal protected nothing. Test: TestReview638StartServesWhenThePrimaryIsUpdatedFirst.

   Fix for 1 and 2 (D63): Apply refuses only what this node can't serve whatever the others are (an unpinned range, checkServable); the member and version checks stay in Check, which the edit path already runs through checkDHCP. In suggested-fix.diff.

3. Medium: a lease a replica granted can't be removed anywhere. The primary answers 404 192.0.2.126 has no lease on this node for a lease its own list shows; the replica answers 409 Make control-plane changes on the cluster primary. Fix (D64): ending a lease changes only the node that granted it, so both remove routes are replicaLocal and dhcpService.RemoveLease asks for the permission without refuseOnReplica. Test: TestReview638ReplicaRemovesItsOwnLeases. The primary's lease table should say which node holds a lease it can't remove; the MCP tool goes through the same service.

4. Low: a node rewrites another node's lease row. The engine loads the mirrored rows with its own, and when a client holding a lease on the replica takes one from the primary, releaseOthers ends the replica's row and saves it: the primary's database gets a counter of its own for the replica's leases and holds the row as released under sequence 1, while the replica holds it active under 5. The copy flips back when the replica's highest row is the one rewritten. The design has only the owner writing a row. Test: TestReview638EngineSavesOnlyItsOwnRows. Fix: one condition in releaseOthers (old.NodeID == exchange.engine.node); the name already follows the newer lease.

With the fix applied all five tests pass, the dhcp, app, cluster, web and config tests pass, and the cluster script passes with nothing filed.

Not filed:

• Removing a member that still has ranges should say so; the range is orphaned until it's moved. That fits 8c's takeover.
• With [dhcp] refused at start the status API says "enabled": false, "clustered": false while sable.toml says enabled.
• Members' addresses are stored as ip:port (192.0.2.1:18153), which ownAddresses can't parse with netip.ParseAddr, so a peer's address never counts as this node's. The probe tag covers it today; netip.ParseAddrPort as a fallback would make the second line of D36 real.
• Every DISCOVER holds an offered address on the other node for 60 s. Fine at home; a pool sized to the network (as the docs now say) absorbs it.