handle nil jobs while sorting unpack jobs

Signed-off-by: Ankita Thomas <[email protected]>

Author: ankitathomas

pkg/controller/bundle/bundle_unpacker.go

@@ -863,24 +863,26 @@ func sortUnpackJobs(jobs []*batchv1.Job, maxRetainedJobs int) (latest *batchv1.J
 	// sort jobs so that latest job is first
 	// with preference for non-failed jobs
 	sort.Slice(jobs, func(i, j int) bool {
+		if jobs[i] == nil || jobs[j] == nil {
+			return jobs[i] != nil
+		}
 		condI, failedI := getCondition(jobs[i], batchv1.JobFailed)
 		condJ, failedJ := getCondition(jobs[j], batchv1.JobFailed)
 		if failedI != failedJ {
 			return !failedI // non-failed job goes first
 		}
-		// sort multiple jobs without the Failed condition
-		if condI == nil && condJ == nil {
-			return jobs[i].CreationTimestamp.After(jobs[j].CreationTimestamp.Time)
-		}
-		if condI == nil {
-			return false
-		}
-		if condJ == nil {
-			return true
-		}
 		return condI.LastTransitionTime.After(condJ.LastTransitionTime.Time)
 	})
+	if jobs[0] == nil {
+		// all nil jobs
+		return
+	}
 	latest = jobs[0]
+	nilJobsIndex := len(jobs) - 1
+	for ; nilJobsIndex >= 0 && jobs[nilJobsIndex] == nil; nilJobsIndex-- {
+	}
+
+	jobs = jobs[:nilJobsIndex+1] // exclude nil jobs from list of jobs to delete
 	if len(jobs) <= maxRetainedJobs {
 		return
 	}
@@ -890,7 +892,7 @@ func sortUnpackJobs(jobs []*batchv1.Job, maxRetainedJobs int) (latest *batchv1.J
 	}
 
 	// cleanup old failed jobs, n-1 recent jobs and the oldest job
-	for i := 0; i < maxRetainedJobs && i+maxRetainedJobs < len(jobs); i++ {
+	for i := 0; i < maxRetainedJobs && i+maxRetainedJobs < len(jobs)-1; i++ {
 		toDelete = append(toDelete, jobs[maxRetainedJobs+i])
 	}
 

pkg/controller/bundle/bundle_unpacker_test.go

@@ -1998,14 +1998,14 @@ func TestSortUnpackJobs(t *testing.T) {
 		}
 	}
 	nilConditionJob := &batchv1.Job{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:   "nc",
-				Labels: map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue, bundleUnpackRefLabel: "test"},
-			},
-			Status: batchv1.JobStatus{
-				Conditions: nil,
-			},
-		}
+		ObjectMeta: metav1.ObjectMeta{
+			Name:   "nc",
+			Labels: map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue, bundleUnpackRefLabel: "test"},
+		},
+		Status: batchv1.JobStatus{
+			Conditions: nil,
+		},
+	}
 	failedJobs := []*batchv1.Job{
 		testJob("f-1", true, 1),
 		testJob("f-2", true, 2),
@@ -2045,6 +2045,7 @@ func TestSortUnpackJobs(t *testing.T) {
 				nil,
 				failedJobs[1],
 			},
+			expectedLatest: failedJobs[2],
 		}, {
 			name:        "nil condition",
 			maxRetained: 3,