From 5228818a1288f8e0239ba52ddec8872d8918f9d0 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 29 Apr 2026 12:25:40 +0000 Subject: [PATCH] integration: reproduce HA primary flap to offline node via docker disconnect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- integration/dockertestutil/network.go | 41 ++++++ integration/route_test.go | 192 ++++++++++++++++++++++++++ integration/tailscale.go | 2 + integration/tsic/tsic.go | 15 ++ 4 files changed, 250 insertions(+) diff --git a/integration/dockertestutil/network.go b/integration/dockertestutil/network.go index 68d472f4..d224c43c 100644 --- a/integration/dockertestutil/network.go +++ b/integration/dockertestutil/network.go @@ -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) { diff --git a/integration/route_test.go b/integration/route_test.go index bb6c9c44..15fbbf6d 100644 --- a/integration/route_test.go +++ b/integration/route_test.go @@ -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") +} diff --git a/integration/tailscale.go b/integration/tailscale.go index 4c9a761b..9f5ea2f6 100644 --- a/integration/tailscale.go +++ b/integration/tailscale.go @@ -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. diff --git a/integration/tsic/tsic.go b/integration/tsic/tsic.go index c0ea8174..4a558a04 100644 --- a/integration/tsic/tsic.go +++ b/integration/tsic/tsic.go @@ -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 {