From da57c93a8a68f993237466c2004ad810de47f47d Mon Sep 17 00:00:00 2001 From: Janet Kuo Date: Mon, 18 Jul 2016 13:24:57 -0700 Subject: [PATCH] Fix errors, verification and test failures; add unit test for sj UpdateStatus --- .../typed/batch/unversioned/batch_client.go | 9 - pkg/controller/job/jobcontroller.go | 13 +- pkg/controller/job/jobcontroller_test.go | 25 --- pkg/controller/job/utils.go | 31 +++ pkg/controller/job/utils_test.go | 49 +++++ pkg/controller/scheduledjob/controller.go | 20 +- .../scheduledjob/controller_test.go | 208 ++++++++++++++++-- pkg/controller/scheduledjob/doc.go | 2 +- pkg/controller/scheduledjob/injection.go | 2 +- pkg/controller/scheduledjob/utils.go | 20 +- pkg/controller/scheduledjob/utils_test.go | 47 +--- pkg/kubectl/cmd/util/clientcache.go | 3 - pkg/kubectl/cmd/util/factory.go | 1 - 13 files changed, 294 insertions(+), 136 deletions(-) create mode 100644 pkg/controller/job/utils.go create mode 100644 pkg/controller/job/utils_test.go diff --git a/pkg/client/clientset_generated/internalclientset/typed/batch/unversioned/batch_client.go b/pkg/client/clientset_generated/internalclientset/typed/batch/unversioned/batch_client.go index 783e8517139..8f2b0e80386 100644 --- a/pkg/client/clientset_generated/internalclientset/typed/batch/unversioned/batch_client.go +++ b/pkg/client/clientset_generated/internalclientset/typed/batch/unversioned/batch_client.go @@ -18,7 +18,6 @@ package unversioned import ( api "k8s.io/kubernetes/pkg/api" - //"k8s.io/kubernetes/pkg/api/unversioned" registered "k8s.io/kubernetes/pkg/apimachinery/registered" restclient "k8s.io/kubernetes/pkg/client/restclient" ) @@ -82,17 +81,9 @@ func setConfigDefaults(config *restclient.Config) error { } // TODO: Unconditionally set the config.Version, until we fix the config. //if config.Version == "" { - // XXX why is above commented out? copyGroupVersion := g.GroupVersion config.GroupVersion = ©GroupVersion //} - // Do we need something like this: - // if config.Version == "" { - // copyGroupVersion := g.GroupVersion - // config.GroupVersion = ©GroupVersion - //} else { - // config.GroupVersion = &unversioned.GroupVersion{Group: "batch", Version: config.Version} - //} config.NegotiatedSerializer = api.Codecs diff --git a/pkg/controller/job/jobcontroller.go b/pkg/controller/job/jobcontroller.go index b4639dec589..6e6b1b3691f 100644 --- a/pkg/controller/job/jobcontroller.go +++ b/pkg/controller/job/jobcontroller.go @@ -113,7 +113,7 @@ func NewJobController(podInformer framework.SharedIndexInformer, kubeClient clie framework.ResourceEventHandlerFuncs{ AddFunc: jm.enqueueController, UpdateFunc: func(old, cur interface{}) { - if job := cur.(*batch.Job); !isJobFinished(job) { + if job := cur.(*batch.Job); !IsJobFinished(job) { jm.enqueueController(job) } }, @@ -347,7 +347,7 @@ func (jm *JobController) syncJob(key string) error { job.Status.StartTime = &now } // if job was finished previously, we don't want to redo the termination - if isJobFinished(&job) { + if IsJobFinished(&job) { return nil } if pastActiveDeadline(&job) { @@ -559,15 +559,6 @@ func filterPods(pods []api.Pod, phase api.PodPhase) int { return result } -func isJobFinished(j *batch.Job) bool { - for _, c := range j.Status.Conditions { - if (c.Type == batch.JobComplete || c.Type == batch.JobFailed) && c.Status == api.ConditionTrue { - return true - } - } - return false -} - // byCreationTimestamp sorts a list by creation timestamp, using their names as a tie breaker. type byCreationTimestamp []batch.Job diff --git a/pkg/controller/job/jobcontroller_test.go b/pkg/controller/job/jobcontroller_test.go index 4468d544051..b2a57d57601 100644 --- a/pkg/controller/job/jobcontroller_test.go +++ b/pkg/controller/job/jobcontroller_test.go @@ -657,31 +657,6 @@ func TestWatchJobs(t *testing.T) { <-received } -func TestIsJobFinished(t *testing.T) { - job := &batch.Job{ - Status: batch.JobStatus{ - Conditions: []batch.JobCondition{{ - Type: batch.JobComplete, - Status: api.ConditionTrue, - }}, - }, - } - - if !isJobFinished(job) { - t.Error("Job was expected to be finished") - } - - job.Status.Conditions[0].Status = api.ConditionFalse - if isJobFinished(job) { - t.Error("Job was not expected to be finished") - } - - job.Status.Conditions[0].Status = api.ConditionUnknown - if isJobFinished(job) { - t.Error("Job was not expected to be finished") - } -} - func TestWatchPods(t *testing.T) { testJob := newJob(2, 2) clientset := fake.NewSimpleClientset(testJob) diff --git a/pkg/controller/job/utils.go b/pkg/controller/job/utils.go new file mode 100644 index 00000000000..64d1f4268a8 --- /dev/null +++ b/pkg/controller/job/utils.go @@ -0,0 +1,31 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package job + +import ( + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/apis/batch" +) + +func IsJobFinished(j *batch.Job) bool { + for _, c := range j.Status.Conditions { + if (c.Type == batch.JobComplete || c.Type == batch.JobFailed) && c.Status == api.ConditionTrue { + return true + } + } + return false +} diff --git a/pkg/controller/job/utils_test.go b/pkg/controller/job/utils_test.go new file mode 100644 index 00000000000..59f2b63f19f --- /dev/null +++ b/pkg/controller/job/utils_test.go @@ -0,0 +1,49 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package job + +import ( + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/apis/batch" +) + +func TestIsJobFinished(t *testing.T) { + job := &batch.Job{ + Status: batch.JobStatus{ + Conditions: []batch.JobCondition{{ + Type: batch.JobComplete, + Status: api.ConditionTrue, + }}, + }, + } + + if !IsJobFinished(job) { + t.Error("Job was expected to be finished") + } + + job.Status.Conditions[0].Status = api.ConditionFalse + if IsJobFinished(job) { + t.Error("Job was not expected to be finished") + } + + job.Status.Conditions[0].Status = api.ConditionUnknown + if IsJobFinished(job) { + t.Error("Job was not expected to be finished") + } +} diff --git a/pkg/controller/scheduledjob/controller.go b/pkg/controller/scheduledjob/controller.go index e62858103f8..87cb9891736 100644 --- a/pkg/controller/scheduledjob/controller.go +++ b/pkg/controller/scheduledjob/controller.go @@ -1,5 +1,5 @@ /* -Copyright 2016 The Kubernetes Authors All rights reserved. +Copyright 2016 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -40,6 +40,7 @@ import ( clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" unversionedcore "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/unversioned" "k8s.io/kubernetes/pkg/client/record" + "k8s.io/kubernetes/pkg/controller/job" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util/metrics" utilruntime "k8s.io/kubernetes/pkg/util/runtime" @@ -84,8 +85,8 @@ func NewScheduledJobControllerFromClient(kubeClient clientset.Interface) *Schedu func (jm *ScheduledJobController) Run(stopCh <-chan struct{}) { defer utilruntime.HandleCrash() glog.Infof("Starting ScheduledJob Manager") - // Check things every 1 second. - go wait.Until(jm.SyncAll, 1*time.Second, stopCh) + // Check things every 10 second. + go wait.Until(jm.SyncAll, 10*time.Second, stopCh) <-stopCh glog.Infof("Shutting down ScheduledJob Manager") } @@ -118,13 +119,11 @@ func (jm *ScheduledJobController) SyncAll() { // SyncOne reconciles a ScheduledJob with a list of any Jobs that it created. // All known jobs created by "sj" should be included in "js". -// Returns a new ScheduledJobStatus if an update to status is required, else nil. // The current time is passed in to facilitate testing. // It has no receiver, to facilitate testing. func SyncOne(sj batch.ScheduledJob, js []batch.Job, now time.Time, jc jobControlInterface, sjc sjControlInterface, recorder record.EventRecorder) { - nameForLog := fmt.Sprintf("namespace/%s/scheduledJob/%s", sj.Namespace, sj.Name) + nameForLog := fmt.Sprintf("%s/%s", sj.Namespace, sj.Name) - glog.V(4).Infof("Not starting job for %s because it is suspended", nameForLog) for _, j := range js { found := inActiveList(sj, j.ObjectMeta.UID) if !found { @@ -140,7 +139,7 @@ func SyncOne(sj batch.ScheduledJob, js []batch.Job, now time.Time, jc jobControl // in the same namespace "adopt" that job. ReplicaSets and their Pods work the same way. // TBS: how to update sj.Status.LastScheduleTime if the adopted job is newer than any we knew about? } else { - if isJobActive(&j) { + if job.IsJobFinished(&j) { deleteFromActiveList(&sj, j.ObjectMeta.UID) // TODO: event to call out failure vs success. recorder.Eventf(&sj, api.EventTypeNormal, "SawCompletedJob", "Saw completed job: %v", j.Name) @@ -152,7 +151,7 @@ func SyncOne(sj batch.ScheduledJob, js []batch.Job, now time.Time, jc jobControl glog.Errorf("Unable to update status for %s: %v", nameForLog, err) } - if sj.Spec.Suspend { + if sj.Spec.Suspend != nil && *sj.Spec.Suspend { glog.V(4).Infof("Not starting job for %s because it is suspended", nameForLog) return } @@ -200,7 +199,6 @@ func SyncOne(sj batch.ScheduledJob, js []batch.Job, now time.Time, jc jobControl return } if sj.Spec.ConcurrencyPolicy == batch.ReplaceConcurrent { - glog.Errorf("Not starting job for %s because of prior execution still running and concurrency policy is Replace and delete is not supported yet", nameForLog) for _, j := range sj.Status.Active { glog.V(4).Infof("Deleting job %s of %s s that was still running at next scheduled start time", j.Name, nameForLog) if err := jc.DeleteJob(j.Namespace, j.Name); err != nil { @@ -231,7 +229,7 @@ func SyncOne(sj batch.ScheduledJob, js []batch.Job, now time.Time, jc jobControl // iteration of SyncAll, we might not see our own status update, and // then post one again. So, we need to use the job name as a lock to // prevent us from making the job twice. TODO: name the job - // deterministically. + // deterministically (via hash of its scheduled time). // Add the just-started job to the status list. ref, err := getRef(jobResp) @@ -240,7 +238,7 @@ func SyncOne(sj batch.ScheduledJob, js []batch.Job, now time.Time, jc jobControl } else { sj.Status.Active = append(sj.Status.Active, *ref) } - sj.Status.LastScheduleTime = &unversioned.Time{scheduledTime} + sj.Status.LastScheduleTime = &unversioned.Time{Time: scheduledTime} if err := sjc.UpdateStatus(&sj); err != nil { glog.Infof("Unable to update status for %s: %v", nameForLog, err) } diff --git a/pkg/controller/scheduledjob/controller_test.go b/pkg/controller/scheduledjob/controller_test.go index 32df1cee4dd..a6bfffc76c3 100644 --- a/pkg/controller/scheduledjob/controller_test.go +++ b/pkg/controller/scheduledjob/controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2016 The Kubernetes Authors All rights reserved. +Copyright 2016 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,13 +17,14 @@ limitations under the License. package scheduledjob import ( + "testing" + "time" + "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/apis/batch" "k8s.io/kubernetes/pkg/client/record" "k8s.io/kubernetes/pkg/types" - "testing" - "time" ) // schedule is hourly on the hour @@ -78,8 +79,8 @@ func scheduledJob() batch.ScheduledJob { Name: "myscheduledjob", Namespace: "snazzycats", UID: types.UID("1a2b3c"), - SelfLink: "/apis/extensions/v1beta1/namespaces/snazzycats/jobs/myscheduledjob", - CreationTimestamp: unversioned.Time{justBeforeTheHour()}, + SelfLink: "/apis/batch/v2alpha1/namespaces/snazzycats/jobs/myscheduledjob", + CreationTimestamp: unversioned.Time{Time: justBeforeTheHour()}, }, Spec: batch.ScheduledJobSpec{ Schedule: "0 0 * * * * ?", @@ -89,25 +90,41 @@ func scheduledJob() batch.ScheduledJob { Labels: map[string]string{"a": "b"}, Annotations: map[string]string{"x": "y"}, }, - Spec: batch.JobSpec{ - Template: api.PodTemplateSpec{ - ObjectMeta: api.ObjectMeta{ - Labels: map[string]string{ - "foo": "bar", - }, - }, - Spec: api.PodSpec{ - Containers: []api.Container{ - {Image: "foo/bar"}, - }, - }, - }, + Spec: jobSpec(), + }, + }, + } +} + +func jobSpec() batch.JobSpec { + return batch.JobSpec{ + Template: api.PodTemplateSpec{ + ObjectMeta: api.ObjectMeta{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + {Image: "foo/bar"}, }, }, }, } } +func newJob(UID string) batch.Job { + return batch.Job{ + ObjectMeta: api.ObjectMeta{ + UID: types.UID(UID), + Name: "foobar", + Namespace: api.NamespaceDefault, + SelfLink: "/apis/batch/v1/namespaces/snazzycats/jobs/myjob", + }, + Spec: jobSpec(), + } +} + var ( shortDead int64 = 10 longDead int64 = 1000000 @@ -173,20 +190,20 @@ func TestSyncOne_RunOrNot(t *testing.T) { t.Log("Test case:", name) sj := scheduledJob() sj.Spec.ConcurrencyPolicy = tc.concurrencyPolicy - sj.Spec.Suspend = tc.suspend + sj.Spec.Suspend = &tc.suspend sj.Spec.Schedule = tc.schedule if tc.deadline != noDead { sj.Spec.StartingDeadlineSeconds = &tc.deadline } if tc.ranPreviously { - sj.ObjectMeta.CreationTimestamp = unversioned.Time{justBeforeThePriorHour()} - sj.Status.LastScheduleTime = &unversioned.Time{justAfterThePriorHour()} + sj.ObjectMeta.CreationTimestamp = unversioned.Time{Time: justBeforeThePriorHour()} + sj.Status.LastScheduleTime = &unversioned.Time{Time: justAfterThePriorHour()} if tc.stillActive { sj.Status.Active = []api.ObjectReference{{}} } } else { - sj.ObjectMeta.CreationTimestamp = unversioned.Time{justBeforeTheHour()} + sj.ObjectMeta.CreationTimestamp = unversioned.Time{Time: justBeforeTheHour()} if tc.stillActive { t.Errorf("Test setup error: this case makes no sense.") } @@ -228,3 +245,150 @@ func TestSyncOne_RunOrNot(t *testing.T) { // TODO: simulation where the controller randomly doesn't run, and randomly has errors starting jobs or deleting jobs, // but over time, all jobs run as expected (assuming Allow and no deadline). + +// TestSyncOne_Status tests sj.UpdateStatus in SyncOne +func TestSyncOne_Status(t *testing.T) { + finishedJob := newJob("1") + finishedJob.Status.Conditions = append(finishedJob.Status.Conditions, batch.JobCondition{Type: batch.JobComplete, Status: api.ConditionTrue}) + unexpectedJob := newJob("2") + + testCases := map[string]struct { + // sj spec + concurrencyPolicy batch.ConcurrencyPolicy + suspend bool + schedule string + deadline int64 + + // sj status + ranPreviously bool + hasFinishedJob bool + + // environment + now time.Time + hasUnexpectedJob bool + + // expectations + expectCreate bool + expectDelete bool + }{ + "never ran, not time, A": {A, F, onTheHour, noDead, F, F, justBeforeTheHour(), F, F, F}, + "never ran, not time, F": {f, F, onTheHour, noDead, F, F, justBeforeTheHour(), F, F, F}, + "never ran, not time, R": {R, F, onTheHour, noDead, F, F, justBeforeTheHour(), F, F, F}, + "never ran, is time, A": {A, F, onTheHour, noDead, F, F, justAfterTheHour(), F, T, F}, + "never ran, is time, F": {f, F, onTheHour, noDead, F, F, justAfterTheHour(), F, T, F}, + "never ran, is time, R": {R, F, onTheHour, noDead, F, F, justAfterTheHour(), F, T, F}, + "never ran, is time, suspended": {A, T, onTheHour, noDead, F, F, justAfterTheHour(), F, F, F}, + "never ran, is time, past deadline": {A, F, onTheHour, shortDead, F, F, justAfterTheHour(), F, F, F}, + "never ran, is time, not past deadline": {A, F, onTheHour, longDead, F, F, justAfterTheHour(), F, T, F}, + + "prev ran but done, not time, A": {A, F, onTheHour, noDead, T, F, justBeforeTheHour(), F, F, F}, + "prev ran but done, not time, finished job, A": {A, F, onTheHour, noDead, T, T, justBeforeTheHour(), F, F, F}, + "prev ran but done, not time, unexpected job, A": {A, F, onTheHour, noDead, T, F, justBeforeTheHour(), T, F, F}, + "prev ran but done, not time, finished job, unexpected job, A": {A, F, onTheHour, noDead, T, T, justBeforeTheHour(), T, F, F}, + "prev ran but done, not time, finished job, F": {f, F, onTheHour, noDead, T, T, justBeforeTheHour(), F, F, F}, + "prev ran but done, not time, unexpected job, R": {R, F, onTheHour, noDead, T, F, justBeforeTheHour(), T, F, F}, + + "prev ran but done, is time, A": {A, F, onTheHour, noDead, T, F, justAfterTheHour(), F, T, F}, + "prev ran but done, is time, finished job, A": {A, F, onTheHour, noDead, T, T, justAfterTheHour(), F, T, F}, + "prev ran but done, is time, unexpected job, A": {A, F, onTheHour, noDead, T, F, justAfterTheHour(), T, T, F}, + "prev ran but done, is time, finished job, unexpected job, A": {A, F, onTheHour, noDead, T, T, justAfterTheHour(), T, T, F}, + "prev ran but done, is time, F": {f, F, onTheHour, noDead, T, F, justAfterTheHour(), F, T, F}, + "prev ran but done, is time, finished job, F": {f, F, onTheHour, noDead, T, T, justAfterTheHour(), F, T, F}, + "prev ran but done, is time, unexpected job, F": {f, F, onTheHour, noDead, T, F, justAfterTheHour(), T, T, F}, + "prev ran but done, is time, finished job, unexpected job, F": {f, F, onTheHour, noDead, T, T, justAfterTheHour(), T, T, F}, + "prev ran but done, is time, R": {R, F, onTheHour, noDead, T, F, justAfterTheHour(), F, T, F}, + "prev ran but done, is time, finished job, R": {R, F, onTheHour, noDead, T, T, justAfterTheHour(), F, T, F}, + "prev ran but done, is time, unexpected job, R": {R, F, onTheHour, noDead, T, F, justAfterTheHour(), T, T, F}, + "prev ran but done, is time, finished job, unexpected job, R": {R, F, onTheHour, noDead, T, T, justAfterTheHour(), T, T, F}, + "prev ran but done, is time, suspended": {A, T, onTheHour, noDead, T, F, justAfterTheHour(), F, F, F}, + "prev ran but done, is time, finished job, suspended": {A, T, onTheHour, noDead, T, T, justAfterTheHour(), F, F, F}, + "prev ran but done, is time, unexpected job, suspended": {A, T, onTheHour, noDead, T, F, justAfterTheHour(), T, F, F}, + "prev ran but done, is time, finished job, unexpected job, suspended": {A, T, onTheHour, noDead, T, T, justAfterTheHour(), T, F, F}, + "prev ran but done, is time, past deadline": {A, F, onTheHour, shortDead, T, F, justAfterTheHour(), F, F, F}, + "prev ran but done, is time, finished job, past deadline": {A, F, onTheHour, shortDead, T, T, justAfterTheHour(), F, F, F}, + "prev ran but done, is time, unexpected job, past deadline": {A, F, onTheHour, shortDead, T, F, justAfterTheHour(), T, F, F}, + "prev ran but done, is time, finished job, unexpected job, past deadline": {A, F, onTheHour, shortDead, T, T, justAfterTheHour(), T, F, F}, + "prev ran but done, is time, not past deadline": {A, F, onTheHour, longDead, T, F, justAfterTheHour(), F, T, F}, + "prev ran but done, is time, finished job, not past deadline": {A, F, onTheHour, longDead, T, T, justAfterTheHour(), F, T, F}, + "prev ran but done, is time, unexpected job, not past deadline": {A, F, onTheHour, longDead, T, F, justAfterTheHour(), T, T, F}, + "prev ran but done, is time, finished job, unexpected job, not past deadline": {A, F, onTheHour, longDead, T, T, justAfterTheHour(), T, T, F}, + } + + for name, tc := range testCases { + t.Log("Test case:", name) + // Setup the test + sj := scheduledJob() + sj.Spec.ConcurrencyPolicy = tc.concurrencyPolicy + sj.Spec.Suspend = &tc.suspend + sj.Spec.Schedule = tc.schedule + if tc.deadline != noDead { + sj.Spec.StartingDeadlineSeconds = &tc.deadline + } + if tc.ranPreviously { + sj.ObjectMeta.CreationTimestamp = unversioned.Time{Time: justBeforeThePriorHour()} + sj.Status.LastScheduleTime = &unversioned.Time{Time: justAfterThePriorHour()} + } else { + if tc.hasFinishedJob || tc.hasUnexpectedJob { + t.Errorf("Test setup error: this case makes no sense.") + } + sj.ObjectMeta.CreationTimestamp = unversioned.Time{Time: justBeforeTheHour()} + } + jobs := []batch.Job{} + if tc.hasFinishedJob { + ref, err := getRef(&finishedJob) + if err != nil { + t.Errorf("Test setup error: failed to get job's ref: %v.", err) + } + sj.Status.Active = []api.ObjectReference{*ref} + jobs = append(jobs, finishedJob) + } + if tc.hasUnexpectedJob { + jobs = append(jobs, unexpectedJob) + } + + jc := &fakeJobControl{} + sjc := &fakeSJControl{} + recorder := record.NewFakeRecorder(10) + + // Run the code + SyncOne(sj, jobs, tc.now, jc, sjc, recorder) + + // Status update happens once when ranging through job list, and another one if create jobs. + expectUpdates := 1 + // Events happens when there's unexpected / finished jobs, and upon job creation / deletion. + expectedEvents := 0 + if tc.expectCreate { + expectUpdates++ + expectedEvents++ + } + if tc.expectDelete { + expectedEvents++ + } + if tc.hasFinishedJob { + expectedEvents++ + } + if tc.hasUnexpectedJob { + expectedEvents++ + } + + if len(recorder.Events) != expectedEvents { + t.Errorf("Expected %d event, actually %v: %#v", expectedEvents, len(recorder.Events), recorder.Events) + } + + if expectUpdates != len(sjc.Updates) { + t.Errorf("expected %d status updates, actually %d", expectUpdates, len(sjc.Updates)) + } + + if tc.hasFinishedJob && inActiveList(sjc.Updates[0], finishedJob.UID) { + t.Errorf("Expected finished job removed from active list, actually active list = %#v.", sjc.Updates[0].Status.Active) + } + + if tc.hasUnexpectedJob && inActiveList(sjc.Updates[0], unexpectedJob.UID) { + t.Errorf("Expected unexpected job not added to active list, actually active list = %#v.", sjc.Updates[0].Status.Active) + } + + if tc.expectCreate && !sjc.Updates[1].Status.LastScheduleTime.Time.Equal(topOfTheHour()) { + t.Errorf("Expected LastScheduleTime updated to %s, got %s.", topOfTheHour(), sjc.Updates[1].Status.LastScheduleTime) + } + } +} diff --git a/pkg/controller/scheduledjob/doc.go b/pkg/controller/scheduledjob/doc.go index 3fafb54d088..649d7e4404e 100644 --- a/pkg/controller/scheduledjob/doc.go +++ b/pkg/controller/scheduledjob/doc.go @@ -1,5 +1,5 @@ /* -Copyright 2015 The Kubernetes Authors All rights reserved. +Copyright 2016 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/pkg/controller/scheduledjob/injection.go b/pkg/controller/scheduledjob/injection.go index 5b81edc049c..fed5c06aa51 100644 --- a/pkg/controller/scheduledjob/injection.go +++ b/pkg/controller/scheduledjob/injection.go @@ -1,5 +1,5 @@ /* -Copyright 2016 The Kubernetes Authors All rights reserved. +Copyright 2016 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/pkg/controller/scheduledjob/utils.go b/pkg/controller/scheduledjob/utils.go index 0c4213c27aa..2adc6bfdf80 100644 --- a/pkg/controller/scheduledjob/utils.go +++ b/pkg/controller/scheduledjob/utils.go @@ -1,5 +1,5 @@ /* -Copyright 2016 The Kubernetes Authors All rights reserved. +Copyright 2016 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -119,9 +119,9 @@ func getNextStartTimeAfter(schedule string, now time.Time) (time.Time, error) { return sched.Next(now), nil } -// getRecentUnmetScheduleTimes gets a slice of times that have passed when a Job should have started but did not +// getRecentUnmetScheduleTimes gets a slice of times (from oldest to latest) that have passed when a Job should have started but did not. // -// If there are too many unstarted times, only the most recent may be returned. +// If there are too many (>100) unstarted times, just give up and return an empty slice. // If there were missed times prior to the last known start time, then those are not returned. func getRecentUnmetScheduleTimes(sj batch.ScheduledJob, now time.Time) ([]time.Time, error) { starts := []time.Time{} @@ -166,25 +166,13 @@ func getRecentUnmetScheduleTimes(sj batch.ScheduledJob, now time.Time) ([]time.T // I've somewhat arbitrarily picked 100, as more than 80, but // but less than "lots". if len(starts) > 100 { + // We can't get the most recent times so just return an empty slice return []time.Time{}, fmt.Errorf("Too many missed start times to list") } } return starts, nil } -func isJobActive(j *batch.Job) bool { - return !isJobFinished(j) -} - -func isJobFinished(j *batch.Job) bool { - for _, c := range j.Status.Conditions { - if (c.Type == batch.JobComplete || c.Type == batch.JobFailed) && c.Status == api.ConditionTrue { - return true - } - } - return false -} - // XXX unit test this // getJobFromTemplate makes a Job from a ScheduledJob diff --git a/pkg/controller/scheduledjob/utils_test.go b/pkg/controller/scheduledjob/utils_test.go index 78bfd02a4bf..226015cf9ed 100644 --- a/pkg/controller/scheduledjob/utils_test.go +++ b/pkg/controller/scheduledjob/utils_test.go @@ -1,5 +1,5 @@ /* -Copyright 2015 The Kubernetes Authors All rights reserved. +Copyright 2016 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -283,7 +283,7 @@ func TestGetRecentUnmetScheduleTimes(t *testing.T) { { // Case 1: no known start times, and none needed yet. // Creation time is before T1. - sj.ObjectMeta.CreationTimestamp = unversioned.Time{T1.Add(-10 * time.Minute)} + sj.ObjectMeta.CreationTimestamp = unversioned.Time{Time: T1.Add(-10 * time.Minute)} // Current time is more than creation time, but less than T1. now := T1.Add(-7 * time.Minute) times, err := getRecentUnmetScheduleTimes(sj, now) @@ -297,7 +297,7 @@ func TestGetRecentUnmetScheduleTimes(t *testing.T) { { // Case 2: no known start times, and one needed. // Creation time is before T1. - sj.ObjectMeta.CreationTimestamp = unversioned.Time{T1.Add(-10 * time.Minute)} + sj.ObjectMeta.CreationTimestamp = unversioned.Time{Time: T1.Add(-10 * time.Minute)} // Current time is after T1 now := T1.Add(2 * time.Second) times, err := getRecentUnmetScheduleTimes(sj, now) @@ -313,9 +313,9 @@ func TestGetRecentUnmetScheduleTimes(t *testing.T) { { // Case 3: known LastScheduleTime, no start needed. // Creation time is before T1. - sj.ObjectMeta.CreationTimestamp = unversioned.Time{T1.Add(-10 * time.Minute)} + sj.ObjectMeta.CreationTimestamp = unversioned.Time{Time: T1.Add(-10 * time.Minute)} // Status shows a start at the expected time. - sj.Status.LastScheduleTime = &unversioned.Time{T1} + sj.Status.LastScheduleTime = &unversioned.Time{Time: T1} // Current time is after T1 now := T1.Add(2 * time.Minute) times, err := getRecentUnmetScheduleTimes(sj, now) @@ -329,9 +329,9 @@ func TestGetRecentUnmetScheduleTimes(t *testing.T) { { // Case 4: known LastScheduleTime, a start needed // Creation time is before T1. - sj.ObjectMeta.CreationTimestamp = unversioned.Time{T1.Add(-10 * time.Minute)} + sj.ObjectMeta.CreationTimestamp = unversioned.Time{Time: T1.Add(-10 * time.Minute)} // Status shows a start at the expected time. - sj.Status.LastScheduleTime = &unversioned.Time{T1} + sj.Status.LastScheduleTime = &unversioned.Time{Time: T1} // Current time is after T1 and after T2 now := T2.Add(5 * time.Minute) times, err := getRecentUnmetScheduleTimes(sj, now) @@ -346,8 +346,8 @@ func TestGetRecentUnmetScheduleTimes(t *testing.T) { } { // Case 5: known LastScheduleTime, two starts needed - sj.ObjectMeta.CreationTimestamp = unversioned.Time{T1.Add(-2 * time.Hour)} - sj.Status.LastScheduleTime = &unversioned.Time{T1.Add(-1 * time.Hour)} + sj.ObjectMeta.CreationTimestamp = unversioned.Time{Time: T1.Add(-2 * time.Hour)} + sj.Status.LastScheduleTime = &unversioned.Time{Time: T1.Add(-1 * time.Hour)} // Current time is after T1 and after T2 now := T2.Add(5 * time.Minute) times, err := getRecentUnmetScheduleTimes(sj, now) @@ -367,8 +367,8 @@ func TestGetRecentUnmetScheduleTimes(t *testing.T) { } { // Case 6: now is way way ahead of last start time. - sj.ObjectMeta.CreationTimestamp = unversioned.Time{T1.Add(-2 * time.Hour)} - sj.Status.LastScheduleTime = &unversioned.Time{T1.Add(-1 * time.Hour)} + sj.ObjectMeta.CreationTimestamp = unversioned.Time{Time: T1.Add(-2 * time.Hour)} + sj.Status.LastScheduleTime = &unversioned.Time{Time: T1.Add(-1 * time.Hour)} now := T2.Add(10 * 24 * time.Hour) _, err := getRecentUnmetScheduleTimes(sj, now) if err == nil { @@ -377,28 +377,3 @@ func TestGetRecentUnmetScheduleTimes(t *testing.T) { } } - -func TestIsJobFinished(t *testing.T) { - job := &batch.Job{ - Status: batch.JobStatus{ - Conditions: []batch.JobCondition{{ - Type: batch.JobComplete, - Status: api.ConditionTrue, - }}, - }, - } - - if !isJobFinished(job) { - t.Error("Job was expected to be finished") - } - - job.Status.Conditions[0].Status = api.ConditionFalse - if isJobFinished(job) { - t.Error("Job was not expected to be finished") - } - - job.Status.Conditions[0].Status = api.ConditionUnknown - if isJobFinished(job) { - t.Error("Job was not expected to be finished") - } -} diff --git a/pkg/kubectl/cmd/util/clientcache.go b/pkg/kubectl/cmd/util/clientcache.go index 1dc50ad8099..81476cabaf0 100644 --- a/pkg/kubectl/cmd/util/clientcache.go +++ b/pkg/kubectl/cmd/util/clientcache.go @@ -17,8 +17,6 @@ limitations under the License. package util import ( - "fmt" - fed_clientset "k8s.io/kubernetes/federation/client/clientset_generated/federation_internalclientset" "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/apimachinery/registered" @@ -84,7 +82,6 @@ func (c *ClientCache) ClientConfigForVersion(version *unversioned.GroupVersion) return nil, err } config.GroupVersion = negotiatedVersion - fmt.Printf(" Negotiated version %v\n", negotiatedVersion) if version != nil { c.configs[*version] = &config diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 251c8a78f5c..a961805cd84 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -352,7 +352,6 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory { }, ClientForMapping: func(mapping *meta.RESTMapping) (resource.RESTClient, error) { cfg, err := clientConfig.ClientConfig() - fmt.Printf("Mapping version: %#v", mappingVersion) if err != nil { return nil, err }