integration: reproduce HA primary flap to offline node via docker disconnect

Add a docker-network-disconnect primitive — `DisconnectFromNetwork` / `ReconnectToNetwork` on `TailscaleClient`, backed by `pool.Client.DisconnectNetwork` — so an integration test can faithfully simulate the cable-pull scenario the issue reporter observed in Proxmox. Iptables-based simulations leave the socket open at the container kernel and miss the bug. Use it in `TestHASubnetRouterFailoverDockerDisconnect` to assert the no-flap invariant: once primary has failed over from r1 to r2 and r2 also loses connectivity, primary must not switch back to the still-offline r1. The assertion fails on the current branch — the all-unhealthy fallback in `electPrimaryRoutes` flips primary to the lowest-NodeID candidate regardless of online state. Locking the failure in via TDD before designing the fix. Updates #3203
2026-07-12 19:09:14 +09:00 · 2026-04-29 12:25:40 +00:00
parent 2d04fff605
commit 5228818a12
4 changed files with 250 additions and 0 deletions
@@ -91,6 +91,47 @@ func AddContainerToNetwork(
 	return nil
 }

+// DisconnectContainerFromNetwork removes the container from network at
+// the docker daemon level. Mirrors a physical cable pull: the
+// container's network interface for that network disappears and any
+// in-flight TCP connections are left half-open, exactly the failure
+// mode iptables-based simulations cannot reproduce.
+func DisconnectContainerFromNetwork(
+	pool *dockertest.Pool,
+	network *dockertest.Network,
+	testContainer string,
+) error {
+	containers, err := pool.Client.ListContainers(docker.ListContainersOptions{
+		All: true,
+		Filters: map[string][]string{
+			"name": {testContainer},
+		},
+	})
+	if err != nil {
+		return err
+	}
+
+	if len(containers) == 0 {
+		return fmt.Errorf("%w: %s", ErrContainerNotFound, testContainer)
+	}
+
+	return pool.Client.DisconnectNetwork(network.Network.ID, docker.NetworkConnectionOptions{
+		Container: containers[0].ID,
+		Force:     true,
+	})
+}
+
+// ReconnectContainerToNetwork is the inverse of
+// DisconnectContainerFromNetwork — re-attaches the container to the
+// network so traffic can flow again.
+func ReconnectContainerToNetwork(
+	pool *dockertest.Pool,
+	network *dockertest.Network,
+	testContainer string,
+) error {
+	return AddContainerToNetwork(pool, network, testContainer)
+}
+
 // RandomFreeHostPort asks the kernel for a free open port that is ready to use.
 // (from https://github.com/phayes/freeport)
 func RandomFreeHostPort() (int, error) {
@@ -4158,3 +4158,195 @@ func TestHASubnetRouterFailoverBothOfflineCablePull(t *testing.T) {
 		assert.Len(c, result, 13)
 	}, propagationTime, 1*time.Second, "client reaches webservice via r2 after recovery")
 }
+
+// TestHASubnetRouterFailoverDockerDisconnect reproduces the report
+// from issue #3203 by simulating a real cable pull at the docker
+// daemon level. The container's network interface is removed via
+// `docker network disconnect`, which leaves any in-flight TCP
+// connection half-open at the peer — exactly the failure mode the
+// reporter observed when disconnecting a Proxmox interface, and the
+// one that iptables-based simulations cannot reproduce because the
+// container's kernel still owns the socket.
+//
+// The behaviour under test is the **no-flap invariant**: once the
+// primary has failed over to r2 and r2 itself loses connectivity,
+// the primary must NOT flap back to r1, which is also offline.
+// Today the all-unhealthy fallback in electPrimaryRoutes flips back
+// to candidates[0] (lowest NodeID) regardless of online state,
+// producing the user-visible "switches back to offline node 1"
+// pathology.
+//
+// This test is expected to FAIL on the current branch — that is the
+// reproduction. A separate change will fix the algorithm.
+func TestHASubnetRouterFailoverDockerDisconnect(t *testing.T) {
+	IntegrationSkip(t)
+
+	propagationTime := integrationutil.ScaledTimeout(120 * time.Second)
+
+	spec := ScenarioSpec{
+		NodesPerUser: 2,
+		Users:        []string{"user1", "user2"},
+		Networks: map[string]NetworkSpec{
+			"usernet1": {Users: []string{"user1"}},
+			"usernet2": {Users: []string{"user2"}},
+		},
+		ExtraService: map[string][]extraServiceFunc{
+			"usernet1": {Webservice},
+		},
+		Versions: []string{"head"},
+	}
+
+	scenario, err := NewScenario(spec)
+	require.NoErrorf(t, err, "failed to create scenario: %s", err)
+
+	err = scenario.CreateHeadscaleEnv(
+		[]tsic.Option{tsic.WithAcceptRoutes()},
+		hsic.WithTestName("rt-hadocker"),
+	)
+	requireNoErrHeadscaleEnv(t, err)
+
+	allClients, err := scenario.ListTailscaleClients()
+	requireNoErrListClients(t, err)
+
+	err = scenario.WaitForTailscaleSync()
+	requireNoErrSync(t, err)
+
+	headscale, err := scenario.Headscale()
+	requireNoErrGetHeadscale(t, err)
+
+	prefp, err := scenario.SubnetOfNetwork("usernet1")
+	require.NoError(t, err)
+
+	pref := *prefp
+
+	usernet1, err := scenario.Network("usernet1")
+	require.NoError(t, err)
+
+	services, err := scenario.Services("usernet1")
+	require.NoError(t, err)
+	require.Len(t, services, 1)
+
+	web := services[0]
+	webip := netip.MustParseAddr(web.GetIPInNetwork(usernet1))
+	weburl := fmt.Sprintf("http://%s/etc/hostname", webip)
+
+	sort.SliceStable(allClients, func(i, j int) bool {
+		return allClients[i].MustStatus().Self.ID < allClients[j].MustStatus().Self.ID
+	})
+
+	subRouter1 := allClients[0]
+	subRouter2 := allClients[1]
+	client := allClients[2]
+
+	for _, r := range []TailscaleClient{subRouter1, subRouter2} {
+		_, _, err = r.Execute([]string{
+			"tailscale", "set", "--advertise-routes=" + pref.String(),
+		})
+		require.NoErrorf(t, err, "advertise route on %s", r.Hostname())
+	}
+
+	err = scenario.WaitForTailscaleSync()
+	requireNoErrSync(t, err)
+
+	var nodes []*v1.Node
+
+	assert.EventuallyWithT(t, func(c *assert.CollectT) {
+		nodes, err = headscale.ListNodes()
+		assert.NoError(c, err)
+		assert.Len(c, nodes, 4)
+	}, propagationTime, 200*time.Millisecond, "nodes registered")
+
+	_, err = headscale.ApproveRoutes(
+		MustFindNode(subRouter1.Hostname(), nodes).GetId(),
+		[]netip.Prefix{pref},
+	)
+	require.NoError(t, err)
+
+	_, err = headscale.ApproveRoutes(
+		MustFindNode(subRouter2.Hostname(), nodes).GetId(),
+		[]netip.Prefix{pref},
+	)
+	require.NoError(t, err)
+
+	nodeID1 := types.NodeID(MustFindNode(subRouter1.Hostname(), nodes).GetId())
+	nodeID2 := types.NodeID(MustFindNode(subRouter2.Hostname(), nodes).GetId())
+
+	// Sanity: r1 (lowest NodeID) starts as primary.
+	assert.EventuallyWithT(t, func(c *assert.CollectT) {
+		pr, err := headscale.PrimaryRoutes()
+		assert.NoError(c, err)
+		assert.Equal(c, map[string]types.NodeID{
+			pref.String(): nodeID1,
+		}, pr.PrimaryRoutes, "r1 should start as primary")
+	}, propagationTime, 200*time.Millisecond, "HA setup")
+
+	// Verify client traffic reaches the webservice via r1 before any
+	// failover so the post-disconnect state is comparable.
+	assert.EventuallyWithT(t, func(c *assert.CollectT) {
+		result, err := client.Curl(weburl)
+		assert.NoError(c, err)
+		assert.Len(c, result, 13)
+	}, propagationTime, 1*time.Second, "client reaches webservice via r1 before disconnects")
+
+	t.Log("=== Cable-pull r1 via docker network disconnect. ===")
+	require.NoError(t, subRouter1.DisconnectFromNetwork(usernet1),
+		"docker disconnect r1 from usernet1")
+
+	// Eventually r2 should take over as primary. With ProbeInterval
+	// 10s + ProbeTimeout 5s the worst-case detection is ~15s, plus
+	// the 10s poll-grace before state.Disconnect runs.
+	assert.EventuallyWithT(t, func(c *assert.CollectT) {
+		pr, err := headscale.PrimaryRoutes()
+		assert.NoError(c, err)
+		assert.Equal(c, map[string]types.NodeID{
+			pref.String(): nodeID2,
+		}, pr.PrimaryRoutes, "r2 should become primary after r1 docker disconnect")
+	}, propagationTime, 1*time.Second, "waiting for r2 promotion")
+
+	t.Log("=== Cable-pull r2 while r1 is still disconnected. ===")
+	require.NoError(t, subRouter2.DisconnectFromNetwork(usernet1),
+		"docker disconnect r2 from usernet1")
+
+	// No-flap assertion (the bug under test).
+	//
+	// Both r1 and r2 are now offline at the network layer. r2 was
+	// the standing primary. The HA prober will mark r2 unhealthy
+	// within ~15s; once it does, the all-unhealthy fallback in the
+	// algorithm flips primary to candidates[0] (r1, lowest NodeID).
+	//
+	// Per the user's report this is wrong — the primary should not
+	// jump to a node that is itself offline. Assert primary stays r2
+	// across a window long enough to cover the prober tick and the
+	// fallback decision.
+	flapWindow := integrationutil.ScaledTimeout(40 * time.Second)
+	require.Never(t, func() bool {
+		pr, err := headscale.PrimaryRoutes()
+		if err != nil {
+			return false
+		}
+
+		owner, ok := pr.PrimaryRoutes[pref.String()]
+
+		return ok && owner == nodeID1
+	}, flapWindow, 1*time.Second,
+		"primary must not flap to offline r1 (issue #3203 reporter follow-up)")
+
+	t.Log("=== Reconnect r2 to docker network. ===")
+	require.NoError(t, subRouter2.ReconnectToNetwork(usernet1),
+		"docker reconnect r2 to usernet1")
+
+	assert.EventuallyWithT(t, func(c *assert.CollectT) {
+		pr, err := headscale.PrimaryRoutes()
+		assert.NoError(c, err)
+		assert.Equal(c, map[string]types.NodeID{
+			pref.String(): nodeID2,
+		}, pr.PrimaryRoutes,
+			"r2 should be primary again after reconnect — issue #3203")
+	}, propagationTime, 1*time.Second, "waiting for r2 to be primary again")
+
+	assert.EventuallyWithT(t, func(c *assert.CollectT) {
+		result, err := client.Curl(weburl)
+		assert.NoError(c, err)
+		assert.Len(c, result, 13)
+	}, propagationTime, 1*time.Second, "client reaches webservice via r2 after recovery")
+}
@@ -58,6 +58,8 @@ type TailscaleClient interface {
 	ReadFile(path string) ([]byte, error)
 	PacketFilter() ([]filter.Match, error)
 	ConnectToNetwork(network *dockertest.Network) error
+	DisconnectFromNetwork(network *dockertest.Network) error
+	ReconnectToNetwork(network *dockertest.Network) error

 	// FailingPeersAsString returns a formatted-ish multi-line-string of peers in the client
 	// and a bool indicating if the clients online count and peer count is equal.
@@ -807,6 +807,21 @@ func (t *TailscaleInContainer) Down() error {
 	return nil
 }

+// DisconnectFromNetwork detaches the container from network at the
+// docker daemon level. The container's network interface for that
+// network disappears and any in-flight TCP connection is left
+// half-open at the peer — the same failure mode a real cable pull
+// produces, which iptables-based simulations cannot reproduce.
+func (t *TailscaleInContainer) DisconnectFromNetwork(network *dockertest.Network) error {
+	return dockertestutil.DisconnectContainerFromNetwork(t.pool, network, t.hostname)
+}
+
+// ReconnectToNetwork is the inverse of DisconnectFromNetwork: it
+// re-attaches the container to network so traffic can flow again.
+func (t *TailscaleInContainer) ReconnectToNetwork(network *dockertest.Network) error {
+	return dockertestutil.ReconnectContainerToNetwork(t.pool, network, t.hostname)
+}
+
 // IPs returns the netip.Addr of the Tailscale instance.
 func (t *TailscaleInContainer) IPs() ([]netip.Addr, error) {
 	if len(t.ips) != 0 {