diff --git a/pkg/controller/scheduledjob/controller.go b/pkg/controller/scheduledjob/controller.go index ae7cb3fe750..956e9eb44fd 100644 --- a/pkg/controller/scheduledjob/controller.go +++ b/pkg/controller/scheduledjob/controller.go @@ -209,7 +209,7 @@ func SyncOne(sj batch.ScheduledJob, js []batch.Job, now time.Time, jc jobControl } } - jobReq, err := getJobFromTemplate(&sj) + jobReq, err := getJobFromTemplate(&sj, scheduledTime) if err != nil { glog.Errorf("Unable to make Job from template in %s: %v", nameForLog, err) return @@ -228,8 +228,8 @@ func SyncOne(sj batch.ScheduledJob, js []batch.Job, now time.Time, jc jobControl // the next time. Actually, if we relist the SJs and Jobs on the next // 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 (via hash of its scheduled time). + // prevent us from making the job twice (name the job with hash of its + // scheduled time). // Add the just-started job to the status list. ref, err := getRef(jobResp) diff --git a/pkg/controller/scheduledjob/utils.go b/pkg/controller/scheduledjob/utils.go index 99d824f58d2..a5d716baf0e 100644 --- a/pkg/controller/scheduledjob/utils.go +++ b/pkg/controller/scheduledjob/utils.go @@ -19,6 +19,7 @@ package scheduledjob import ( "encoding/json" "fmt" + "hash/adler32" "time" "github.com/golang/glog" @@ -29,6 +30,7 @@ import ( "k8s.io/kubernetes/pkg/apis/batch" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/types" + hashutil "k8s.io/kubernetes/pkg/util/hash" ) // Utilities for dealing with Jobs and ScheduledJobs and time. @@ -186,7 +188,7 @@ func addSeconds(schedule string) string { // XXX unit test this // getJobFromTemplate makes a Job from a ScheduledJob -func getJobFromTemplate(sj *batch.ScheduledJob) (*batch.Job, error) { +func getJobFromTemplate(sj *batch.ScheduledJob, scheduledTime time.Time) (*batch.Job, error) { // TODO: consider adding the following labels: // nominal-start-time=$RFC_3339_DATE_OF_INTENDED_START -- for user convenience // scheduled-job-name=$SJ_NAME -- for user convenience @@ -197,16 +199,14 @@ func getJobFromTemplate(sj *batch.ScheduledJob) (*batch.Job, error) { return nil, err } annotations[CreatedByAnnotation] = string(createdByRefJson) - // TODO: instead of using generateName, use a deterministic hash of the nominal - // start time, to prevent same job being created twice. - // We want job names for a given nominal start time to have a predictable name to avoid - prefix := fmt.Sprintf("%s-", sj.Name) + // We want job names for a given nominal start time to have a deterministic name to avoid the same job being created twice + name := fmt.Sprintf("%s-%d", sj.Name, getTimeHash(scheduledTime)) job := &batch.Job{ ObjectMeta: api.ObjectMeta{ - Labels: labels, - Annotations: annotations, - GenerateName: prefix, + Labels: labels, + Annotations: annotations, + Name: name, }, } if err := api.Scheme.Convert(&sj.Spec.JobTemplate.Spec, &job.Spec); err != nil { @@ -215,6 +215,12 @@ func getJobFromTemplate(sj *batch.ScheduledJob) (*batch.Job, error) { return job, nil } +func getTimeHash(scheduledTime time.Time) uint32 { + timeHasher := adler32.New() + hashutil.DeepHashObject(timeHasher, scheduledTime) + return timeHasher.Sum32() +} + // makeCreatedByRefJson makes a json string with an object reference for use in "created-by" annotation value func makeCreatedByRefJson(object runtime.Object) (string, error) { createdByRef, err := api.GetReference(object) diff --git a/pkg/controller/scheduledjob/utils_test.go b/pkg/controller/scheduledjob/utils_test.go index d65fe207f91..0280b76bba4 100644 --- a/pkg/controller/scheduledjob/utils_test.go +++ b/pkg/controller/scheduledjob/utils_test.go @@ -18,6 +18,7 @@ package scheduledjob import ( //"fmt" + "strings" "testing" "time" @@ -72,12 +73,12 @@ func TestGetJobFromTemplate(t *testing.T) { } var job *batch.Job - job, err := getJobFromTemplate(&sj) + job, err := getJobFromTemplate(&sj, time.Time{}) if err != nil { t.Errorf("Did not expect error: %s", err) } - if job.ObjectMeta.GenerateName != "myscheduledjob-" { - t.Errorf("Wrong GenerateName") + if !strings.HasPrefix(job.ObjectMeta.Name, "myscheduledjob-") { + t.Errorf("Wrong Name") } if len(job.ObjectMeta.Labels) != 1 { t.Errorf("Wrong number of labels")