Merge pull request #76455 from joelsmith/master

Fix potential test flakes in HPA tests TestEventNotCreated and TestAvoidUncessaryUpdates

pkg/controller/podautoscaler/horizontal_test.go

@@ -330,6 +330,7 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa
 	})
 
 	fakeClient.AddReactor("update", "horizontalpodautoscalers", func(action core.Action) (handled bool, ret runtime.Object, err error) {
+		handled, obj, err := func() (handled bool, ret *autoscalingv1.HorizontalPodAutoscaler, err error) {
 			tc.Lock()
 			defer tc.Unlock()
 
@@ -360,8 +361,12 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa
 			assert.Equal(t, tc.expectedConditions, actualConditions, "the status conditions should have been as expected")
 			tc.statusUpdated = true
 			// Every time we reconcile HPA object we are updating status.
-		tc.processed <- obj.Name
 			return true, obj, nil
+		}()
+		if obj != nil {
+			tc.processed <- obj.Name
+		}
+		return handled, obj, err
 	})
 
 	fakeScaleClient := &scalefake.FakeScaleClient{}
@@ -701,15 +706,25 @@ func (tc *testCase) runTestWithController(t *testing.T, hpaController *Horizonta
 	go hpaController.Run(stop)
 
 	tc.Lock()
-	if tc.verifyEvents {
+	shouldWait := tc.verifyEvents
 	tc.Unlock()
+
+	if shouldWait {
 		// We need to wait for events to be broadcasted (sleep for longer than record.sleepDuration).
-		time.Sleep(2 * time.Second)
-	} else {
-		tc.Unlock()
+		timeoutTime := time.Now().Add(2 * time.Second)
+		for now := time.Now(); timeoutTime.After(now); now = time.Now() {
+			sleepUntil := timeoutTime.Sub(now)
+			select {
+			case <-tc.processed:
+				// drain the chan of any sent events to keep it from filling before the timeout
+			case <-time.After(sleepUntil):
+				// timeout reached, ready to verifyResults
 			}
+		}
+	} else {
 		// Wait for HPA to be processed.
 		<-tc.processed
+	}
 	tc.verifyResults(t)
 }
 
@@ -2418,7 +2433,9 @@ func TestAvoidUncessaryUpdates(t *testing.T) {
 			// wait a tick and then mark that we're finished (otherwise, we have no
 			// way to indicate that we're finished, because the function decides not to do anything)
 			time.Sleep(1 * time.Second)
+			tc.Lock()
 			tc.statusUpdated = true
+			tc.Unlock()
 			tc.processed <- "test-hpa"
 		}()
 
@@ -2493,8 +2510,6 @@ func TestAvoidUncessaryUpdates(t *testing.T) {
 		return true, objv1, nil
 	})
 	testClient.PrependReactor("update", "horizontalpodautoscalers", func(action core.Action) (handled bool, ret runtime.Object, err error) {
-		tc.Lock()
-		defer tc.Unlock()
 		assert.Fail(t, "should not have attempted to update the HPA when nothing changed")
 		// mark that we've processed this HPA
 		tc.processed <- ""

pkg/controller/podautoscaler/legacy_horizontal_test.go

@@ -332,6 +332,7 @@ func (tc *legacyTestCase) prepareTestClient(t *testing.T) (*fake.Clientset, *sca
 	})
 
 	fakeClient.AddReactor("update", "horizontalpodautoscalers", func(action core.Action) (handled bool, ret runtime.Object, err error) {
+		obj := func() *autoscalingv1.HorizontalPodAutoscaler {
 			tc.Lock()
 			defer tc.Unlock()
 
@@ -345,6 +346,8 @@ func (tc *legacyTestCase) prepareTestClient(t *testing.T) (*fake.Clientset, *sca
 				}
 			}
 			tc.statusUpdated = true
+			return obj
+		}()
 		// Every time we reconcile HPA object we are updating status.
 		tc.processed <- obj.Name
 		return true, obj, nil