diff --git a/pkg/apis/batch/validation/validation.go b/pkg/apis/batch/validation/validation.go index 03135c2a206..8466b92e038 100644 --- a/pkg/apis/batch/validation/validation.go +++ b/pkg/apis/batch/validation/validation.go @@ -17,6 +17,8 @@ limitations under the License. package validation import ( + "fmt" + "github.com/robfig/cron" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -124,8 +126,14 @@ func validateJobSpec(spec *batch.JobSpec, fldPath *field.Path, opts apivalidatio } allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpec(&spec.Template, fldPath.Child("template"), opts)...) - if spec.Template.Spec.RestartPolicy != api.RestartPolicyOnFailure && - spec.Template.Spec.RestartPolicy != api.RestartPolicyNever { + + // spec.Template.Spec.RestartPolicy can be defaulted as RestartPolicyAlways + // by SetDefaults_PodSpec function when the user does not explicitly specify a value for it, + // so we check both empty and RestartPolicyAlways cases here + if spec.Template.Spec.RestartPolicy == api.RestartPolicyAlways || spec.Template.Spec.RestartPolicy == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("template", "spec", "restartPolicy"), + fmt.Sprintf("valid values: %q, %q", api.RestartPolicyOnFailure, api.RestartPolicyNever))) + } else if spec.Template.Spec.RestartPolicy != api.RestartPolicyOnFailure && spec.Template.Spec.RestartPolicy != api.RestartPolicyNever { allErrs = append(allErrs, field.NotSupported(fldPath.Child("template", "spec", "restartPolicy"), spec.Template.Spec.RestartPolicy, []string{string(api.RestartPolicyOnFailure), string(api.RestartPolicyNever)})) } diff --git a/pkg/apis/batch/validation/validation_test.go b/pkg/apis/batch/validation/validation_test.go index e261c831a72..8c357db1722 100644 --- a/pkg/apis/batch/validation/validation_test.go +++ b/pkg/apis/batch/validation/validation_test.go @@ -195,7 +195,7 @@ func TestValidateJob(t *testing.T) { }, }, }, - "spec.template.spec.restartPolicy: Unsupported value": { + "spec.template.spec.restartPolicy: Required value": { ObjectMeta: metav1.ObjectMeta{ Name: "myjob", Namespace: metav1.NamespaceDefault, @@ -216,6 +216,27 @@ func TestValidateJob(t *testing.T) { }, }, }, + "spec.template.spec.restartPolicy: Unsupported value": { + ObjectMeta: metav1.ObjectMeta{ + Name: "myjob", + Namespace: metav1.NamespaceDefault, + UID: types.UID("1a2b3c"), + }, + Spec: batch.JobSpec{ + Selector: validManualSelector, + ManualSelector: newBool(true), + Template: api.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: validManualSelector.MatchLabels, + }, + Spec: api.PodSpec{ + RestartPolicy: "Invalid", + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: api.TerminationMessageReadFile}}, + }, + }, + }, + }, } for _, setFeature := range []bool{true, false} { @@ -586,7 +607,7 @@ func TestValidateCronJob(t *testing.T) { }, }, }, - "spec.jobTemplate.spec.template.spec.restartPolicy: Unsupported value": { + "spec.jobTemplate.spec.template.spec.restartPolicy: Required value": { ObjectMeta: metav1.ObjectMeta{ Name: "mycronjob", Namespace: metav1.NamespaceDefault, @@ -608,6 +629,28 @@ func TestValidateCronJob(t *testing.T) { }, }, }, + "spec.jobTemplate.spec.template.spec.restartPolicy: Unsupported value": { + ObjectMeta: metav1.ObjectMeta{ + Name: "mycronjob", + Namespace: metav1.NamespaceDefault, + UID: types.UID("1a2b3c"), + }, + Spec: batch.CronJobSpec{ + Schedule: "* * * * ?", + ConcurrencyPolicy: batch.AllowConcurrent, + JobTemplate: batch.JobTemplateSpec{ + Spec: batch.JobSpec{ + Template: api.PodTemplateSpec{ + Spec: api.PodSpec{ + RestartPolicy: "Invalid", + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: api.TerminationMessageReadFile}}, + }, + }, + }, + }, + }, + }, } if utilfeature.DefaultFeatureGate.Enabled(features.TTLAfterFinished) { errorCases["spec.jobTemplate.spec.ttlSecondsAfterFinished:must be greater than or equal to 0"] = batch.CronJob{