TUN-8427: Fix BackoffHandler's internally shared clock structure

A clock structure was used to help support unit testing timetravel
but it is a globally shared object and is likely unsafe to share
across tests. Reordering of the tests seemed to have intermittent
failures for the TestWaitForBackoffFallback specifically on windows
builds.

Adjusting this to be a shim inside the BackoffHandler struct should
resolve shared object overrides in unit testing.

Additionally, added the reset retries functionality to be inline with
the ResetNow function of the BackoffHandler to align better with
expected functionality of the method.

Removes unused reconnectCredentialManager.

Author: DevinCarr

retry/backoffhandler.go

@@ -6,17 +6,16 @@ import (
 	"time"
 )
 
+const (
+	DefaultBaseTime time.Duration = time.Second
+)
+
 // Redeclare time functions so they can be overridden in tests.
-type clock struct {
+type Clock struct {
 	Now   func() time.Time
 	After func(d time.Duration) <-chan time.Time
 }
 
-var Clock = clock{
-	Now:   time.Now,
-	After: time.After,
-}
-
 // BackoffHandler manages exponential backoff and limits the maximum number of retries.
 // The base time period is 1 second, doubling with each retry.
 // After initial success, a grace period can be set to reset the backoff timer if
@@ -25,15 +24,26 @@ var Clock = clock{
 type BackoffHandler struct {
 	// MaxRetries sets the maximum number of retries to perform. The default value
 	// of 0 disables retry completely.
-	MaxRetries uint
+	maxRetries uint
 	// RetryForever caps the exponential backoff period according to MaxRetries
 	// but allows you to retry indefinitely.
-	RetryForever bool
+	retryForever bool
 	// BaseTime sets the initial backoff period.
-	BaseTime time.Duration
+	baseTime time.Duration
 
 	retries       uint
 	resetDeadline time.Time
+
+	Clock Clock
+}
+
+func NewBackoff(maxRetries uint, baseTime time.Duration, retryForever bool) BackoffHandler {
+	return BackoffHandler{
+		maxRetries:   maxRetries,
+		baseTime:     baseTime,
+		retryForever: retryForever,
+		Clock:        Clock{Now: time.Now, After: time.After},
+	}
 }
 
 func (b BackoffHandler) GetMaxBackoffDuration(ctx context.Context) (time.Duration, bool) {
@@ -44,11 +54,11 @@ func (b BackoffHandler) GetMaxBackoffDuration(ctx context.Context) (time.Duratio
 		return time.Duration(0), false
 	default:
 	}
-	if !b.resetDeadline.IsZero() && Clock.Now().After(b.resetDeadline) {
+	if !b.resetDeadline.IsZero() && b.Clock.Now().After(b.resetDeadline) {
 		// b.retries would be set to 0 at this point
 		return time.Second, true
 	}
-	if b.retries >= b.MaxRetries && !b.RetryForever {
+	if b.retries >= b.maxRetries && !b.retryForever {
 		return time.Duration(0), false
 	}
 	maxTimeToWait := b.GetBaseTime() * 1 << (b.retries + 1)
@@ -58,20 +68,20 @@ func (b BackoffHandler) GetMaxBackoffDuration(ctx context.Context) (time.Duratio
 // BackoffTimer returns a channel that sends the current time when the exponential backoff timeout expires.
 // Returns nil if the maximum number of retries have been used.
 func (b *BackoffHandler) BackoffTimer() <-chan time.Time {
-	if !b.resetDeadline.IsZero() && Clock.Now().After(b.resetDeadline) {
+	if !b.resetDeadline.IsZero() && b.Clock.Now().After(b.resetDeadline) {
 		b.retries = 0
 		b.resetDeadline = time.Time{}
 	}
-	if b.retries >= b.MaxRetries {
-		if !b.RetryForever {
+	if b.retries >= b.maxRetries {
+		if !b.retryForever {
 			return nil
 		}
 	} else {
 		b.retries++
 	}
 	maxTimeToWait := time.Duration(b.GetBaseTime() * 1 << (b.retries))
 	timeToWait := time.Duration(rand.Int63n(maxTimeToWait.Nanoseconds()))
-	return Clock.After(timeToWait)
+	return b.Clock.After(timeToWait)
 }
 
 // Backoff is used to wait according to exponential backoff. Returns false if the
@@ -94,16 +104,16 @@ func (b *BackoffHandler) Backoff(ctx context.Context) bool {
 func (b *BackoffHandler) SetGracePeriod() time.Duration {
 	maxTimeToWait := b.GetBaseTime() * 2 << (b.retries + 1)
 	timeToWait := time.Duration(rand.Int63n(maxTimeToWait.Nanoseconds()))
-	b.resetDeadline = Clock.Now().Add(timeToWait)
+	b.resetDeadline = b.Clock.Now().Add(timeToWait)
 
 	return timeToWait
 }
 
 func (b BackoffHandler) GetBaseTime() time.Duration {
-	if b.BaseTime == 0 {
-		return time.Second
+	if b.baseTime == 0 {
+		return DefaultBaseTime
 	}
-	return b.BaseTime
+	return b.baseTime
 }
 
 // Retries returns the number of retries consumed so far.
@@ -112,9 +122,10 @@ func (b *BackoffHandler) Retries() int {
 }
 
 func (b *BackoffHandler) ReachedMaxRetries() bool {
-	return b.retries == b.MaxRetries
+	return b.retries == b.maxRetries
 }
 
 func (b *BackoffHandler) ResetNow() {
-	b.resetDeadline = time.Now()
+	b.resetDeadline = b.Clock.Now()
+	b.retries = 0
 }

retry/backoffhandler_test.go

@@ -13,10 +13,9 @@ func immediateTimeAfter(time.Duration) <-chan time.Time {
 }
 
 func TestBackoffRetries(t *testing.T) {
-	// make backoff return immediately
-	Clock.After = immediateTimeAfter
 	ctx := context.Background()
-	backoff := BackoffHandler{MaxRetries: 3}
+	// make backoff return immediately
+	backoff := BackoffHandler{maxRetries: 3, Clock: Clock{time.Now, immediateTimeAfter}}
 	if !backoff.Backoff(ctx) {
 		t.Fatalf("backoff failed immediately")
 	}
@@ -32,10 +31,10 @@ func TestBackoffRetries(t *testing.T) {
 }
 
 func TestBackoffCancel(t *testing.T) {
-	// prevent backoff from returning normally
-	Clock.After = func(time.Duration) <-chan time.Time { return make(chan time.Time) }
 	ctx, cancelFunc := context.WithCancel(context.Background())
-	backoff := BackoffHandler{MaxRetries: 3}
+	// prevent backoff from returning normally
+	after := func(time.Duration) <-chan time.Time { return make(chan time.Time) }
+	backoff := BackoffHandler{maxRetries: 3, Clock: Clock{time.Now, after}}
 	cancelFunc()
 	if backoff.Backoff(ctx) {
 		t.Fatalf("backoff allowed after cancel")
@@ -46,13 +45,12 @@ func TestBackoffCancel(t *testing.T) {
 }
 
 func TestBackoffGracePeriod(t *testing.T) {
+	ctx := context.Background()
 	currentTime := time.Now()
 	// make Clock.Now return whatever we like
-	Clock.Now = func() time.Time { return currentTime }
+	now := func() time.Time { return currentTime }
 	// make backoff return immediately
-	Clock.After = immediateTimeAfter
-	ctx := context.Background()
-	backoff := BackoffHandler{MaxRetries: 1}
+	backoff := BackoffHandler{maxRetries: 1, Clock: Clock{now, immediateTimeAfter}}
 	if !backoff.Backoff(ctx) {
 		t.Fatalf("backoff failed immediately")
 	}
@@ -70,10 +68,9 @@ func TestBackoffGracePeriod(t *testing.T) {
 }
 
 func TestGetMaxBackoffDurationRetries(t *testing.T) {
-	// make backoff return immediately
-	Clock.After = immediateTimeAfter
 	ctx := context.Background()
-	backoff := BackoffHandler{MaxRetries: 3}
+	// make backoff return immediately
+	backoff := BackoffHandler{maxRetries: 3, Clock: Clock{time.Now, immediateTimeAfter}}
 	if _, ok := backoff.GetMaxBackoffDuration(ctx); !ok {
 		t.Fatalf("backoff failed immediately")
 	}
@@ -95,10 +92,9 @@ func TestGetMaxBackoffDurationRetries(t *testing.T) {
 }
 
 func TestGetMaxBackoffDuration(t *testing.T) {
-	// make backoff return immediately
-	Clock.After = immediateTimeAfter
 	ctx := context.Background()
-	backoff := BackoffHandler{MaxRetries: 3}
+	// make backoff return immediately
+	backoff := BackoffHandler{maxRetries: 3, Clock: Clock{time.Now, immediateTimeAfter}}
 	if duration, ok := backoff.GetMaxBackoffDuration(ctx); !ok || duration > time.Second*2 {
 		t.Fatalf("backoff (%s) didn't return < 2 seconds on first retry", duration)
 	}
@@ -117,10 +113,9 @@ func TestGetMaxBackoffDuration(t *testing.T) {
 }
 
 func TestBackoffRetryForever(t *testing.T) {
-	// make backoff return immediately
-	Clock.After = immediateTimeAfter
 	ctx := context.Background()
-	backoff := BackoffHandler{MaxRetries: 3, RetryForever: true}
+	// make backoff return immediately
+	backoff := BackoffHandler{maxRetries: 3, retryForever: true, Clock: Clock{time.Now, immediateTimeAfter}}
 	if duration, ok := backoff.GetMaxBackoffDuration(ctx); !ok || duration > time.Second*2 {
 		t.Fatalf("backoff (%s) didn't return < 2 seconds on first retry", duration)
 	}