From eec6bf24f745249636955e738e92a9a72b56da3b Mon Sep 17 00:00:00 2001
From: Alex Dadgar <alex.dadgar@gmail.com>
Date: Sun, 23 Jul 2017 16:21:25 -0700
Subject: [PATCH] Make test Vault pick random ports

---
 client/consul_template_test.go         |   2 +-
 client/fingerprint/vault_test.go       |   2 +-
 client/vaultclient/vaultclient_test.go |   2 +-
 nomad/vault_test.go                    | 120 ++++++++++++++++---------
 testutil/vault.go                      | 100 +++++++++++++++++++--
 5 files changed, 175 insertions(+), 51 deletions(-)

diff --git a/client/consul_template_test.go b/client/consul_template_test.go
index c8b453cab3..fcc3665a9b 100644
--- a/client/consul_template_test.go
+++ b/client/consul_template_test.go
@@ -136,7 +136,7 @@ func newTestHarness(t *testing.T, templates []*structs.Template, consul, vault b
 	}
 
 	if vault {
-		harness.vault = testutil.NewTestVault(t).Start()
+		harness.vault = testutil.NewTestVault(t)
 		harness.config.VaultConfig = harness.vault.Config
 		harness.vaultToken = harness.vault.RootToken
 	}
diff --git a/client/fingerprint/vault_test.go b/client/fingerprint/vault_test.go
index 2036b3e1f5..a6835b937d 100644
--- a/client/fingerprint/vault_test.go
+++ b/client/fingerprint/vault_test.go
@@ -9,7 +9,7 @@ import (
 )
 
 func TestVaultFingerprint(t *testing.T) {
-	tv := testutil.NewTestVault(t).Start()
+	tv := testutil.NewTestVault(t)
 	defer tv.Stop()
 
 	fp := NewVaultFingerprint(testLogger())
diff --git a/client/vaultclient/vaultclient_test.go b/client/vaultclient/vaultclient_test.go
index 455f4894d2..dc0b63aa85 100644
--- a/client/vaultclient/vaultclient_test.go
+++ b/client/vaultclient/vaultclient_test.go
@@ -13,7 +13,7 @@ import (
 
 func TestVaultClient_TokenRenewals(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	logger := log.New(os.Stderr, "TEST: ", log.Lshortfile|log.LstdFlags)
diff --git a/nomad/vault_test.go b/nomad/vault_test.go
index f099bd51e1..cdf87146db 100644
--- a/nomad/vault_test.go
+++ b/nomad/vault_test.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
+	"math/rand"
 	"os"
 	"reflect"
 	"strings"
@@ -13,6 +14,7 @@ import (
 
 	"golang.org/x/time/rate"
 
+	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/nomad/structs/config"
@@ -169,37 +171,66 @@ func TestVaultClient_BadConfig(t *testing.T) {
 	}
 }
 
+// started seperately.
 // Test that the Vault Client can establish a connection even if it is started
 // before Vault is available.
 func TestVaultClient_EstablishConnection(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t)
-	defer v.Stop()
+	for i := 10; i >= 0; i-- {
+		v := testutil.NewTestVaultDelayed(t)
+		logger := log.New(os.Stderr, "", log.LstdFlags)
+		v.Config.ConnectionRetryIntv = 100 * time.Millisecond
+		client, err := NewVaultClient(v.Config, logger, nil)
+		if err != nil {
+			t.Fatalf("failed to build vault client: %v", err)
+		}
 
-	logger := log.New(os.Stderr, "", log.LstdFlags)
-	v.Config.ConnectionRetryIntv = 100 * time.Millisecond
-	client, err := NewVaultClient(v.Config, logger, nil)
-	if err != nil {
-		t.Fatalf("failed to build vault client: %v", err)
-	}
-	defer client.Stop()
+		// Sleep a little while and check that no connection has been established.
+		time.Sleep(100 * time.Duration(testutil.TestMultiplier()) * time.Millisecond)
+		if established, _ := client.ConnectionEstablished(); established {
+			t.Fatalf("ConnectionEstablished() returned true before Vault server started")
+		}
 
-	// Sleep a little while and check that no connection has been established.
-	time.Sleep(100 * time.Duration(testutil.TestMultiplier()) * time.Millisecond)
+		// Start Vault
+		if err := v.Start(); err != nil {
+			v.Stop()
+			client.Stop()
 
-	if established, _ := client.ConnectionEstablished(); established {
-		t.Fatalf("ConnectionEstablished() returned true before Vault server started")
-	}
+			if i == 0 {
+				t.Fatalf("Failed to start vault: %v", err)
+			}
 
-	// Start Vault
-	v.Start()
+			wait := time.Duration(rand.Int31n(2000)) * time.Millisecond
+			time.Sleep(wait)
+			continue
+		}
 
-	waitForConnection(client, t)
+		var waitErr error
+		testutil.WaitForResult(func() (bool, error) {
+			return client.ConnectionEstablished()
+		}, func(err error) {
+			waitErr = err
+		})
+
+		v.Stop()
+		client.Stop()
+		if waitErr != nil {
+			if i == 0 {
+				t.Fatalf("Failed to start vault: %v", err)
+			}
+
+			wait := time.Duration(rand.Int31n(2000)) * time.Millisecond
+			time.Sleep(wait)
+			continue
+		}
+
+		break
+	}
 }
 
 func TestVaultClient_ValidateRole(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	// Set the configs token in a new test role
@@ -252,7 +283,7 @@ func TestVaultClient_ValidateRole(t *testing.T) {
 
 func TestVaultClient_ValidateRole_NonExistant(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	v.Config.Token = defaultTestVaultWhitelistRoleAndToken(v, t, 5)
@@ -292,7 +323,7 @@ func TestVaultClient_ValidateRole_NonExistant(t *testing.T) {
 
 func TestVaultClient_ValidateToken(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	// Set the configs token in a new test role
@@ -346,7 +377,7 @@ func TestVaultClient_ValidateToken(t *testing.T) {
 
 func TestVaultClient_SetActive(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	logger := log.New(os.Stderr, "", log.LstdFlags)
@@ -376,10 +407,10 @@ func TestVaultClient_SetActive(t *testing.T) {
 // Test that we can update the config and things keep working
 func TestVaultClient_SetConfig(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
-	v2 := testutil.NewTestVault(t).Start()
+	v2 := testutil.NewTestVault(t)
 	defer v2.Stop()
 
 	// Set the configs token in a new test role
@@ -413,7 +444,7 @@ func TestVaultClient_SetConfig(t *testing.T) {
 // Test that we can disable vault
 func TestVaultClient_SetConfig_Disable(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	logger := log.New(os.Stderr, "", log.LstdFlags)
@@ -447,7 +478,7 @@ func TestVaultClient_SetConfig_Disable(t *testing.T) {
 
 func TestVaultClient_RenewalLoop(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	// Set the configs token in a new test role
@@ -528,7 +559,7 @@ func TestVaultClient_LookupToken_Invalid(t *testing.T) {
 
 func TestVaultClient_LookupToken_Root(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	logger := log.New(os.Stderr, "", log.LstdFlags)
@@ -590,7 +621,7 @@ func TestVaultClient_LookupToken_Root(t *testing.T) {
 
 func TestVaultClient_LookupToken_Role(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	// Set the configs token in a new test role
@@ -655,7 +686,7 @@ func TestVaultClient_LookupToken_Role(t *testing.T) {
 
 func TestVaultClient_LookupToken_RateLimit(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	logger := log.New(os.Stderr, "", log.LstdFlags)
@@ -714,7 +745,7 @@ func TestVaultClient_LookupToken_RateLimit(t *testing.T) {
 
 func TestVaultClient_CreateToken_Root(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	logger := log.New(os.Stderr, "", log.LstdFlags)
@@ -758,7 +789,7 @@ func TestVaultClient_CreateToken_Root(t *testing.T) {
 
 func TestVaultClient_CreateToken_Whitelist_Role(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	// Set the configs token in a new test role
@@ -806,7 +837,7 @@ func TestVaultClient_CreateToken_Whitelist_Role(t *testing.T) {
 
 func TestVaultClient_CreateToken_Root_Target_Role(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	// Create the test role
@@ -867,7 +898,7 @@ func TestVaultClient_CreateToken_Blacklist_Role(t *testing.T) {
 		t.Skipf("Vault has a regression in v0.6.4 that this test hits")
 	}
 
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	// Set the configs token in a new test role
@@ -916,7 +947,7 @@ func TestVaultClient_CreateToken_Blacklist_Role(t *testing.T) {
 
 func TestVaultClient_CreateToken_Role_InvalidToken(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	// Set the configs token in a new test role
@@ -956,7 +987,7 @@ func TestVaultClient_CreateToken_Role_InvalidToken(t *testing.T) {
 
 func TestVaultClient_CreateToken_Role_Unrecoverable(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	// Set the configs token in a new test role
@@ -991,11 +1022,14 @@ func TestVaultClient_CreateToken_Role_Unrecoverable(t *testing.T) {
 
 func TestVaultClient_CreateToken_Prestart(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t)
-	defer v.Stop()
+	vconfig := &config.VaultConfig{
+		Enabled: helper.BoolToPtr(true),
+		Token:   structs.GenerateUUID(),
+		Addr:    "http://127.0.0.1:0",
+	}
 
 	logger := log.New(os.Stderr, "", log.LstdFlags)
-	client, err := NewVaultClient(v.Config, logger, nil)
+	client, err := NewVaultClient(vconfig, logger, nil)
 	if err != nil {
 		t.Fatalf("failed to build vault client: %v", err)
 	}
@@ -1021,9 +1055,13 @@ func TestVaultClient_CreateToken_Prestart(t *testing.T) {
 
 func TestVaultClient_RevokeTokens_PreEstablishs(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t)
+	vconfig := &config.VaultConfig{
+		Enabled: helper.BoolToPtr(true),
+		Token:   structs.GenerateUUID(),
+		Addr:    "http://127.0.0.1:0",
+	}
 	logger := log.New(os.Stderr, "", log.LstdFlags)
-	client, err := NewVaultClient(v.Config, logger, nil)
+	client, err := NewVaultClient(vconfig, logger, nil)
 	if err != nil {
 		t.Fatalf("failed to build vault client: %v", err)
 	}
@@ -1061,7 +1099,7 @@ func TestVaultClient_RevokeTokens_PreEstablishs(t *testing.T) {
 
 func TestVaultClient_RevokeTokens_Root(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	purged := 0
@@ -1126,7 +1164,7 @@ func TestVaultClient_RevokeTokens_Root(t *testing.T) {
 
 func TestVaultClient_RevokeTokens_Role(t *testing.T) {
 	t.Parallel()
-	v := testutil.NewTestVault(t).Start()
+	v := testutil.NewTestVault(t)
 	defer v.Stop()
 
 	// Set the configs token in a new test role
diff --git a/testutil/vault.go b/testutil/vault.go
index c6935e37e8..799bcef4dd 100644
--- a/testutil/vault.go
+++ b/testutil/vault.go
@@ -36,6 +36,92 @@ type TestVault struct {
 
 // NewTestVault returns a new TestVault instance that has yet to be started
 func NewTestVault(t *testing.T) *TestVault {
+	for i := 10; i >= 0; i-- {
+		port := getPort()
+		token := structs.GenerateUUID()
+		bind := fmt.Sprintf("-dev-listen-address=127.0.0.1:%d", port)
+		http := fmt.Sprintf("http://127.0.0.1:%d", port)
+		root := fmt.Sprintf("-dev-root-token-id=%s", token)
+
+		bin := "vault"
+		if runtime.GOOS == "windows" {
+			bin = "vault.exe"
+		}
+		cmd := exec.Command(bin, "server", "-dev", bind, root)
+		cmd.Stdout = os.Stdout
+		cmd.Stderr = os.Stderr
+
+		// Build the config
+		conf := vapi.DefaultConfig()
+		conf.Address = http
+
+		// Make the client and set the token to the root token
+		client, err := vapi.NewClient(conf)
+		if err != nil {
+			t.Fatalf("failed to build Vault API client: %v", err)
+		}
+		client.SetToken(token)
+
+		enable := true
+		tv := &TestVault{
+			cmd:       cmd,
+			t:         t,
+			Addr:      bind,
+			HTTPAddr:  http,
+			RootToken: token,
+			Client:    client,
+			Config: &config.VaultConfig{
+				Enabled: &enable,
+				Token:   token,
+				Addr:    http,
+			},
+		}
+
+		if err := tv.cmd.Start(); err != nil {
+			tv.t.Fatalf("failed to start vault: %v", err)
+		}
+
+		// Start the waiter
+		tv.waitCh = make(chan error, 1)
+		go func() {
+			err := tv.cmd.Wait()
+			tv.waitCh <- err
+		}()
+
+		// Ensure Vault started
+		var startErr error
+		select {
+		case startErr = <-tv.waitCh:
+		case <-time.After(time.Duration(500*TestMultiplier()) * time.Millisecond):
+		}
+
+		if startErr != nil && i == 0 {
+			t.Fatalf("failed to start vault: %v", startErr)
+		} else if startErr != nil {
+			wait := time.Duration(rand.Int31n(2000)) * time.Millisecond
+			time.Sleep(wait)
+			continue
+		}
+
+		waitErr := tv.waitForAPI()
+		if waitErr != nil && i == 0 {
+			t.Fatalf("failed to start vault: %v", waitErr)
+		} else if waitErr != nil {
+			wait := time.Duration(rand.Int31n(2000)) * time.Millisecond
+			time.Sleep(wait)
+			continue
+		}
+
+		return tv
+	}
+
+	return nil
+}
+
+// NewTestVaultDelayed returns a test Vault server that has not been started.
+// Start must be called and it is the callers responsibility to deal with any
+// port conflicts that may occur and retry accordingly.
+func NewTestVaultDelayed(t *testing.T) *TestVault {
 	port := getPort()
 	token := structs.GenerateUUID()
 	bind := fmt.Sprintf("-dev-listen-address=127.0.0.1:%d", port)
@@ -81,7 +167,7 @@ func NewTestVault(t *testing.T) *TestVault {
 
 // Start starts the test Vault server and waits for it to respond to its HTTP
 // API
-func (tv *TestVault) Start() *TestVault {
+func (tv *TestVault) Start() error {
 	if err := tv.cmd.Start(); err != nil {
 		tv.t.Fatalf("failed to start vault: %v", err)
 	}
@@ -96,12 +182,11 @@ func (tv *TestVault) Start() *TestVault {
 	// Ensure Vault started
 	select {
 	case err := <-tv.waitCh:
-		tv.t.Fatal(err.Error())
+		return err
 	case <-time.After(time.Duration(500*TestMultiplier()) * time.Millisecond):
 	}
 
-	tv.waitForAPI()
-	return tv
+	return tv.waitForAPI()
 }
 
 // Stop stops the test Vault server
@@ -120,7 +205,8 @@ func (tv *TestVault) Stop() {
 
 // waitForAPI waits for the Vault HTTP endpoint to start
 // responding. This is an indication that the agent has started.
-func (tv *TestVault) waitForAPI() {
+func (tv *TestVault) waitForAPI() error {
+	var waitErr error
 	WaitForResult(func() (bool, error) {
 		inited, err := tv.Client.Sys().InitStatus()
 		if err != nil {
@@ -128,9 +214,9 @@ func (tv *TestVault) waitForAPI() {
 		}
 		return inited, nil
 	}, func(err error) {
-		defer tv.Stop()
-		tv.t.Fatalf("err: %s", err)
+		waitErr = err
 	})
+	return waitErr
 }
 
 func getPort() int {
-- 
GitLab