Merge pull request #51519 from hzxuzhonghu/cronjob

Automatic merge from submit-queue

update deprecated interface and fix bug not return when list pod failed in cronjob_controller.go

**What this PR does / why we need it**:
remove some unused redundant code, and fix bug: when list pod failed, 
job still deleted but pod may still exist  in func `deleteJob`

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

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 *batchv2alpha1.CronJob, js []batchv1.Job, now time.Time, jc jobC
 		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 *batchv2alpha1.CronJob, js []batchv1.Job, now time.Time, jc jobC
 	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 *batchv2alpha1.CronJob, job *batchv1.Job, jc jobControlInterfa
 	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 {

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))
 		}

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
 }