diff --git a/integration/README.md b/integration/README.md index 5511a113f..b3089a061 100644 --- a/integration/README.md +++ b/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 diff --git a/integration/acl_test.go b/integration/acl_test.go index f02320c1f..1c5994c74 100644 --- a/integration/acl_test.go +++ b/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) diff --git a/integration/general_test.go b/integration/general_test.go index 9bdfd8c71..09d16247f 100644 --- a/integration/general_test.go +++ b/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)