api: cleanup use of deprecated waiter functions (#15608)

api/agent_test.go

@@ -10,7 +10,7 @@ import (
 	"time"
 
 	"github.com/hashicorp/nomad/api/internal/testutil"
-	"github.com/kr/pretty"
+	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -269,39 +269,14 @@ func TestAgent_Health(t *testing.T) {
 // functionality for a specific client node
 func TestAgent_MonitorWithNode(t *testing.T) {
 	testutil.Parallel(t)
-	rpcPort := 0
+
 	c, s := makeClient(t, nil, func(c *testutil.TestServerConfig) {
-		rpcPort = c.Ports.RPC
-		c.Client = &testutil.ClientConfig{
-			Enabled: true,
-		}
+		c.DevMode = true
 	})
 	defer s.Stop()
 
-	require.NoError(t, c.Agent().SetServers([]string{fmt.Sprintf("127.0.0.1:%d", rpcPort)}))
-
 	agent := c.Agent()
-
-	index := uint64(0)
-	var node *NodeListStub
-	// grab a node
-	testutil.WaitForResult(func() (bool, error) {
-		nodes, qm, err := c.Nodes().List(&QueryOptions{WaitIndex: index})
-		if err != nil {
-			return false, err
-		}
-		index = qm.LastIndex
-		if len(nodes) != 1 {
-			return false, fmt.Errorf("expected 1 node but found: %s", pretty.Sprint(nodes))
-		}
-		if nodes[0].Status != "ready" {
-			return false, fmt.Errorf("node not ready: %s", nodes[0].Status)
-		}
-		node = nodes[0]
-		return true, nil
-	}, func(err error) {
-		t.Fatalf("err: %v", err)
-	})
+	node := oneNodeFromNodeList(t, c.Nodes())
 
 	doneCh := make(chan struct{})
 	q := &QueryOptions{
@@ -316,7 +291,7 @@ func TestAgent_MonitorWithNode(t *testing.T) {
 
 	// make a request to generate some logs
 	_, err := agent.NodeName()
-	require.NoError(t, err)
+	must.NoError(t, err)
 
 	// Wait for a log message
 OUTER:
@@ -329,7 +304,7 @@ OUTER:
 		case err := <-errCh:
 			t.Errorf("Error: %v", err)
 		case <-time.After(2 * time.Second):
-			require.Fail(t, "failed to get a DEBUG log message")
+			t.Fatal("failed to get a DEBUG log message")
 		}
 	}
 }

api/allocations_test.go

@@ -14,7 +14,9 @@ import (
 )
 
 func TestAllocations_List(t *testing.T) {
+	testutil.RequireRoot(t)
 	testutil.Parallel(t)
+
 	c, s := makeClient(t, nil, func(c *testutil.TestServerConfig) {
 		c.DevMode = true
 	})
@@ -106,7 +108,9 @@ func TestAllocations_PrefixList(t *testing.T) {
 }
 
 func TestAllocations_List_Resources(t *testing.T) {
+	testutil.RequireRoot(t)
 	testutil.Parallel(t)
+
 	c, s := makeClient(t, nil, func(c *testutil.TestServerConfig) {
 		c.DevMode = true
 	})

api/evaluations_test.go

@@ -1,10 +1,13 @@
 package api
 
 import (
+	"fmt"
 	"sort"
 	"testing"
 
 	"github.com/hashicorp/nomad/api/internal/testutil"
+	"github.com/shoenig/test/must"
+	"github.com/shoenig/test/wait"
 	"github.com/stretchr/testify/require"
 )
 
@@ -40,15 +43,18 @@ func TestEvaluations_List(t *testing.T) {
 
 	// wait until the 2nd eval shows up before we try paging
 	results := []*Evaluation{}
-	testutil.WaitForResult(func() (bool, error) {
+
+	f := func() error {
 		results, _, err = e.List(nil)
-		if len(results) < 2 || err != nil {
-			return false, err
+		if err != nil {
+			return fmt.Errorf("failed to list evaluations: %w", err)
 		}
-		return true, nil
-	}, func(err error) {
-		t.Fatalf("err: %s", err)
-	})
+		if len(results) < 2 {
+			return fmt.Errorf("fewer than 2 results, got: %d", len(results))
+		}
+		return nil
+	}
+	must.Wait(t, wait.InitialSuccess(wait.ErrorFunc(f)))
 
 	// query first page
 	result, qm, err = e.List(&QueryOptions{

api/fs_test.go

@@ -12,12 +12,13 @@ import (
 
 	"github.com/docker/go-units"
 	"github.com/hashicorp/nomad/api/internal/testutil"
-	"github.com/kr/pretty"
 	"github.com/shoenig/test"
 	"github.com/shoenig/test/must"
+	"github.com/shoenig/test/wait"
 )
 
 func TestFS_Logs(t *testing.T) {
+	testutil.RequireRoot(t)
 	testutil.Parallel(t)
 
 	c, s := makeClient(t, nil, func(c *testutil.TestServerConfig) {
@@ -25,32 +26,14 @@ func TestFS_Logs(t *testing.T) {
 	})
 	defer s.Stop()
 
-	index := uint64(0)
-	testutil.WaitForResult(func() (bool, error) {
-		nodes, qm, err := c.Nodes().List(&QueryOptions{WaitIndex: index})
-		if err != nil {
-			return false, err
-		}
-		index = qm.LastIndex
-		if len(nodes) != 1 {
-			return false, fmt.Errorf("expected 1 node but found: %s", pretty.Sprint(nodes))
-		}
-		if nodes[0].Status != "ready" {
-			return false, fmt.Errorf("node not ready: %s", nodes[0].Status)
-		}
-		if _, ok := nodes[0].Drivers["mock_driver"]; !ok {
-			return false, errors.New("mock_driver not ready")
-		}
-		return true, nil
-	}, func(err error) {
-		t.Fatalf("err: %v", err)
-	})
+	node := oneNodeFromNodeList(t, c.Nodes())
+	index := node.ModifyIndex
 
 	var input strings.Builder
 	input.Grow(units.MB)
 	lines := 80 * units.KB
 	for i := 0; i < lines; i++ {
-		fmt.Fprintf(&input, "%d\n", i)
+		_, _ = fmt.Fprintf(&input, "%d\n", i)
 	}
 
 	job := &Job{
@@ -79,41 +62,46 @@ func TestFS_Logs(t *testing.T) {
 	must.NoError(t, err)
 
 	index = jobResp.EvalCreateIndex
-	evals := c.Evaluations()
-	testutil.WaitForResult(func() (bool, error) {
-		evalResp, qm, err := evals.Info(jobResp.EvalID, &QueryOptions{WaitIndex: index})
+	evaluations := c.Evaluations()
+
+	f := func() error {
+		resp, qm, err := evaluations.Info(jobResp.EvalID, &QueryOptions{WaitIndex: index})
 		if err != nil {
-			return false, err
-		}
-		if evalResp.BlockedEval != "" {
-			t.Fatalf("Eval blocked: %s", pretty.Sprint(evalResp))
+			return fmt.Errorf("failed to get evaluation info: %w", err)
 		}
+		must.Eq(t, "", resp.BlockedEval)
 		index = qm.LastIndex
-		if evalResp.Status != "complete" {
-			return false, fmt.Errorf("eval status: %v", evalResp.Status)
+		if resp.Status != "complete" {
+			return fmt.Errorf("evaluation status is not complete, got: %s", resp.Status)
 		}
-		return true, nil
-	}, func(err error) {
-		t.Fatalf("err: %v", err)
-	})
+		return nil
+	}
+	must.Wait(t, wait.InitialSuccess(
+		wait.ErrorFunc(f),
+		wait.Timeout(10*time.Second),
+		wait.Gap(1*time.Second),
+	))
 
 	allocID := ""
-	testutil.WaitForResult(func() (bool, error) {
+	g := func() error {
 		allocs, _, err := jobs.Allocations(*job.ID, true, &QueryOptions{WaitIndex: index})
 		if err != nil {
-			return false, err
+			return fmt.Errorf("failed to get allocations: %w", err)
 		}
-		if len(allocs) != 1 {
-			return false, fmt.Errorf("unexpected number of allocs: %d", len(allocs))
+		if n := len(allocs); n != 1 {
+			return fmt.Errorf("expected 1 allocation, got: %d", n)
 		}
 		if allocs[0].ClientStatus != "complete" {
-			return false, fmt.Errorf("alloc not complete: %s", allocs[0].ClientStatus)
+			return fmt.Errorf("allocation not complete: %s", allocs[0].ClientStatus)
 		}
 		allocID = allocs[0].ID
-		return true, nil
-	}, func(err error) {
-		t.Fatalf("err: %v", err)
-	})
+		return nil
+	}
+	must.Wait(t, wait.InitialSuccess(
+		wait.ErrorFunc(g),
+		wait.Timeout(10*time.Second),
+		wait.Gap(1*time.Second),
+	))
 
 	alloc, _, err := c.Allocations().Info(allocID, nil)
 	must.NoError(t, err)
@@ -135,7 +123,7 @@ func TestFS_Logs(t *testing.T) {
 				result.Write(f.Data)
 			case err := <-errors:
 				// Don't Fatal here as the other assertions may
-				// contain helpeful information.
+				// contain helpful information.
 				t.Errorf("Error: %v", err)
 			}
 		}

api/internal/testutil/server.go

@@ -275,21 +275,25 @@ func (s *TestServer) Stop() {
 // responding. This is an indication that the agent has started,
 // but will likely return before a leader is elected.
 func (s *TestServer) waitForAPI() {
-	WaitForResult(func() (bool, error) {
-		// Using this endpoint as it is does not have restricted access
+	f := func() error {
 		resp, err := s.HTTPClient.Get(s.url("/v1/metrics"))
 		if err != nil {
-			return false, err
+			return fmt.Errorf("failed to get metrics: %w", err)
 		}
-		defer resp.Body.Close()
-		if err := s.requireOK(resp); err != nil {
-			return false, err
+		defer func() { _ = resp.Body.Close() }()
+		if err = s.requireOK(resp); err != nil {
+			return fmt.Errorf("metrics response is not ok: %w", err)
 		}
-		return true, nil
-	}, func(err error) {
-		defer s.Stop()
-		s.t.Fatalf("err: %s", err)
-	})
+		return nil
+	}
+	test.Wait(s.t,
+		wait.InitialSuccess(
+			wait.ErrorFunc(f),
+			wait.Timeout(10*time.Second),
+			wait.Gap(1*time.Second),
+		),
+		must.Sprint("failed to wait for api"),
+	)
 }
 
 // waitForLeader waits for the Nomad server's HTTP API to become
@@ -309,7 +313,6 @@ func (s *TestServer) waitForLeader() {
 		}
 		return nil
 	}
-
 	test.Wait(s.t,
 		wait.InitialSuccess(
 			wait.ErrorFunc(f),
@@ -318,8 +321,6 @@ func (s *TestServer) waitForLeader() {
 		),
 		must.Sprint("failed to wait for leader"),
 	)
-
-	// todo(shoenig): should be able to stop s on failure
 }
 
 // waitForClient waits for the Nomad client to be ready. The function returns
@@ -328,36 +329,32 @@ func (s *TestServer) waitForClient() {
 	if !s.Config.DevMode {
 		return
 	}
-
-	WaitForResult(func() (bool, error) {
+	f := func() error {
 		resp, err := s.HTTPClient.Get(s.url("/v1/nodes"))
 		if err != nil {
-			return false, err
+			return fmt.Errorf("failed to get nodes: %w", err)
 		}
-		defer resp.Body.Close()
-		if err := s.requireOK(resp); err != nil {
-			return false, err
+		defer func() { _ = resp.Body.Close() }()
+		if err = s.requireOK(resp); err != nil {
+			return fmt.Errorf("nodes response not ok: %w", err)
 		}
-
 		var decoded []struct {
 			ID     string
 			Status string
 		}
-
-		dec := json.NewDecoder(resp.Body)
-		if err := dec.Decode(&decoded); err != nil {
-			return false, err
+		if err = json.NewDecoder(resp.Body).Decode(&decoded); err != nil {
+			return fmt.Errorf("failed to decode nodes response: %w", err)
 		}
-
-		if len(decoded) != 1 || decoded[0].Status != "ready" {
-			return false, fmt.Errorf("Node not ready: %v", decoded)
-		}
-
-		return true, nil
-	}, func(err error) {
-		defer s.Stop()
-		s.t.Fatalf("err: %s", err)
-	})
+		return nil
+	}
+	test.Wait(s.t,
+		wait.InitialSuccess(
+			wait.ErrorFunc(f),
+			wait.Timeout(10*time.Second),
+			wait.Gap(1*time.Second),
+		),
+		must.Sprint("failed to wait for client (node)"),
+	)
 }
 
 // url is a helper function which takes a relative URL and
@@ -369,7 +366,7 @@ func (s *TestServer) url(path string) string {
 // requireOK checks the HTTP response code and ensures it is acceptable.
 func (s *TestServer) requireOK(resp *http.Response) error {
 	if resp.StatusCode != 200 {
-		return fmt.Errorf("Bad status code: %d", resp.StatusCode)
+		return fmt.Errorf("bad status code: %d", resp.StatusCode)
 	}
 	return nil
 }
@@ -377,16 +374,14 @@ func (s *TestServer) requireOK(resp *http.Response) error {
 // put performs a new HTTP PUT request.
 func (s *TestServer) put(path string, body io.Reader) *http.Response {
 	req, err := http.NewRequest("PUT", s.url(path), body)
-	if err != nil {
-		s.t.Fatalf("err: %s", err)
-	}
+	must.NoError(s.t, err)
+
 	resp, err := s.HTTPClient.Do(req)
-	if err != nil {
-		s.t.Fatalf("err: %s", err)
-	}
-	if err := s.requireOK(resp); err != nil {
-		defer resp.Body.Close()
-		s.t.Fatal(err)
+	must.NoError(s.t, err)
+
+	if err = s.requireOK(resp); err != nil {
+		_ = resp.Body.Close()
+		must.NoError(s.t, err)
 	}
 	return resp
 }
@@ -394,23 +389,20 @@ func (s *TestServer) put(path string, body io.Reader) *http.Response {
 // get performs a new HTTP GET request.
 func (s *TestServer) get(path string) *http.Response {
 	resp, err := s.HTTPClient.Get(s.url(path))
-	if err != nil {
-		s.t.Fatalf("err: %s", err)
-	}
-	if err := s.requireOK(resp); err != nil {
-		defer resp.Body.Close()
-		s.t.Fatal(err)
+	must.NoError(s.t, err)
+
+	if err = s.requireOK(resp); err != nil {
+		_ = resp.Body.Close()
+		must.NoError(s.t, err)
 	}
 	return resp
 }
 
 // encodePayload returns a new io.Reader wrapping the encoded contents
 // of the payload, suitable for passing directly to a new request.
-func (s *TestServer) encodePayload(payload interface{}) io.Reader {
+func (s *TestServer) encodePayload(payload any) io.Reader {
 	var encoded bytes.Buffer
-	enc := json.NewEncoder(&encoded)
-	if err := enc.Encode(payload); err != nil {
-		s.t.Fatalf("err: %s", err)
-	}
+	err := json.NewEncoder(&encoded).Encode(payload)
+	must.NoError(s.t, err)
 	return &encoded
 }

api/internal/testutil/slow.go

@@ -3,6 +3,7 @@ package testutil
 import (
 	"os"
 	"strconv"
+	"syscall"
 	"testing"
 )
 
@@ -23,3 +24,9 @@ func SkipSlow(t *testing.T, reason string) {
 func Parallel(t *testing.T) {
 	t.Parallel() // :)
 }
+
+func RequireRoot(t *testing.T) {
+	if syscall.Getuid() != 0 {
+		t.Skip("test requires root")
+	}
+}

api/internal/testutil/wait.go

@@ -1,74 +0,0 @@
-package testutil
-
-import (
-	"os"
-	"time"
-)
-
-type testFn func() (bool, error)
-type errorFn func(error)
-
-func WaitForResult(test testFn, error errorFn) {
-	WaitForResultRetries(500*TestMultiplier(), test, error)
-}
-
-func WaitForResultRetries(retries int64, test testFn, error errorFn) {
-	for retries > 0 {
-		time.Sleep(10 * time.Millisecond)
-		retries--
-
-		success, err := test()
-		if success {
-			return
-		}
-
-		if retries == 0 {
-			error(err)
-		}
-	}
-}
-
-// AssertUntil asserts the test function passes throughout the given duration.
-// Otherwise error is called on failure.
-func AssertUntil(until time.Duration, test testFn, error errorFn) {
-	deadline := time.Now().Add(until)
-	for time.Now().Before(deadline) {
-		success, err := test()
-		if !success {
-			error(err)
-			return
-		}
-		// Sleep some arbitrary fraction of the deadline
-		time.Sleep(until / 30)
-	}
-}
-
-// TestMultiplier returns a multiplier for retries and waits given environment
-// the tests are being run under.
-func TestMultiplier() int64 {
-	if IsCI() {
-		return 4
-	}
-
-	return 1
-}
-
-// Timeout takes the desired timeout and increases it if running in Travis
-func Timeout(original time.Duration) time.Duration {
-	return original * time.Duration(TestMultiplier())
-}
-
-func IsCI() bool {
-	_, ok := os.LookupEnv("CI")
-	return ok
-}
-
-func IsTravis() bool {
-	_, ok := os.LookupEnv("TRAVIS")
-	return ok
-}
-
-func IsAppVeyor() bool {
-	_, ok := os.LookupEnv("APPVEYOR")
-	return ok
-}

api/jobs_test.go

@@ -1,6 +1,7 @@
 package api
 
 import (
+	"fmt"
 	"reflect"
 	"sort"
 	"strings"
@@ -9,6 +10,8 @@ import (
 
 	"github.com/hashicorp/nomad/api/internal/testutil"
 	"github.com/kr/pretty"
+	"github.com/shoenig/test/must"
+	"github.com/shoenig/test/wait"
 	"github.com/stretchr/testify/require"
 )
 
@@ -1789,55 +1792,55 @@ func TestJobs_ForceEvaluate(t *testing.T) {
 
 func TestJobs_PeriodicForce(t *testing.T) {
 	testutil.Parallel(t)
+
 	c, s := makeClient(t, nil, nil)
 	defer s.Stop()
+
 	jobs := c.Jobs()
 
 	// Force-eval on a nonexistent job fails
 	_, _, err := jobs.PeriodicForce("job1", nil)
-	if err == nil || !strings.Contains(err.Error(), "not found") {
-		t.Fatalf("expected not found error, got: %#v", err)
-	}
+	must.ErrorContains(t, err, "not found")
 
 	// Create a new job
 	job := testPeriodicJob()
 	_, _, err = jobs.Register(job, nil)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
+	must.NoError(t, err)
 
-	testutil.WaitForResult(func() (bool, error) {
+	f := func() error {
 		out, _, err := jobs.Info(*job.ID, nil)
-		if err != nil || out == nil || *out.ID != *job.ID {
-			return false, err
+		if err != nil {
+			return fmt.Errorf("failed to get jobs info: %w", err)
 		}
-		return true, nil
-	}, func(err error) {
-		t.Fatalf("err: %s", err)
-	})
+		if out == nil {
+			return fmt.Errorf("jobs info response is nil")
+		}
+		if *out.ID != *job.ID {
+			return fmt.Errorf("expected job ids to match, out: %s, job: %s", *out.ID, *job.ID)
+		}
+		return nil
+	}
+	must.Wait(t, wait.InitialSuccess(
+		wait.ErrorFunc(f),
+		wait.Timeout(10*time.Second),
+		wait.Gap(1*time.Second),
+	))
 
 	// Try force again
 	evalID, wm, err := jobs.PeriodicForce(*job.ID, nil)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
+	must.NoError(t, err)
+
 	assertWriteMeta(t, wm)
 
-	if evalID == "" {
-		t.Fatalf("empty evalID")
-	}
+	must.NotEq(t, "", evalID)
 
 	// Retrieve the eval
-	evals := c.Evaluations()
-	eval, qm, err := evals.Info(evalID, nil)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
+	evaluations := c.Evaluations()
+	eval, qm, err := evaluations.Info(evalID, nil)
+	must.NoError(t, err)
+
 	assertQueryMeta(t, qm)
-	if eval.ID == evalID {
-		return
-	}
-	t.Fatalf("evaluation %q missing", evalID)
+	must.Eq(t, eval.ID, evalID)
 }
 
 func TestJobs_Plan(t *testing.T) {

api/nodes_test.go

@@ -24,6 +24,9 @@ func queryNodeList(t *testing.T, nodes *Nodes) ([]*NodeListStub, *QueryMeta) {
 		if err != nil {
 			return fmt.Errorf("failed to list nodes: %w", err)
 		}
+		if len(nodeListStub) == 0 {
+			return fmt.Errorf("no nodes yet")
+		}
 		return nil
 	}
 

api/operator_metrics_test.go

@@ -9,7 +9,9 @@ import (
 
 func TestOperator_MetricsSummary(t *testing.T) {
 	testutil.Parallel(t)
-	c, s := makeClient(t, nil, nil)
+	c, s := makeClient(t, nil, func(c *testutil.TestServerConfig) {
+		c.DevMode = true
+	})
 	defer s.Stop()
 
 	operator := c.Operator()
@@ -33,6 +35,7 @@ func TestOperator_MetricsSummary(t *testing.T) {
 func TestOperator_Metrics_Prometheus(t *testing.T) {
 	testutil.Parallel(t)
 	c, s := makeClient(t, nil, func(c *testutil.TestServerConfig) {
+		c.DevMode = true
 		c.Telemetry = &testutil.Telemetry{PrometheusMetrics: true}
 	})
 	defer s.Stop()

api/regions_test.go

@@ -3,8 +3,11 @@ package api
 import (
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/nomad/api/internal/testutil"
+	"github.com/shoenig/test/must"
+	"github.com/shoenig/test/wait"
 )
 
 func TestRegionsList(t *testing.T) {
@@ -20,24 +23,28 @@ func TestRegionsList(t *testing.T) {
 	defer s2.Stop()
 
 	// Join the servers
-	if _, err := c2.Agent().Join(s1.SerfAddr); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	_, err := c2.Agent().Join(s1.SerfAddr)
+	must.NoError(t, err)
 
-	// Regions returned and sorted
-	testutil.WaitForResult(func() (bool, error) {
+	f := func() error {
 		regions, err := c1.Regions().List()
 		if err != nil {
-			return false, err
+			return fmt.Errorf("failed to get regions: %w", err)
 		}
 		if n := len(regions); n != 2 {
-			return false, fmt.Errorf("expected 2 regions, got: %d", n)
+			return fmt.Errorf("expected 2 regions, got %d", n)
 		}
-		if regions[0] != "regionA" || regions[1] != "regionB" {
-			return false, fmt.Errorf("bad: %#v", regions)
+		if regions[0] != "regionA" {
+			return fmt.Errorf("unexpected first region, got: %s", regions[0])
 		}
-		return true, nil
-	}, func(err error) {
-		t.Fatalf("err: %v", err)
-	})
+		if regions[1] != "regionB" {
+			return fmt.Errorf("unexpected second region, got: %s", regions[1])
+		}
+		return nil
+	}
+	must.Wait(t, wait.InitialSuccess(
+		wait.ErrorFunc(f),
+		wait.Timeout(10*time.Second),
+		wait.Gap(1*time.Second),
+	))
 }