From 7e7ee9011ac25bc48d19449f69f5f3f94d5772d5 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Fri, 28 Aug 2020 19:31:17 +0000
Subject: [PATCH] Fix FakeClock::Reset to always succeed

Current FakeClock::Reset only successfully resets the timer and
returns true when the timer has neither fired nore been stopped.

This is incorrect behavior as real clocks allow for reseting
in both of those situations (as long has the channel has been drained).

This is useful in tests that use a fake clock to test
wait.BackofManager, as the timer resets after each iteration of
Backoff() after the timer has already fired.
---
 .../apimachinery/pkg/util/clock/clock.go       | 18 ++++++++++++++++--
 .../apimachinery/pkg/util/clock/clock_test.go  |  7 ++++---
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/staging/src/k8s.io/apimachinery/pkg/util/clock/clock.go b/staging/src/k8s.io/apimachinery/pkg/util/clock/clock.go
index 6cf13d83d10..3e1e2517b42 100644
--- a/staging/src/k8s.io/apimachinery/pkg/util/clock/clock.go
+++ b/staging/src/k8s.io/apimachinery/pkg/util/clock/clock.go
@@ -348,7 +348,13 @@ func (f *fakeTimer) Stop() bool {
 // Reset conditionally updates the firing time of the timer.  If the
 // timer has neither fired nor been stopped then this call resets the
 // timer to the fake clock's "now" + d and returns true, otherwise
-// this call returns false.  This is like time.Timer::Reset.
+// it creates a new waiter, adds it to the clock, and returns true.
+//
+// It is not possible to return false, because a fake timer can be reset
+// from any state (waiting to fire, already fired, and stopped).
+//
+// See the GoDoc for time.Timer::Reset for more context on why
+// the return value of Reset() is not useful.
 func (f *fakeTimer) Reset(d time.Duration) bool {
 	f.fakeClock.lock.Lock()
 	defer f.fakeClock.lock.Unlock()
@@ -360,7 +366,15 @@ func (f *fakeTimer) Reset(d time.Duration) bool {
 			return true
 		}
 	}
-	return false
+	// No existing waiter, timer has already fired or been reset.
+	// We should still enable Reset() to succeed by creating a
+	// new waiter and adding it to the clock's waiters.
+	newWaiter := fakeClockWaiter{
+		targetTime: f.fakeClock.time.Add(d),
+		destChan:   seekChan,
+	}
+	f.fakeClock.waiters = append(f.fakeClock.waiters, newWaiter)
+	return true
 }
 
 // Ticker defines the Ticker interface
diff --git a/staging/src/k8s.io/apimachinery/pkg/util/clock/clock_test.go b/staging/src/k8s.io/apimachinery/pkg/util/clock/clock_test.go
index 16967d9a0c3..04870743ffc 100644
--- a/staging/src/k8s.io/apimachinery/pkg/util/clock/clock_test.go
+++ b/staging/src/k8s.io/apimachinery/pkg/util/clock/clock_test.go
@@ -217,8 +217,8 @@ func TestFakeTimer(t *testing.T) {
 		t.Errorf("unexpected channel read")
 	default:
 	}
-	if twoSec.Reset(time.Second) {
-		t.Errorf("Expected twoSec.Reset() to return false")
+	if !twoSec.Reset(time.Second) {
+		t.Errorf("Expected twoSec.Reset() to return true")
 	}
 	if !treSec.Reset(time.Second) {
 		t.Errorf("Expected treSec.Reset() to return true")
@@ -238,8 +238,9 @@ func TestFakeTimer(t *testing.T) {
 	case <-oneSec.C():
 		t.Errorf("unexpected channel read")
 	case <-twoSec.C():
-		t.Errorf("unexpected channel read")
+		// Expected!
 	default:
+		t.Errorf("unexpected channel non-read")
 	}
 	select {
 	case <-treSec.C():