diff --git a/.github/workflows/test-integration.yaml b/.github/workflows/test-integration.yaml index 3ac7e616..0fe2d069 100644 --- a/.github/workflows/test-integration.yaml +++ b/.github/workflows/test-integration.yaml @@ -303,6 +303,7 @@ jobs: - TestPingAllByHostname - TestTaildrop - TestUpdateHostnameFromClient + - TestHostinfoChangeReachesPeersOnlyWhenPeerVisible - TestExpireNode - TestSetNodeExpiryInFuture - TestDisableNodeExpiry diff --git a/integration/general_test.go b/integration/general_test.go index f33c69a5..1f89d831 100644 --- a/integration/general_test.go +++ b/integration/general_test.go @@ -941,6 +941,214 @@ func TestUpdateHostnameFromClient(t *testing.T) { }, time.Second, 50*time.Millisecond, "hostname updates should be reflected in node list with new suffix") } +// peersMapResponseType is the [change.Change.Type] label that +// headscale_mapresponse_generated_total carries for a whole-peer resend, +// which is what [change.NodeAdded] produces. +const peersMapResponseType = "peers" + +// mapResponseCountsByType reads headscale_mapresponse_generated_total keyed by +// its response_type label. curl runs inside the container because the metrics +// listener is published to the Docker host, not to the network the test +// process shares with the server. +func mapResponseCountsByType(headscale ControlServer) (map[string]float64, error) { + const ( + metricName = "headscale_mapresponse_generated_total" + labelPrefix = `response_type="` + ) + + out, err := headscale.Execute( + []string{"curl", "-s", "http://localhost:9090/metrics"}, + ) + if err != nil { + return nil, fmt.Errorf("fetching metrics: %w", err) + } + + counts := make(map[string]float64) + + for _, line := range strings.Split(out, "\n") { + if !strings.HasPrefix(line, metricName+"{") { + continue + } + + labelStart := strings.Index(line, labelPrefix) + if labelStart < 0 { + continue + } + + rest := line[labelStart+len(labelPrefix):] + + labelEnd := strings.Index(rest, `"`) + if labelEnd < 0 { + continue + } + + valueStart := strings.LastIndex(line, " ") + if valueStart < 0 { + continue + } + + value, err := strconv.ParseFloat(strings.TrimSpace(line[valueStart+1:]), 64) + if err != nil { + return nil, fmt.Errorf("parsing metric line %q: %w", line, err) + } + + counts[rest[:labelEnd]] = value + } + + return counts, nil +} + +// quiescedMapResponseCounts waits until the whole-peer map response counter +// stops moving and then returns the whole counter family. A read taken +// mid-fan-out charges someone else's responses to the change under test. +func quiescedMapResponseCounts( + t *testing.T, + headscale ControlServer, + waitingFor string, +) map[string]float64 { + t.Helper() + + var counts map[string]float64 + + previous := -1.0 + + require.EventuallyWithT(t, func(ct *assert.CollectT) { + current, err := mapResponseCountsByType(headscale) + assert.NoError(ct, err) + assert.NotEmpty(ct, current, "metrics should carry the map response counter") + assert.Equal(ct, previous, current[peersMapResponseType], + "whole-peer fan-out is still moving") + + previous = current[peersMapResponseType] + counts = current + }, integrationutil.HAConvergeTimeout, 2*time.Second, waitingFor) + + return counts +} + +// TestHostinfoChangeReachesPeersOnlyWhenPeerVisible pins which Hostinfo edits +// are worth a whole-peer resend. +// +// Peers render only a handful of Hostinfo fields, so a client toggling one they +// never read should cost their peers nothing, while a field they do render has +// to arrive. Shields-up and hostname sit on either side of that line. +// +// The observable is headscale_mapresponse_generated_total{response_type="peers"}: +// [change.NodeAdded] is the whole-peer broadcast the map request path emits for +// a Hostinfo edit peers read, and [change.Change.Type] labels it "peers". +func TestHostinfoChangeReachesPeersOnlyWhenPeerVisible(t *testing.T) { + IntegrationSkip(t) + + spec := ScenarioSpec{ + NodesPerUser: 2, + Users: []string{"user1"}, + } + + scenario, err := NewScenario(spec) + require.NoErrorf(t, err, "failed to create scenario") + defer scenario.ShutdownAssertNoPanics(t) + + err = scenario.CreateHeadscaleEnv( + []tsic.Option{}, + hsic.WithTestName("hostinfopeervisible"), + ) + requireNoErrHeadscaleEnv(t, err) + + allClients, err := scenario.ListTailscaleClients() + requireNoErrListClients(t, err) + require.Len(t, allClients, 2, "one client changes Hostinfo, the other observes") + + err = scenario.WaitForTailscaleSync() + requireNoErrSync(t, err) + + headscale, err := scenario.Headscale() + requireNoErrGetHeadscale(t, err) + + subject, observer := allClients[0], allClients[1] + + subjectStableID := string(subject.MustStatus().Self.ID) + subjectNodeID, err := strconv.ParseUint(subjectStableID, 10, 64) + require.NoErrorf(t, err, + "headscale issues numeric stable node IDs, got %q", subjectStableID) + + baseline := quiescedMapResponseCounts(t, headscale, + "registration fan-out should settle before the baseline counter read") + + // Shields-up moves Hostinfo.ShieldsUp and nothing a peer reads. + _, _, err = subject.Execute([]string{"tailscale", "set", "--shields-up=true"}) + require.NoErrorf(t, err, "failed to raise shields on %s", subject.Hostname()) + + // Storing Hostinfo and telling peers about it are separate decisions, so + // the NodeStore is the sync point that proves the map request landed + // whatever the server chose to broadcast. + require.EventuallyWithT(t, func(ct *assert.CollectT) { + store, err := headscale.DebugNodeStore() + assert.NoError(ct, err) + + node, ok := store[types.NodeID(subjectNodeID)] + if !assert.True(ct, ok, "node %d should be in the NodeStore", subjectNodeID) { + return + } + + if !assert.NotNil(ct, node.Hostinfo, "node %d should carry Hostinfo", subjectNodeID) { + return + } + + assert.True(ct, node.Hostinfo.ShieldsUp, + "headscale should have processed the shields-up map request") + }, integrationutil.HAConvergeTimeout, 1*time.Second, + "headscale should record the shields-up Hostinfo change") + + afterShields := quiescedMapResponseCounts(t, headscale, + "map response counter should settle after the shields-up change") + + t.Logf("map response counts: baseline=%v afterShields=%v", baseline, afterShields) + + // assert, not require: the peer-visible half below is the other side of + // the same contract and its evidence is worth having in the same run. + assert.Equal(t, + baseline[peersMapResponseType], + afterShields[peersMapResponseType], + "shields-up touches no Hostinfo field a peer reads, so no peer should be handed the whole node again") + + // Hostname is rendered by every peer, so this one has to travel. + const newHostname = "shields-up-renamed" + + _, _, err = subject.Execute([]string{"tailscale", "set", "--hostname=" + newHostname}) + require.NoErrorf(t, err, "failed to set hostname on %s", subject.Hostname()) + + require.EventuallyWithT(t, func(ct *assert.CollectT) { + status, err := observer.Status() + assert.NoError(ct, err) + + var seen bool + + for _, peer := range status.Peer { + if string(peer.ID) != subjectStableID { + continue + } + + seen = true + + assert.Equal(ct, newHostname, peer.HostName, + "peer %s should be seen under its new hostname", subjectStableID) + } + + assert.True(ct, seen, "observer should still have node %s as a peer", subjectStableID) + }, integrationutil.HAConvergeTimeout, 1*time.Second, + "hostname is peer-visible, so the rename must reach the peer") + + afterHostname := quiescedMapResponseCounts(t, headscale, + "map response counter should settle after the hostname change") + + t.Logf("map response counts after hostname change: %v", afterHostname) + + require.Greater(t, + afterHostname[peersMapResponseType], + afterShields[peersMapResponseType], + "a hostname change is peer-visible and must fan out as a whole-peer resend") +} + func TestExpireNode(t *testing.T) { IntegrationSkip(t)