Merge pull request #30420 from janetkuo/sj-job-determi

Automatic merge from submit-queue

Name jobs created by sj deterministically

```release-note
Name the job created by scheduledjob (sj) deterministically with sj's name and a hash of job's scheduled time.
```

@erictune @soltysh

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.kubernetes.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.kubernetes.io/reviews/kubernetes/kubernetes/30420)
<!-- Reviewable:end -->

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)

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)

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