diff --git a/api/lifetime_watcher.go b/api/lifetime_watcher.go index 5f90de00a..aecf18dfa 100644 --- a/api/lifetime_watcher.go +++ b/api/lifetime_watcher.go @@ -337,25 +337,15 @@ func (r *LifetimeWatcher) doRenewWithOptions(tokenMode bool, nonRenewable bool, var sleepDuration time.Duration - if errorBackoff != nil { - sleepDuration = errorBackoff.NextBackOff() - if sleepDuration == backoff.Stop { - return err - } - } else { - // We keep evaluating a new grace period so long as the lease is - // extending. Once it stops extending, we've hit the max and need to - // rely on the grace duration. - if remainingLeaseDuration > priorDuration { - r.calculateGrace(remainingLeaseDuration, time.Duration(r.increment)*time.Second) - } - priorDuration = remainingLeaseDuration - - // The sleep duration is set to 2/3 of the current lease duration plus - // 1/3 of the current grace period, which adds jitter. - sleepDuration = time.Duration(float64(remainingLeaseDuration.Nanoseconds())*2/3 + float64(r.grace.Nanoseconds())/3) + if errorBackoff == nil { + sleepDuration = r.calculateSleepDuration(remainingLeaseDuration, priorDuration) + } else if errorBackoff.NextBackOff() == backoff.Stop { + return err } + // remainingLeaseDuration becomes the priorDuration for the next loop + priorDuration = remainingLeaseDuration + // If we are within grace, return now; or, if the amount of time we // would sleep would land us in the grace period. This helps with short // tokens; for example, you don't want a current lease duration of 4 @@ -377,6 +367,21 @@ func (r *LifetimeWatcher) doRenewWithOptions(tokenMode bool, nonRenewable bool, } } +// calculateSleepDuration calculates the amount of time the LifeTimeWatcher should sleep +// before re-entering its loop. +func (r *LifetimeWatcher) calculateSleepDuration(remainingLeaseDuration, priorDuration time.Duration) time.Duration { + // We keep evaluating a new grace period so long as the lease is + // extending. Once it stops extending, we've hit the max and need to + // rely on the grace duration. + if remainingLeaseDuration > priorDuration { + r.calculateGrace(remainingLeaseDuration, time.Duration(r.increment)*time.Second) + } + + // The sleep duration is set to 2/3 of the current lease duration plus + // 1/3 of the current grace period, which adds jitter. + return time.Duration(float64(remainingLeaseDuration.Nanoseconds())*2/3 + float64(r.grace.Nanoseconds())/3) +} + // 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. diff --git a/api/renewer_test.go b/api/renewer_test.go index 3b28d8546..4d5ac2ec2 100644 --- a/api/renewer_test.go +++ b/api/renewer_test.go @@ -3,7 +3,11 @@ package api import ( "errors" "fmt" + "math" + "math/rand" + "reflect" "testing" + "testing/quick" "time" "github.com/go-test/deep" @@ -233,3 +237,47 @@ func TestLifetimeWatcher(t *testing.T) { }) } } + +// TestCalcSleepPeriod uses property based testing to evaluate the calculateSleepDuration +// function of LifeTimeWatchers, but also incidentally tests "calculateGrace". +// This is on account of "calculateSleepDuration" performing the "calculateGrace" +// function in particular instances. +// Both of these functions support the vital functionality of the LifeTimeWatcher +// and therefore should be tested rigorously. +func TestCalcSleepPeriod(t *testing.T) { + c := quick.Config{ + MaxCount: 1000, + Values: func(values []reflect.Value, r *rand.Rand) { + leaseDuration := r.Intn(math.MaxInt64) + remainingLeaseDuration := r.Intn(leaseDuration) + priorDuration := remainingLeaseDuration + increment := r.Intn(leaseDuration + 1) + + values[0] = reflect.ValueOf(r) + values[1] = reflect.ValueOf(time.Duration(leaseDuration)) + values[2] = reflect.ValueOf(time.Duration(priorDuration)) + values[3] = reflect.ValueOf(time.Duration(remainingLeaseDuration)) + values[4] = reflect.ValueOf(increment) // integer truncation... could be interesting. + }, + } + + // tests that "calculateSleepDuration" will always return a value less than + // the remaining lease duration given a random leaseDuration, priorDuration, remainingLeaseDuration, and increment. + // Inputs are generated so that: + // leaseDuration > priorDuration > remainingLeaseDuration + // and remainingLeaseDuration > increment + if err := quick.Check(func(r *rand.Rand, leaseDuration, priorDuration, remainingLeaseDuration time.Duration, increment int) bool { + lw := LifetimeWatcher{ + grace: 0, + increment: increment, + random: r, + } + + lw.calculateGrace(remainingLeaseDuration, time.Duration(increment)) + + // ensure that we sleep for less than the remaining lease. + return lw.calculateSleepDuration(remainingLeaseDuration, priorDuration) < remainingLeaseDuration + }, &c); err != nil { + t.Error(err) + } +} diff --git a/changelog/17919.txt b/changelog/17919.txt new file mode 100644 index 000000000..8fbb41db4 --- /dev/null +++ b/changelog/17919.txt @@ -0,0 +1,3 @@ +```release-note:improvement +api: property based testing for LifetimeWatcher sleep duration calculation +```