LifetimeWatcher should retry renew failures until end of lease #11445
Conversation
|
@ncabatoff Any reason why you are using |
@andrejvanderzee We're already using v3 elsewhere in Vault and I didn't see anything revolutionary in v4 I wanted. |
Co-authored-by: Vishal Nayak <vishalnayak@users.noreply.github.com>
|
Thank you @ncabatoff and @vishalnayak for picking this up! |
| @@ -5,6 +5,7 @@ go 1.13 | |||
| replace github.com/hashicorp/vault/sdk => ../sdk | |||
|
|
|||
| require ( | |||
| github.com/cenkalti/backoff/v3 v3.0.0 | |||
There was a problem hiding this comment.
I'm not familiar with the library and changes between versions, but why not the latest version, v4?
There was a problem hiding this comment.
We're already using v3 elsewhere and IIRC the differences between the two versions are disruptive yet insignificant. Like adopting the new version would require changes elsewhere but doesn't fix any bugs or provide any new functionality. At least that's my recollection from when I looked into it.
| @@ -268,18 +268,20 @@ func (r *LifetimeWatcher) doRenew() error { | |||
| default: | |||
| } | |||
|
|
|||
| var leaseDuration time.Duration | |||
| var remainingLeaseDuration time.Duration | |||
| fallbackLeaseDuration := initialTime.Add(priorDuration).Sub(time.Now()) | |||
There was a problem hiding this comment.
One last thought, I think the value of this only gets used in one place now, so you could delete the variable to simplify a bit.
Co-authored-by: Andrej van der Zee <andrejvanderzee@gmail.com>
This is based on @andrejvanderzee's work in #11008.