Retry flaky tests (#19088)

agent/health_endpoint_test.go

@@ -442,19 +442,21 @@ func TestHealthServiceChecks_NodeMetaFilter(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	req, _ = http.NewRequest("GET", "/v1/health/checks/consul?dc=dc1&node-meta=somekey:somevalue", nil)
-	resp = httptest.NewRecorder()
-	obj, err = a.srv.HealthServiceChecks(resp, req)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	assertIndex(t, resp)
+	retry.Run(t, func(r *retry.R) {
+		req, _ = http.NewRequest("GET", "/v1/health/checks/consul?dc=dc1&node-meta=somekey:somevalue", nil)
+		resp = httptest.NewRecorder()
+		obj, err = a.srv.HealthServiceChecks(resp, req)
+		if err != nil {
+			r.Fatalf("err: %v", err)
+		}
+		assertIndex(r, resp)
 
-	// Should be 1 health check for consul
-	nodes = obj.(structs.HealthChecks)
-	if len(nodes) != 1 {
-		t.Fatalf("bad: %v", obj)
-	}
+		// Should be 1 health check for consul
+		nodes = obj.(structs.HealthChecks)
+		if len(nodes) != 1 {
+			r.Fatalf("bad: %v", obj)
+		}
+	})
 }
 
 func TestHealthServiceChecks_Filtering(t *testing.T) {

agent/http_test.go

@@ -1628,8 +1628,10 @@ func TestAllowedNets(t *testing.T) {
 }
 
 // assertIndex tests that X-Consul-Index is set and non-zero
-func assertIndex(t *testing.T, resp *httptest.ResponseRecorder) {
-	t.Helper()
+func assertIndex(t require.TestingT, resp *httptest.ResponseRecorder) {
+	if tt, ok := t.(*testing.T); ok {
+		tt.Helper()
+	}
 	require.NoError(t, checkIndex(resp))
 }
 

agent/metrics_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/hashicorp/consul/agent/rpc/middleware"
 	"github.com/hashicorp/consul/lib/retry"
 	"github.com/hashicorp/consul/sdk/testutil"
+	testretry "github.com/hashicorp/consul/sdk/testutil/retry"
 	"github.com/hashicorp/consul/testrpc"
 	"github.com/hashicorp/consul/tlsutil"
 )
@@ -30,8 +31,11 @@ func skipIfShortTesting(t *testing.T) {
 	}
 }
 
-func recordPromMetrics(t *testing.T, a *TestAgent, respRec *httptest.ResponseRecorder) {
-	t.Helper()
+func recordPromMetrics(t require.TestingT, a *TestAgent, respRec *httptest.ResponseRecorder) {
+	if tt, ok := t.(*testing.T); ok {
+		tt.Helper()
+	}
+
 	req, err := http.NewRequest("GET", "/v1/agent/metrics?format=prometheus", nil)
 	require.NoError(t, err, "Failed to generate new http request.")
 
@@ -483,28 +487,24 @@ func TestHTTPHandlers_AgentMetrics_WAL_Prometheus(t *testing.T) {
 		defer a.Shutdown()
 		testrpc.WaitForLeader(t, a.RPC, "dc1")
 
-		respRec := httptest.NewRecorder()
-		recordPromMetrics(t, a, respRec)
+		testretry.Run(t, func(r *testretry.R) {
+			respRec := httptest.NewRecorder()
+			recordPromMetrics(r, a, respRec)
+
+			out := respRec.Body.String()
+			require.Contains(r, out, "agent_5_raft_wal_head_truncations")
+			require.Contains(r, out, "agent_5_raft_wal_last_segment_age_seconds")
+			require.Contains(r, out, "agent_5_raft_wal_log_appends")
+			require.Contains(r, out, "agent_5_raft_wal_log_entries_read")
+			require.Contains(r, out, "agent_5_raft_wal_log_entries_written")
+			require.Contains(r, out, "agent_5_raft_wal_log_entry_bytes_read")
+			require.Contains(r, out, "agent_5_raft_wal_log_entry_bytes_written")
+			require.Contains(r, out, "agent_5_raft_wal_segment_rotations")
+			require.Contains(r, out, "agent_5_raft_wal_stable_gets")
+			require.Contains(r, out, "agent_5_raft_wal_stable_sets")
+			require.Contains(r, out, "agent_5_raft_wal_tail_truncations")
+		})
 
-		out := respRec.Body.String()
-		defer func() {
-			if t.Failed() {
-				t.Log("--- Failed output START ---")
-				t.Log(out)
-				t.Log("--- Failed output END ---")
-			}
-		}()
-		require.Contains(t, out, "agent_5_raft_wal_head_truncations")
-		require.Contains(t, out, "agent_5_raft_wal_last_segment_age_seconds")
-		require.Contains(t, out, "agent_5_raft_wal_log_appends")
-		require.Contains(t, out, "agent_5_raft_wal_log_entries_read")
-		require.Contains(t, out, "agent_5_raft_wal_log_entries_written")
-		require.Contains(t, out, "agent_5_raft_wal_log_entry_bytes_read")
-		require.Contains(t, out, "agent_5_raft_wal_log_entry_bytes_written")
-		require.Contains(t, out, "agent_5_raft_wal_segment_rotations")
-		require.Contains(t, out, "agent_5_raft_wal_stable_gets")
-		require.Contains(t, out, "agent_5_raft_wal_stable_sets")
-		require.Contains(t, out, "agent_5_raft_wal_tail_truncations")
 	})
 
 	t.Run("server without WAL enabled emits no WAL metrics", func(t *testing.T) {
@@ -590,22 +590,17 @@ func TestHTTPHandlers_AgentMetrics_LogVerifier_Prometheus(t *testing.T) {
 		defer a.Shutdown()
 		testrpc.WaitForLeader(t, a.RPC, "dc1")
 
-		respRec := httptest.NewRecorder()
-		recordPromMetrics(t, a, respRec)
-
-		out := respRec.Body.String()
-		defer func() {
-			if t.Failed() {
-				t.Log("--- Failed output START ---")
-				t.Log(out)
-				t.Log("--- Failed output END ---")
-			}
-		}()
-		require.Contains(t, out, "agent_5_raft_logstore_verifier_checkpoints_written")
-		require.Contains(t, out, "agent_5_raft_logstore_verifier_dropped_reports")
-		require.Contains(t, out, "agent_5_raft_logstore_verifier_ranges_verified")
-		require.Contains(t, out, "agent_5_raft_logstore_verifier_read_checksum_failures")
-		require.Contains(t, out, "agent_5_raft_logstore_verifier_write_checksum_failures")
+		testretry.Run(t, func(r *testretry.R) {
+			respRec := httptest.NewRecorder()
+			recordPromMetrics(r, a, respRec)
+
+			out := respRec.Body.String()
+			require.Contains(r, out, "agent_5_raft_logstore_verifier_checkpoints_written")
+			require.Contains(r, out, "agent_5_raft_logstore_verifier_dropped_reports")
+			require.Contains(r, out, "agent_5_raft_logstore_verifier_ranges_verified")
+			require.Contains(r, out, "agent_5_raft_logstore_verifier_read_checksum_failures")
+			require.Contains(r, out, "agent_5_raft_logstore_verifier_write_checksum_failures")
+		})
 	})
 
 	t.Run("server with verifier disabled emits no extra metrics", func(t *testing.T) {

agent/testagent.go

@@ -405,6 +405,19 @@ func (a *TestAgent) consulConfig() *consul.Config {
 	return c
 }
 
+// Using sdk/freeport with *retry.R is not possible without changing
+// function signatures. We use this shim instead to save the headache
+// of syncing sdk submodule updates.
+type retryShim struct {
+	*retry.R
+
+	name string
+}
+
+func (r *retryShim) Name() string {
+	return r.name
+}
+
 // pickRandomPorts selects random ports from fixed size random blocks of
 // ports. This does not eliminate the chance for port conflict but
 // reduces it significantly with little overhead. Furthermore, asking
@@ -414,7 +427,10 @@ func (a *TestAgent) consulConfig() *consul.Config {
 // Instead of relying on one set of ports to be sufficient we retry
 // starting the agent with different ports on port conflict.
 func randomPortsSource(t *testing.T, useHTTPS bool) string {
-	ports := freeport.GetN(t, 7)
+	var ports []int
+	retry.RunWith(retry.TwoSeconds(), t, func(r *retry.R) {
+		ports = freeport.GetN(&retryShim{r, t.Name()}, 7)
+	})
 
 	var http, https int
 	if useHTTPS {