WIP: Vault Agent: Add configuration for maximum connection attempts #8135
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kalafut
reviewed
Jan 10, 2020
command/agent.go
Outdated
| Name: "max-connection-attempts", | ||
| Target: &c.flagMaxConnectionAttempts, | ||
| Default: 0, | ||
| EnvVar: "VAULT_MAX_CONNECTION_ATTEMPTS", |
There was a problem hiding this comment.
Why is connection attempts preferred over a timeout (duration) as a configuration? As a user I know what "60s" means, but I don't know how long a connection attempt takes.
There was a problem hiding this comment.
That's a fair question. The original request for this was for something akin to VAULT_MAX_RETRIES. A timeout could likely work just as well
There was a problem hiding this comment.
Timeout works for me!
kalafut
reviewed
Jan 17, 2020
| ah.logger.Info("starting auth handler") | ||
| defer func() { | ||
| am.Shutdown() | ||
| close(ah.OutputCh) | ||
| close(ah.DoneCh) | ||
| close(ah.TemplateTokenCh) | ||
| ah.logger.Info("auth handler stopped") | ||
| if connTimer != nil { | ||
| if !connTimer.Stop() { |
There was a problem hiding this comment.
I see this example in the Go docs but I don't really understand it, and it feels like we could hang in the channel receive?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WIP
Vault Agent can be configured to attempt to authenticate with a Vault cluster that is not responsive. When this occurs, the goroutine running the Auth Method can enter an infinite loop of attempting to connect and receiving tcp dial errors like so:
This repeats until either Vault becomes responsive or the Agent process is terminated. Note that this is different than a failed auth attempt or responses from a sealed Vault; the dial error occurs when no connection can be made at the tcp layer. Because of that fact the existing
VAULT_CLIENT_TIMEOUTandVAULT_MAX_RETRIESconfiguration options are never even considered, and Agent attempts to authenticate indefinitely.This pull request adds an additional configuration,
max-connection-attemptswhich is specific to Vault Agent. This option can limit the number of failed connection attempts before exiting the Agent process. The default is0, which indicates "unlimited", so that the current behavior is unchanged and this change is backwards compatible.The maximum connection attempts can be set with the
-max-connection-attemptscommand line flag, theVAULT_MAX_CONNECTION_ATTEMPTSenvironment variable, or themax_connection_attemptsconfiguration variable, in that order of precedence.Remaining items: