Merge pull request #9264 from shubham-pampattiwar/fix-backup-q-accum

Fix Schedule Backup Queue Accumulation During Extended Blocking Scenarios

changelogs/unreleased/9264-shubham-pampattiwar

@@ -0,0 +1 @@
+Fix schedule controller to prevent backup queue accumulation during extended blocking scenarios by properly handling empty backup phases

pkg/controller/schedule_controller.go

@@ -229,7 +229,7 @@ func (c *scheduleReconciler) checkIfBackupInNewOrProgress(schedule *velerov1.Sch
 	}
 
 	for _, backup := range backupList.Items {
-		if backup.Status.Phase == velerov1.BackupPhaseNew || backup.Status.Phase == velerov1.BackupPhaseInProgress {
+		if backup.Status.Phase == "" || backup.Status.Phase == velerov1.BackupPhaseNew || backup.Status.Phase == velerov1.BackupPhaseInProgress {
 			log.Debugf("%s/%s still has backups that are in InProgress or New...", schedule.Namespace, schedule.Name)
 			return true
 		}

pkg/controller/schedule_controller_test.go

@@ -149,6 +149,13 @@ func TestReconcileOfSchedule(t *testing.T) {
 			expectedPhase: string(velerov1.SchedulePhaseEnabled),
 			backup:        builder.ForBackup("ns", "name-20220905120000").ObjectMeta(builder.WithLabels(velerov1.ScheduleNameLabel, "name")).Phase(velerov1.BackupPhaseNew).Result(),
 		},
+		{
+			name:          "schedule already has backup with empty phase (not yet reconciled).",
+			schedule:      newScheduleBuilder(velerov1.SchedulePhaseEnabled).CronSchedule("@every 5m").LastBackupTime("2000-01-01 00:00:00").Result(),
+			fakeClockTime: "2017-01-01 12:00:00",
+			expectedPhase: string(velerov1.SchedulePhaseEnabled),
+			backup:        builder.ForBackup("ns", "name-20220905120000").ObjectMeta(builder.WithLabels(velerov1.ScheduleNameLabel, "name")).Phase("").Result(),
+		},
 	}
 
 	for _, test := range tests {
@@ -215,10 +222,10 @@ func TestReconcileOfSchedule(t *testing.T) {
 			backups := &velerov1.BackupList{}
 			require.NoError(t, client.List(ctx, backups))
 
-			// If backup associated with schedule's status is in New or InProgress,
+			// If backup associated with schedule's status is in New or InProgress or empty phase,
 			// new backup shouldn't be submitted.
 			if test.backup != nil &&
-				(test.backup.Status.Phase == velerov1.BackupPhaseNew || test.backup.Status.Phase == velerov1.BackupPhaseInProgress) {
+				(test.backup.Status.Phase == "" || test.backup.Status.Phase == velerov1.BackupPhaseNew || test.backup.Status.Phase == velerov1.BackupPhaseInProgress) {
 				assert.Len(t, backups.Items, 1)
 				require.NoError(t, client.Delete(ctx, test.backup))
 			}
@@ -479,4 +486,19 @@ func TestCheckIfBackupInNewOrProgress(t *testing.T) {
 	reconciler = NewScheduleReconciler("namespace", logger, client, metrics.NewServerMetrics(), false)
 	result = reconciler.checkIfBackupInNewOrProgress(testSchedule)
 	assert.True(t, result)
+
+	// Clean backup in InProgress phase.
+	err = client.Delete(ctx, inProgressBackup)
+	require.NoError(t, err, "fail to delete backup in InProgress phase in TestCheckIfBackupInNewOrProgress: %v", err)
+
+	// Create backup with empty phase (not yet reconciled).
+	emptyPhaseBackup := builder.ForBackup("ns", "backup-3").
+		ObjectMeta(builder.WithLabels(velerov1.ScheduleNameLabel, "name")).
+		Phase("").Result()
+	err = client.Create(ctx, emptyPhaseBackup)
+	require.NoError(t, err, "fail to create backup with empty phase in TestCheckIfBackupInNewOrProgress: %v", err)
+
+	reconciler = NewScheduleReconciler("namespace", logger, client, metrics.NewServerMetrics(), false)
+	result = reconciler.checkIfBackupInNewOrProgress(testSchedule)
+	assert.True(t, result)
 }