From b80a8db6d3791f29cd6ad42eff94b728349f6024 Mon Sep 17 00:00:00 2001 From: hzxuzhonghu Date: Tue, 29 Aug 2017 20:13:18 +0800 Subject: [PATCH] update Deprecated code and fix bug not return when list pod failed --- pkg/controller/cronjob/cronjob_controller.go | 6 ++++- .../cronjob/cronjob_controller_test.go | 25 +++++++++++++++++-- pkg/controller/cronjob/injection.go | 7 ++++-- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/pkg/controller/cronjob/cronjob_controller.go b/pkg/controller/cronjob/cronjob_controller.go index 2ed8ddf6b28..682ae721a4a 100644 --- a/pkg/controller/cronjob/cronjob_controller.go +++ b/pkg/controller/cronjob/cronjob_controller.go @@ -72,7 +72,7 @@ func NewCronJobController(kubeClient clientset.Interface) *CronJobController { // TODO: remove the wrapper when every clients have moved to use the clientset. eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: v1core.New(kubeClient.Core().RESTClient()).Events("")}) - if kubeClient != nil && kubeClient.Core().RESTClient().GetRateLimiter() != nil { + if kubeClient != nil && kubeClient.CoreV1().RESTClient().GetRateLimiter() != nil { metrics.RegisterMetricAndTrackRateLimiterUsage("cronjob_controller", kubeClient.Core().RESTClient().GetRateLimiter()) } @@ -253,10 +253,12 @@ func syncOne(sj *batchv1beta1.CronJob, js []batchv1.Job, now time.Time, jc jobCo glog.V(4).Infof("Not starting job for %s because it is suspended", nameForLog) return } + times, err := getRecentUnmetScheduleTimes(*sj, now) if err != nil { recorder.Eventf(sj, v1.EventTypeWarning, "FailedNeedsStart", "Cannot determine if job needs to be started: %v", err) glog.Errorf("Cannot determine if %s needs to be started: %v", nameForLog, err) + return } // TODO: handle multiple unmet start times, from oldest to newest, updating status as needed. if len(times) == 0 { @@ -266,6 +268,7 @@ func syncOne(sj *batchv1beta1.CronJob, js []batchv1.Job, now time.Time, jc jobCo if len(times) > 1 { glog.V(4).Infof("Multiple unmet start times for %s so only starting last one", nameForLog) } + scheduledTime := times[len(times)-1] tooLate := false if sj.Spec.StartingDeadlineSeconds != nil { @@ -376,6 +379,7 @@ func deleteJob(sj *batchv1beta1.CronJob, job *batchv1.Job, jc jobControlInterfac podList, err := pc.ListPods(job.Namespace, options) if err != nil { recorder.Eventf(sj, v1.EventTypeWarning, "FailedList", "List job-pods: %v", err) + return false } errList := []error{} for _, pod := range podList.Items { diff --git a/pkg/controller/cronjob/cronjob_controller_test.go b/pkg/controller/cronjob/cronjob_controller_test.go index ec7ead4e789..f04819fd1c1 100644 --- a/pkg/controller/cronjob/cronjob_controller_test.go +++ b/pkg/controller/cronjob/cronjob_controller_test.go @@ -17,6 +17,7 @@ limitations under the License. package cronjob import ( + "errors" "sort" "strconv" "strings" @@ -34,9 +35,10 @@ import ( _ "k8s.io/kubernetes/pkg/apis/batch/install" ) -// schedule is hourly on the hour var ( - onTheHour string = "0 * * * ?" + // schedule is hourly on the hour + onTheHour string = "0 * * * ?" + errorSchedule string = "obvious error schedule" ) func justBeforeTheHour() time.Time { @@ -197,6 +199,9 @@ func TestSyncOne_RunOrNot(t *testing.T) { expectActive int expectedWarnings int }{ + "never ran, not valid schedule, A": {A, F, errorSchedule, noDead, F, F, justBeforeTheHour(), F, F, 0, 1}, + "never ran, not valid schedule, F": {f, F, errorSchedule, noDead, F, F, justBeforeTheHour(), F, F, 0, 1}, + "never ran, not valid schedule, R": {f, F, errorSchedule, noDead, F, F, justBeforeTheHour(), F, F, 0, 1}, "never ran, not time, A": {A, F, onTheHour, noDead, F, F, justBeforeTheHour(), F, F, 0, 0}, "never ran, not time, F": {f, F, onTheHour, noDead, F, F, justBeforeTheHour(), F, F, 0, 0}, "never ran, not time, R": {R, F, onTheHour, noDead, F, F, justBeforeTheHour(), F, F, 0, 0}, @@ -480,6 +485,16 @@ func TestCleanupFinishedJobs_DeleteOrNot(t *testing.T) { {"2016-05-19T08:00:00Z", F, F, F, F}, {"2016-05-19T09:00:00Z", F, F, F, F}, }, justBeforeTheHour(), &limitZero, &limitZero, 6}, + + "failed list pod err": { + []CleanupJobSpec{ + {"2016-05-19T04:00:00Z", T, F, F, F}, + {"2016-05-19T05:00:00Z", T, F, F, F}, + {"2016-05-19T06:00:00Z", T, T, F, F}, + {"2016-05-19T07:00:00Z", T, T, F, F}, + {"2016-05-19T08:00:00Z", T, F, F, F}, + {"2016-05-19T09:00:00Z", T, F, F, F}, + }, justBeforeTheHour(), &limitZero, &limitZero, 0}, } for name, tc := range testCases { @@ -551,6 +566,9 @@ func TestCleanupFinishedJobs_DeleteOrNot(t *testing.T) { pc := &fakePodControl{} sjc := &fakeSJControl{} recorder := record.NewFakeRecorder(10) + if name == "failed list pod err" { + pc.Err = errors.New("fakePodControl err") + } cleanupFinishedJobs(&sj, js, jc, sjc, pc, recorder) @@ -569,6 +587,9 @@ func TestCleanupFinishedJobs_DeleteOrNot(t *testing.T) { // Check for events expectedEvents := len(jobsToDelete) + if name == "failed list pod err" { + expectedEvents = len(tc.jobSpecs) + } if len(recorder.Events) != expectedEvents { t.Errorf("%s: expected %d event, actually %v", name, expectedEvents, len(recorder.Events)) } diff --git a/pkg/controller/cronjob/injection.go b/pkg/controller/cronjob/injection.go index 47563588687..7108879a8f9 100644 --- a/pkg/controller/cronjob/injection.go +++ b/pkg/controller/cronjob/injection.go @@ -216,11 +216,11 @@ type realPodControl struct { var _ podControlInterface = &realPodControl{} func (r realPodControl) ListPods(namespace string, opts metav1.ListOptions) (*v1.PodList, error) { - return r.KubeClient.Core().Pods(namespace).List(opts) + return r.KubeClient.CoreV1().Pods(namespace).List(opts) } func (r realPodControl) DeletePod(namespace string, name string) error { - return r.KubeClient.Core().Pods(namespace).Delete(name, nil) + return r.KubeClient.CoreV1().Pods(namespace).Delete(name, nil) } type fakePodControl struct { @@ -235,6 +235,9 @@ var _ podControlInterface = &fakePodControl{} func (f *fakePodControl) ListPods(namespace string, opts metav1.ListOptions) (*v1.PodList, error) { f.Lock() defer f.Unlock() + if f.Err != nil { + return nil, f.Err + } return &v1.PodList{Items: f.Pods}, nil }