diff --git a/pkg/controller/cronjob/BUILD b/pkg/controller/cronjob/BUILD index 3ceb655c3ec..d74052d1295 100644 --- a/pkg/controller/cronjob/BUILD +++ b/pkg/controller/cronjob/BUILD @@ -54,6 +54,7 @@ go_test( "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/client-go/tools/record:go_default_library", ], ) diff --git a/pkg/controller/cronjob/cronjob_controller_test.go b/pkg/controller/cronjob/cronjob_controller_test.go index f04819fd1c1..bc08af54e74 100644 --- a/pkg/controller/cronjob/cronjob_controller_test.go +++ b/pkg/controller/cronjob/cronjob_controller_test.go @@ -18,7 +18,6 @@ package cronjob import ( "errors" - "sort" "strconv" "strings" "testing" @@ -29,6 +28,7 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" // For the cronjob controller to do conversions. _ "k8s.io/kubernetes/pkg/api/install" @@ -524,7 +524,7 @@ func TestCleanupFinishedJobs_DeleteOrNot(t *testing.T) { // Create jobs js := []batchv1.Job{} - jobsToDelete := []string{} + jobsToDelete := sets.NewString() sj.Status.Active = []v1.ObjectReference{} for i, spec := range tc.jobSpecs { @@ -558,7 +558,7 @@ func TestCleanupFinishedJobs_DeleteOrNot(t *testing.T) { js = append(js, *job) if spec.ExpectDelete { - jobsToDelete = append(jobsToDelete, job.Name) + jobsToDelete.Insert(job.Name) } } @@ -576,12 +576,9 @@ func TestCleanupFinishedJobs_DeleteOrNot(t *testing.T) { if len(jc.DeleteJobName) != len(jobsToDelete) { t.Errorf("%s: expected %d job deleted, actually %d", name, len(jobsToDelete), len(jc.DeleteJobName)) } else { - sort.Strings(jobsToDelete) - sort.Strings(jc.DeleteJobName) - for i, expectedJobName := range jobsToDelete { - if expectedJobName != jc.DeleteJobName[i] { - t.Errorf("%s: expected job %s deleted, actually %v -- %v vs %v", name, expectedJobName, jc.DeleteJobName[i], jc.DeleteJobName, jobsToDelete) - } + jcDeleteJobName := sets.NewString(jc.DeleteJobName...) + if !jcDeleteJobName.Equal(jobsToDelete) { + t.Errorf("%s: expected jobs: %v deleted, actually: %v deleted", name, jobsToDelete, jcDeleteJobName) } } diff --git a/pkg/controller/cronjob/utils.go b/pkg/controller/cronjob/utils.go index 2416dbeb946..5909659def1 100644 --- a/pkg/controller/cronjob/utils.go +++ b/pkg/controller/cronjob/utils.go @@ -144,7 +144,7 @@ func getRecentUnmetScheduleTimes(sj batchv1beta1.CronJob, now time.Time) ([]time for t := sched.Next(earliestTime); !t.After(now); t = sched.Next(t) { starts = append(starts, t) - // An object might miss several starts. For example, if + // An object might miss several starts. For example, if // controller gets wedged on friday at 5:01pm when everyone has // gone home, and someone comes in on tuesday AM and discovers // the problem and restarts the controller, then all the hourly @@ -159,7 +159,7 @@ func getRecentUnmetScheduleTimes(sj batchv1beta1.CronJob, now time.Time) ([]time // of this controller. In that case, we want to not try to list // all the missed start times. // - // I've somewhat arbitrarily picked 100, as more than 80, but + // I've somewhat arbitrarily picked 100, as more than 80, // but less than "lots". if len(starts) > 100 { // We can't get the most recent times so just return an empty slice