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

Exponential backoff on api.LifetimeWatcher does not work #26382

Closed
andrewstucki opened this issue Apr 11, 2024 · 0 comments · Fixed by #26383
Closed

Exponential backoff on api.LifetimeWatcher does not work #26382

andrewstucki opened this issue Apr 11, 2024 · 0 comments · Fixed by #26383
Labels
bug Used to indicate a potential bug

Comments

@andrewstucki
Copy link
Contributor

Describe the bug
We were using the Vault LifetimeWatcher from the api package in an internal project and noticed an issue with the backoff behavior of token renewal that was causing a bunch of our tests to fail when we upgraded to a new version of Vault.

The bug is here:

var sleepDuration time.Duration
if errorBackoff == nil {
sleepDuration = r.calculateSleepDuration(remainingLeaseDuration, priorDuration)
} else if errorBackoff.NextBackOff() == backoff.Stop {
return err
}

sleepDuration appears to be the time.Duration used prior to re-running the renewal loop. In the case when errBackoff is nil, then a simple backoff duration is calculated based on the call to calculateSleepDuration. If errBackoff is not nil then sleepDuration is never set and the timeout in the following select block immediately fires again.

In our testing environment this was caught because our mock Vault server was returning an invalid response, so the renew operation was failing and we were getting an inordinate amount of immediate retries.

The fix is just refactoring the above block to capture the errBackoff.NextBackoff() value as sleepDuration. I'll open a PR shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants