Merge pull request #3434 from hashicorp/f-flaky

Fix flaky tests

GNUmakefile

@@ -221,15 +221,14 @@ test: ## Run the Nomad test suite and/or the Nomad UI test suite
 		fi
 
 .PHONY: test-nomad
-test-nomad: LOCAL_PACKAGES = $(shell go list ./... | grep -v '/vendor/')
 test-nomad: dev ## Run Nomad test suites
 	@echo "==> Running Nomad test suites:"
 	@NOMAD_TEST_RKT=1 \
 		go test \
 			-cover \
 			-timeout=900s \
 			-tags="nomad_test $(if $(HAS_LXC),lxc)" \
-			$(LOCAL_PACKAGES)
+			./...
 
 .PHONY: clean
 clean: GOPATH=$(shell go env GOPATH)

client/client_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/consul/lib/freeport"
 	memdb "github.com/hashicorp/go-memdb"
 	"github.com/hashicorp/nomad/client/config"
 	"github.com/hashicorp/nomad/client/fingerprint"
@@ -28,10 +29,6 @@ import (
 	ctestutil "github.com/hashicorp/nomad/client/testutil"
 )
 
-func getPort() int {
-	return 1030 + int(rand.Int31n(6440))
-}
-
 func testACLServer(t *testing.T, cb func(*nomad.Config)) (*nomad.Server, string, *structs.ACLToken) {
 	server, addr := testServer(t, func(c *nomad.Config) {
 		c.ACLEnabled = true
@@ -78,12 +75,13 @@ func testServer(t *testing.T, cb func(*nomad.Config)) (*nomad.Server, string) {
 	}
 
 	for i := 10; i >= 0; i-- {
+		ports := freeport.GetT(t, 2)
 		config.RPCAddr = &net.TCPAddr{
 			IP:   []byte{127, 0, 0, 1},
-			Port: getPort(),
+			Port: ports[0],
 		}
 		config.NodeName = fmt.Sprintf("Node %d", config.RPCAddr.Port)
-		config.SerfConfig.MemberlistConfig.BindPort = getPort()
+		config.SerfConfig.MemberlistConfig.BindPort = ports[1]
 
 		// Create server
 		server, err := nomad.NewServer(config, catalog, logger)

client/driver/docker_test.go

@@ -16,6 +16,7 @@ import (
 	"time"
 
 	docker "github.com/fsouza/go-dockerclient"
+	"github.com/hashicorp/consul/lib/freeport"
 	sockaddr "github.com/hashicorp/go-sockaddr"
 	"github.com/hashicorp/nomad/client/allocdir"
 	"github.com/hashicorp/nomad/client/config"
@@ -42,17 +43,12 @@ func dockerIsRemote(t *testing.T) bool {
 	return false
 }
 
-// Ports used by tests
-var (
-	docker_reserved = 2000 + int(rand.Int31n(10000))
-	docker_dynamic  = 2000 + int(rand.Int31n(10000))
-)
-
 // Returns a task with a reserved and dynamic port. The ports are returned
 // respectively.
-func dockerTask() (*structs.Task, int, int) {
-	docker_reserved += 1
-	docker_dynamic += 1
+func dockerTask(t *testing.T) (*structs.Task, int, int) {
+	ports := freeport.GetT(t, 2)
+	dockerReserved := ports[0]
+	dockerDynamic := ports[1]
 	return &structs.Task{
 		Name:   "redis-demo",
 		Driver: "docker",
@@ -72,12 +68,12 @@ func dockerTask() (*structs.Task, int, int) {
 			Networks: []*structs.NetworkResource{
 				{
 					IP:            "127.0.0.1",
-					ReservedPorts: []structs.Port{{Label: "main", Value: docker_reserved}},
-					DynamicPorts:  []structs.Port{{Label: "REDIS", Value: docker_dynamic}},
+					ReservedPorts: []structs.Port{{Label: "main", Value: dockerReserved}},
+					DynamicPorts:  []structs.Port{{Label: "REDIS", Value: dockerDynamic}},
 				},
 			},
 		},
-	}, docker_reserved, docker_dynamic
+	}, dockerReserved, dockerDynamic
 }
 
 // dockerSetup does all of the basic setup you need to get a running docker
@@ -563,9 +559,9 @@ func TestDockerDriver_StartN(t *testing.T) {
 		t.Skip("Docker not connected")
 	}
 
-	task1, _, _ := dockerTask()
-	task2, _, _ := dockerTask()
-	task3, _, _ := dockerTask()
+	task1, _, _ := dockerTask(t)
+	task2, _, _ := dockerTask(t)
+	task3, _, _ := dockerTask(t)
 	taskList := []*structs.Task{task1, task2, task3}
 
 	handles := make([]DriverHandle, len(taskList))
@@ -617,15 +613,15 @@ func TestDockerDriver_StartNVersions(t *testing.T) {
 		t.Skip("Docker not connected")
 	}
 
-	task1, _, _ := dockerTask()
+	task1, _, _ := dockerTask(t)
 	task1.Config["image"] = "busybox"
 	task1.Config["load"] = "busybox.tar"
 
-	task2, _, _ := dockerTask()
+	task2, _, _ := dockerTask(t)
 	task2.Config["image"] = "busybox:musl"
 	task2.Config["load"] = "busybox_musl.tar"
 
-	task3, _, _ := dockerTask()
+	task3, _, _ := dockerTask(t)
 	task3.Config["image"] = "busybox:glibc"
 	task3.Config["load"] = "busybox_glibc.tar"
 
@@ -795,7 +791,7 @@ func TestDockerDriver_Labels(t *testing.T) {
 		t.Skip("Docker not connected")
 	}
 
-	task, _, _ := dockerTask()
+	task, _, _ := dockerTask(t)
 	task.Config["labels"] = []map[string]string{
 		{
 			"label1": "value1",
@@ -830,7 +826,7 @@ func TestDockerDriver_ForcePull_IsInvalidConfig(t *testing.T) {
 		t.Skip("Docker not connected")
 	}
 
-	task, _, _ := dockerTask()
+	task, _, _ := dockerTask(t)
 	task.Config["force_pull"] = "nothing"
 
 	ctx := testDockerDriverContexts(t, task)
@@ -851,7 +847,7 @@ func TestDockerDriver_ForcePull(t *testing.T) {
 		t.Skip("Docker not connected")
 	}
 
-	task, _, _ := dockerTask()
+	task, _, _ := dockerTask(t)
 	task.Config["force_pull"] = "true"
 
 	client, handle, cleanup := dockerSetup(t, task)
@@ -873,7 +869,7 @@ func TestDockerDriver_SecurityOpt(t *testing.T) {
 		t.Skip("Docker not connected")
 	}
 
-	task, _, _ := dockerTask()
+	task, _, _ := dockerTask(t)
 	task.Config["security_opt"] = []string{"seccomp=unconfined"}
 
 	client, handle, cleanup := dockerSetup(t, task)
@@ -899,7 +895,7 @@ func TestDockerDriver_DNS(t *testing.T) {
 		t.Skip("Docker not connected")
 	}
 
-	task, _, _ := dockerTask()
+	task, _, _ := dockerTask(t)
 	task.Config["dns_servers"] = []string{"8.8.8.8", "8.8.4.4"}
 	task.Config["dns_search_domains"] = []string{"example.com", "example.org", "example.net"}
 	task.Config["dns_options"] = []string{"ndots:1"}
@@ -935,7 +931,7 @@ func TestDockerDriver_MACAddress(t *testing.T) {
 		t.Skip("Docker not connected")
 	}
 
-	task, _, _ := dockerTask()
+	task, _, _ := dockerTask(t)
 	task.Config["mac_address"] = "00:16:3e:00:00:00"
 
 	client, handle, cleanup := dockerSetup(t, task)
@@ -961,7 +957,7 @@ func TestDockerWorkDir(t *testing.T) {
 		t.Skip("Docker not connected")
 	}
 
-	task, _, _ := dockerTask()
+	task, _, _ := dockerTask(t)
 	task.Config["work_dir"] = "/some/path"
 
 	client, handle, cleanup := dockerSetup(t, task)
@@ -994,7 +990,7 @@ func TestDockerDriver_PortsNoMap(t *testing.T) {
 		t.Skip("Docker not connected")
 	}
 
-	task, res, dyn := dockerTask()
+	task, res, dyn := dockerTask(t)
 
 	client, handle, cleanup := dockerSetup(t, task)
 	defer cleanup()
@@ -1051,7 +1047,7 @@ func TestDockerDriver_PortsMapping(t *testing.T) {
 		t.Skip("Docker not connected")
 	}
 
-	task, res, dyn := dockerTask()
+	task, res, dyn := dockerTask(t)
 	task.Config["port_map"] = []map[string]string{
 		{
 			"main":  "8080",

client/driver/raw_exec_test.go

@@ -265,7 +265,7 @@ func TestRawExecDriver_HandlerExec(t *testing.T) {
 		Driver: "raw_exec",
 		Config: map[string]interface{}{
 			"command": testtask.Path(),
-			"args":    []string{"sleep", "9000"},
+			"args":    []string{"sleep", "9000s"},
 		},
 		LogConfig: &structs.LogConfig{
 			MaxFiles:      10,
@@ -310,6 +310,12 @@ func TestRawExecDriver_HandlerExec(t *testing.T) {
 		t.Fatalf("expected output to contain %q but found: %q", expected, out)
 	}
 
+	select {
+	case res := <-resp.Handle.WaitCh():
+		t.Fatalf("Shouldn't be exited: %v", res.String())
+	default:
+	}
+
 	if err := resp.Handle.Kill(); err != nil {
 		t.Fatalf("error killing exec handle: %v", err)
 	}

command/agent/agent_test.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"io/ioutil"
 	"log"
-	"net"
 	"os"
 	"strings"
 	"testing"
@@ -15,20 +14,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func getPort() int {
-	addr, err := net.ResolveTCPAddr("tcp", "127.0.0.1:0")
-	if err != nil {
-		panic(err)
-	}
-
-	l, err := net.ListenTCP("tcp", addr)
-	if err != nil {
-		panic(err)
-	}
-	defer l.Close()
-	return l.Addr().(*net.TCPAddr).Port
-}
-
 func tmpDir(t testing.TB) string {
 	dir, err := ioutil.TempDir("", "nomad")
 	if err != nil {
@@ -39,7 +24,7 @@ func tmpDir(t testing.TB) string {
 
 func TestAgent_RPCPing(t *testing.T) {
 	t.Parallel()
-	agent := NewTestAgent(t.Name(), nil)
+	agent := NewTestAgent(t, t.Name(), nil)
 	defer agent.Shutdown()
 
 	var out struct{}

command/agent/command_test.go

@@ -82,7 +82,7 @@ func TestCommand_Args(t *testing.T) {
 // TODO Why is this failing
 func TestRetryJoin(t *testing.T) {
 	t.Parallel()
-	agent := NewTestAgent(t.Name(), nil)
+	agent := NewTestAgent(t, t.Name(), nil)
 	defer agent.Shutdown()
 
 	doneCh := make(chan struct{})

command/agent/http_test.go

@@ -27,7 +27,7 @@ import (
 // makeHTTPServer returns a test server whose logs will be written to
 // the passed writer. If the writer is nil, the logs are written to stderr.
 func makeHTTPServer(t testing.TB, cb func(c *Config)) *TestAgent {
-	return NewTestAgent(t.Name(), cb)
+	return NewTestAgent(t, t.Name(), cb)
 }
 
 func BenchmarkHTTPRequests(b *testing.B) {

command/agent/keyring_test.go

@@ -13,7 +13,7 @@ func TestAgent_LoadKeyrings(t *testing.T) {
 	key := "tbLJg26ZJyJ9pK3qhc9jig=="
 
 	// Should be no configured keyring file by default
-	agent1 := NewTestAgent(t.Name(), nil)
+	agent1 := NewTestAgent(t, t.Name(), nil)
 	defer agent1.Shutdown()
 
 	c := agent1.server.GetConfig()
@@ -26,6 +26,7 @@ func TestAgent_LoadKeyrings(t *testing.T) {
 
 	// Server should auto-load WAN keyring files
 	agent2 := &TestAgent{
+		T:    t,
 		Name: t.Name() + "2",
 		Key:  key,
 	}

command/agent/metrics_endpoint_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	metrics "github.com/armon/go-metrics"
+	"github.com/hashicorp/nomad/testutil"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -41,12 +42,17 @@ func TestHTTP_Metrics(t *testing.T) {
 		assert.Nil(err)
 		respW = httptest.NewRecorder()
 
-		resp, err := s.Server.MetricsRequest(respW, req)
-		assert.Nil(err)
-
-		res := resp.(metrics.MetricsSummary)
-
-		gauges := res.Gauges
-		assert.NotEqual(0, len(gauges))
+		testutil.WaitForResult(func() (bool, error) {
+			resp, err := s.Server.MetricsRequest(respW, req)
+			if err != nil {
+				return false, err
+			}
+			respW.Flush()
+
+			res := resp.(metrics.MetricsSummary)
+			return len(res.Gauges) != 0, nil
+		}, func(err error) {
+			t.Fatalf("should have metrics: %v", err)
+		})
 	})
 }