Fix flaky TestClean unit tests for storagebundle REST server (antrea-…

…io#5065)

The test was using a Sleep statement, which was actually too short for
some CI machines, causing flakes.

Instead we use a virtual clock for the test, which means that the test
runtime is no longer dependent on the "duration" parameter for the
`clean` function. We also use polling to validate assertions.

Signed-off-by: Antonin Bas <[email protected]>

pkg/apiserver/registry/system/supportbundle/rest.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/klog/v2"
+	clockutils "k8s.io/utils/clock"
 	"k8s.io/utils/exec"
 
 	agentquerier "antrea.io/antrea/pkg/agent/querier"
@@ -49,6 +50,8 @@ var (
 	defaultFS       = afero.NewOsFs()
 	defaultExecutor = exec.New()
 	newAgentDumper  = support.NewAgentDumper
+
+	clock clockutils.Clock = &clockutils.RealClock{}
 )
 
 // NewControllerStorage creates a support bundle storage for working on antrea controller.
@@ -284,7 +287,7 @@ func (r *supportBundleREST) collectController(ctx context.Context, since string)
 func (r *supportBundleREST) clean(ctx context.Context, bundlePath string, duration time.Duration) {
 	select {
 	case <-ctx.Done():
-	case <-time.After(duration):
+	case <-clock.After(duration):
 		func() {
 			r.statusLocker.Lock()
 			defer r.statusLocker.Unlock()

pkg/apiserver/registry/system/supportbundle/rest_test.go

@@ -27,6 +27,8 @@ import (
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	clockutils "k8s.io/utils/clock"
+	clocktesting "k8s.io/utils/clock/testing"
 	"k8s.io/utils/exec"
 	exectesting "k8s.io/utils/exec/testing"
 
@@ -72,29 +74,43 @@ func TestClean(t *testing.T) {
 	}{
 		"CleanByCancellation": {
 			needCancel: true,
-			duration:   time.Hour,
+			duration:   1 * time.Hour,
 		},
 		"CleanByTimeout": {
-			duration: 10 * time.Millisecond,
+			duration: 1 * time.Second,
 		},
 	} {
 		t.Run(name, func(t *testing.T) {
+			fakeClock := clocktesting.NewFakeClock(time.Now())
+			clock = fakeClock
+			defer func() {
+				clock = &clockutils.RealClock{}
+			}()
 			f, err := defaultFS.Create("test.tar.gz")
 			require.NoError(t, err)
 			defer defaultFS.Remove(f.Name())
 			require.NoError(t, f.Close())
 			storage := NewControllerStorage()
 			ctx, cancelFunc := context.WithCancel(context.Background())
 			defer cancelFunc()
+			require.False(t, fakeClock.HasWaiters())
+			go storage.SupportBundle.clean(ctx, f.Name(), tc.duration)
+			// Wait for the clean function to have called clock.After:
+			// until it does, we cannot advance the fake clock.
+			require.Eventually(t, func() bool {
+				return fakeClock.HasWaiters()
+			}, 1*time.Second, 10*time.Millisecond)
 			if tc.needCancel {
 				cancelFunc()
+			} else {
+				fakeClock.Step(tc.duration)
 			}
-			go storage.SupportBundle.clean(ctx, f.Name(), tc.duration)
-			time.Sleep(200 * time.Millisecond)
-			exist, err := afero.Exists(defaultFS, f.Name())
-			require.NoError(t, err)
-			require.False(t, exist)
-			require.Equal(t, system.SupportBundleStatusNone, storage.SupportBundle.cache.Status)
+			assert.EventuallyWithT(t, func(c *assert.CollectT) {
+				exist, err := afero.Exists(defaultFS, f.Name())
+				require.NoError(t, err)
+				assert.False(c, exist)
+			}, 1*time.Second, 10*time.Millisecond, "Supportbundle file was not deleted")
+			assert.Equal(t, system.SupportBundleStatusNone, storage.SupportBundle.cache.Status)
 		})
 	}
 }