diff --git a/command/agent/operator_endpoint_test.go b/command/agent/operator_endpoint_test.go index b4166d730..cbfed0435 100644 --- a/command/agent/operator_endpoint_test.go +++ b/command/agent/operator_endpoint_test.go @@ -15,11 +15,12 @@ import ( "testing" "time" - "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" + "github.com/shoenig/test/must" + "github.com/shoenig/test/wait" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -215,27 +216,36 @@ func TestOperator_ServerHealth(t *testing.T) { }, func(s *TestAgent) { body := bytes.NewBuffer(nil) req, _ := http.NewRequest("GET", "/v1/operator/autopilot/health", body) - retry.Run(t, func(r *retry.R) { + f := func() error { resp := httptest.NewRecorder() obj, err := s.Server.OperatorServerHealth(resp, req) if err != nil { - r.Fatalf("err: %v", err) + return fmt.Errorf("failed to get operator server health: %w", err) } - if resp.Code != 200 { - r.Fatalf("bad code: %d", resp.Code) + if code := resp.Code; code != 200 { + return fmt.Errorf("response code not 200, got: %d", code) } - out, ok := obj.(*api.OperatorHealthReply) - if !ok { - r.Fatalf("unexpected: %T", obj) + out := obj.(*api.OperatorHealthReply) + if n := len(out.Servers); n != 1 { + return fmt.Errorf("expected 1 server, got: %d", n) } - if len(out.Servers) != 1 || - !out.Servers[0].Healthy || - out.Servers[0].Name != s.server.LocalMember().Name || - out.Servers[0].SerfStatus != "alive" || - out.FailureTolerance != 0 { - r.Fatalf("bad: %v, %q", out, s.server.LocalMember().Name) + s1, s2 := out.Servers[0].Name, s.server.LocalMember().Name + if s1 != s2 { + return fmt.Errorf("expected server names to match, got %s and %s", s1, s2) } - }) + if out.Servers[0].SerfStatus != "alive" { + return fmt.Errorf("expected serf status to be alive, got: %s", out.Servers[0].SerfStatus) + } + if out.FailureTolerance != 0 { + return fmt.Errorf("expected failure tolerance of 0, got: %d", out.FailureTolerance) + } + return nil + } + must.Wait(t, wait.InitialSuccess( + wait.ErrorFunc(f), + wait.Timeout(10*time.Second), + wait.Gap(1*time.Second), + )) }) } @@ -247,25 +257,33 @@ func TestOperator_ServerHealth_Unhealthy(t *testing.T) { }, func(s *TestAgent) { body := bytes.NewBuffer(nil) req, _ := http.NewRequest("GET", "/v1/operator/autopilot/health", body) - retry.Run(t, func(r *retry.R) { + f := func() error { resp := httptest.NewRecorder() obj, err := s.Server.OperatorServerHealth(resp, req) if err != nil { - r.Fatalf("err: %v", err) + return fmt.Errorf("failed to get operator server health: %w", err) } - if resp.Code != 429 { - r.Fatalf("bad code: %d, %v", resp.Code, obj.(*api.OperatorHealthReply)) + if code := resp.Code; code != 429 { + return fmt.Errorf("expected code 429, got: %d", code) } - out, ok := obj.(*api.OperatorHealthReply) - if !ok { - r.Fatalf("unexpected: %T", obj) + out := obj.(*api.OperatorHealthReply) + if n := len(out.Servers); n != 1 { + return fmt.Errorf("expected 1 server, got: %d", n) } - if len(out.Servers) != 1 || - out.Healthy || - out.Servers[0].Name != s.server.LocalMember().Name { - r.Fatalf("bad: %#v", out.Servers) + if out.Healthy { + return fmt.Errorf("expected server to be unhealthy") } - }) + s1, s2 := out.Servers[0].Name, s.server.LocalMember().Name + if s1 != s2 { + return fmt.Errorf("expected server names to match, got %s and %s", s1, s2) + } + return nil + } + must.Wait(t, wait.InitialSuccess( + wait.ErrorFunc(f), + wait.Timeout(10*time.Second), + wait.Gap(1*time.Second), + )) }) } diff --git a/internal/testing/apitests/operator_autopilot_test.go b/internal/testing/apitests/operator_autopilot_test.go index 84945ec55..e194d6f07 100644 --- a/internal/testing/apitests/operator_autopilot_test.go +++ b/internal/testing/apitests/operator_autopilot_test.go @@ -4,52 +4,55 @@ import ( "fmt" "testing" - "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/testutil" - "github.com/stretchr/testify/require" + "github.com/shoenig/test/must" + "github.com/shoenig/test/wait" ) func TestAPI_OperatorAutopilotGetSetConfiguration(t *testing.T) { ci.Parallel(t) - require := require.New(t) + c, s := makeClient(t, nil, nil) defer s.Stop() operator := c.Operator() var config *api.AutopilotConfiguration - retry.Run(t, func(r *retry.R) { - var err error + var err error + + f := func() error { config, _, err = operator.AutopilotGetConfiguration(nil) - r.Check(err) - }) - require.True(config.CleanupDeadServers) + return err + } + must.Wait(t, wait.InitialSuccess(wait.ErrorFunc(f))) + must.True(t, config.CleanupDeadServers) // Change a config setting newConf := &api.AutopilotConfiguration{CleanupDeadServers: false} - _, err := operator.AutopilotSetConfiguration(newConf, nil) - require.Nil(err) + _, err = operator.AutopilotSetConfiguration(newConf, nil) + must.NoError(t, err) config, _, err = operator.AutopilotGetConfiguration(nil) - require.Nil(err) - require.False(config.CleanupDeadServers) + must.NoError(t, err) + must.False(t, config.CleanupDeadServers) } func TestAPI_OperatorAutopilotCASConfiguration(t *testing.T) { ci.Parallel(t) - require := require.New(t) + c, s := makeClient(t, nil, nil) defer s.Stop() operator := c.Operator() var config *api.AutopilotConfiguration - retry.Run(t, func(r *retry.R) { - var err error + var err error + f := func() error { config, _, err = operator.AutopilotGetConfiguration(nil) - r.Check(err) - }) - require.True(config.CleanupDeadServers) + return err + } + must.Wait(t, wait.InitialSuccess(wait.ErrorFunc(f))) + must.True(t, config.CleanupDeadServers) // Pass an invalid ModifyIndex { @@ -57,9 +60,10 @@ func TestAPI_OperatorAutopilotCASConfiguration(t *testing.T) { CleanupDeadServers: false, ModifyIndex: config.ModifyIndex - 1, } - resp, _, err := operator.AutopilotCASConfiguration(newConf, nil) - require.Nil(err) - require.False(resp) + var apState bool + apState, _, err = operator.AutopilotCASConfiguration(newConf, nil) + must.NoError(t, err) + must.False(t, apState) } // Pass a valid ModifyIndex @@ -68,9 +72,10 @@ func TestAPI_OperatorAutopilotCASConfiguration(t *testing.T) { CleanupDeadServers: false, ModifyIndex: config.ModifyIndex, } - resp, _, err := operator.AutopilotCASConfiguration(newConf, nil) - require.Nil(err) - require.True(resp) + var apState bool + apState, _, err = operator.AutopilotCASConfiguration(newConf, nil) + must.NoError(t, err) + must.True(t, apState) } } diff --git a/internal/testing/apitests/operator_test.go b/internal/testing/apitests/operator_test.go index fe0f1f173..203713f30 100644 --- a/internal/testing/apitests/operator_test.go +++ b/internal/testing/apitests/operator_test.go @@ -3,28 +3,29 @@ package apitests import ( "testing" - "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/ci" - "github.com/stretchr/testify/require" + "github.com/shoenig/test/must" + "github.com/shoenig/test/wait" ) func TestAPI_OperatorSchedulerGetSetConfiguration(t *testing.T) { ci.Parallel(t) - require := require.New(t) + c, s := makeClient(t, nil, nil) defer s.Stop() operator := c.Operator() var config *api.SchedulerConfigurationResponse - retry.Run(t, func(r *retry.R) { - var err error + var err error + f := func() error { config, _, err = operator.SchedulerGetConfiguration(nil) - r.Check(err) - }) - require.True(config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) - require.False(config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) - require.False(config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) + return err + } + must.Wait(t, wait.InitialSuccess(wait.ErrorFunc(f))) + must.True(t, config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) + must.False(t, config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) + must.False(t, config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) // Change a config setting newConf := &api.SchedulerConfiguration{ @@ -35,34 +36,34 @@ func TestAPI_OperatorSchedulerGetSetConfiguration(t *testing.T) { }, } resp, wm, err := operator.SchedulerSetConfiguration(newConf, nil) - require.Nil(err) - require.NotZero(wm.LastIndex) - // non CAS requests should update on success - require.True(resp.Updated) + must.NoError(t, err) + must.Positive(t, wm.LastIndex) + must.True(t, resp.Updated) // non CAS requests should update on success config, _, err = operator.SchedulerGetConfiguration(nil) - require.Nil(err) - require.False(config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) - require.True(config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) - require.True(config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) + must.NoError(t, err) + must.False(t, config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) + must.True(t, config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) + must.True(t, config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) } func TestAPI_OperatorSchedulerCASConfiguration(t *testing.T) { ci.Parallel(t) - require := require.New(t) + c, s := makeClient(t, nil, nil) defer s.Stop() operator := c.Operator() var config *api.SchedulerConfigurationResponse - retry.Run(t, func(r *retry.R) { - var err error + var err error + f := func() error { config, _, err = operator.SchedulerGetConfiguration(nil) - r.Check(err) - }) - require.True(config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) - require.False(config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) - require.False(config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) + return err + } + must.Wait(t, wait.InitialSuccess(wait.ErrorFunc(f))) + must.True(t, config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) + must.False(t, config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) + must.False(t, config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) // Pass an invalid ModifyIndex { @@ -70,10 +71,12 @@ func TestAPI_OperatorSchedulerCASConfiguration(t *testing.T) { PreemptionConfig: api.PreemptionConfig{SystemSchedulerEnabled: false, BatchSchedulerEnabled: true}, ModifyIndex: config.SchedulerConfig.ModifyIndex - 1, } - resp, wm, err := operator.SchedulerCASConfiguration(newConf, nil) - require.Nil(err) - require.NotZero(wm.LastIndex) - require.False(resp.Updated) + var resp *api.SchedulerSetConfigurationResponse + var wm *api.WriteMeta + resp, wm, err = operator.SchedulerCASConfiguration(newConf, nil) + must.NoError(t, err) + must.Positive(t, wm.LastIndex) + must.False(t, resp.Updated) } // Pass a valid ModifyIndex @@ -82,15 +85,17 @@ func TestAPI_OperatorSchedulerCASConfiguration(t *testing.T) { PreemptionConfig: api.PreemptionConfig{SystemSchedulerEnabled: false, BatchSchedulerEnabled: true}, ModifyIndex: config.SchedulerConfig.ModifyIndex, } - resp, wm, err := operator.SchedulerCASConfiguration(newConf, nil) - require.Nil(err) - require.NotZero(wm.LastIndex) - require.True(resp.Updated) + var resp *api.SchedulerSetConfigurationResponse + var wm *api.WriteMeta + resp, wm, err = operator.SchedulerCASConfiguration(newConf, nil) + must.NoError(t, err) + must.Positive(t, wm.LastIndex) + must.True(t, resp.Updated) } - config, _, err := operator.SchedulerGetConfiguration(nil) - require.Nil(err) - require.False(config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) - require.True(config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) - require.False(config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) + config, _, err = operator.SchedulerGetConfiguration(nil) + must.NoError(t, err) + must.False(t, config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) + must.True(t, config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) + must.False(t, config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) } diff --git a/nomad/leader_test.go b/nomad/leader_test.go index 7df547136..6d8b1f993 100644 --- a/nomad/leader_test.go +++ b/nomad/leader_test.go @@ -8,11 +8,11 @@ import ( "testing" "time" - "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/go-hclog" memdb "github.com/hashicorp/go-memdb" "github.com/hashicorp/go-version" "github.com/shoenig/test/must" + "github.com/shoenig/test/wait" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -768,11 +768,8 @@ func TestLeader_ClusterID_upgradePath(t *testing.T) { // A check that ClusterID is not available yet noIDYet := func() { for _, s := range servers { - retry.Run(t, func(r *retry.R) { - if _, err := s.s.ClusterID(); err == nil { - r.Error("expected error") - } - }) + _, err := s.s.ClusterID() + must.Error(t, err) } } @@ -863,23 +860,32 @@ func TestLeader_ClusterID_noUpgrade(t *testing.T) { } func agreeClusterID(t *testing.T, servers []*Server) { - retries := &retry.Timer{Timeout: 60 * time.Second, Wait: 1 * time.Second} - ids := make([]string, 3) - for i, s := range servers { - retry.RunWith(retries, t, func(r *retry.R) { - id, err := s.ClusterID() - if err != nil { - r.Error(err.Error()) - return - } - if !helper.IsUUID(id) { - r.Error("not a UUID") - return - } - ids[i] = id - }) + must.Len(t, 3, servers) + + f := func() error { + id1, err1 := servers[0].ClusterID() + if err1 != nil { + return err1 + } + id2, err2 := servers[1].ClusterID() + if err2 != nil { + return err2 + } + id3, err3 := servers[2].ClusterID() + if err3 != nil { + return err3 + } + if id1 != id2 || id2 != id3 { + return fmt.Errorf("ids do not match, id1: %s, id2: %s, id3: %s", id1, id2, id3) + } + return nil } - require.True(t, ids[0] == ids[1] && ids[1] == ids[2], "ids[0] %s, ids[1] %s, ids[2] %s", ids[0], ids[1], ids[2]) + + must.Wait(t, wait.InitialSuccess( + wait.ErrorFunc(f), + wait.Timeout(60*time.Second), + wait.Gap(1*time.Second), + )) } func TestLeader_ReplicateACLPolicies(t *testing.T) {