servertest: wait for stable via-route netmaps

Updates #3524

(cherry picked from commit f1293a805a)

hscontrol/servertest/via_compat_test.go

@@ -240,6 +240,9 @@ func runViaMapCompat(t *testing.T, c *testcapture.Capture) {
 	saasAddrs := saasAddrsByNode(c, clients)
 
 	// Compare each viewer's [tailcfg.MapResponse] against the golden [netmap.NetworkMap].
+	// Route approvals reach clients asynchronously, so the peer-count wait
+	// above says nothing about routes, primaries or filters: retry the
+	// whole comparison until the viewer's netmap has caught up.
 	for viewerName, cl := range clients {
 		capture := c.Captures[viewerName]
 		if capture.Netmap == nil {
@@ -247,17 +250,69 @@ func runViaMapCompat(t *testing.T, c *testcapture.Capture) {
 		}
 
 		t.Run(viewerName, func(t *testing.T) {
-			nm := cl.Netmap()
-			require.NotNil(t, nm, "netmap is nil")
+			requireNetmapHolds(t, func(tt require.TestingT) {
+				nm := cl.Netmap()
+				require.NotNil(tt, nm, "netmap is nil")
 
-			require.Equal(t, selfOffersExit(capture.Netmap), selfOffersExit(nm),
-				"SelfNode exit routes should match SaaS")
+				require.Equal(tt, selfOffersExit(capture.Netmap), selfOffersExit(nm),
+					"SelfNode exit routes should match SaaS")
 
-			compareNetmap(t, nm, capture, clients, saasAddrs)
+				compareNetmap(tt, nm, capture, clients, saasAddrs)
+			})
 		})
 	}
 }
 
+// requireNetmapHolds retries check until it passes on several consecutive
+// ticks. One passing tick is not enough: a netmap that matches and then
+// drifts, such as a primary flipping away, is the bug these oracles guard.
+func requireNetmapHolds(t *testing.T, check func(require.TestingT)) {
+	t.Helper()
+
+	// Long enough for a batch still in flight to land.
+	const holdTicks = 5
+
+	held := 0
+
+	require.EventuallyWithT(t, func(collect *assert.CollectT) {
+		tick := &failRecorder{CollectT: collect}
+
+		// Deferred because FailNow exits the goroutine.
+		defer func() {
+			if tick.failed {
+				held = 0
+
+				return
+			}
+
+			held++
+			if held < holdTicks {
+				collect.Errorf("netmap held for %d of %d ticks", held, holdTicks)
+			}
+		}()
+
+		check(tick)
+	}, 30*time.Second, 100*time.Millisecond)
+}
+
+// failRecorder notes whether a tick failed, which [assert.CollectT] does
+// not expose.
+type failRecorder struct {
+	*assert.CollectT
+
+	failed bool
+}
+
+func (r *failRecorder) Errorf(format string, args ...any) {
+	r.failed = true
+	r.CollectT.Errorf(format, args...)
+}
+
+func (r *failRecorder) FailNow() {
+	r.failed = true
+	r.CollectT.FailNow()
+}
+
 // compareNetmap compares the headscale [tailcfg.MapResponse] against the
 // captured [netmap.NetworkMap] data in an IP-independent way. It validates:
 //   - Peer visibility (which peers are present, by hostname)
@@ -265,14 +320,18 @@ func runViaMapCompat(t *testing.T, c *testcapture.Capture) {
 //   - Number of Tailscale IPs per peer (should be 2: one v4 + one v6)
 //   - PrimaryRoutes per peer
 //   - PacketFilter (source, destination, ports) triples
+//
+// It takes [require.TestingT] so it can run inside [requireNetmapHolds].
 func compareNetmap(
-	t *testing.T,
+	t require.TestingT,
 	got *netmap.NetworkMap,
 	want testcapture.Node,
 	clients map[string]*servertest.TestClient,
 	saasAddrs map[netip.Addr]string,
 ) {
-	t.Helper()
+	if h, ok := t.(interface{ Helper() }); ok {
+		h.Helper()
+	}
 
 	require.NotNil(t, want.Netmap, "golden Netmap is nil")
 
@@ -412,13 +471,15 @@ func compareNetmap(
 // Tailscale IPs differ between SaaS and headscale allocation, so both
 // sides are re-keyed by peer identity.
 func comparePacketFilter(
-	t *testing.T,
+	t require.TestingT,
 	got *netmap.NetworkMap,
 	want testcapture.Node,
 	clients map[string]*servertest.TestClient,
 	saasAddrs map[netip.Addr]string,
 ) {
-	t.Helper()
+	if h, ok := t.(interface{ Helper() }); ok {
+		h.Helper()
+	}
 
 	hsAddrs := hsAddrsByPeer(clients)
 
@@ -486,12 +547,14 @@ func capTriple(srcs, dst string, c tailcfg.PeerCapability) string {
 // peerIdents resolves a prefix into sorted canonical identity tokens,
 // see [addIdentsForSrc].
 func peerIdents(
-	t *testing.T,
+	t require.TestingT,
 	pfx netip.Prefix,
 	addrToPeer map[netip.Addr]string,
 	ruleIndex int,
 ) []string {
-	t.Helper()
+	if h, ok := t.(interface{ Helper() }); ok {
+		h.Helper()
+	}
 
 	seen := map[string]struct{}{}
 	addIdentsForSrc(t, pfx, addrToPeer, ruleIndex, seen)
@@ -561,12 +624,14 @@ func hsAddrsByPeer(clients map[string]*servertest.TestClient) map[netip.Addr]str
 // or /128 (IPv6) expands to the union of its contained peers.
 // Unresolvable Tailscale-range sources fail the test.
 func canonicaliseSrcStrings(
-	t *testing.T,
+	t require.TestingT,
 	srcs []string,
 	addrToPeer map[netip.Addr]string,
 	ruleIndex int,
 ) []string {
-	t.Helper()
+	if h, ok := t.(interface{ Helper() }); ok {
+		h.Helper()
+	}
 
 	seen := map[string]struct{}{}
 
@@ -591,12 +656,14 @@ func canonicaliseSrcStrings(
 // [canonicaliseSrcStrings], reading already-parsed [netip.Prefix] values
 // from [tailcfg.Match.Srcs].
 func canonicaliseSrcPrefixes(
-	t *testing.T,
+	t require.TestingT,
 	srcs []netip.Prefix,
 	addrToPeer map[netip.Addr]string,
 	ruleIndex int,
 ) []string {
-	t.Helper()
+	if h, ok := t.(interface{ Helper() }); ok {
+		h.Helper()
+	}
 
 	seen := map[string]struct{}{}
 
@@ -618,13 +685,15 @@ func canonicaliseSrcPrefixes(
 // through literally; a Tailscale-range prefix expands to the union
 // of peer names whose addresses fall within it.
 func addIdentsForSrc(
-	t *testing.T,
+	t require.TestingT,
 	pfx netip.Prefix,
 	addrToPeer map[netip.Addr]string,
 	ruleIndex int,
 	seen map[string]struct{},
 ) {
-	t.Helper()
+	if h, ok := t.(interface{ Helper() }); ok {
+		h.Helper()
+	}
 
 	if !prefixInTailscaleRange(pfx) {
 		seen[pfx.String()] = struct{}{}

hscontrol/servertest/via_ha_compat_test.go

@@ -186,6 +186,9 @@ func runViaHACompat(t *testing.T, c *testcapture.Capture) {
 	}
 
 	// Compare each viewer's [tailcfg.MapResponse] against golden [netmap.NetworkMap].
+	// Route approvals reach clients asynchronously, so the peer-count wait
+	// above says nothing about routes, primaries or filters: retry the
+	// whole comparison until the viewer's netmap has caught up.
 	for viewerName, cl := range clients {
 		capture := c.Captures[viewerName]
 		if capture.Netmap == nil {
@@ -193,7 +196,9 @@ func runViaHACompat(t *testing.T, c *testcapture.Capture) {
 		}
 
 		t.Run(viewerName, func(t *testing.T) {
-			compareCaptureNetmap(t, cl, capture, clients)
+			requireNetmapHolds(t, func(tt require.TestingT) {
+				compareCaptureNetmap(tt, cl, capture, clients)
+			})
 		})
 	}
 }
@@ -201,13 +206,16 @@ func runViaHACompat(t *testing.T, c *testcapture.Capture) {
 // compareCaptureNetmap compares headscale's [tailcfg.MapResponse] against a
 // [testcapture.Node]'s [netmap.NetworkMap] data. Same logic as [compareNetmap] but
 // reads from typed [testcapture] fields instead of goldenFile strings.
+// It takes [require.TestingT] so it can run inside [requireNetmapHolds].
 func compareCaptureNetmap(
-	t *testing.T,
+	t require.TestingT,
 	viewer *servertest.TestClient,
 	want testcapture.Node,
 	clients map[string]*servertest.TestClient,
 ) {
-	t.Helper()
+	if h, ok := t.(interface{ Helper() }); ok {
+		h.Helper()
+	}
 
 	nm := viewer.Netmap()
 	require.NotNil(t, nm, "viewer has no netmap")