Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vault agent - respect consul-template retry configuration #9059

Closed
wants to merge 1 commit into from

Conversation

clatour
Copy link

@clatour clatour commented May 21, 2020

The consul template retry stanza from configuration was not being passed
through appropriately, this should fix that.

Since a duration is not a valid HCL type, the swap from a simple hcl
decode had to be changed to mapstructure.

The consul template retry stanza from configuration was not being passed
through appropriately, this should fix that.

Since a duration is not a valid HCL type, the swap from a simple hcl
decode had to be changed to `mapstructure`.
@hashicorp-cla
Copy link

hashicorp-cla commented May 21, 2020

CLA assistant check
All committers have signed the CLA.

@ncabatoff
Copy link
Collaborator

Hi @clatour,

Thanks for the PR!

I'm not too familiar with Agent's use of templating, but I suspect this may not fully address the original issue. A brief look at the code suggests that the consul-template lib's connection to Vault is managed independently of Agent's auto-auth code. So even if you make consul-template give up trying to talk to Vault via this config, Agent itself would still be spinning its wheels trying to talk to Vault. There's an incomplete PR (#8135) that tried to address that other bit.

Have you tested that this fix solves #6001 for you?

@clatour
Copy link
Author

clatour commented May 26, 2020

Hi @ncabatoff, on second glance, I think I may have linked the wrong issue. This patch does not alleviate the retry issue when the host is invalid or unreachable, only respects the retry configuration block for the vault agent configuration.

@avoidik
Copy link
Contributor

avoidik commented May 29, 2020

hi @clatour there are two issues at the moment with retry-logic in vault agent:

  1. an issue when the consul rendering server inside the vault agent service have failed to render templates and was terminated, but vault-agent service itself left in a half-working state (e.g. auto-auth works, rendering templates don't), and the only solution is to restart it vault agent - command execution over 30s disables template server completely #9108
  2. vault not available due to sealed, not initialized, so vault agent pushes the maximum exceeded attempts also switching in to a half-working state where the only solution is to restart it Vault Agent: Add option to wait for sealed=false, intialized=true #8970

theoretically, this two are the events to trigger restart of vault-agent conditionally, but retry may help to resolve them unconditionally without any sophisticated logic implemented

@clatour clatour changed the title vault agent - respect retry configuration vault agent - respect consul-template retry configuration May 29, 2020
@clatour
Copy link
Author

clatour commented May 29, 2020

Hi @avoidik,

I have edited my initial comment (and title) to remove the link to the outstanding vault agent retry issue. This is solely a patch to respect some configuration that is supported by consul template, and was documented as supported in vault-agent, but did not look to be wired through.

Currently the only instance in which I am using vault-agent is rendering templates as part of a deployment, where I’d like vault-agent to quickly set up, render template(s), and tear down. Similar to the idea of consul-templates once flag. So I’m not super familiar with any of the other retry mechanisms that exist when running vault-agent as a daemon.

What this solves for my use case is largely when a secret doesn’t exist or the template can’t otherwise be rendered, I’d like the consul-template component to allow itself to be configured to fail faster. Which I think is more related to your first point, but as-implemented would just cause the template rendering to be able to be shut down after a configurable retry, and not actually helping the retry/restart of that component overall.

@VioletHynes
Copy link
Contributor

As this PR is older and cannot currently be merged due to merge conflicts, I’m going to go ahead and close it. Please feel free to open a new PR if your desired changes are still applicable!

However, additionally, I think there's a good chance the problem you're trying to solve was solved here: #16970

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants