From aebf9a0663f0f9d9ba8837c2bb3f7dc9587b733e Mon Sep 17 00:00:00 2001
From: Cory Snider <csnider@mirantis.com>
Date: Mon, 14 Sep 2026 15:28:25 -0400
Subject: [PATCH 1/6] d/libn/networkdb: try the rejoin before waiting on a tick

retryJoin ticks before making its first attempt, so it does nothing at
all unless its context outlives retryInterval. rejoinClusterBootStrap
hands it a context bounded by rejoinClusterDuration, which makes those
two a coupling that nothing states and nothing checks: set the duration
to a second and every rejoin becomes a select between a ticker and a
deadline which expire together, decided at random.

TestNetworkDBIslands asks for exactly that, scaling the 10s default down
by ten to the 1s retryInterval. It is skipped, so nothing has been
noticing, but the trap is there for any caller which wants a short
rejoin budget: it buys nothing.

Attempt the join first and tick afterwards. A budget then buys at least
one attempt however small it is, the two knobs are independent again,
and the wait before a rejoin is one second shorter.

Assisted-by: Claude Opus 5
Signed-off-by: Cory Snider <csnider@mirantis.com>
---
 daemon/libnetwork/networkdb/cluster.go      | 23 ++++++-----
 daemon/libnetwork/networkdb/cluster_test.go | 42 +++++++++++++++++++++
 2 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/daemon/libnetwork/networkdb/cluster.go b/daemon/libnetwork/networkdb/cluster.go
index 43d29f949a..2de39a1e53 100644
--- a/daemon/libnetwork/networkdb/cluster.go
+++ b/daemon/libnetwork/networkdb/cluster.go
@@ -174,22 +174,25 @@ func (nDB *NetworkDB) clusterInit() error {
 	return nil
 }
 
+// retryJoin joins members, retrying every retryInterval until it succeeds or
+// ctx is done. The first attempt is made straight away: waiting out a tick
+// first would make the whole call a no-op whenever ctx expires within
+// retryInterval, and it is the caller who chooses that budget.
 func (nDB *NetworkDB) retryJoin(ctx context.Context, members []string) {
 	t := time.NewTicker(retryInterval)
 	defer t.Stop()
 
-	for {
+	for ctx.Err() == nil {
+		if _, err := nDB.memberlist.Join(members); err != nil {
+			log.G(ctx).Errorf("Failed to join memberlist %s on retry: %v", members, err)
+		} else if err := nDB.sendNodeEvent(NodeEventTypeJoin); err != nil {
+			log.G(ctx).Errorf("failed to send node join on retry: %v", err)
+		} else {
+			return
+		}
+
 		select {
 		case <-t.C:
-			if _, err := nDB.memberlist.Join(members); err != nil {
-				log.G(ctx).Errorf("Failed to join memberlist %s on retry: %v", members, err)
-				continue
-			}
-			if err := nDB.sendNodeEvent(NodeEventTypeJoin); err != nil {
-				log.G(ctx).Errorf("failed to send node join on retry: %v", err)
-				continue
-			}
-			return
 		case <-ctx.Done():
 			return
 		}
diff --git a/daemon/libnetwork/networkdb/cluster_test.go b/daemon/libnetwork/networkdb/cluster_test.go
index 0df489cdcd..46195331ea 100644
--- a/daemon/libnetwork/networkdb/cluster_test.go
+++ b/daemon/libnetwork/networkdb/cluster_test.go
@@ -1,19 +1,24 @@
 package networkdb
 
 import (
+	"context"
 	"iter"
 	"maps"
 	"math"
 	"math/bits"
 	"math/rand/v2"
+	"net"
 	"slices"
+	"strconv"
 	"strings"
 	"testing"
 	"testing/synctest"
 	"time"
 
+	"github.com/moby/moby/v2/daemon/internal/stringid"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
+	"gotest.tools/v3/poll"
 	"pgregory.net/rapid"
 )
 
@@ -258,3 +263,40 @@ func distributionStats(vals iter.Seq[int]) (mean, stdev, minv, maxv float64) {
 	stdev = math.Sqrt(sumSq/float64(n) - mean*mean)
 	return mean, stdev, minv, maxv
 }
+
+// TestRetryJoinAttemptsBeforeTicking pins that retryJoin makes its first
+// attempt straight away. Waiting out a tick first makes the whole call a no-op
+// for any caller whose budget is shorter than retryInterval, and no test which
+// drives a real cluster would notice: they all allow a budget several ticks
+// long.
+func TestRetryJoinAttemptsBeforeTicking(t *testing.T) {
+	conf := DefaultConfig()
+	conf.BindAddr = "127.0.0.1"
+	conf.AdvertiseAddr = conf.BindAddr
+
+	launch := func(name string) *NetworkDB {
+		t.Helper()
+		c := *conf
+		c.Hostname = name
+		c.NodeID = stringid.TruncateID(stringid.GenerateRandomID())
+		c.BindPort = int(dbPort.Add(1))
+		return launchNode(t, c)
+	}
+	// Deliberately not joined to each other: retryJoin is the only thing which
+	// puts them in touch.
+	a, b := launch("node1"), launch("node2")
+	defer closeNetworkDBInstances(t, []*NetworkDB{a, b})
+
+	ctx, cancel := context.WithTimeout(t.Context(), retryInterval/10)
+	defer cancel()
+	a.retryJoin(ctx, []string{net.JoinHostPort(b.config.AdvertiseAddr, strconv.Itoa(b.config.BindPort))})
+
+	poll.WaitOn(t, func(t poll.LogT) poll.Result {
+		a.RLock()
+		defer a.RUnlock()
+		if _, ok := a.nodes[b.config.NodeID]; ok {
+			return poll.Success()
+		}
+		return poll.Continue("waiting for the join made inside the budget to have taken")
+	}, poll.WithDelay(100*time.Millisecond), poll.WithTimeout(30*time.Second))
+}

From 15a2cf140e9d64dbc594981cab27ad5a7062eafb Mon Sep 17 00:00:00 2001
From: Cory Snider <csnider@mirantis.com>
Date: Mon, 14 Sep 2026 15:28:49 -0400
Subject: [PATCH 2/6] d/libn/networkdb: ask for the bootstrap peers when
 rejoining

The bootstrap list was a snapshot taken once, when the agent came up,
and nothing ever refreshed it: agentSetup returns early once the agent
exists, so the later cluster events bounce off it, and Join is the only
writer of the list.

A swarm which replaces a manager at a new address therefore leaves every
node holding an address which will never answer again, and no address
for the manager which replaced it. The one path which could still merge
a split cluster is aimed where the cluster used to be.

Make the list a question asked at rejoin time instead.
Config.BootstrapPeers reports the peers to bootstrap from, and
libnetwork answers it from the cluster provider, which tracks the
manager set as it changes. It is called with no NetworkDB lock held, so
a caller's own locks are never taken beneath ours; libnetwork likewise
releases the controller lock before calling out to the cluster.

What it reports is last known good, which is what matters when the
control plane is the thing that broke: swarmkit keeps the manager list
in memory and on disk, a failed connection only re-weights an entry,
and the one path which drops one runs off a dispatcher session message.
No session, no pruning -- so a node which has lost the control plane
goes on bootstrapping off the managers it last saw, which is the point
of gossiping in the first place. TestRemoteAddrListIsLastKnownGood pins
the half of that which is ours: asked afresh every call, passed on as
reported.

A NetworkDB built without one never bootstraps its way back. Join still
gets it into the cluster, and reconnectNode goes on retrying the peers
it saw fail, so a node which falls out can still return by itself -- but
once reapDeadNode has forgotten those there is nothing left to rejoin
through. That is the whole of what the daemon's old behaviour was worth
-- a snapshot which went stale the moment the managers did -- so there
is no reason to keep it as a mode, and cmd/networkdb-test drives its
joins explicitly and never waited on a rejoin anyway.

It also removes a cost of the change which follows this one. Once every
bootstrap address is weighed on its own, an address which has gone for
good would otherwise sit on a stale snapshot and be retried every rejoin
interval -- which a node latched onto one of its other bootstrap nodes
never used to do. Asking the cluster drops such an address as soon as
the cluster does.

Assisted-by: Claude Opus 5
Signed-off-by: Cory Snider <csnider@mirantis.com>
---
 daemon/libnetwork/agent.go                    |  48 +++++++-
 daemon/libnetwork/agent_test.go               |  69 ++++++++++++
 daemon/libnetwork/networkdb/cluster.go        |  24 ++--
 daemon/libnetwork/networkdb/networkdb.go      |  23 ++--
 daemon/libnetwork/networkdb/networkdb_test.go | 104 ++++++++++++++++++
 5 files changed, 246 insertions(+), 22 deletions(-)

diff --git a/daemon/libnetwork/agent.go b/daemon/libnetwork/agent.go
index d7798f20ae..bf68a7a1a9 100644
--- a/daemon/libnetwork/agent.go
+++ b/daemon/libnetwork/agent.go
@@ -233,12 +233,7 @@ func (c *Controller) agentSetup(clusterProvider cluster.Provider) error {
 	bindAddr := clusterProvider.GetLocalAddress()
 	advAddr := clusterProvider.GetAdvertiseAddress()
 	dataAddr := clusterProvider.GetDataPathAddress()
-	remoteList := clusterProvider.GetRemoteAddressList()
-	remoteAddrList := make([]string, 0, len(remoteList))
-	for _, remote := range remoteList {
-		addr, _, _ := net.SplitHostPort(remote)
-		remoteAddrList = append(remoteAddrList, addr)
-	}
+	remoteAddrList := remoteAddrs(clusterProvider)
 
 	listen := clusterProvider.GetListenAddress()
 	listenAddr, _, _ := net.SplitHostPort(listen)
@@ -330,6 +325,10 @@ func (c *Controller) agentInit(listenAddr, bindAddrOrInterface, advertiseAddr, d
 	netDBConf.BindAddr = listenAddr
 	netDBConf.AdvertiseAddr = advertiseAddr
 	netDBConf.Keys = keys
+	// Answered afresh every time a rejoin is considered, so that a node which
+	// has to bootstrap its way back into the gossip cluster aims at the
+	// managers the cluster has now rather than the ones it had at startup.
+	netDBConf.BootstrapPeers = c.remoteAddrList
 	if c.Config().NetworkControlPlaneMTU != 0 {
 		// Consider the MTU remove the IP hdr (IPv4 or IPv6) and the TCP/UDP hdr.
 		// To be on the safe side let's cut 100 bytes
@@ -383,6 +382,43 @@ func (c *Controller) agentInit(listenAddr, bindAddrOrInterface, advertiseAddr, d
 	return nil
 }
 
+// remoteAddrList is [remoteAddrs] for whichever cluster provider is current,
+// so that a NetworkDB holding on to it as a callback asks the cluster the node
+// is in now rather than the one it was in when the agent came up.
+//
+// It reports nil once the node has left the cluster and the provider has gone,
+// which NetworkDB reads as "no bootstrap nodes known" and skips its rejoin.
+func (c *Controller) remoteAddrList() []string {
+	c.mu.Lock()
+	cp := c.cfg.ClusterProvider
+	c.mu.Unlock()
+	if cp == nil {
+		return nil
+	}
+	// Deliberately outside the lock above: this calls out into the cluster,
+	// which takes locks of its own.
+	return remoteAddrs(cp)
+}
+
+// remoteAddrs is the addresses cp reports for the cluster's other managers, as
+// NetworkDB wants them: bare IPs, with the port the cluster provider reports
+// dropped, because a peer is assumed to gossip on the same port we do.
+func remoteAddrs(cp cluster.Provider) []string {
+	remoteList := cp.GetRemoteAddressList()
+	remoteAddrList := make([]string, 0, len(remoteList))
+	for _, remote := range remoteList {
+		addr, _, err := net.SplitHostPort(remote)
+		if err != nil {
+			// Nothing to drop: the provider named an address without a port.
+			// Pass it through rather than the empty string SplitHostPort
+			// leaves behind, which is not an address at all.
+			addr = remote
+		}
+		remoteAddrList = append(remoteAddrList, addr)
+	}
+	return remoteAddrList
+}
+
 func (c *Controller) agentJoin(remoteAddrList []string) error {
 	agent := c.getAgent()
 	if agent == nil {
diff --git a/daemon/libnetwork/agent_test.go b/daemon/libnetwork/agent_test.go
index 8015ee5075..4528c699f1 100644
--- a/daemon/libnetwork/agent_test.go
+++ b/daemon/libnetwork/agent_test.go
@@ -5,11 +5,15 @@ import (
 	"net"
 	"net/netip"
 	"slices"
+	"sync"
 	"testing"
 
 	"github.com/gogo/protobuf/proto"
 	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 
+	"github.com/moby/moby/v2/daemon/libnetwork/cluster"
+	"github.com/moby/moby/v2/daemon/libnetwork/config"
 	"github.com/moby/moby/v2/daemon/libnetwork/networkdb"
 )
 
@@ -414,3 +418,68 @@ func TestHandleEPTableEvent(t *testing.T) {
 		})
 	}
 }
+
+// clusterProviderStub answers GetRemoteAddressList and nothing else: every
+// other method of the interface is left to the nil embedded value, so a test
+// which reaches one panics rather than quietly passing.
+type clusterProviderStub struct {
+	cluster.Provider
+	mu      sync.Mutex
+	remotes []string
+	calls   int
+}
+
+func (p *clusterProviderStub) GetRemoteAddressList() []string {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	p.calls++
+	return slices.Clone(p.remotes)
+}
+
+func TestRemoteAddrs(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		reported []string
+		want     []string
+	}{
+		{name: "none", reported: nil, want: []string{}},
+		{name: "host and port", reported: []string{"10.0.0.1:2377"}, want: []string{"10.0.0.1"}},
+		// The daemon's own providers report these, and SplitHostPort leaves
+		// the empty string behind for them, which is not an address at all.
+		{name: "no port", reported: []string{"10.0.0.1"}, want: []string{"10.0.0.1"}},
+		{name: "ipv6", reported: []string{"[fd00::1]:2377"}, want: []string{"fd00::1"}},
+		{name: "mixed", reported: []string{"10.0.0.1:2377", "10.0.0.2"}, want: []string{"10.0.0.1", "10.0.0.2"}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Check(t, is.DeepEqual(remoteAddrs(&clusterProviderStub{remotes: tc.reported}), tc.want))
+		})
+	}
+}
+
+// TestRemoteAddrListIsLastKnownGood pins the half of the bootstrap-peer
+// contract which is libnetwork's: the list is asked for afresh on every call
+// and passed on as the provider reported it, with nothing cached, dropped or
+// second-guessed here. A node which has lost the Swarm control plane therefore
+// keeps bootstrapping off the managers it last saw, which is what lets gossip
+// carry on without one.
+//
+// Swarmkit holds up the other half: its manager list lives in memory and on
+// disk, a failed connection only re-weights an entry, and the one path which
+// removes one runs off a dispatcher session message -- so an outage freezes
+// the list rather than draining it.
+func TestRemoteAddrListIsLastKnownGood(t *testing.T) {
+	cp := &clusterProviderStub{remotes: []string{"10.0.0.1:2377", "10.0.0.2:2377"}}
+	c := &Controller{cfg: &config.Config{ClusterProvider: cp}}
+
+	want := []string{"10.0.0.1", "10.0.0.2"}
+	for i := range 3 {
+		assert.Check(t, is.DeepEqual(c.remoteAddrList(), want), "call %d", i)
+	}
+	assert.Check(t, is.Equal(cp.calls, 3), "the peers must be asked for every time, not snapshotted")
+
+	// Once the node leaves the cluster the provider goes with it, and NetworkDB
+	// is told there is nothing to bootstrap from rather than being handed a
+	// list which can no longer be answered for.
+	c.cfg.ClusterProvider = nil
+	assert.Check(t, is.Nil(c.remoteAddrList()))
+}
diff --git a/daemon/libnetwork/networkdb/cluster.go b/daemon/libnetwork/networkdb/cluster.go
index 2de39a1e53..ec3558e086 100644
--- a/daemon/libnetwork/networkdb/cluster.go
+++ b/daemon/libnetwork/networkdb/cluster.go
@@ -203,9 +203,12 @@ func (nDB *NetworkDB) clusterJoin(members []string) error {
 	mlist := nDB.memberlist
 
 	if _, err := mlist.Join(members); err != nil {
-		// In case of failure, we no longer need to explicitly call retryJoin.
-		// rejoinClusterBootStrap, which runs every nDB.config.rejoinClusterInterval,
-		// will retryJoin for nDB.config.rejoinClusterDuration.
+		// No explicit retryJoin here: rejoinClusterBootStrap runs every
+		// nDB.config.rejoinClusterInterval and will retryJoin for
+		// nDB.config.rejoinClusterDuration. Note that it works from
+		// Config.BootstrapPeers, not from members, so it only picks this
+		// join up again if the caller also names these addresses there --
+		// and a NetworkDB configured without BootstrapPeers never retries.
 		return fmt.Errorf("could not join node to memberlist: %v", err)
 	}
 
@@ -280,20 +283,25 @@ func (nDB *NetworkDB) reapDeadNode() {
 // if not, call the cluster join to merge 2 separate clusters that are formed when all managers
 // stopped/started at the same time
 func (nDB *NetworkDB) rejoinClusterBootStrap() {
-	nDB.RLock()
-	if len(nDB.bootStrapIP) == 0 {
-		nDB.RUnlock()
+	// Ask who the bootstrap nodes are before taking the lock: the answer comes
+	// from the caller, whose own locks must not be taken beneath ours.
+	if nDB.config.BootstrapPeers == nil {
+		return
+	}
+	peers := nDB.config.BootstrapPeers()
+	if len(peers) == 0 {
 		return
 	}
 
+	nDB.RLock()
 	myself, ok := nDB.nodes[nDB.config.NodeID]
 	if !ok {
 		nDB.RUnlock()
 		log.G(context.TODO()).Warnf("rejoinClusterBootstrap unable to find local node info using ID:%v", nDB.config.NodeID)
 		return
 	}
-	bootStrapIPs := make([]string, 0, len(nDB.bootStrapIP))
-	for _, bootIP := range nDB.bootStrapIP {
+	bootStrapIPs := make([]string, 0, len(peers))
+	for _, bootIP := range peers {
 		// bootstrap IPs are usually IP:port from the Join
 		bootstrapIP, err := netip.ParseAddrPort(bootIP)
 		if err != nil {
diff --git a/daemon/libnetwork/networkdb/networkdb.go b/daemon/libnetwork/networkdb/networkdb.go
index 7b1c54c4e9..7379e6082f 100644
--- a/daemon/libnetwork/networkdb/networkdb.go
+++ b/daemon/libnetwork/networkdb/networkdb.go
@@ -107,10 +107,6 @@ type NetworkDB struct {
 	// Reference to the memberlist's keyring to add & remove keys
 	keyring *memberlist.Keyring
 
-	// bootStrapIP is the list of IPs that can be used to bootstrap
-	// the gossip.
-	bootStrapIP []string
-
 	// lastStatsTimestamp is the last timestamp when the stats got printed
 	lastStatsTimestamp time.Time
 
@@ -284,6 +280,21 @@ type Config struct {
 	// Only set by tests.
 	rngSeed *[32]byte
 
+	// BootstrapPeers reports the addresses this node should try to rejoin the
+	// cluster through, or nil if none are known. It is consulted every time a
+	// rejoin is considered, so a caller which can name the current peers keeps
+	// this node from bootstrapping off addresses which have since gone away.
+	//
+	// It is called from the rejoin timer with no NetworkDB lock held, so it
+	// must be safe for concurrent use, and it must not block for long.
+	//
+	// When nil this node never bootstraps its way back. Join still gets it
+	// into the cluster, and reconnectNode goes on retrying the peers it saw
+	// fail, so a node which falls out can still return by itself -- but once
+	// reapDeadNode has forgotten those peers there is nothing left to rejoin
+	// through.
+	BootstrapPeers func() []string
+
 	// StatsPrintPeriod the period to use to print queue stats
 	// Default is 5min
 	StatsPrintPeriod time.Duration
@@ -383,10 +394,6 @@ func newNetworkDB(c *Config) *NetworkDB {
 // Join joins this NetworkDB instance with a list of peer NetworkDB
 // instances passed by the caller in the form of addr:port
 func (nDB *NetworkDB) Join(members []string) error {
-	nDB.Lock()
-	nDB.bootStrapIP = append([]string(nil), members...)
-	log.G(context.TODO()).Infof("The new bootstrap node list is:%v", nDB.bootStrapIP)
-	nDB.Unlock()
 	return nDB.clusterJoin(members)
 }
 
diff --git a/daemon/libnetwork/networkdb/networkdb_test.go b/daemon/libnetwork/networkdb/networkdb_test.go
index ae77d8c116..274039518c 100644
--- a/daemon/libnetwork/networkdb/networkdb_test.go
+++ b/daemon/libnetwork/networkdb/networkdb_test.go
@@ -7,6 +7,7 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"sync"
 	"sync/atomic"
 	"testing"
 	"text/tabwriter"
@@ -91,6 +92,44 @@ func closeNetworkDBInstances(t TestingT, dbs []*NetworkDB) {
 	}
 }
 
+// addrOf is db's gossip address, in the ip:port form a bootstrap list takes.
+func addrOf(db *NetworkDB) string {
+	return net.JoinHostPort(db.config.AdvertiseAddr, strconv.Itoa(db.config.BindPort))
+}
+
+// peerList answers [Config.BootstrapPeers] with something a test can change as
+// it goes, so a node is only told where to rejoin once the test wants it to
+// try. Its zero value answers "no bootstrap nodes known".
+type peerList struct {
+	mu    sync.Mutex
+	addrs []string
+}
+
+// get is the [Config.BootstrapPeers] callback. It is called from the rejoin
+// timer, so it copies rather than handing out the slice set below.
+func (p *peerList) get() []string {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	return append([]string(nil), p.addrs...)
+}
+
+func (p *peerList) set(addrs ...string) {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	p.addrs = addrs
+}
+
+// waitFor polls cond until it holds, failing the test if it never does.
+func waitFor(t TestingT, what string, cond func() bool) {
+	t.Helper()
+	poll.WaitOn(t, func(t poll.LogT) poll.Result {
+		if cond() {
+			return poll.Success()
+		}
+		return poll.Continue("waiting for %s", what)
+	}, poll.WithDelay(100*time.Millisecond), poll.WithTimeout(60*time.Second))
+}
+
 func (nDB *NetworkDB) verifyNodeExistence(t *testing.T, node string, present bool) {
 	t.Helper()
 	for range 80 {
@@ -1087,3 +1126,68 @@ func TestNetworkDBIslands(t *testing.T) {
 	poll.WaitOn(t, check, poll.WithDelay(time.Second), poll.WithTimeout(pollTimeout()))
 	closeNetworkDBInstances(t, dbs)
 }
+
+// TestRejoinClusterBootStrapUsesCurrentPeers checks that a rejoin targets the
+// peers the caller names now, not the ones it named when the node started.
+//
+// The bootstrap list was a snapshot taken once, at agent init, and nothing
+// refreshed it. A swarm which replaces a manager at a new address therefore
+// left every node holding an address which would never answer again, and no
+// address for the manager which replaced it -- so the one path which can still
+// merge a split cluster was aimed where the cluster used to be.
+//
+// Both bootstrap nodes are put beyond reach here and a replacement brought up
+// somewhere the cluster has never seen, so gossip cannot carry the news: naming
+// it in the live list is the only way the survivor can find it.
+func TestRejoinClusterBootStrapUsesCurrentPeers(t *testing.T) {
+	var peers peerList
+
+	conf := DefaultConfig()
+	// Shortened so the test is not sitting through the default minute-long
+	// rejoin interval.
+	conf.rejoinClusterInterval = 2 * time.Second
+	conf.rejoinClusterDuration = 3 * time.Second
+	conf.BootstrapPeers = peers.get
+
+	dbs := createNetworkDBInstances(t, 3, "node", conf)
+	survivor := dbs[2]
+
+	// Closed nodes are not put back here, so the teardown tracks what is still
+	// running rather than closing the whole slice: NetworkDB.Close is not
+	// idempotent, and a second call panics inside memberlist.Leave.
+	live := append([]*NetworkDB(nil), dbs...)
+	defer func() { closeNetworkDBInstances(t, live) }()
+
+	peers.set(addrOf(dbs[0]), addrOf(dbs[1]))
+
+	dbs[0].Close()
+	dbs[1].Close()
+	live = []*NetworkDB{survivor}
+	waitFor(t, "the survivor to be left on its own", func() bool {
+		survivor.RLock()
+		defer survivor.RUnlock()
+		return len(survivor.nodes) == 1
+	})
+
+	// The replacement has no bootstrap list of its own -- no callback, and it
+	// joins nothing -- so it cannot reach out. Only the survivor can close the
+	// gap, and only if it is told where to look.
+	plain := DefaultConfig()
+	plain.rejoinClusterInterval = conf.rejoinClusterInterval
+	plain.rejoinClusterDuration = conf.rejoinClusterDuration
+	plain.Hostname = "replacement"
+	plain.NodeID = stringid.TruncateID(stringid.GenerateRandomID())
+	plain.BindPort = int(dbPort.Add(1))
+	plain.BindAddr = "127.0.0.1"
+	plain.AdvertiseAddr = plain.BindAddr
+	replacement := launchNode(t, *plain)
+	live = append(live, replacement)
+
+	peers.set(addrOf(replacement))
+	waitFor(t, "the survivor to rejoin through the replacement", func() bool {
+		survivor.RLock()
+		defer survivor.RUnlock()
+		_, ok := survivor.nodes[replacement.config.NodeID]
+		return ok
+	})
+}

From a2719f21ee0d3c6d4b6d3b6b1f128a13c87ebbe9 Mon Sep 17 00:00:00 2001
From: Cory Snider <csnider@mirantis.com>
Date: Mon, 14 Sep 2026 16:06:41 -0400
Subject: [PATCH 3/6] d/libn/networkdb: rejoin each bootstrap node
 independently

rejoinClusterBootStrap abandoned the entire bootstrap list as soon as
any address in it was found in the cluster. That is the wrong question
to ask: being attached to one bootstrap node says nothing about the
others. memberlist.Join() compounds it, reporting an error only when
every address it was given fails, so even an attempted rejoin can leave
a node attached to part of its list and satisfying the check from then
on.

A partition which outlives node reaping is where that turns permanent.
Split a cluster so that a bootstrap node is left on each side and
neither half can heal itself: reconnectNode retries the peers it saw
fail, but reapDeadNode forgets them after nodeReapInterval, and this
function is then all that is left to merge the halves. Latched on the
bootstrap node it can still see, each side never tries the ones it
cannot, and they stay split after the network is healthy again -- until
some node restarts and joins both.

Weigh each address on its own instead, and rejoin the ones which are
not currently peers. The existing empty-list guard already covers the
case where they are all present.

This is one of the two things wrong with the long-standing
TestNetworkDBIslands flake: that test relaunches three bootstrap nodes
at their old addresses, and failed whenever a rejoin happened to land
between two of the relaunches, which is why it failed a few percent of
the time rather than always. The other -- a departed incarnation left
in the books until a reap a day later -- went with the node-state work
this is stacked on.

It stays skipped all the same. It needs bootstrap peers of its own now
that Join no longer records any, and gets them here, but it is a poor
guard for the fault: with the fault left in, it passed 40 runs out of
40. Its FIXME now says so.

TestRejoinClusterBootStrapStragglers is what holds the line instead. It
reproduces the fault directly, by bringing the two bootstrap nodes back
one at a time rather than racing them, and so fails 10 times out of 10
without this change and passes 100 out of 100 with it.

Assisted-by: Claude Opus 5
Signed-off-by: Cory Snider <csnider@mirantis.com>
---
 daemon/libnetwork/networkdb/cluster.go        |  57 +++++---
 daemon/libnetwork/networkdb/networkdb_test.go | 136 +++++++++++++++++-
 2 files changed, 170 insertions(+), 23 deletions(-)

diff --git a/daemon/libnetwork/networkdb/cluster.go b/daemon/libnetwork/networkdb/cluster.go
index ec3558e086..c7bf5e00fb 100644
--- a/daemon/libnetwork/networkdb/cluster.go
+++ b/daemon/libnetwork/networkdb/cluster.go
@@ -282,6 +282,20 @@ func (nDB *NetworkDB) reapDeadNode() {
 // rejoinClusterBootStrap is called periodically to check if all bootStrap nodes are active in the cluster,
 // if not, call the cluster join to merge 2 separate clusters that are formed when all managers
 // stopped/started at the same time
+//
+// Each bootstrap address is considered on its own: being attached to one of
+// them says nothing about the others, and memberlist.Join reports an error only
+// when every address it was given fails, so a rejoin which reached part of the
+// list still returns success. Only the addresses which are not already peers
+// are worth rejoining, and they stay worth rejoining for as long as they are
+// absent.
+//
+// That matters most for a partition which outlives node reaping. reconnectNode
+// retries the peers it saw fail, but reapDeadNode forgets them after
+// nodeReapInterval, leaving this function as the only thing which can still
+// merge the two halves. With a bootstrap node left on each side, each half is
+// attached to one bootstrap address throughout, so it has to keep trying the
+// ones it cannot see or the split outlives the network fault which caused it.
 func (nDB *NetworkDB) rejoinClusterBootStrap() {
 	// Ask who the bootstrap nodes are before taking the lock: the answer comes
 	// from the caller, whose own locks must not be taken beneath ours.
@@ -293,13 +307,22 @@ func (nDB *NetworkDB) rejoinClusterBootStrap() {
 		return
 	}
 
+	// Ourselves included: a bootstrap address which is our own is one we have
+	// no way to rejoin and no reason to, so it belongs in here rather than
+	// being skipped over and then treated as an absent peer.
 	nDB.RLock()
-	myself, ok := nDB.nodes[nDB.config.NodeID]
-	if !ok {
+	if _, ok := nDB.nodes[nDB.config.NodeID]; !ok {
 		nDB.RUnlock()
 		log.G(context.TODO()).Warnf("rejoinClusterBootstrap unable to find local node info using ID:%v", nDB.config.NodeID)
 		return
 	}
+	joined := make(map[netip.AddrPort]struct{}, len(nDB.nodes))
+	for _, node := range nDB.nodes {
+		nodeIP, _ := netip.AddrFromSlice(node.Addr)
+		joined[netip.AddrPortFrom(nodeIP.Unmap(), node.Port)] = struct{}{}
+	}
+	nDB.RUnlock()
+
 	bootStrapIPs := make([]string, 0, len(peers))
 	for _, bootIP := range peers {
 		// bootstrap IPs are usually IP:port from the Join
@@ -312,28 +335,24 @@ func (nDB *NetworkDB) rejoinClusterBootStrap() {
 				bootstrapIP = netip.AddrPortFrom(addr, uint16(nDB.config.BindPort))
 			}
 		}
-		if bootstrapIP.IsValid() {
-			for _, node := range nDB.nodes {
-				if node == myself {
-					continue
-				}
-				nodeIP, _ := netip.AddrFromSlice(node.Addr)
-				if bootstrapIP == netip.AddrPortFrom(nodeIP.Unmap(), node.Port) {
-					// One of the bootstrap nodes (and not myself) is part of the cluster, return
-					nDB.RUnlock()
-					return
-				}
-			}
-			bootStrapIPs = append(bootStrapIPs, bootIP)
+		if !bootstrapIP.IsValid() {
+			continue
 		}
+		if _, ok := joined[bootstrapIP]; ok {
+			// This bootstrap node is already part of the cluster, so it
+			// needs no rejoining. The others still might.
+			continue
+		}
+		bootStrapIPs = append(bootStrapIPs, bootIP)
 	}
-	nDB.RUnlock()
 	if len(bootStrapIPs) == 0 {
-		// this will also avoid to call the Join with an empty list erasing the current bootstrap ip list
-		log.G(context.TODO()).Debug("rejoinClusterBootStrap did not find any valid IP")
+		// Every bootstrap node is already a peer, or none of them parsed.
+		// memberlist.Join reports success on an empty list, so without this
+		// retryJoin would announce a join it never made.
+		log.G(context.TODO()).Debug("rejoinClusterBootStrap has no bootstrap node to rejoin")
 		return
 	}
-	// None of the bootStrap nodes are in the cluster, call memberlist join
+	// Some of the bootStrap nodes are not in the cluster, call memberlist join
 	log.G(context.TODO()).Debugf("rejoinClusterBootStrap, calling cluster join with bootStrap %v", bootStrapIPs)
 	ctx, cancel := context.WithTimeout(nDB.ctx, nDB.config.rejoinClusterDuration)
 	defer cancel()
diff --git a/daemon/libnetwork/networkdb/networkdb_test.go b/daemon/libnetwork/networkdb/networkdb_test.go
index 274039518c..859f8eeb33 100644
--- a/daemon/libnetwork/networkdb/networkdb_test.go
+++ b/daemon/libnetwork/networkdb/networkdb_test.go
@@ -1019,6 +1019,12 @@ 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 {
@@ -1033,23 +1039,30 @@ func TestNetworkDBIslands(t *testing.T) {
 		return defaultTimeout
 	}
 
+	var peers peerList
+
 	_ = log.SetLevel(log.DebugLevel)
 	conf := DefaultConfig()
 	// Shorten durations to speed up test execution.
 	conf.rejoinClusterDuration = conf.rejoinClusterDuration / 10
 	conf.rejoinClusterInterval = conf.rejoinClusterInterval / 10
+	// Answered empty until the bootstrap nodes are gone, so that nothing tries
+	// to rejoin anything while the cluster is still whole.
+	conf.BootstrapPeers = peers.get
 	dbs := createNetworkDBInstances(t, 5, "node", conf)
 
-	// Get the node IP used currently
-	node := dbs[0].nodes[dbs[0].config.NodeID]
-	baseIPStr := node.Addr.String()
+	// Get the node IP used currently. The memberlist event delegate writes
+	// this map from its own goroutines while the cluster gossips, so it is
+	// not ours to read unlocked.
+	dbs[0].RLock()
+	baseIPStr := dbs[0].nodes[dbs[0].config.NodeID].Addr.String()
+	dbs[0].RUnlock()
 	// Node 0,1,2 are going to be the 3 bootstrap nodes
 	members := []string{
 		fmt.Sprintf("%s:%d", baseIPStr, dbs[0].config.BindPort),
 		fmt.Sprintf("%s:%d", baseIPStr, dbs[1].config.BindPort),
 		fmt.Sprintf("%s:%d", baseIPStr, dbs[2].config.BindPort),
 	}
-	// Rejoining will update the list of the bootstrap members
 	for i := 3; i < 5; i++ {
 		t.Logf("Re-joining: %d", i)
 		assert.Check(t, dbs[i].Join(members))
@@ -1095,11 +1108,20 @@ func TestNetworkDBIslands(t *testing.T) {
 	}
 	poll.WaitOn(t, check, poll.WithDelay(time.Second), poll.WithTimeout(pollTimeout()))
 
+	// Only nodes 3 and 4 are left, and only they know where the bootstrap
+	// nodes live, so naming them now makes those two the only thing which can
+	// bring the cluster back together.
+	peers.set(members...)
+
 	// Spawn again the first 3 nodes with different names but same IP:port
 	for i := range 3 {
 		log.G(t.Context()).Infof("node %d coming back", i)
 		conf := *dbs[i].config
 		conf.NodeID = stringid.TruncateID(stringid.GenerateRandomID())
+		// A returning node is given no bootstrap peers of its own: it does no
+		// join, exactly as the comment below says, so the reconvergence has to
+		// come from the two which stayed.
+		conf.BootstrapPeers = nil
 		dbs[i] = launchNode(t, conf)
 	}
 
@@ -1191,3 +1213,109 @@ func TestRejoinClusterBootStrapUsesCurrentPeers(t *testing.T) {
 		return ok
 	})
 }
+
+// TestRejoinClusterBootStrapStragglers checks that a bootstrap node which is
+// already in the cluster does not suppress the rejoin of the ones which are
+// not.
+//
+// rejoinClusterBootStrap used to abandon the whole bootstrap list as soon as
+// any one address in it was found in the cluster, which is the wrong question
+// to ask: being attached to one bootstrap node says nothing about the others.
+// memberlist.Join compounds it, reporting failure only when every address
+// fails, so even an attempted rejoin can leave a node attached to part of its
+// list and satisfying the check from then on.
+//
+// That turns permanent once a partition outlives node reaping: reconnectNode
+// retries the peers it saw fail, reapDeadNode forgets them after
+// nodeReapInterval, and this function is all that is left to merge the halves
+// -- latched, on both sides, on the bootstrap node each can still see. It is
+// also the root cause of the TestNetworkDBIslands flakes, which hit it whenever
+// a rejoin lands between two of that test's three relaunches.
+//
+// Here the two bootstrap nodes are brought back one at a time, with a wait in
+// between for the survivor to reconnect to the first. That wait is what makes
+// the fault deterministic: reaching it means a rejoin has already run and
+// latched, so before the fix the second node could never be picked up.
+func TestRejoinClusterBootStrapStragglers(t *testing.T) {
+	var peers peerList
+
+	conf := DefaultConfig()
+	// Shortened so the test is not sitting through the default minute-long
+	// rejoin interval twice.
+	conf.rejoinClusterInterval = 2 * time.Second
+	conf.rejoinClusterDuration = 3 * time.Second
+	// Left empty while the cluster forms, so no node rejoins anything before
+	// the departures below give it something to do.
+	conf.BootstrapPeers = peers.get
+
+	dbs := createNetworkDBInstances(t, 3, "node", conf)
+	survivor := dbs[2]
+
+	// The two below are closed and only later replaced, so the teardown tracks
+	// what is still running rather than closing the whole slice: NetworkDB.Close
+	// is not idempotent, and a second call panics inside memberlist.Leave --
+	// which would bury the failure that got us here.
+	live := append([]*NetworkDB(nil), dbs...)
+	defer func() { closeNetworkDBInstances(t, live) }()
+
+	// Both of the others are bootstrap nodes, so that a rejoin has something
+	// to be selective about.
+	peers.set(addrOf(dbs[0]), addrOf(dbs[1]))
+
+	sees := func(target *NetworkDB) bool {
+		survivor.RLock()
+		defer survivor.RUnlock()
+		_, ok := survivor.nodes[target.config.NodeID]
+		return ok
+	}
+
+	departed := []string{dbs[0].config.NodeID, dbs[1].config.NodeID}
+	dbs[0].Close()
+	dbs[1].Close()
+	live = []*NetworkDB{survivor}
+	waitFor(t, "the survivor to be left on its own", func() bool {
+		survivor.RLock()
+		defer survivor.RUnlock()
+		return len(survivor.nodes) == 1
+	})
+
+	// Both left cleanly, so nothing will try to reconnect them on the
+	// survivor's behalf. If memberlist had instead declared them failed,
+	// reconnectNode would pull them back and this test would pass whether or
+	// not the bug is present, so assert the precondition rather than assume
+	// it.
+	survivor.RLock()
+	for _, id := range departed {
+		_, failed := survivor.failedNodes[id]
+		assert.Check(t, !failed, "node %s should have left cleanly, not failed", id)
+	}
+	survivor.RUnlock()
+
+	relaunch := func(i int) *NetworkDB {
+		t.Helper()
+		conf := *dbs[i].config
+		conf.NodeID = stringid.TruncateID(stringid.GenerateRandomID())
+		// The returning node is given no way to reach out, so the survivor
+		// is the only thing that can close the gap.
+		conf.BootstrapPeers = nil
+		dbs[i] = launchNode(t, conf)
+		live = append(live, dbs[i])
+		return dbs[i]
+	}
+
+	// First one back, at the address it had before. Only the survivor can
+	// reattach it: the returning node has no bootstrap peers of its own.
+	//
+	// That one-sided arrangement is the point. It leaves the survivor in the
+	// position every node is in once a partition has outlived reapDeadNode --
+	// the peers it lost are forgotten, reconnectNode has nothing left to
+	// retry, and the bootstrap list is the only way back -- without needing a
+	// day of waiting to get there.
+	first := relaunch(0)
+	waitFor(t, "the survivor to rejoin the first bootstrap node", func() bool { return sees(first) })
+
+	// Second one back, now that a rejoin has definitely run with the first
+	// already in the cluster.
+	second := relaunch(1)
+	waitFor(t, "the survivor to rejoin the second bootstrap node", func() bool { return sees(second) })
+}

From 9294a50aca207b11664753b170679bf5d948b917 Mon Sep 17 00:00:00 2001
From: Cory Snider <csnider@mirantis.com>
Date: Mon, 14 Sep 2026 16:06:41 -0400
Subject: [PATCH 4/6] d/libn/networkdb: bootstrap off the port we gossip on

A bootstrap address which names no port is read as being on
config.BindPort. Nothing sets that in the daemon: agentInit builds the
config from DefaultConfig, which leaves BindPort zero, and clusterInit
only passes it to memberlist when it is non-zero, so memberlist picks
its own default and the node gossips on 7946 while config.BindPort
stays 0.

Portless is exactly what the daemon's bootstrap addresses are.
Controller.remoteAddrList drops the port the cluster provider reports,
because that is the control-plane port and not the one a peer gossips
on. So every bootstrap address the daemon has ever supplied was
compared as ip:0 against peers at ip:7946, and matched nothing.

The membership test in rejoinClusterBootStrap has therefore never done
anything on a real swarm. Back when one recognised address abandoned
the whole list, that meant the early return never fired and every node
called retryJoin against every manager once a rejoin interval --
memberlist.Join push/pulls each address it is given, so that is a full
state exchange with every manager every minute, plus a node-join
broadcast, for a cluster with nothing wrong with it. Now that each
address is weighed on its own, the same addresses are simply never
filtered out.

Read a portless address as being on the port we advertise ourselves,
which is what the surrounding code already says it assumes. Compare the
two unmapped as well, while we are reading addresses: memberlist knows a
peer by whatever form its address arrived in, and the peer side was
already being unmapped before the comparison, so a 4-in-6 bootstrap
address matched nothing and was rejoined every interval -- the same cost
this commit is removing, reached by a different route.

Neither reading is exercised by the tests which drive a real cluster.
They set BindPort explicitly and name their bootstrap nodes as ip:port,
the one form which needs no interpretation at all. So the address
handling moves into bootstrapPeersToRejoin, where
TestBootstrapPeersToRejoin can cover it directly -- no memberlist, and
BindPort left zero the way the daemon leaves it.

Note this is what makes weighing each bootstrap address on its own
load-bearing rather than merely tidy. With the membership test working
for the first time, a node which stayed attached to one bootstrap node
would otherwise have latched onto it and stopped trying the rest.

Assisted-by: Claude Opus 5
Signed-off-by: Cory Snider <csnider@mirantis.com>
---
 daemon/libnetwork/networkdb/cluster.go      | 90 ++++++++++++---------
 daemon/libnetwork/networkdb/cluster_test.go | 51 ++++++++++++
 daemon/libnetwork/networkdb/networkdb.go    |  4 +-
 3 files changed, 106 insertions(+), 39 deletions(-)

diff --git a/daemon/libnetwork/networkdb/cluster.go b/daemon/libnetwork/networkdb/cluster.go
index c7bf5e00fb..040eb4f0ec 100644
--- a/daemon/libnetwork/networkdb/cluster.go
+++ b/daemon/libnetwork/networkdb/cluster.go
@@ -279,6 +279,57 @@ func (nDB *NetworkDB) reapDeadNode() {
 	}
 }
 
+// bootstrapPeersToRejoin is the subset of peers which are not currently in the
+// cluster, and so are worth rejoining. Ourselves is counted as in it: a
+// bootstrap address which is our own is one we have no way to rejoin and no
+// reason to, rather than an absent peer.
+//
+// An address which names no port is read as being on the port we gossip on,
+// and not config.BindPort: that is only a request, left zero by every caller
+// which is happy with memberlist's default. Addresses are compared unmapped,
+// because memberlist knows its peers by whatever form their address arrived
+// in, so a 4-in-6 address has to match the same address written as IPv4.
+func (nDB *NetworkDB) bootstrapPeersToRejoin(peers []string) []string {
+	nDB.RLock()
+	defer nDB.RUnlock()
+
+	myself, ok := nDB.nodes[nDB.config.NodeID]
+	if !ok {
+		log.G(context.TODO()).Warnf("rejoinClusterBootstrap unable to find local node info using ID:%v", nDB.config.NodeID)
+		return nil
+	}
+	gossipPort := myself.Port
+
+	joined := make(map[netip.AddrPort]struct{}, len(nDB.nodes))
+	for _, node := range nDB.nodes {
+		nodeIP, _ := netip.AddrFromSlice(node.Addr)
+		joined[netip.AddrPortFrom(nodeIP.Unmap(), node.Port)] = struct{}{}
+	}
+
+	rejoin := make([]string, 0, len(peers))
+	for _, bootIP := range peers {
+		// bootstrap IPs are usually IP:port from the Join
+		bootstrapIP, err := netip.ParseAddrPort(bootIP)
+		if err != nil {
+			// try to parse it as an IP without port
+			// Note this seems to be the case for swarm that do not specify any port
+			addr, err := netip.ParseAddr(bootIP)
+			if err != nil {
+				continue
+			}
+			// A peer is assumed to gossip on the same port we do.
+			bootstrapIP = netip.AddrPortFrom(addr, gossipPort)
+		}
+		if _, ok := joined[netip.AddrPortFrom(bootstrapIP.Addr().Unmap(), bootstrapIP.Port())]; ok {
+			// This bootstrap node is already part of the cluster, so it
+			// needs no rejoining. The others still might.
+			continue
+		}
+		rejoin = append(rejoin, bootIP)
+	}
+	return rejoin
+}
+
 // rejoinClusterBootStrap is called periodically to check if all bootStrap nodes are active in the cluster,
 // if not, call the cluster join to merge 2 separate clusters that are formed when all managers
 // stopped/started at the same time
@@ -307,44 +358,7 @@ func (nDB *NetworkDB) rejoinClusterBootStrap() {
 		return
 	}
 
-	// Ourselves included: a bootstrap address which is our own is one we have
-	// no way to rejoin and no reason to, so it belongs in here rather than
-	// being skipped over and then treated as an absent peer.
-	nDB.RLock()
-	if _, ok := nDB.nodes[nDB.config.NodeID]; !ok {
-		nDB.RUnlock()
-		log.G(context.TODO()).Warnf("rejoinClusterBootstrap unable to find local node info using ID:%v", nDB.config.NodeID)
-		return
-	}
-	joined := make(map[netip.AddrPort]struct{}, len(nDB.nodes))
-	for _, node := range nDB.nodes {
-		nodeIP, _ := netip.AddrFromSlice(node.Addr)
-		joined[netip.AddrPortFrom(nodeIP.Unmap(), node.Port)] = struct{}{}
-	}
-	nDB.RUnlock()
-
-	bootStrapIPs := make([]string, 0, len(peers))
-	for _, bootIP := range peers {
-		// bootstrap IPs are usually IP:port from the Join
-		bootstrapIP, err := netip.ParseAddrPort(bootIP)
-		if err != nil {
-			// try to parse it as an IP without port
-			// Note this seems to be the case for swarm that do not specify any port
-			addr, err := netip.ParseAddr(bootIP)
-			if err == nil {
-				bootstrapIP = netip.AddrPortFrom(addr, uint16(nDB.config.BindPort))
-			}
-		}
-		if !bootstrapIP.IsValid() {
-			continue
-		}
-		if _, ok := joined[bootstrapIP]; ok {
-			// This bootstrap node is already part of the cluster, so it
-			// needs no rejoining. The others still might.
-			continue
-		}
-		bootStrapIPs = append(bootStrapIPs, bootIP)
-	}
+	bootStrapIPs := nDB.bootstrapPeersToRejoin(peers)
 	if len(bootStrapIPs) == 0 {
 		// Every bootstrap node is already a peer, or none of them parsed.
 		// memberlist.Join reports success on an empty list, so without this
diff --git a/daemon/libnetwork/networkdb/cluster_test.go b/daemon/libnetwork/networkdb/cluster_test.go
index 46195331ea..3e2d291d1e 100644
--- a/daemon/libnetwork/networkdb/cluster_test.go
+++ b/daemon/libnetwork/networkdb/cluster_test.go
@@ -15,6 +15,7 @@ import (
 	"testing/synctest"
 	"time"
 
+	"github.com/hashicorp/memberlist"
 	"github.com/moby/moby/v2/daemon/internal/stringid"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
@@ -264,6 +265,56 @@ func distributionStats(vals iter.Seq[int]) (mean, stdev, minv, maxv float64) {
 	return mean, stdev, minv, maxv
 }
 
+// TestBootstrapPeersToRejoin covers how a bootstrap address is read before it
+// is compared with the cluster's peers. Both readings are load-bearing and
+// neither is exercised by the tests which drive a real cluster: those name
+// their bootstrap nodes as ip:port and set BindPort explicitly, so the address
+// arrives in the one form which needs no interpretation.
+func TestBootstrapPeersToRejoin(t *testing.T) {
+	// A node gossiping on 7946 with config.BindPort left zero, which is what
+	// the daemon does: it never sets BindPort, so memberlist picks its own
+	// default and the port we gossip on is not the one in the config.
+	db := newNetworkDB(DefaultConfig())
+	defer db.broadcaster.Close()
+	assert.Assert(t, is.Equal(db.config.BindPort, 0), "the point of the test is that BindPort is unset")
+
+	db.config.NodeID = "self"
+	// net.ParseIP yields the 4-in-6 form, which is how memberlist holds an
+	// IPv4 peer address.
+	db.nodes["self"] = &node{Node: memberlist.Node{Name: "self", Addr: net.ParseIP("192.0.2.1"), Port: 7946}}
+	db.nodes["peer"] = &node{Node: memberlist.Node{Name: "peer", Addr: net.ParseIP("192.0.2.2"), Port: 7946}}
+
+	for _, tc := range []struct {
+		name  string
+		peers []string
+		want  []string
+	}{
+		{name: "none known", peers: nil, want: []string{}},
+		{name: "peer by ip:port", peers: []string{"192.0.2.2:7946"}, want: []string{}},
+		// Read on the port we gossip on. Read on config.BindPort instead and
+		// this is 192.0.2.2:0, which matches nothing and is rejoined forever.
+		{name: "peer without a port", peers: []string{"192.0.2.2"}, want: []string{}},
+		// The peer side is unmapped before it is compared, so the bootstrap
+		// side has to be too, or these never match.
+		{name: "peer as 4-in-6 with port", peers: []string{"[::ffff:192.0.2.2]:7946"}, want: []string{}},
+		{name: "peer as 4-in-6 without a port", peers: []string{"::ffff:192.0.2.2"}, want: []string{}},
+		{name: "ourselves", peers: []string{"192.0.2.1"}, want: []string{}},
+		{name: "unparseable", peers: []string{"not-an-address"}, want: []string{}},
+		{name: "an absent bootstrap node", peers: []string{"192.0.2.9:7946"}, want: []string{"192.0.2.9:7946"}},
+		{name: "a peer on another port", peers: []string{"192.0.2.2:9999"}, want: []string{"192.0.2.2:9999"}},
+		// Being attached to one bootstrap node says nothing about the others.
+		{
+			name:  "only the ones which are absent",
+			peers: []string{"192.0.2.2", "192.0.2.9:7946", "192.0.2.1"},
+			want:  []string{"192.0.2.9:7946"},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Check(t, is.DeepEqual(db.bootstrapPeersToRejoin(tc.peers), tc.want))
+		})
+	}
+}
+
 // TestRetryJoinAttemptsBeforeTicking pins that retryJoin makes its first
 // attempt straight away. Waiting out a tick first makes the whole call a no-op
 // for any caller whose budget is shorter than retryInterval, and no test which
diff --git a/daemon/libnetwork/networkdb/networkdb.go b/daemon/libnetwork/networkdb/networkdb.go
index 7379e6082f..fcc442551f 100644
--- a/daemon/libnetwork/networkdb/networkdb.go
+++ b/daemon/libnetwork/networkdb/networkdb.go
@@ -210,7 +210,9 @@ type Config struct {
 	AdvertiseAddr string
 
 	// BindPort is the local node's port to which we bind to for
-	// cluster communication.
+	// cluster communication. Zero leaves the choice to memberlist, so it is
+	// not the port this node ends up gossiping on: for that, ask memberlist
+	// what it advertises.
 	BindPort int
 
 	// Keys to be added to the Keyring of the memberlist. Key at index

From eeec2b053172bbfcf17c49bbca72c73bcee042d0 Mon Sep 17 00:00:00 2001
From: Cory Snider <csnider@mirantis.com>
Date: Mon, 14 Sep 2026 19:37:26 -0400
Subject: [PATCH 5/6] 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>
---
 daemon/libnetwork/networkdb/networkdb_test.go | 44 ++++++++++---------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/daemon/libnetwork/networkdb/networkdb_test.go b/daemon/libnetwork/networkdb/networkdb_test.go
index 859f8eeb33..3277f9c5ea 100644
--- a/daemon/libnetwork/networkdb/networkdb_test.go
+++ b/daemon/libnetwork/networkdb/networkdb_test.go
@@ -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()

From c5e676c07b9f3149378eec4510434b71398358af Mon Sep 17 00:00:00 2001
From: Cory Snider <csnider@mirantis.com>
Date: Thu, 17 Sep 2026 19:13:47 -0400
Subject: [PATCH 6/6] d/libn/networkdb: close test clusters even when a test
 fails

Most of these tests close their instances with a plain call on the last
line, so a t.Fatal anywhere above it -- which is how every poll.WaitOn
in the package reports a timeout -- leaves the whole cluster running.
The nodes keep their sockets and their timer goroutines for the rest of
the binary, and the ones which have bootstrap peers go on dialling
addresses that are gone, underneath every test which runs after them.

Defer the close instead. Two of them close instances part-way through
as a step of the test rather than as tidying up, so those track what is
still running: Close is not idempotent, and a second call panics inside
memberlist.Leave rather than returning an error, which would bury the
failure that got us there.

Assisted-by: Claude Opus 5
Signed-off-by: Cory Snider <csnider@mirantis.com>
---
 daemon/libnetwork/networkdb/networkdb_test.go | 38 +++++++++++++------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/daemon/libnetwork/networkdb/networkdb_test.go b/daemon/libnetwork/networkdb/networkdb_test.go
index 3277f9c5ea..7cd513e134 100644
--- a/daemon/libnetwork/networkdb/networkdb_test.go
+++ b/daemon/libnetwork/networkdb/networkdb_test.go
@@ -249,6 +249,7 @@ func TestNetworkDBSimple(t *testing.T) {
 
 func TestNetworkDBJoinLeaveNetwork(t *testing.T) {
 	dbs := createNetworkDBInstances(t, 2, "node", DefaultConfig())
+	defer closeNetworkDBInstances(t, dbs)
 
 	err := dbs[0].JoinNetwork("network1")
 	assert.NilError(t, err)
@@ -259,11 +260,11 @@ func TestNetworkDBJoinLeaveNetwork(t *testing.T) {
 	assert.NilError(t, err)
 
 	dbs[1].verifyNetworkExistence(t, dbs[0].config.NodeID, "network1", false)
-	closeNetworkDBInstances(t, dbs)
 }
 
 func TestNetworkDBJoinLeaveNetworks(t *testing.T) {
 	dbs := createNetworkDBInstances(t, 2, "node", DefaultConfig())
+	defer closeNetworkDBInstances(t, dbs)
 
 	n := 10
 	for i := 1; i <= n; i++ {
@@ -302,11 +303,11 @@ func TestNetworkDBJoinLeaveNetworks(t *testing.T) {
 		dbs[0].verifyNetworkExistence(t, dbs[1].config.NodeID, fmt.Sprintf("network1%d", i), false)
 	}
 
-	closeNetworkDBInstances(t, dbs)
 }
 
 func TestNetworkDBCRUDTableEntry(t *testing.T) {
 	dbs := createNetworkDBInstances(t, 3, "node", DefaultConfig())
+	defer closeNetworkDBInstances(t, dbs)
 
 	err := dbs[0].JoinNetwork("network1")
 	assert.NilError(t, err)
@@ -332,7 +333,6 @@ func TestNetworkDBCRUDTableEntry(t *testing.T) {
 
 	dbs[1].verifyEntryExistence(t, "test_table", "network1", "test_key", "", false)
 
-	closeNetworkDBInstances(t, dbs)
 }
 
 // TestCreateEntryOverTombstone checks that a key can be created again while
@@ -416,6 +416,7 @@ func (nDB *NetworkDB) dumpTable(t *testing.T, tname string) {
 
 func TestNetworkDBCRUDTableEntries(t *testing.T) {
 	dbs := createNetworkDBInstances(t, 2, "node", DefaultConfig())
+	defer closeNetworkDBInstances(t, dbs)
 
 	err := dbs[0].JoinNetwork("network1")
 	assert.NilError(t, err)
@@ -489,11 +490,15 @@ func TestNetworkDBCRUDTableEntries(t *testing.T) {
 		dbs[n].dumpTable(t, "test_table")
 	}
 
-	closeNetworkDBInstances(t, dbs)
 }
 
 func TestNetworkDBNodeLeave(t *testing.T) {
 	dbs := createNetworkDBInstances(t, 2, "node", DefaultConfig())
+	// Closing is what this test does rather than how it tidies up, so the
+	// teardown tracks what is still running: NetworkDB.Close is not
+	// idempotent, and a second call panics inside memberlist.Leave.
+	live := append([]*NetworkDB(nil), dbs...)
+	defer func() { closeNetworkDBInstances(t, live) }()
 
 	err := dbs[0].JoinNetwork("network1")
 	assert.NilError(t, err)
@@ -507,12 +512,15 @@ func TestNetworkDBNodeLeave(t *testing.T) {
 	dbs[1].verifyEntryExistence(t, "test_table", "network1", "test_key", "test_value", true)
 
 	dbs[0].Close()
+	live = []*NetworkDB{dbs[1]}
 	dbs[1].verifyEntryExistence(t, "test_table", "network1", "test_key", "test_value", false)
 	dbs[1].Close()
+	live = nil
 }
 
 func TestNetworkDBWatch(t *testing.T) {
 	dbs := createNetworkDBInstances(t, 2, "node", DefaultConfig())
+	defer closeNetworkDBInstances(t, dbs)
 	err := dbs[0].JoinNetwork("network1")
 	assert.NilError(t, err)
 
@@ -537,11 +545,11 @@ func TestNetworkDBWatch(t *testing.T) {
 	testWatch(t, ch.C, "test_table", "network1", "test_key", "test_updated_value", "")
 
 	cancel()
-	closeNetworkDBInstances(t, dbs)
 }
 
 func TestNetworkDBBulkSync(t *testing.T) {
 	dbs := createNetworkDBInstances(t, 2, "node", DefaultConfig())
+	defer closeNetworkDBInstances(t, dbs)
 
 	err := dbs[0].JoinNetwork("network1")
 	assert.NilError(t, err)
@@ -568,7 +576,6 @@ func TestNetworkDBBulkSync(t *testing.T) {
 		assert.NilError(t, err)
 	}
 
-	closeNetworkDBInstances(t, dbs)
 }
 
 // TestBulkSyncWithFailedPeer checks that a bulk sync is still sent to a peer
@@ -652,6 +659,7 @@ func TestNetworkDBCRUDMediumCluster(t *testing.T) {
 	n := 5
 
 	dbs := createNetworkDBInstances(t, n, "node", DefaultConfig())
+	defer closeNetworkDBInstances(t, dbs)
 
 	// Shake out any data races.
 	done := make(chan struct{})
@@ -717,11 +725,11 @@ func TestNetworkDBCRUDMediumCluster(t *testing.T) {
 		assert.Check(t, is.Contains(err.Error(), "deleted and pending garbage collection"), err)
 	}
 
-	closeNetworkDBInstances(t, dbs)
 }
 
 func TestNetworkDBNodeJoinLeaveIteration(t *testing.T) {
 	dbs := createNetworkDBInstances(t, 2, "node", DefaultConfig())
+	defer closeNetworkDBInstances(t, dbs)
 
 	dbChangeWitness := func(nDB *NetworkDB) func(network string, expectNodeCount int) {
 		staleNetworkTime := nDB.networkClock.Time()
@@ -793,7 +801,6 @@ func TestNetworkDBNodeJoinLeaveIteration(t *testing.T) {
 	dbs[1].verifyNetworkExistence(t, dbs[0].config.NodeID, "network1", true)
 	witness1("network1", 2)
 
-	closeNetworkDBInstances(t, dbs)
 }
 
 func TestNetworkDBGarbageCollection(t *testing.T) {
@@ -803,6 +810,7 @@ func TestNetworkDBGarbageCollection(t *testing.T) {
 	config.StatsPrintPeriod = 15 * time.Second
 
 	dbs := createNetworkDBInstances(t, 3, "node", config)
+	defer closeNetworkDBInstances(t, dbs)
 
 	// 2 Nodes join network
 	err := dbs[0].JoinNetwork("network1")
@@ -857,7 +865,6 @@ func TestNetworkDBGarbageCollection(t *testing.T) {
 		dbs[i].Unlock()
 	}
 
-	closeNetworkDBInstances(t, dbs)
 }
 
 func checkNodeIsForgotten(t *testing.T, db *NetworkDB, nodeName string, msgAndArgs ...any) {
@@ -959,6 +966,7 @@ func TestNodeReincarnation(t *testing.T) {
 
 func TestParallelCreate(t *testing.T) {
 	dbs := createNetworkDBInstances(t, 1, "node", DefaultConfig())
+	defer closeNetworkDBInstances(t, dbs)
 
 	startCh := make(chan int)
 	doneCh := make(chan error)
@@ -983,11 +991,11 @@ func TestParallelCreate(t *testing.T) {
 	// Only 1 write should have succeeded
 	assert.Check(t, is.Equal(uint32(1), success.Load()))
 
-	closeNetworkDBInstances(t, dbs)
 }
 
 func TestParallelDelete(t *testing.T) {
 	dbs := createNetworkDBInstances(t, 1, "node", DefaultConfig())
+	defer closeNetworkDBInstances(t, dbs)
 
 	err := dbs[0].CreateEntry("testTable", "testNetwork", "key", []byte("value"))
 	assert.NilError(t, err)
@@ -1015,7 +1023,6 @@ func TestParallelDelete(t *testing.T) {
 	// Only 1 write should have succeeded
 	assert.Check(t, is.Equal(uint32(1), success.Load()))
 
-	closeNetworkDBInstances(t, dbs)
 }
 
 func TestNetworkDBIslands(t *testing.T) {
@@ -1042,6 +1049,12 @@ func TestNetworkDBIslands(t *testing.T) {
 	// to rejoin anything while the cluster is still whole.
 	conf.BootstrapPeers = peers.get
 	dbs := createNetworkDBInstances(t, 5, "node", conf)
+	// The first three leave part-way through and are relaunched, so the
+	// teardown tracks what is still running rather than closing the whole
+	// slice: NetworkDB.Close is not idempotent, and a second call panics
+	// inside memberlist.Leave.
+	live := append([]*NetworkDB(nil), dbs...)
+	defer func() { closeNetworkDBInstances(t, live) }()
 
 	// Get the node IP used currently. The memberlist event delegate writes
 	// this map from its own goroutines while the cluster gossips, so it is
@@ -1069,6 +1082,7 @@ func TestNetworkDBIslands(t *testing.T) {
 		departed[i] = dbs[i].config.NodeID
 		dbs[i].Close()
 	}
+	live = []*NetworkDB{dbs[3], dbs[4]}
 
 	checkDBs := make(map[string]*NetworkDB)
 	for i := 3; i < 5; i++ {
@@ -1115,6 +1129,7 @@ func TestNetworkDBIslands(t *testing.T) {
 		// come from the two which stayed.
 		conf.BootstrapPeers = nil
 		dbs[i] = launchNode(t, conf)
+		live = append(live, dbs[i])
 	}
 
 	// Every node has to end up seeing all five of the current nodes as
@@ -1148,7 +1163,6 @@ func TestNetworkDBIslands(t *testing.T) {
 		return poll.Success()
 	}
 	poll.WaitOn(t, check, poll.WithDelay(time.Second), poll.WithTimeout(pollTimeout()))
-	closeNetworkDBInstances(t, dbs)
 }
 
 // TestRejoinClusterBootStrapUsesCurrentPeers checks that a rejoin targets the