Merge pull request #30227 from soltysh/remove_seconds

Automatic merge from submit-queue

Remove seconds from scheduled jobs cron format

@erictune @janetkuo as promised this removes the seconds from the cron format

<!-- 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/30227)
<!-- Reviewable:end -->

pkg/apis/batch/validation/validation.go

@@ -205,8 +205,12 @@ func validateConcurrencyPolicy(concurrencyPolicy *batch.ConcurrencyPolicy, fldPa
 
 func validateScheduleFormat(schedule string, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
-	_, err := cron.Parse(schedule)
+	// TODO soltysh: this should be removed when https://github.com/robfig/cron/issues/58 is fixed
-	if err != nil {
+	tmpSchedule := schedule
+	if len(schedule) > 0 && schedule[0] != '@' {
+		tmpSchedule = "0 " + schedule
+	}
+	if _, err := cron.Parse(tmpSchedule); err != nil {
 		allErrs = append(allErrs, field.Invalid(fldPath, schedule, err.Error()))
 	}
 

pkg/apis/batch/validation/validation_test.go

@@ -317,7 +317,23 @@ func TestValidateScheduledJob(t *testing.T) {
 				UID:       types.UID("1a2b3c"),
 			},
 			Spec: batch.ScheduledJobSpec{
-				Schedule:          "* * * * * ?",
+				Schedule:          "* * * * ?",
+				ConcurrencyPolicy: batch.AllowConcurrent,
+				JobTemplate: batch.JobTemplateSpec{
+					Spec: batch.JobSpec{
+						Template: validPodTemplateSpec,
+					},
+				},
+			},
+		},
+		"non-standard scheduled": {
+			ObjectMeta: api.ObjectMeta{
+				Name:      "myscheduledjob",
+				Namespace: api.NamespaceDefault,
+				UID:       types.UID("1a2b3c"),
+			},
+			Spec: batch.ScheduledJobSpec{
+				Schedule:          "@hourly",
 				ConcurrencyPolicy: batch.AllowConcurrent,
 				JobTemplate: batch.JobTemplateSpec{
 					Spec: batch.JobSpec{
@@ -376,7 +392,7 @@ func TestValidateScheduledJob(t *testing.T) {
 				UID:       types.UID("1a2b3c"),
 			},
 			Spec: batch.ScheduledJobSpec{
-				Schedule:                "* * * * * ?",
+				Schedule:                "* * * * ?",
 				ConcurrencyPolicy:       batch.AllowConcurrent,
 				StartingDeadlineSeconds: &negative64,
 				JobTemplate: batch.JobTemplateSpec{
@@ -393,7 +409,7 @@ func TestValidateScheduledJob(t *testing.T) {
 				UID:       types.UID("1a2b3c"),
 			},
 			Spec: batch.ScheduledJobSpec{
-				Schedule: "* * * * * ?",
+				Schedule: "* * * * ?",
 				JobTemplate: batch.JobTemplateSpec{
 					Spec: batch.JobSpec{
 						Template: validPodTemplateSpec,
@@ -408,7 +424,7 @@ func TestValidateScheduledJob(t *testing.T) {
 				UID:       types.UID("1a2b3c"),
 			},
 			Spec: batch.ScheduledJobSpec{
-				Schedule:          "* * * * * ?",
+				Schedule:          "* * * * ?",
 				ConcurrencyPolicy: batch.AllowConcurrent,
 				JobTemplate: batch.JobTemplateSpec{
 					Spec: batch.JobSpec{
@@ -425,7 +441,7 @@ func TestValidateScheduledJob(t *testing.T) {
 				UID:       types.UID("1a2b3c"),
 			},
 			Spec: batch.ScheduledJobSpec{
-				Schedule:          "* * * * * ?",
+				Schedule:          "* * * * ?",
 				ConcurrencyPolicy: batch.AllowConcurrent,
 				JobTemplate: batch.JobTemplateSpec{
 
@@ -443,7 +459,7 @@ func TestValidateScheduledJob(t *testing.T) {
 				UID:       types.UID("1a2b3c"),
 			},
 			Spec: batch.ScheduledJobSpec{
-				Schedule:          "* * * * * ?",
+				Schedule:          "* * * * ?",
 				ConcurrencyPolicy: batch.AllowConcurrent,
 				JobTemplate: batch.JobTemplateSpec{
 					Spec: batch.JobSpec{
@@ -460,7 +476,7 @@ func TestValidateScheduledJob(t *testing.T) {
 				UID:       types.UID("1a2b3c"),
 			},
 			Spec: batch.ScheduledJobSpec{
-				Schedule:          "* * * * * ?",
+				Schedule:          "* * * * ?",
 				ConcurrencyPolicy: batch.AllowConcurrent,
 				JobTemplate: batch.JobTemplateSpec{
 					Spec: batch.JobSpec{
@@ -477,7 +493,7 @@ func TestValidateScheduledJob(t *testing.T) {
 				UID:       types.UID("1a2b3c"),
 			},
 			Spec: batch.ScheduledJobSpec{
-				Schedule:          "* * * * * ?",
+				Schedule:          "* * * * ?",
 				ConcurrencyPolicy: batch.AllowConcurrent,
 				JobTemplate: batch.JobTemplateSpec{
 					Spec: batch.JobSpec{
@@ -494,7 +510,7 @@ func TestValidateScheduledJob(t *testing.T) {
 				UID:       types.UID("1a2b3c"),
 			},
 			Spec: batch.ScheduledJobSpec{
-				Schedule:          "* * * * * ?",
+				Schedule:          "* * * * ?",
 				ConcurrencyPolicy: batch.AllowConcurrent,
 				JobTemplate: batch.JobTemplateSpec{
 					Spec: batch.JobSpec{

pkg/controller/scheduledjob/controller_test.go

@@ -29,7 +29,7 @@ import (
 
 // schedule is hourly on the hour
 var (
-	onTheHour string = "0 0 * * * ?"
+	onTheHour string = "0 * * * ?"
 )
 
 func justBeforeTheHour() time.Time {
@@ -83,7 +83,7 @@ func scheduledJob() batch.ScheduledJob {
 			CreationTimestamp: unversioned.Time{Time: justBeforeTheHour()},
 		},
 		Spec: batch.ScheduledJobSpec{
-			Schedule:          "0 0 * * * * ?",
+			Schedule:          "* * * * ?",
 			ConcurrencyPolicy: batch.AllowConcurrent,
 			JobTemplate: batch.JobTemplateSpec{
 				ObjectMeta: api.ObjectMeta{

pkg/controller/scheduledjob/utils.go

@@ -111,8 +111,8 @@ func getNextStartTimeAfter(schedule string, now time.Time) (time.Time, error) {
 	// How to handle concurrency control.
 	// How to detect changes to schedules or deleted schedules and then
 	// update the jobs?
-
+	tmpSched := addSeconds(schedule)
-	sched, err := cron.Parse(schedule)
+	sched, err := cron.Parse(tmpSched)
 	if err != nil {
 		return time.Unix(0, 0), fmt.Errorf("Unparseable schedule: %s : %s", schedule, err)
 	}
@@ -125,7 +125,8 @@ func getNextStartTimeAfter(schedule string, now time.Time) (time.Time, error) {
 // 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{}
-	sched, err := cron.Parse(sj.Spec.Schedule)
+	tmpSched := addSeconds(sj.Spec.Schedule)
+	sched, err := cron.Parse(tmpSched)
 	if err != nil {
 		return starts, fmt.Errorf("Unparseable schedule: %s : %s", sj.Spec.Schedule, err)
 	}
@@ -173,6 +174,15 @@ func getRecentUnmetScheduleTimes(sj batch.ScheduledJob, now time.Time) ([]time.T
 	return starts, nil
 }
 
+// TODO soltysh: this should be removed when https://github.com/robfig/cron/issues/58 is fixed
+func addSeconds(schedule string) string {
+	tmpSched := schedule
+	if len(schedule) > 0 && schedule[0] != '@' {
+		tmpSched = "0 " + schedule
+	}
+	return tmpSched
+}
+
 // XXX unit test this
 
 // getJobFromTemplate makes a Job from a ScheduledJob

pkg/controller/scheduledjob/utils_test.go

@@ -44,7 +44,7 @@ func TestGetJobFromTemplate(t *testing.T) {
 			SelfLink:  "/apis/extensions/v1beta1/namespaces/snazzycats/jobs/myscheduledjob",
 		},
 		Spec: batch.ScheduledJobSpec{
-			Schedule:          "0 0 * * * * ?",
+			Schedule:          "* * * * ?",
 			ConcurrencyPolicy: batch.AllowConcurrent,
 			JobTemplate: batch.JobTemplateSpec{
 				ObjectMeta: api.ObjectMeta{
@@ -256,7 +256,7 @@ func TestGroupJobsByParent(t *testing.T) {
 
 func TestGetRecentUnmetScheduleTimes(t *testing.T) {
 	// schedule is hourly on the hour
-	schedule := "0 0 * * * ?"
+	schedule := "0 * * * ?"
 	// T1 is a scheduled start time of that schedule
 	T1, err := time.Parse(time.RFC3339, "2016-05-19T10:00:00Z")
 	if err != nil {

pkg/kubectl/cmd/run.go

@@ -72,7 +72,7 @@ var (
 		kubectl run pi --image=perl --restart=OnFailure -- perl -Mbignum=bpi -wle 'print bpi(2000)'
 
 		# Start the scheduled job to compute π to 2000 places and print it out every 5 minutes.
-		kubectl run pi --schedule="* 0/5 * * * ?" --image=perl --restart=OnFailure -- perl -Mbignum=bpi -wle 'print bpi(2000)'`)
+		kubectl run pi --schedule="0/5 * * * ?" --image=perl --restart=OnFailure -- perl -Mbignum=bpi -wle 'print bpi(2000)'`)
 )
 
 func NewCmdRun(f *cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer) *cobra.Command {

pkg/registry/scheduledjob/etcd/etcd_test.go

@@ -45,7 +45,7 @@ func validNewScheduledJob() *batch.ScheduledJob {
 			Namespace: api.NamespaceDefault,
 		},
 		Spec: batch.ScheduledJobSpec{
-			Schedule:          "* * * * * ?",
+			Schedule:          "* * * * ?",
 			ConcurrencyPolicy: batch.AllowConcurrent,
 			JobTemplate: batch.JobTemplateSpec{
 				Spec: batch.JobSpec{
@@ -92,7 +92,7 @@ func TestUpdate(t *testing.T) {
 	storage, _, server := newStorage(t)
 	defer server.Terminate(t)
 	test := registrytest.New(t, storage.Store)
-	schedule := "1 1 1 1 1 ?"
+	schedule := "1 1 1 1 ?"
 	test.TestUpdate(
 		// valid
 		validNewScheduledJob(),

pkg/registry/scheduledjob/strategy_test.go

@@ -54,7 +54,7 @@ func TestScheduledJobStrategy(t *testing.T) {
 			Namespace: api.NamespaceDefault,
 		},
 		Spec: batch.ScheduledJobSpec{
-			Schedule:          "* * * * * ?",
+			Schedule:          "* * * * ?",
 			ConcurrencyPolicy: batch.AllowConcurrent,
 			JobTemplate: batch.JobTemplateSpec{
 				Spec: batch.JobSpec{
@@ -76,7 +76,7 @@ func TestScheduledJobStrategy(t *testing.T) {
 	updatedScheduledJob := &batch.ScheduledJob{
 		ObjectMeta: api.ObjectMeta{Name: "bar", ResourceVersion: "4"},
 		Spec: batch.ScheduledJobSpec{
-			Schedule: "5 5 5 5 * ?",
+			Schedule: "5 5 5 * ?",
 		},
 		Status: batch.ScheduledJobStatus{
 			LastScheduleTime: &now,
@@ -109,7 +109,7 @@ func TestScheduledJobStatusStrategy(t *testing.T) {
 			Containers:    []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}},
 		},
 	}
-	oldSchedule := "* * * * * ?"
+	oldSchedule := "* * * * ?"
 	oldScheduledJob := &batch.ScheduledJob{
 		ObjectMeta: api.ObjectMeta{
 			Name:            "myscheduledjob",
@@ -134,7 +134,7 @@ func TestScheduledJobStatusStrategy(t *testing.T) {
 			ResourceVersion: "9",
 		},
 		Spec: batch.ScheduledJobSpec{
-			Schedule:          "5 5 5 * * ?",
+			Schedule:          "5 5 * * ?",
 			ConcurrencyPolicy: batch.AllowConcurrent,
 			JobTemplate: batch.JobTemplateSpec{
 				Spec: batch.JobSpec{