d/libn/networkdb: unskip TestNetworkDBIslands

The skip can go: the rejoin latch behind the flake is fixed earlier in
this branch, and the departed incarnation which used to linger in the
books went with the work this is stacked on. What is left is a closing
check which counts rather than names -- every node had to hold exactly
five active nodes and no failed ones -- and neither count is an
invariant. A node which starts after another has departed can still be
told of that departed incarnation by gossip already in flight, where it
sits in failedNodes until something supersedes it at its address, so
which nodes carry such a straggler is a matter of timing. Nor does a
count of five ever say that the five are the ones which ought to be
there.

Assert what reconverging means instead: every node sees all five of the
current nodes as active, by ID rather than by count. That is what the
relaunched three cannot bring about for themselves, since they do no
join, and it is indifferent to whatever stragglers a node picked up on
the way. The rejoin fault this test has always been aimed at still
shows, as a node which never appears.

Both checks are clean on this base -- 80 runs of the counting one, 60
of this one -- so the rewrite buys robustness rather than fixing a
failure in front of us.

Assisted-by: Claude Opus 5
Signed-off-by: Cory Snider <csnider@mirantis.com>
This commit is contained in:
Cory Snider committed 2026-09-17 19:39:05 -04:00
1 parent 9294a50aca
commit eeec2b0531
1 file changed
+23 -21
+23 -21
View File
@@ -1019,14 +1019,6 @@ func TestParallelDelete(t *testing.T) {
}
func TestNetworkDBIslands(t *testing.T) {
// FIXME: kept skipped for now. It is a poor guard for the rejoin fault it
// was written for: with that fault present it passed 40 runs out of 40.
// Its closing check also counts node-list entries rather than naming the
// nodes it expects, so it answers a question about timing as much as one
// about convergence. TestRejoinClusterBootStrapStragglers covers the
// fault directly.
t.Skip("FIXME: flaky test; see https://github.com/moby/moby/issues/42459")
pollTimeout := func() time.Duration {
const defaultTimeout = 120 * time.Second
dl, ok := t.Deadline()
@@ -1125,22 +1117,32 @@ func TestNetworkDBIslands(t *testing.T) {
dbs[i] = launchNode(t, conf)
}
// Give some time for the reconnect routine to run, it runs every 6s.
// Every node has to end up seeing all five of the current nodes as
// active, which is what reconverging means and what the relaunched three
// have no way to bring about for themselves: they did no join.
//
// Asserted by identity rather than by counting the node lists. A node
// which starts after another has departed can still be told of that
// departed incarnation by gossip already in flight, and it sits in
// failedNodes until something supersedes it at its address. Which nodes
// carry such a straggler is a matter of timing, so counting them tests
// the schedule rather than the convergence.
current := make([]string, 0, len(dbs))
for _, db := range dbs {
current = append(current, db.config.NodeID)
}
check = func(t poll.LogT) poll.Result {
// Verify that the cluster is again all connected. Note that the 3 previous node did not do any join.
// The two groups converge on the same state now: the survivors have
// nothing left to remember the departed incarnations by, so there is
// no longer an asymmetry between them and the nodes which came back.
for i := range 5 {
db := dbs[i]
for _, db := range dbs {
db.RLock()
nNodes, nFailed := len(db.nodes), len(db.failedNodes)
db.RUnlock()
if nNodes != 5 {
return poll.Continue("%s:Waiting to connect to all nodes", dbs[i].config.Hostname)
var missing []string
for _, id := range current {
if _, ok := db.nodes[id]; !ok {
missing = append(missing, id)
}
}
if nFailed != 0 {
return poll.Continue("%s:Waiting for 0 failedNodes", dbs[i].config.Hostname)
db.RUnlock()
if len(missing) > 0 {
return poll.Continue("%s: waiting to see %v active", db.config.Hostname, missing)
}
}
return poll.Success()