Skip to content

Retry flaky tests #19088

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kisunji merged 3 commits into from
Oct 5, 2023
Merged

Retry flaky tests #19088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d157d93
Retry tests
Oct 5, 2023
9da2d0c
Retry tests
Oct 5, 2023
91ca74d
Retry port grabs
Oct 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions agent/health_endpoint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions agent/http_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down
73 changes: 34 additions & 39 deletions agent/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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.")

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
18 changes: 17 additions & 1 deletion agent/testagent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down