Merge pull request #3957 from hashicorp/b-vault-retry

Retry validating token from vault till success
hashicorp · Mar 9, 2018 · 14afd04 · 14afd04
2 parents bc4ae22 + d50338a
commit 14afd04
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 30 deletions.
diff --git a/nomad/vault.go b/nomad/vault.go
@@ -406,35 +406,37 @@ func (v *vaultClient) establishConnection() {
 	// Create the retry timer and set initial duration to zero so it fires
 	// immediately
 	retryTimer := time.NewTimer(0)
-
+	initStatus := false
 OUTER:
 	for {
 		select {
 		case <-v.tomb.Dying():
 			return
 		case <-retryTimer.C:
 			// Ensure the API is reachable
-			if _, err := v.client.Sys().InitStatus(); err != nil {
-				v.logger.Printf("[WARN] vault: failed to contact Vault API. Retrying in %v: %v",
-					v.config.ConnectionRetryIntv, err)
+			if !initStatus {
+				if _, err := v.client.Sys().InitStatus(); err != nil {
+					v.logger.Printf("[WARN] vault: failed to contact Vault API. Retrying in %v: %v",
+						v.config.ConnectionRetryIntv, err)
+					retryTimer.Reset(v.config.ConnectionRetryIntv)
+					continue OUTER
+				}
+				initStatus = true
+			}
+			// Retry validating the token till success
+			if err := v.parseSelfToken(); err != nil {
+				v.logger.Printf("[ERR] vault: failed to validate self token/role. Retrying in %v: %v", v.config.ConnectionRetryIntv, err)
 				retryTimer.Reset(v.config.ConnectionRetryIntv)
+				v.l.Lock()
+				v.connEstablished = true
+				v.connEstablishedErr = fmt.Errorf("Nomad Server failed to establish connections to Vault: %v", err)
+				v.l.Unlock()
 				continue OUTER
 			}
-
 			break OUTER
 		}
 	}
 
-	// Retrieve our token, validate it and parse the lease duration
-	if err := v.parseSelfToken(); err != nil {
-		v.logger.Printf("[ERR] vault: failed to validate self token/role and not retrying: %v", err)
-		v.l.Lock()
-		v.connEstablished = false
-		v.connEstablishedErr = err
-		v.l.Unlock()
-		return
-	}
-
 	// Set the wrapping function such that token creation is wrapped now
 	// that we know our role
 	v.client.SetWrappingLookupFunc(v.getWrappingFn())
@@ -844,8 +846,8 @@ func (v *vaultClient) CreateToken(ctx context.Context, a *structs.Allocation, ta
 	// Check if we have established a connection with Vault
 	if established, err := v.ConnectionEstablished(); !established && err == nil {
 		return nil, structs.NewRecoverableError(fmt.Errorf("Connection to Vault has not been established"), true)
-	} else if !established {
-		return nil, fmt.Errorf("Connection to Vault failed: %v", err)
+	} else if err != nil {
+		return nil, err
 	}
 
 	// Track how long the request takes
@@ -922,8 +924,8 @@ func (v *vaultClient) LookupToken(ctx context.Context, token string) (*vapi.Secr
 	// Check if we have established a connection with Vault
 	if established, err := v.ConnectionEstablished(); !established && err == nil {
 		return nil, structs.NewRecoverableError(fmt.Errorf("Connection to Vault has not been established"), true)
-	} else if !established {
-		return nil, fmt.Errorf("Connection to Vault failed: %v", err)
+	} else if err != nil {
+		return nil, err
 	}
 
 	// Track how long the request takes
@@ -1041,8 +1043,8 @@ func (v *vaultClient) parallelRevoke(ctx context.Context, accessors []*structs.V
 	// Check if we have established a connection with Vault
 	if established, err := v.ConnectionEstablished(); !established && err == nil {
 		return structs.NewRecoverableError(fmt.Errorf("Connection to Vault has not been established"), true)
-	} else if !established {
-		return fmt.Errorf("Connection to Vault failed: %v", err)
+	} else if err != nil {
+		return err
 	}
 
 	g, pCtx := errgroup.WithContext(ctx)

diff --git a/nomad/vault_test.go b/nomad/vault_test.go
@@ -260,8 +260,8 @@ func TestVaultClient_ValidateRole(t *testing.T) {
 	var connErr error
 	testutil.WaitForResult(func() (bool, error) {
 		conn, connErr = client.ConnectionEstablished()
-		if conn {
-			return false, fmt.Errorf("Should not connect")
+		if !conn {
+			return false, fmt.Errorf("Should connect")
 		}
 
 		if connErr == nil {
@@ -303,8 +303,8 @@ func TestVaultClient_ValidateRole_NonExistant(t *testing.T) {
 	var connErr error
 	testutil.WaitForResult(func() (bool, error) {
 		conn, connErr = client.ConnectionEstablished()
-		if conn {
-			return false, fmt.Errorf("Should not connect")
+		if !conn {
+			return false, fmt.Errorf("Should connect")
 		}
 
 		if connErr == nil {
@@ -351,8 +351,8 @@ func TestVaultClient_ValidateToken(t *testing.T) {
 	var connErr error
 	testutil.WaitForResult(func() (bool, error) {
 		conn, connErr = client.ConnectionEstablished()
-		if conn {
-			return false, fmt.Errorf("Should not connect")
+		if !conn {
+			return false, fmt.Errorf("Should connect")
 		}
 
 		if connErr == nil {
@@ -967,10 +967,9 @@ func TestVaultClient_CreateToken_Role_InvalidToken(t *testing.T) {
 
 	testutil.WaitForResult(func() (bool, error) {
 		established, err := client.ConnectionEstablished()
-		if established {
-			return false, fmt.Errorf("Shouldn't establish")
+		if !established {
+			return false, fmt.Errorf("Should establish")
 		}
-
 		return err != nil, nil
 	}, func(err error) {
 		t.Fatalf("Connection not established")