LifeTimeWatcher SleepDuration calculation testing (#17919)

* factor out sleep duration calc
* property based sleep duration test

Co-authored-by: peteski22 <peter.wilson@hashicorp.com>

api/lifetime_watcher.go

@@ -337,25 +337,15 @@ func (r *LifetimeWatcher) doRenewWithOptions(tokenMode bool, nonRenewable bool,
 
 		var sleepDuration time.Duration
 
-		if errorBackoff != nil {
+		if errorBackoff == nil {
-			sleepDuration = errorBackoff.NextBackOff()
+			sleepDuration = r.calculateSleepDuration(remainingLeaseDuration, priorDuration)
-			if sleepDuration == backoff.Stop {
+		} else if errorBackoff.NextBackOff() == backoff.Stop {
-				return err
+			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)
 		}
 
+		// 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.

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)
+	}
+}

changelog/17919.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+api: property based testing for LifetimeWatcher sleep duration calculation
+```