integration: cover node deletion ending the long poll

A deleted node is served a self node with no StableID, so Status().Self.ID
goes empty; TestACLPolicyPropagationOverTime must match on hostname instead.

Updates #3410

integration/README.md

@@ -29,8 +29,8 @@ act pull_request -W .github/workflows/test-integration.yaml
 ```
 
 Each test runs as a separate workflow on GitHub Actions. To add a new
-test, run `go generate` inside `../cmd/gh-action-integration-generator/`
-and commit the generated workflow file.
+test, run `go generate` inside `../.github/workflows/` and commit the
+regenerated `test-integration.yaml`.
 
 ## Framework overview
 

integration/acl_test.go

@@ -3,7 +3,6 @@ package integration
 import (
 	"fmt"
 	"net/netip"
-	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -2008,8 +2007,9 @@ func TestACLPolicyPropagationOverTime(t *testing.T) {
 
 		// Get the node list and find the newest node (highest ID)
 		var (
-			nodeList       []*clientv1.Node
-			nodeToDeleteID uint64
+			nodeList         []*clientv1.Node
+			nodeToDeleteID   uint64
+			nodeToDeleteName string
 		)
 
 		assert.EventuallyWithT(t, func(ct *assert.CollectT) {
@@ -2021,6 +2021,7 @@ func TestACLPolicyPropagationOverTime(t *testing.T) {
 			for _, node := range nodeList {
 				if mustParseID(node.Id) > nodeToDeleteID {
 					nodeToDeleteID = mustParseID(node.Id)
+					nodeToDeleteName = node.Name
 				}
 			}
 		}, integrationutil.ScaledTimeout(10*time.Second), integrationutil.SlowPoll, "iteration %d: Phase 2b - listing nodes before deletion", iteration)
@@ -2034,15 +2035,11 @@ func TestACLPolicyPropagationOverTime(t *testing.T) {
 		// This is necessary for WaitForTailscaleSyncPerUser to calculate correct peer counts
 		t.Logf("Iteration %d: Phase 2b - Removing deleted client from scenario", iteration)
 
+		// Match on hostname, not on Status().Self.ID: a deleted node is served
+		// an expired self node carrying no StableID, so Self.ID is empty by the
+		// time this runs.
 		for clientName, client := range scenario.users["user1"].Clients {
-			status := client.MustStatus()
-
-			nodeID, err := strconv.ParseUint(string(status.Self.ID), 10, 64)
-			if err != nil {
-				continue
-			}
-
-			if nodeID == nodeToDeleteID {
+			if client.Hostname() == nodeToDeleteName {
 				delete(scenario.users["user1"].Clients, clientName)
 				t.Logf("Iteration %d: Phase 2b - Removed client %s (node ID %d) from scenario", iteration, clientName, nodeToDeleteID)
 

integration/general_test.go

@@ -1500,6 +1500,87 @@ func TestPingAllByIPManyUpDown(t *testing.T) {
 	}
 }
 
+// TestNodeDeletionEndsLongPoll verifies that deleting a node ends its map
+// session and tells the client to re-authenticate. Before the fix the deleted
+// node's long poll was orphaned: the client stayed Running forever against a
+// node that no longer existed, and the server could not shut down while that
+// stream was open.
+//
+// See: https://github.com/juanfont/headscale/issues/3410
+func TestNodeDeletionEndsLongPoll(t *testing.T) {
+	IntegrationSkip(t)
+	t.Parallel()
+
+	spec := ScenarioSpec{
+		NodesPerUser: len(MustTestVersions),
+		Users:        []string{"user1"},
+	}
+
+	scenario, err := NewScenario(spec)
+	require.NoError(t, err)
+	defer scenario.ShutdownAssertNoPanics(t)
+
+	err = scenario.CreateHeadscaleEnv([]tsic.Option{}, hsic.WithTestName("deletelongpoll"))
+	requireNoErrHeadscaleEnv(t, err)
+
+	err = scenario.WaitForTailscaleSync()
+	requireNoErrSync(t, err)
+
+	headscale, err := scenario.Headscale()
+	require.NoError(t, err)
+
+	allClients, err := scenario.ListTailscaleClients()
+	requireNoErrListClients(t, err)
+	require.NotEmpty(t, allClients)
+
+	for _, client := range allClients {
+		require.NoError(t, client.WaitForRunning(integrationutil.StatusReadyTimeout),
+			"client %s must be Running before the deletion", client.Hostname())
+	}
+
+	deleted := allClients[0]
+
+	var deletedID uint64
+
+	assert.EventuallyWithT(t, func(ct *assert.CollectT) {
+		nodes, err := headscale.ListNodes()
+		assert.NoError(ct, err)
+		assert.Len(ct, nodes, len(allClients))
+
+		for _, node := range nodes {
+			if node.Name == deleted.Hostname() {
+				deletedID = mustParseID(node.Id)
+			}
+		}
+
+		assert.NotZero(ct, deletedID)
+	}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "node list should name every client before deletion")
+
+	require.NoError(t, headscale.DeleteNode(deletedID))
+
+	assert.EventuallyWithT(t, func(ct *assert.CollectT) {
+		status, err := deleted.Status()
+		assert.NoError(ct, err)
+
+		if status != nil {
+			assert.Equal(ct, "NeedsLogin", status.BackendState)
+		}
+	}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll,
+		"deleted node must stop polling and ask for a new login")
+
+	for _, client := range allClients[1:] {
+		assert.EventuallyWithT(t, func(ct *assert.CollectT) {
+			status, err := client.Status()
+			assert.NoError(ct, err)
+
+			if status != nil {
+				assert.Equal(ct, "Running", status.BackendState)
+			}
+		}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll,
+			"deleting a peer must not disturb the remaining clients")
+	}
+}
+
 func Test2118DeletingOnlineNodePanics(t *testing.T) {
 	IntegrationSkip(t)