diff --git a/pkg/controller/bundle/bundle_unpacker.go b/pkg/controller/bundle/bundle_unpacker.go index c9195a9c54..82a648f037 100644 --- a/pkg/controller/bundle/bundle_unpacker.go +++ b/pkg/controller/bundle/bundle_unpacker.go @@ -863,6 +863,9 @@ 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 { @@ -870,7 +873,16 @@ func sortUnpackJobs(jobs []*batchv1.Job, maxRetainedJobs int) (latest *batchv1.J } 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]) } diff --git a/pkg/controller/bundle/bundle_unpacker_test.go b/pkg/controller/bundle/bundle_unpacker_test.go index 4e3468b210..d36bba773b 100644 --- a/pkg/controller/bundle/bundle_unpacker_test.go +++ b/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,