integration: pin which Hostinfo changes reach peers

Updates #3417

(cherry picked from commit 6a9f6c3bd7)

.github/workflows/test-integration.yaml

@@ -292,6 +292,7 @@ jobs:
           - TestPingAllByHostname
           - TestTaildrop
           - TestUpdateHostnameFromClient
+          - TestHostinfoChangeReachesPeersOnlyWhenPeerVisible
           - TestExpireNode
           - TestSetNodeExpiryInFuture
           - TestDisableNodeExpiry

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)