Cleanups in controller utils

1. Squash two identical sorters byTime
2. Move helper for searching active jobs into utils to exist next to its
  counterpart
This commit is contained in:
Maciej Szulik 2022-06-28 16:59:38 +02:00
parent 46f3821bf4
commit cb491a8d0f
No known key found for this signature in database
GPG Key ID: F15E55D276FA84C4
2 changed files with 19 additions and 37 deletions

View File

@ -431,13 +431,13 @@ func (jm *ControllerV2) syncCronJob(
childrenJobs := make(map[types.UID]bool)
for _, j := range jobs {
childrenJobs[j.ObjectMeta.UID] = true
found := inActiveList(*cronJob, j.ObjectMeta.UID)
found := inActiveList(cronJob, j.ObjectMeta.UID)
if !found && !IsJobFinished(j) {
cjCopy, err := jm.cronJobControl.GetCronJob(ctx, cronJob.Namespace, cronJob.Name)
if err != nil {
return nil, nil, updateStatus, err
}
if inActiveList(*cjCopy, j.ObjectMeta.UID) {
if inActiveList(cjCopy, j.ObjectMeta.UID) {
cronJob = cjCopy
continue
}
@ -553,11 +553,11 @@ func (jm *ControllerV2) syncCronJob(
t := nextScheduledTimeDuration(*cronJob, sched, now)
return cronJob, t, updateStatus, nil
}
if isJobInActiveList(&batchv1.Job{
if inActiveListByName(cronJob, &batchv1.Job{
ObjectMeta: metav1.ObjectMeta{
Name: getJobName(cronJob, *scheduledTime),
Namespace: cronJob.Namespace,
}}, cronJob.Status.Active) || cronJob.Status.LastScheduleTime.Equal(&metav1.Time{Time: *scheduledTime}) {
}}) || cronJob.Status.LastScheduleTime.Equal(&metav1.Time{Time: *scheduledTime}) {
klog.V(4).InfoS("Not starting job because the scheduled time is already processed", "cronjob", klog.KRef(cronJob.GetNamespace(), cronJob.GetName()), "schedule", scheduledTime)
t := nextScheduledTimeDuration(*cronJob, sched, now)
return cronJob, t, updateStatus, nil
@ -724,7 +724,7 @@ func (jm *ControllerV2) removeOldestJobs(cj *batchv1.CronJob, js []*batchv1.Job,
klog.V(4).InfoS("Cleaning up jobs from CronJob list", "deletejobnum", numToDelete, "jobnum", len(js), "cronjob", klog.KRef(cj.GetNamespace(), cj.GetName()))
sort.Sort(byJobStartTimeStar(js))
sort.Sort(byJobStartTime(js))
for i := 0; i < numToDelete; i++ {
klog.V(4).InfoS("Removing job from CronJob list", "job", js[i].Name, "cronjob", klog.KRef(cj.GetNamespace(), cj.GetName()))
if deleteJob(cj, js[i], jm.jobControl, jm.recorder) {
@ -734,17 +734,6 @@ func (jm *ControllerV2) removeOldestJobs(cj *batchv1.CronJob, js []*batchv1.Job,
return updateStatus
}
// isJobInActiveList take a job and checks if activeJobs has a job with the same
// name and namespace.
func isJobInActiveList(job *batchv1.Job, activeJobs []corev1.ObjectReference) bool {
for _, j := range activeJobs {
if j.Name == job.Name && j.Namespace == job.Namespace {
return true
}
}
return false
}
// deleteJob reaps a job, deleting the job, the pods and the reference in the active list
func deleteJob(cj *batchv1.CronJob, job *batchv1.Job, jc jobControlInterface, recorder record.EventRecorder) bool {
nameForLog := fmt.Sprintf("%s/%s", cj.Namespace, cj.Name)

View File

@ -33,7 +33,8 @@ import (
// Utilities for dealing with Jobs and CronJobs and time.
func inActiveList(cj batchv1.CronJob, uid types.UID) bool {
// inActiveList checks if cronjob's .status.active has a job with the same UID.
func inActiveList(cj *batchv1.CronJob, uid types.UID) bool {
for _, j := range cj.Status.Active {
if j.UID == uid {
return true
@ -42,6 +43,17 @@ func inActiveList(cj batchv1.CronJob, uid types.UID) bool {
return false
}
// inActiveListByName checks if cronjob's status.active has a job with the same
// name and namespace.
func inActiveListByName(cj *batchv1.CronJob, job *batchv1.Job) bool {
for _, j := range cj.Status.Active {
if j.Name == job.Name && j.Namespace == job.Namespace {
return true
}
}
return false
}
func deleteFromActiveList(cj *batchv1.CronJob, uid types.UID) {
if cj == nil {
return
@ -200,7 +212,7 @@ func IsJobFinished(j *batchv1.Job) bool {
}
// byJobStartTime sorts a list of jobs by start timestamp, using their names as a tie breaker.
type byJobStartTime []batchv1.Job
type byJobStartTime []*batchv1.Job
func (o byJobStartTime) Len() int { return len(o) }
func (o byJobStartTime) Swap(i, j int) { o[i], o[j] = o[j], o[i] }
@ -217,22 +229,3 @@ func (o byJobStartTime) Less(i, j int) bool {
}
return o[i].Status.StartTime.Before(o[j].Status.StartTime)
}
// byJobStartTimeStar sorts a list of jobs by start timestamp, using their names as a tie breaker.
type byJobStartTimeStar []*batchv1.Job
func (o byJobStartTimeStar) Len() int { return len(o) }
func (o byJobStartTimeStar) Swap(i, j int) { o[i], o[j] = o[j], o[i] }
func (o byJobStartTimeStar) Less(i, j int) bool {
if o[i].Status.StartTime == nil && o[j].Status.StartTime != nil {
return false
}
if o[i].Status.StartTime != nil && o[j].Status.StartTime == nil {
return true
}
if o[i].Status.StartTime.Equal(o[j].Status.StartTime) {
return o[i].Name < o[j].Name
}
return o[i].Status.StartTime.Before(o[j].Status.StartTime)
}