Only default Job fields when feature gates are enabled

Also use pointer for completionMode enum

pkg/apis/batch/fuzzer/fuzzer.go

@@ -47,11 +47,11 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
 			} else {
 				j.ManualSelector = nil
 			}
-			if c.Rand.Int31()%2 == 0 {
-				j.CompletionMode = batch.NonIndexedCompletion
-			} else {
-				j.CompletionMode = batch.IndexedCompletion
+			mode := batch.NonIndexedCompletion
+			if c.RandBool() {
+				mode = batch.IndexedCompletion
 			}
+			j.CompletionMode = &mode
 			// We're fuzzing the internal JobSpec type, not the v1 type, so we don't
 			// need to fuzz the nil value.
 			j.Suspend = pointer.BoolPtr(c.RandBool())

pkg/apis/batch/types.go

@@ -189,7 +189,7 @@ type JobSpec struct {
 	// If the Job controller observes a mode that it doesn't recognize, the
 	// controller skips updates for the Job.
 	// +optional
-	CompletionMode CompletionMode
+	CompletionMode *CompletionMode
 
 	// Suspend specifies whether the Job controller should create Pods or not. If
 	// a Job is created with suspend set to true, no Pods are created by the Job

pkg/apis/batch/v1/defaults.go

@@ -19,6 +19,8 @@ package v1
 import (
 	batchv1 "k8s.io/api/batch/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	"k8s.io/kubernetes/pkg/features"
 	utilpointer "k8s.io/utils/pointer"
 )
 
@@ -43,10 +45,11 @@ func SetDefaults_Job(obj *batchv1.Job) {
 	if labels != nil && len(obj.Labels) == 0 {
 		obj.Labels = labels
 	}
-	if len(obj.Spec.CompletionMode) == 0 {
-		obj.Spec.CompletionMode = batchv1.NonIndexedCompletion
+	if utilfeature.DefaultFeatureGate.Enabled(features.IndexedJob) && obj.Spec.CompletionMode == nil {
+		mode := batchv1.NonIndexedCompletion
+		obj.Spec.CompletionMode = &mode
 	}
-	if obj.Spec.Suspend == nil {
+	if utilfeature.DefaultFeatureGate.Enabled(features.SuspendJob) && obj.Spec.Suspend == nil {
 		obj.Spec.Suspend = utilpointer.BoolPtr(false)
 	}
 }

pkg/apis/batch/v1/defaults_test.go

@@ -25,9 +25,12 @@ import (
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/util/feature"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	_ "k8s.io/kubernetes/pkg/apis/batch/install"
 	_ "k8s.io/kubernetes/pkg/apis/core/install"
+	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/utils/pointer"
 
 	. "k8s.io/kubernetes/pkg/apis/batch/v1"
@@ -36,9 +39,11 @@ import (
 func TestSetDefaultJob(t *testing.T) {
 	defaultLabels := map[string]string{"default": "default"}
 	tests := map[string]struct {
-		original     *batchv1.Job
-		expected     *batchv1.Job
-		expectLabels bool
+		indexedJobEnabled bool
+		suspendJobEnabled bool
+		original          *batchv1.Job
+		expected          *batchv1.Job
+		expectLabels      bool
 	}{
 		"All unspecified -> sets all to default values": {
 			original: &batchv1.Job{
@@ -50,19 +55,17 @@ func TestSetDefaultJob(t *testing.T) {
 			},
 			expected: &batchv1.Job{
 				Spec: batchv1.JobSpec{
-					Completions:    pointer.Int32Ptr(1),
-					Parallelism:    pointer.Int32Ptr(1),
-					BackoffLimit:   pointer.Int32Ptr(6),
-					CompletionMode: batchv1.NonIndexedCompletion,
-					Suspend:        pointer.BoolPtr(false),
+					Completions:  pointer.Int32Ptr(1),
+					Parallelism:  pointer.Int32Ptr(1),
+					BackoffLimit: pointer.Int32Ptr(6),
 				},
 			},
 			expectLabels: true,
 		},
-		"suspend set, everything else is defaulted": {
+		"All unspecified, indexed job enabled -> sets all to default values": {
+			indexedJobEnabled: true,
 			original: &batchv1.Job{
 				Spec: batchv1.JobSpec{
-					Suspend: pointer.BoolPtr(true),
 					Template: v1.PodTemplateSpec{
 						ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels},
 					},
@@ -73,13 +76,51 @@ func TestSetDefaultJob(t *testing.T) {
 					Completions:    pointer.Int32Ptr(1),
 					Parallelism:    pointer.Int32Ptr(1),
 					BackoffLimit:   pointer.Int32Ptr(6),
-					CompletionMode: batchv1.NonIndexedCompletion,
-					Suspend:        pointer.BoolPtr(true),
+					CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
 				},
 			},
 			expectLabels: true,
 		},
-		"All unspecified -> all pointers, CompletionMode are defaulted and no default labels": {
+		"All unspecified, suspend job enabled -> sets all to default values": {
+			suspendJobEnabled: true,
+			original: &batchv1.Job{
+				Spec: batchv1.JobSpec{
+					Template: v1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels},
+					},
+				},
+			},
+			expected: &batchv1.Job{
+				Spec: batchv1.JobSpec{
+					Completions:  pointer.Int32Ptr(1),
+					Parallelism:  pointer.Int32Ptr(1),
+					BackoffLimit: pointer.Int32Ptr(6),
+					Suspend:      pointer.BoolPtr(false),
+				},
+			},
+			expectLabels: true,
+		},
+		"suspend set, everything else is defaulted": {
+			suspendJobEnabled: true,
+			original: &batchv1.Job{
+				Spec: batchv1.JobSpec{
+					Suspend: pointer.BoolPtr(true),
+					Template: v1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels},
+					},
+				},
+			},
+			expected: &batchv1.Job{
+				Spec: batchv1.JobSpec{
+					Completions:  pointer.Int32Ptr(1),
+					Parallelism:  pointer.Int32Ptr(1),
+					BackoffLimit: pointer.Int32Ptr(6),
+					Suspend:      pointer.BoolPtr(true),
+				},
+			},
+			expectLabels: true,
+		},
+		"All unspecified -> all pointers are defaulted and no default labels": {
 			original: &batchv1.Job{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: map[string]string{"mylabel": "myvalue"},
@@ -92,11 +133,9 @@ func TestSetDefaultJob(t *testing.T) {
 			},
 			expected: &batchv1.Job{
 				Spec: batchv1.JobSpec{
-					Completions:    pointer.Int32Ptr(1),
-					Parallelism:    pointer.Int32Ptr(1),
-					BackoffLimit:   pointer.Int32Ptr(6),
-					CompletionMode: batchv1.NonIndexedCompletion,
-					Suspend:        pointer.BoolPtr(false),
+					Completions:  pointer.Int32Ptr(1),
+					Parallelism:  pointer.Int32Ptr(1),
+					BackoffLimit: pointer.Int32Ptr(6),
 				},
 			},
 		},
@@ -111,10 +150,8 @@ func TestSetDefaultJob(t *testing.T) {
 			},
 			expected: &batchv1.Job{
 				Spec: batchv1.JobSpec{
-					Parallelism:    pointer.Int32Ptr(0),
-					BackoffLimit:   pointer.Int32Ptr(6),
-					CompletionMode: batchv1.NonIndexedCompletion,
-					Suspend:        pointer.BoolPtr(false),
+					Parallelism:  pointer.Int32Ptr(0),
+					BackoffLimit: pointer.Int32Ptr(6),
 				},
 			},
 			expectLabels: true,
@@ -130,10 +167,8 @@ func TestSetDefaultJob(t *testing.T) {
 			},
 			expected: &batchv1.Job{
 				Spec: batchv1.JobSpec{
-					Parallelism:    pointer.Int32Ptr(2),
-					BackoffLimit:   pointer.Int32Ptr(6),
-					CompletionMode: batchv1.NonIndexedCompletion,
-					Suspend:        pointer.BoolPtr(false),
+					Parallelism:  pointer.Int32Ptr(2),
+					BackoffLimit: pointer.Int32Ptr(6),
 				},
 			},
 			expectLabels: true,
@@ -149,11 +184,9 @@ func TestSetDefaultJob(t *testing.T) {
 			},
 			expected: &batchv1.Job{
 				Spec: batchv1.JobSpec{
-					Completions:    pointer.Int32Ptr(2),
-					Parallelism:    pointer.Int32Ptr(1),
-					BackoffLimit:   pointer.Int32Ptr(6),
-					CompletionMode: batchv1.NonIndexedCompletion,
-					Suspend:        pointer.BoolPtr(false),
+					Completions:  pointer.Int32Ptr(2),
+					Parallelism:  pointer.Int32Ptr(1),
+					BackoffLimit: pointer.Int32Ptr(6),
 				},
 			},
 			expectLabels: true,
@@ -169,11 +202,9 @@ func TestSetDefaultJob(t *testing.T) {
 			},
 			expected: &batchv1.Job{
 				Spec: batchv1.JobSpec{
-					Completions:    pointer.Int32Ptr(1),
-					Parallelism:    pointer.Int32Ptr(1),
-					BackoffLimit:   pointer.Int32Ptr(5),
-					CompletionMode: batchv1.NonIndexedCompletion,
-					Suspend:        pointer.BoolPtr(false),
+					Completions:  pointer.Int32Ptr(1),
+					Parallelism:  pointer.Int32Ptr(1),
+					BackoffLimit: pointer.Int32Ptr(5),
 				},
 			},
 			expectLabels: true,
@@ -184,8 +215,8 @@ func TestSetDefaultJob(t *testing.T) {
 					Completions:    pointer.Int32Ptr(8),
 					Parallelism:    pointer.Int32Ptr(9),
 					BackoffLimit:   pointer.Int32Ptr(10),
-					CompletionMode: batchv1.NonIndexedCompletion,
-					Suspend:        pointer.BoolPtr(true),
+					CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
+					Suspend:        pointer.BoolPtr(false),
 					Template: v1.PodTemplateSpec{
 						ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels},
 					},
@@ -196,8 +227,8 @@ func TestSetDefaultJob(t *testing.T) {
 					Completions:    pointer.Int32Ptr(8),
 					Parallelism:    pointer.Int32Ptr(9),
 					BackoffLimit:   pointer.Int32Ptr(10),
-					CompletionMode: batchv1.NonIndexedCompletion,
-					Suspend:        pointer.BoolPtr(true),
+					CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
+					Suspend:        pointer.BoolPtr(false),
 					Template: v1.PodTemplateSpec{
 						ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels},
 					},
@@ -211,7 +242,7 @@ func TestSetDefaultJob(t *testing.T) {
 					Completions:    pointer.Int32Ptr(11),
 					Parallelism:    pointer.Int32Ptr(10),
 					BackoffLimit:   pointer.Int32Ptr(9),
-					CompletionMode: batchv1.IndexedCompletion,
+					CompletionMode: completionModePtr(batchv1.IndexedCompletion),
 					Suspend:        pointer.BoolPtr(true),
 					Template: v1.PodTemplateSpec{
 						ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels},
@@ -223,7 +254,7 @@ func TestSetDefaultJob(t *testing.T) {
 					Completions:    pointer.Int32Ptr(11),
 					Parallelism:    pointer.Int32Ptr(10),
 					BackoffLimit:   pointer.Int32Ptr(9),
-					CompletionMode: batchv1.IndexedCompletion,
+					CompletionMode: completionModePtr(batchv1.IndexedCompletion),
 					Suspend:        pointer.BoolPtr(true),
 				},
 			},
@@ -233,6 +264,8 @@ func TestSetDefaultJob(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
+			defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.IndexedJob, test.indexedJobEnabled)()
+			defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.SuspendJob, test.suspendJobEnabled)()
 
 			original := test.original
 			expected := test.expected
@@ -256,8 +289,8 @@ func TestSetDefaultJob(t *testing.T) {
 					t.Errorf("Unexpected equality: %v", actual.Labels)
 				}
 			}
-			if actual.Spec.CompletionMode != expected.Spec.CompletionMode {
-				t.Errorf("Got CompletionMode: %v, want: %v", actual.Spec.CompletionMode, expected.Spec.CompletionMode)
+			if diff := cmp.Diff(expected.Spec.CompletionMode, actual.Spec.CompletionMode); diff != "" {
+				t.Errorf("Unexpected CompletionMode (-want,+got):\n%s", diff)
 			}
 		})
 	}
@@ -304,7 +337,7 @@ func TestSetDefaultCronJob(t *testing.T) {
 			expected: &batchv1.CronJob{
 				Spec: batchv1.CronJobSpec{
 					ConcurrencyPolicy:          batchv1.AllowConcurrent,
-					Suspend:                    newBool(false),
+					Suspend:                    pointer.BoolPtr(false),
 					SuccessfulJobsHistoryLimit: pointer.Int32Ptr(3),
 					FailedJobsHistoryLimit:     pointer.Int32Ptr(1),
 				},
@@ -314,7 +347,7 @@ func TestSetDefaultCronJob(t *testing.T) {
 			original: &batchv1.CronJob{
 				Spec: batchv1.CronJobSpec{
 					ConcurrencyPolicy:          batchv1.ForbidConcurrent,
-					Suspend:                    newBool(true),
+					Suspend:                    pointer.BoolPtr(true),
 					SuccessfulJobsHistoryLimit: pointer.Int32Ptr(5),
 					FailedJobsHistoryLimit:     pointer.Int32Ptr(5),
 				},
@@ -322,7 +355,7 @@ func TestSetDefaultCronJob(t *testing.T) {
 			expected: &batchv1.CronJob{
 				Spec: batchv1.CronJobSpec{
 					ConcurrencyPolicy:          batchv1.ForbidConcurrent,
-					Suspend:                    newBool(true),
+					Suspend:                    pointer.BoolPtr(true),
 					SuccessfulJobsHistoryLimit: pointer.Int32Ptr(5),
 					FailedJobsHistoryLimit:     pointer.Int32Ptr(5),
 				},
@@ -354,8 +387,6 @@ func TestSetDefaultCronJob(t *testing.T) {
 	}
 }
 
-func newBool(val bool) *bool {
-	p := new(bool)
-	*p = val
-	return p
+func completionModePtr(m batchv1.CompletionMode) *batchv1.CompletionMode {
+	return &m
 }

pkg/apis/batch/v1/zz_generated.conversion.go

@@ -392,7 +392,7 @@ func autoConvert_v1_JobSpec_To_batch_JobSpec(in *v1.JobSpec, out *batch.JobSpec,
 		return err
 	}
 	out.TTLSecondsAfterFinished = (*int32)(unsafe.Pointer(in.TTLSecondsAfterFinished))
-	out.CompletionMode = batch.CompletionMode(in.CompletionMode)
+	out.CompletionMode = (*batch.CompletionMode)(unsafe.Pointer(in.CompletionMode))
 	out.Suspend = (*bool)(unsafe.Pointer(in.Suspend))
 	return nil
 }
@@ -408,7 +408,7 @@ func autoConvert_batch_JobSpec_To_v1_JobSpec(in *batch.JobSpec, out *v1.JobSpec,
 		return err
 	}
 	out.TTLSecondsAfterFinished = (*int32)(unsafe.Pointer(in.TTLSecondsAfterFinished))
-	out.CompletionMode = v1.CompletionMode(in.CompletionMode)
+	out.CompletionMode = (*v1.CompletionMode)(unsafe.Pointer(in.CompletionMode))
 	out.Suspend = (*bool)(unsafe.Pointer(in.Suspend))
 	return nil
 }

pkg/apis/batch/validation/validation.go

@@ -129,12 +129,12 @@ func validateJobSpec(spec *batch.JobSpec, fldPath *field.Path, opts apivalidatio
 	if spec.TTLSecondsAfterFinished != nil {
 		allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.TTLSecondsAfterFinished), fldPath.Child("ttlSecondsAfterFinished"))...)
 	}
-	// CompletionMode might be empty when IndexedJob feature gate is disabled.
-	if spec.CompletionMode != "" {
-		if spec.CompletionMode != batch.NonIndexedCompletion && spec.CompletionMode != batch.IndexedCompletion {
+	// CompletionMode might be nil when IndexedJob feature gate is disabled.
+	if spec.CompletionMode != nil {
+		if *spec.CompletionMode != batch.NonIndexedCompletion && *spec.CompletionMode != batch.IndexedCompletion {
 			allErrs = append(allErrs, field.NotSupported(fldPath.Child("completionMode"), spec.CompletionMode, []string{string(batch.NonIndexedCompletion), string(batch.IndexedCompletion)}))
 		}
-		if spec.CompletionMode == batch.IndexedCompletion {
+		if *spec.CompletionMode == batch.IndexedCompletion {
 			if spec.Completions == nil {
 				allErrs = append(allErrs, field.Required(fldPath.Child("completions"), fmt.Sprintf("when completion mode is %s", batch.IndexedCompletion)))
 			}

pkg/apis/batch/validation/validation_test.go

@@ -86,7 +86,7 @@ func TestValidateJob(t *testing.T) {
 			},
 			Spec: batch.JobSpec{
 				Selector:       validManualSelector,
-				ManualSelector: newBool(true),
+				ManualSelector: pointer.BoolPtr(true),
 				Template:       validPodTemplateSpecForManual,
 			},
 		},
@@ -110,7 +110,7 @@ func TestValidateJob(t *testing.T) {
 			Spec: batch.JobSpec{
 				Selector:       validGeneratedSelector,
 				Template:       validPodTemplateSpecForGenerated,
-				CompletionMode: batch.NonIndexedCompletion,
+				CompletionMode: completionModePtr(batch.NonIndexedCompletion),
 			},
 		},
 		"valid Indexed completion mode": {
@@ -122,7 +122,7 @@ func TestValidateJob(t *testing.T) {
 			Spec: batch.JobSpec{
 				Selector:       validGeneratedSelector,
 				Template:       validPodTemplateSpecForGenerated,
-				CompletionMode: batch.IndexedCompletion,
+				CompletionMode: completionModePtr(batch.IndexedCompletion),
 				Completions:    pointer.Int32Ptr(2),
 				Parallelism:    pointer.Int32Ptr(100000),
 			},
@@ -192,7 +192,7 @@ func TestValidateJob(t *testing.T) {
 			},
 			Spec: batch.JobSpec{
 				Selector:       validManualSelector,
-				ManualSelector: newBool(true),
+				ManualSelector: pointer.BoolPtr(true),
 				Template: api.PodTemplateSpec{
 					ObjectMeta: metav1.ObjectMeta{
 						Labels: map[string]string{"y": "z"},
@@ -213,7 +213,7 @@ func TestValidateJob(t *testing.T) {
 			},
 			Spec: batch.JobSpec{
 				Selector:       validManualSelector,
-				ManualSelector: newBool(true),
+				ManualSelector: pointer.BoolPtr(true),
 				Template: api.PodTemplateSpec{
 					ObjectMeta: metav1.ObjectMeta{
 						Labels: map[string]string{"controller-uid": "4d5e6f"},
@@ -234,7 +234,7 @@ func TestValidateJob(t *testing.T) {
 			},
 			Spec: batch.JobSpec{
 				Selector:       validManualSelector,
-				ManualSelector: newBool(true),
+				ManualSelector: pointer.BoolPtr(true),
 				Template: api.PodTemplateSpec{
 					ObjectMeta: metav1.ObjectMeta{
 						Labels: validManualSelector.MatchLabels,
@@ -255,7 +255,7 @@ func TestValidateJob(t *testing.T) {
 			},
 			Spec: batch.JobSpec{
 				Selector:       validManualSelector,
-				ManualSelector: newBool(true),
+				ManualSelector: pointer.BoolPtr(true),
 				Template: api.PodTemplateSpec{
 					ObjectMeta: metav1.ObjectMeta{
 						Labels: validManualSelector.MatchLabels,
@@ -289,7 +289,7 @@ func TestValidateJob(t *testing.T) {
 			Spec: batch.JobSpec{
 				Selector:       validGeneratedSelector,
 				Template:       validPodTemplateSpecForGenerated,
-				CompletionMode: batch.IndexedCompletion,
+				CompletionMode: completionModePtr(batch.IndexedCompletion),
 			},
 		},
 		"spec.parallelism: must be less than or equal to 100000 when completion mode is Indexed": {
@@ -301,7 +301,7 @@ func TestValidateJob(t *testing.T) {
 			Spec: batch.JobSpec{
 				Selector:       validGeneratedSelector,
 				Template:       validPodTemplateSpecForGenerated,
-				CompletionMode: batch.IndexedCompletion,
+				CompletionMode: completionModePtr(batch.IndexedCompletion),
 				Completions:    pointer.Int32Ptr(2),
 				Parallelism:    pointer.Int32Ptr(100001),
 			},
@@ -404,12 +404,12 @@ func TestValidateJobUpdate(t *testing.T) {
 				Spec: batch.JobSpec{
 					Selector:       validGeneratedSelector,
 					Template:       validPodTemplateSpecForGenerated,
-					CompletionMode: batch.IndexedCompletion,
+					CompletionMode: completionModePtr(batch.IndexedCompletion),
 					Completions:    pointer.Int32Ptr(2),
 				},
 			},
 			update: func(job *batch.Job) {
-				job.Spec.CompletionMode = batch.NonIndexedCompletion
+				job.Spec.CompletionMode = completionModePtr(batch.NonIndexedCompletion)
 			},
 			err: &field.Error{
 				Type:  field.ErrorTypeInvalid,
@@ -762,7 +762,7 @@ func TestValidateCronJob(t *testing.T) {
 				ConcurrencyPolicy: batch.AllowConcurrent,
 				JobTemplate: batch.JobTemplateSpec{
 					Spec: batch.JobSpec{
-						ManualSelector: newBool(true),
+						ManualSelector: pointer.BoolPtr(true),
 						Template:       validPodTemplateSpec,
 					},
 				},
@@ -865,8 +865,6 @@ func TestValidateCronJob(t *testing.T) {
 	}
 }
 
-func newBool(val bool) *bool {
-	p := new(bool)
-	*p = val
-	return p
+func completionModePtr(m batch.CompletionMode) *batch.CompletionMode {
+	return &m
 }

pkg/apis/batch/zz_generated.deepcopy.go

@@ -271,6 +271,11 @@ func (in *JobSpec) DeepCopyInto(out *JobSpec) {
 		*out = new(int32)
 		**out = **in
 	}
+	if in.CompletionMode != nil {
+		in, out := &in.CompletionMode, &out.CompletionMode
+		*out = new(CompletionMode)
+		**out = **in
+	}
 	if in.Suspend != nil {
 		in, out := &in.Suspend, &out.Suspend
 		*out = new(bool)