-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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-6575 Vault agent respects retry config even with caching set #16970
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just dropped a minor question.
I did some additional testing with a K8S persistent cache enabled just to be extra 100% comfy, i.e. Agent running in K8S with the following config:
Caching and retries both seem to be working as expected, so I'm especially encouraged that this works as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Note: this PR closes #15725 |
I believe that the reason we ignored the retry config is because the cache's client does its own set of retries, which can be set via |
Anecdotally (I could be missing something?), through testing it, this does not work (and this was reported as not working by a customer, too). Setting this environment variable does not seem to do anything to affect retries in Agent, whether or not caching is turned on or off. |
Ah that's right. When caching is enabled, templating hands off retries to that part of Agent since we recently made templating requests route through the cache's listener instead of to Vault directly (to take advantage of Agent's caching capability and reduce number of template requests against Vault). |
It seems like ignoring retry config with caching set was originally intentional behaviour, but it definitely doesn't seem right to me nor I guess the many, many customers that have reported it as a bug.
With this change, Agent will respect the retry settings, regardless of whether caching is enabled (in other words, enabling caching will have the same retry behaviour as not doing). Note that it will still continue to check the cache and the layering still works fine.
For those interested (as we're new to Agent on Core), I used the following config HCL for this change (I modified bits to reproduce different cases, of course):