-
Notifications
You must be signed in to change notification settings - Fork 2k
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: fix renewal time #5479
vault: fix renewal time #5479
Conversation
LGTM! |
516def8
to
48dc7c0
Compare
90e3f3c
to
821e0a4
Compare
Renewal time was being calculated as 10s+Intn(lease-10s), so the renewal time could be very rapid or within 1s of the deadline: [10s, lease) This commit fixes the renewal time by calculating it as: (lease/2) +/- 10s For a lease of 60s this means the renewal will occur in [20s, 40s).
821e0a4
to
888304b
Compare
@schmichael is this also a bug in the 0.8.x line? |
@jippi Yes, unfortunately this bug goes wwwwwway back. |
@schmichael any chance to backport to 0.8? we've been suffering quite a lot from Nomad<>Vault nomad-servers & allocs losing their credentials to DBs and similar, and this seem like a pretty good bet on that issue. I would estimate our issues has caused 4-7h of total downtime in prod last 2 years |
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Renewal time was being calculated as 10s+Intn(lease-10s), so the renewal
time could be very rapid or within 1s of the deadline: [10s, lease)
This commit fixes the renewal time by calculating it as:
For a lease of 60s this means the renewal will occur in [20s, 40s).
Fixes #5471