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

Backport of Respect increment value in grace period calculations (api/LifetimeWatcher) into release/1.9.x #14938

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions api/lifetime_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ type LifetimeWatcherInput struct {

// The new TTL, in seconds, that should be set on the lease. The TTL set
// here may or may not be honored by the vault server, based on Vault
// configuration or any associated max TTL values.
// configuration or any associated max TTL values. If specified, the
// minimum of this value and the remaining lease duration will be used
// for grace period calculations.
Increment int

// RenewBehavior controls what happens when a renewal errors or the
Expand Down Expand Up @@ -257,7 +259,7 @@ func (r *LifetimeWatcher) doRenewWithOptions(tokenMode bool, nonRenewable bool,

initialTime := time.Now()
priorDuration := time.Duration(initLeaseDuration) * time.Second
r.calculateGrace(priorDuration)
r.calculateGrace(priorDuration, time.Duration(r.increment)*time.Second)
var errorBackoff backoff.BackOff

for {
Expand Down Expand Up @@ -345,7 +347,7 @@ func (r *LifetimeWatcher) doRenewWithOptions(tokenMode bool, nonRenewable bool,
// extending. Once it stops extending, we've hit the max and need to
// rely on the grace duration.
if remainingLeaseDuration > priorDuration {
r.calculateGrace(remainingLeaseDuration)
r.calculateGrace(remainingLeaseDuration, time.Duration(r.increment)*time.Second)
}
priorDuration = remainingLeaseDuration

Expand Down Expand Up @@ -373,16 +375,21 @@ func (r *LifetimeWatcher) doRenewWithOptions(tokenMode bool, nonRenewable bool,
}
}

// calculateGrace calculates the grace period based on a reasonable set of
// assumptions given the total lease time; it also adds some jitter to not have
// clients be in sync.
func (r *LifetimeWatcher) calculateGrace(leaseDuration time.Duration) {
if leaseDuration <= 0 {
// calculateGrace calculates the grace period based on the minimum of the
// remaining lease duration and the token increment value; it also adds some
// jitter to not have clients be in sync.
func (r *LifetimeWatcher) calculateGrace(leaseDuration, increment time.Duration) {
minDuration := leaseDuration
if minDuration > increment && increment > 0 {
minDuration = increment
}

if minDuration <= 0 {
r.grace = 0
return
}

leaseNanos := float64(leaseDuration.Nanoseconds())
leaseNanos := float64(minDuration.Nanoseconds())
jitterMax := 0.1 * leaseNanos

// For a given lease duration, we want to allow 80-90% of that to elapse,
Expand Down
105 changes: 58 additions & 47 deletions api/renewer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,28 +24,28 @@ func TestRenewer_NewRenewer(t *testing.T) {
err bool
}{
{
"nil",
nil,
nil,
true,
name: "nil",
i: nil,
e: nil,
err: true,
},
{
"missing_secret",
&RenewerInput{
name: "missing_secret",
i: &RenewerInput{
Secret: nil,
},
nil,
true,
e: nil,
err: true,
},
{
"default_grace",
&RenewerInput{
name: "default_grace",
i: &RenewerInput{
Secret: &Secret{},
},
&Renewer{
e: &Renewer{
secret: &Secret{},
},
false,
err: false,
},
}

Expand Down Expand Up @@ -98,67 +98,78 @@ func TestLifetimeWatcher(t *testing.T) {
expectRenewal bool
}{
{
time.Second,
"no_error",
60,
60,
func(_ string, _ int) (*Secret, error) {
maxTestTime: time.Second,
name: "no_error",
leaseDurationSeconds: 60,
incrementSeconds: 60,
renew: func(_ string, _ int) (*Secret, error) {
return renewedSecret, nil
},
nil,
true,
expectError: nil,
expectRenewal: true,
},
{
5 * time.Second,
"one_error",
15,
15,
func(_ string, _ int) (*Secret, error) {
maxTestTime: time.Second,
name: "short_increment_duration",
leaseDurationSeconds: 60,
incrementSeconds: 10,
renew: func(_ string, _ int) (*Secret, error) {
return renewedSecret, nil
},
expectError: nil,
expectRenewal: true,
},
{
maxTestTime: 5 * time.Second,
name: "one_error",
leaseDurationSeconds: 15,
incrementSeconds: 15,
renew: func(_ string, _ int) (*Secret, error) {
if caseOneErrorCount == 0 {
caseOneErrorCount++
return nil, fmt.Errorf("renew failure")
}
return renewedSecret, nil
},
nil,
true,
expectError: nil,
expectRenewal: true,
},
{
15 * time.Second,
"many_errors",
15,
15,
func(_ string, _ int) (*Secret, error) {
maxTestTime: 15 * time.Second,
name: "many_errors",
leaseDurationSeconds: 15,
incrementSeconds: 15,
renew: func(_ string, _ int) (*Secret, error) {
if caseManyErrorsCount == 3 {
return renewedSecret, nil
}
caseManyErrorsCount++
return nil, fmt.Errorf("renew failure")
},
nil,
true,
expectError: nil,
expectRenewal: true,
},
{
15 * time.Second,
"only_errors",
15,
15,
func(_ string, _ int) (*Secret, error) {
maxTestTime: 15 * time.Second,
name: "only_errors",
leaseDurationSeconds: 15,
incrementSeconds: 15,
renew: func(_ string, _ int) (*Secret, error) {
return nil, fmt.Errorf("renew failure")
},
nil,
false,
expectError: nil,
expectRenewal: false,
},
{
15 * time.Second,
"negative_lease_duration",
-15,
15,
func(_ string, _ int) (*Secret, error) {
maxTestTime: 15 * time.Second,
name: "negative_lease_duration",
leaseDurationSeconds: -15,
incrementSeconds: 15,
renew: func(_ string, _ int) (*Secret, error) {
return renewedSecret, nil
},
nil,
true,
expectError: nil,
expectRenewal: true,
},
}

Expand Down
3 changes: 3 additions & 0 deletions changelog/14836.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
api: Respect increment value in grace period calculations in LifetimeWatcher
```