[OCPBUGS-34173]: fix sorting unpack jobs (#3299)

* fix sorting unpack jobs

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

* Add test cases to reproduce panic

Signed-off-by: Catherine Chan-Tse <[email protected]>

* handle nil jobs while sorting unpack jobs

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

---------

Signed-off-by: Ankita Thomas <[email protected]>
Signed-off-by: Catherine Chan-Tse <[email protected]>
Co-authored-by: Catherine Chan-Tse <[email protected]>

Author: ankitathomas

pkg/controller/bundle/bundle_unpacker.go

@@ -863,14 +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
 		}
 		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
 	}
@@ -880,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

@@ -1997,6 +1997,15 @@ 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,
+		},
+	}
 	failedJobs := []*batchv1.Job{
 		testJob("f-1", true, 1),
 		testJob("f-2", true, 2),
@@ -2028,6 +2037,24 @@ func TestSortUnpackJobs(t *testing.T) {
 		}, {
 			name:        "empty job list",
 			maxRetained: 1,
+		}, {
+			name:        "nil job in list",
+			maxRetained: 1,
+			jobs: []*batchv1.Job{
+				failedJobs[2],
+				nil,
+				failedJobs[1],
+			},
+			expectedLatest: failedJobs[2],
+		}, {
+			name:        "nil condition",
+			maxRetained: 3,
+			jobs: []*batchv1.Job{
+				failedJobs[2],
+				nilConditionJob,
+				failedJobs[1],
+			},
+			expectedLatest: nilConditionJob,
 		}, {
 			name:        "retain oldest",
 			maxRetained: 1,