From 5c640440aa0741685b21445bdd0fe221b89c2a77 Mon Sep 17 00:00:00 2001
From: Mark Mandel <markmandel@google.com>
Date: Thu, 5 Dec 2024 13:45:33 -0800
Subject: [PATCH] Flake: e2e/TestScheduleAutoscaler (#4058)

Found that `nextCronMinuteBetween` would return an invalid crontab
of "59-0 * * * *" at minute 59 of the day. Implemented a fix so that
it now return "0-1 * * * *" which takes a little bit longer, but no
longer errors out.

Also updated test to fail fast rather than let the test continue if this
error happens, to aid in debugging in the future.
---
 test/e2e/fleetautoscaler_test.go | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/test/e2e/fleetautoscaler_test.go b/test/e2e/fleetautoscaler_test.go
index 2ea0adc25f..b8c2e98827 100644
--- a/test/e2e/fleetautoscaler_test.go
+++ b/test/e2e/fleetautoscaler_test.go
@@ -1621,9 +1621,8 @@ func TestScheduleAutoscaler(t *testing.T) {
 	stable := framework.AgonesClient.AgonesV1()
 	fleets := stable.Fleets(framework.Namespace)
 	flt, err := fleets.Create(ctx, defaultFleet(framework.Namespace), metav1.CreateOptions{})
-	if assert.NoError(t, err) {
-		defer fleets.Delete(context.Background(), flt.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint:errcheck
-	}
+	require.NoError(t, err)
+	defer fleets.Delete(context.Background(), flt.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint:errcheck
 
 	framework.AssertFleetCondition(t, flt, e2e.FleetReadyCount(flt.Spec.Replicas))
 
@@ -1633,7 +1632,7 @@ func TestScheduleAutoscaler(t *testing.T) {
 	scheduleAutoscaler := defaultAutoscalerSchedule(t, flt)
 	scheduleAutoscaler.Spec.Policy.Schedule.ActivePeriod.StartCron = nextCronMinute(time.Now())
 	fas, err := fleetautoscalers.Create(ctx, scheduleAutoscaler, metav1.CreateOptions{})
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	framework.AssertFleetCondition(t, flt, e2e.FleetReadyCount(5))
 	fleetautoscalers.Delete(ctx, fas.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint:errcheck
@@ -1646,7 +1645,7 @@ func TestScheduleAutoscaler(t *testing.T) {
 	scheduleAutoscaler = defaultAutoscalerSchedule(t, flt)
 	scheduleAutoscaler.Spec.Policy.Schedule.ActivePeriod.StartCron = nextCronMinuteBetween(time.Now())
 	fas, err = fleetautoscalers.Create(ctx, scheduleAutoscaler, metav1.CreateOptions{})
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	framework.AssertFleetCondition(t, flt, e2e.FleetReadyCount(5))
 	fleetautoscalers.Delete(ctx, fas.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint:errcheck
@@ -1842,8 +1841,13 @@ func nextCronMinute(currentTime time.Time) string {
 // nextCronMinuteBetween returns the minute between the very next minute
 // e.g. if the current time is 12:00, this method will return "1-2 * * * *"
 // meaning between 12:01 - 12:02
+// if the current minute if "59" since 59-0 is invalid, we'll return "0-1 * * * *" and wait for a bit longer on e2e tests.
 func nextCronMinuteBetween(currentTime time.Time) string {
 	nextMinute := currentTime.Add(time.Minute).Minute()
+	if nextMinute == 59 {
+		return "0-1 * * * *"
+	}
+
 	secondMinute := currentTime.Add(2 * time.Minute).Minute()
 	return fmt.Sprintf("%d-%d * * * *", nextMinute, secondMinute)
 }