From 9010b0e0f4bdf2c733c73ac03bd3c4e64c3dbbad Mon Sep 17 00:00:00 2001
From: Seth Vargo <sethvargo@gmail.com>
Date: Thu, 1 Dec 2016 17:24:26 +0200
Subject: [PATCH] Use a random port instead of idx in testutil

The testutil server uses an atomic incrementer to generate unique port
numbers. This works great until tests are run in parallel, _across
packages_. Because each package starts at the same "offset" idx, they
collide.

One way to overcome this is to run each packages' test in isolation, but
that makes the test suite much longer as it does not maximize
parallelization. Alternatively, instead of having "predictable" ports,
we can let the OS choose a random open port automatically.

This still has a (albeit smaller) race condition in that the OS could
return an open port twice, before the server has a chance to actually
start and occupy said port. In practice, I have not been able to hit
this race condition, so it either doesn't happen or it happens far less
frequently that the existing implementation.

I'm not sure how I feel about the panic, but this is just test code, so
I'm including to say it's okay?
---
 testutil/server.go | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/testutil/server.go b/testutil/server.go
index 727f1088e8..8ab196eca2 100644
--- a/testutil/server.go
+++ b/testutil/server.go
@@ -23,15 +23,11 @@ import (
 	"os"
 	"os/exec"
 	"strings"
-	"sync/atomic"
 
 	"github.com/hashicorp/consul/consul/structs"
 	"github.com/hashicorp/go-cleanhttp"
 )
 
-// offset is used to atomically increment the port numbers.
-var offset uint64
-
 // TestPerformanceConfig configures the performance parameters.
 type TestPerformanceConfig struct {
 	RaftMultiplier uint `json:"raft_multiplier,omitempty"`
@@ -82,10 +78,8 @@ type ServerConfigCallback func(c *TestServerConfig)
 // defaultServerConfig returns a new TestServerConfig struct
 // with all of the listen ports incremented by one.
 func defaultServerConfig() *TestServerConfig {
-	idx := int(atomic.AddUint64(&offset, 1))
-
 	return &TestServerConfig{
-		NodeName:          fmt.Sprintf("node%d", idx),
+		NodeName:          fmt.Sprintf("node%d", randomPort()),
 		DisableCheckpoint: true,
 		Performance: &TestPerformanceConfig{
 			RaftMultiplier: 1,
@@ -96,16 +90,26 @@ func defaultServerConfig() *TestServerConfig {
 		Bind:      "127.0.0.1",
 		Addresses: &TestAddressConfig{},
 		Ports: &TestPortConfig{
-			DNS:     20000 + idx,
-			HTTP:    21000 + idx,
-			RPC:     22000 + idx,
-			SerfLan: 23000 + idx,
-			SerfWan: 24000 + idx,
-			Server:  25000 + idx,
+			DNS:     randomPort(),
+			HTTP:    randomPort(),
+			RPC:     randomPort(),
+			SerfLan: randomPort(),
+			SerfWan: randomPort(),
+			Server:  randomPort(),
 		},
 	}
 }
 
+// randomPort asks the kernel for a random port to use.
+func randomPort() int {
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		panic(err)
+	}
+	defer l.Close()
+	return l.Addr().(*net.TCPAddr).Port
+}
+
 // TestService is used to serialize a service definition.
 type TestService struct {
 	ID      string   `json:",omitempty"`