From 2738a1cc67e71c5cb21dcc8aee673c9b7c59b23a Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Thu, 8 Mar 2018 16:53:54 -0600 Subject: [PATCH 1/4] Retry when vault is sealed --- nomad/vault.go | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/nomad/vault.go b/nomad/vault.go index 4b4fb55563c..94ac7c8253a 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -7,6 +7,7 @@ import ( "log" "math/rand" "regexp" + "strings" "sync" "sync/atomic" "time" @@ -406,7 +407,7 @@ 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 { @@ -414,27 +415,37 @@ OUTER: 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) - retryTimer.Reset(v.config.ConnectionRetryIntv) - continue OUTER + 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 + } + // Retrieve our token, validate it and parse the lease duration + if err := v.parseSelfToken(); err != nil { + // Retry if vault is sealed + //TODO(preetha) : is there a nicer way to figure out whether vault is sealed + if strings.Contains(err.Error(), "is sealed") { + v.logger.Printf("[ERR] vault: failed to validate self token/role. Retrying in %v: %v", v.config.ConnectionRetryIntv, err) + retryTimer.Reset(v.config.ConnectionRetryIntv) + continue OUTER + } else { + // Irrecoverable error so we return + 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 + } } - 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()) From 208cce803951ed47d0a4251d2118565075cb0ccc Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Thu, 8 Mar 2018 20:27:49 -0600 Subject: [PATCH 2/4] Always retry on token validation instead of special casing vault sealing --- nomad/vault.go | 37 ++++++++++++++----------------------- nomad/vault_test.go | 17 ++++++++--------- 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/nomad/vault.go b/nomad/vault.go index 94ac7c8253a..aecfb3665fa 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -7,7 +7,6 @@ import ( "log" "math/rand" "regexp" - "strings" "sync" "sync/atomic" "time" @@ -424,23 +423,15 @@ OUTER: } initStatus = true } - // Retrieve our token, validate it and parse the lease duration + // Retry parsing the token till success if err := v.parseSelfToken(); err != nil { - // Retry if vault is sealed - //TODO(preetha) : is there a nicer way to figure out whether vault is sealed - if strings.Contains(err.Error(), "is sealed") { - v.logger.Printf("[ERR] vault: failed to validate self token/role. Retrying in %v: %v", v.config.ConnectionRetryIntv, err) - retryTimer.Reset(v.config.ConnectionRetryIntv) - continue OUTER - } else { - // Irrecoverable error so we return - 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 - } + 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("Connection to Vault failed: %v", err) + v.l.Unlock() + continue OUTER } break OUTER } @@ -855,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 @@ -933,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 @@ -1052,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 index aef0508b3f0..a5c904e9f46 100644 --- 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") From 646780cde68c6ab00dbf07d3e38bf34f62df7b3c Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Fri, 9 Mar 2018 08:56:54 -0600 Subject: [PATCH 3/4] update comment --- nomad/vault.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nomad/vault.go b/nomad/vault.go index aecfb3665fa..0c0a5a0afe5 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -423,7 +423,7 @@ OUTER: } initStatus = true } - // Retry parsing the token till success + // 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) From d50338a9f5cea837ef522e8e3b4bb92fa1bf0981 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Fri, 9 Mar 2018 14:25:53 -0600 Subject: [PATCH 4/4] Update error message --- nomad/vault.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nomad/vault.go b/nomad/vault.go index 0c0a5a0afe5..7f7b6bf6b44 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -429,7 +429,7 @@ OUTER: retryTimer.Reset(v.config.ConnectionRetryIntv) v.l.Lock() v.connEstablished = true - v.connEstablishedErr = fmt.Errorf("Connection to Vault failed: %v", err) + v.connEstablishedErr = fmt.Errorf("Nomad Server failed to establish connections to Vault: %v", err) v.l.Unlock() continue OUTER }