Graduate JobMutableNodeSchedulingDirectives feature to GA

pkg/features/kube_features.go

@@ -402,6 +402,7 @@ const (
 
 	// owner: @ahg
 	// beta: v1.23
+	// stable: v1.27
 	//
 	// Allow updating node scheduling directives in the pod template of jobs. Specifically,
 	// node affinity, selector and tolerations. This is allowed only for suspended jobs
@@ -963,7 +964,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	JobPodFailurePolicy: {Default: true, PreRelease: featuregate.Beta},
 
-	JobMutableNodeSchedulingDirectives: {Default: true, PreRelease: featuregate.Beta},
+	JobMutableNodeSchedulingDirectives: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.29
 
 	JobReadyPods: {Default: true, PreRelease: featuregate.Beta},
 

pkg/registry/batch/job/strategy.go

@@ -183,8 +183,7 @@ func validationOptionsForJob(newJob, oldJob *batch.Job) batchvalidation.JobValid
 		// only for suspended jobs that never started before.
 		suspended := oldJob.Spec.Suspend != nil && *oldJob.Spec.Suspend
 		notStarted := oldJob.Status.StartTime == nil
-		opts.AllowMutableSchedulingDirectives = utilfeature.DefaultFeatureGate.Enabled(features.JobMutableNodeSchedulingDirectives) &&
+		opts.AllowMutableSchedulingDirectives = suspended && notStarted
-			suspended && notStarted
 	}
 	return opts
 }

pkg/registry/batch/job/strategy_test.go

@@ -472,10 +472,9 @@ func TestJobStrategy_ValidateUpdate(t *testing.T) {
 	}
 	now := metav1.Now()
 	cases := map[string]struct {
-		job                                *batch.Job
+		job      *batch.Job
-		update                             func(*batch.Job)
+		update   func(*batch.Job)
-		wantErrs                           field.ErrorList
+		wantErrs field.ErrorList
-		mutableSchedulingDirectivesEnabled bool
 	}{
 		"update parallelism": {
 			job: &batch.Job{
@@ -603,7 +602,6 @@ func TestJobStrategy_ValidateUpdate(t *testing.T) {
 			wantErrs: field.ErrorList{
 				{Type: field.ErrorTypeInvalid, Field: "spec.template"},
 			},
-			mutableSchedulingDirectivesEnabled: true,
 		},
 		"updating node selector for suspended but previously started job disallowed": {
 			job: &batch.Job{
@@ -630,7 +628,6 @@ func TestJobStrategy_ValidateUpdate(t *testing.T) {
 			wantErrs: field.ErrorList{
 				{Type: field.ErrorTypeInvalid, Field: "spec.template"},
 			},
-			mutableSchedulingDirectivesEnabled: true,
 		},
 		"updating node selector for suspended and not previously started job allowed": {
 			job: &batch.Job{
@@ -651,31 +648,6 @@ func TestJobStrategy_ValidateUpdate(t *testing.T) {
 			update: func(job *batch.Job) {
 				job.Spec.Template.Spec.NodeSelector = map[string]string{"foo": "bar"}
 			},
-			mutableSchedulingDirectivesEnabled: true,
-		},
-		"updating node selector whilte gate disabled disallowed": {
-			job: &batch.Job{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:            "myjob",
-					Namespace:       metav1.NamespaceDefault,
-					ResourceVersion: "0",
-					Annotations:     map[string]string{"foo": "bar"},
-				},
-				Spec: batch.JobSpec{
-					Selector:       validSelector,
-					Template:       validPodTemplateSpec,
-					ManualSelector: pointer.BoolPtr(true),
-					Parallelism:    pointer.Int32Ptr(1),
-					Suspend:        pointer.BoolPtr(true),
-				},
-			},
-			update: func(job *batch.Job) {
-				job.Spec.Template.Spec.NodeSelector = map[string]string{"foo": "bar"}
-			},
-			wantErrs: field.ErrorList{
-				{Type: field.ErrorTypeInvalid, Field: "spec.template"},
-			},
-			mutableSchedulingDirectivesEnabled: false,
 		},
 		"invalid label selector": {
 			job: &batch.Job{
@@ -701,7 +673,6 @@ func TestJobStrategy_ValidateUpdate(t *testing.T) {
 	}
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.JobMutableNodeSchedulingDirectives, tc.mutableSchedulingDirectivesEnabled)()
 			newJob := tc.job.DeepCopy()
 			tc.update(newJob)
 			errs := Strategy.ValidateUpdate(ctx, newJob, tc.job)

test/integration/job/job_test.go

@@ -1574,78 +1574,63 @@ func TestSuspendJobControllerRestart(t *testing.T) {
 }
 
 func TestNodeSelectorUpdate(t *testing.T) {
-	for name, featureGate := range map[string]bool{
+	closeFn, restConfig, clientSet, ns := setup(t, "suspend")
-		"feature gate disabled": false,
+	defer closeFn()
-		"feature gate enabled":  true,
+	ctx, cancel := startJobControllerAndWaitForCaches(restConfig)
-	} {
+	defer cancel()
-		t.Run(name, func(t *testing.T) {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.JobMutableNodeSchedulingDirectives, featureGate)()
 
-			closeFn, restConfig, clientSet, ns := setup(t, "suspend")
+	job, err := createJobWithDefaults(ctx, clientSet, ns.Name, &batchv1.Job{Spec: batchv1.JobSpec{
-			defer closeFn()
+		Parallelism: pointer.Int32(1),
-			ctx, cancel := startJobControllerAndWaitForCaches(restConfig)
+		Suspend:     pointer.BoolPtr(true),
-			defer cancel()
+	}})
-
+	if err != nil {
-			job, err := createJobWithDefaults(ctx, clientSet, ns.Name, &batchv1.Job{Spec: batchv1.JobSpec{
+		t.Fatalf("Failed to create Job: %v", err)
-				Parallelism: pointer.Int32(1),
-				Suspend:     pointer.BoolPtr(true),
-			}})
-			if err != nil {
-				t.Fatalf("Failed to create Job: %v", err)
-			}
-			jobName := job.Name
-			jobNamespace := job.Namespace
-			jobClient := clientSet.BatchV1().Jobs(jobNamespace)
-
-			// (1) Unsuspend and set node selector in the same update.
-			nodeSelector := map[string]string{"foo": "bar"}
-			_, err = updateJob(ctx, jobClient, jobName, func(j *batchv1.Job) {
-				j.Spec.Template.Spec.NodeSelector = nodeSelector
-				j.Spec.Suspend = pointer.BoolPtr(false)
-			})
-			if !featureGate {
-				if err == nil || !strings.Contains(err.Error(), "spec.template: Invalid value") {
-					t.Errorf("Expected \"spec.template: Invalid value\" error, got: %v", err)
-				}
-			} else if featureGate && err != nil {
-				t.Errorf("Unexpected error: %v", err)
-			}
-
-			// (2) Check that the pod was created using the expected node selector.
-			if featureGate {
-				var pod *v1.Pod
-				if err := wait.PollImmediate(waitInterval, wait.ForeverTestTimeout, func() (bool, error) {
-					pods, err := clientSet.CoreV1().Pods(jobNamespace).List(ctx, metav1.ListOptions{})
-					if err != nil {
-						t.Fatalf("Failed to list Job Pods: %v", err)
-					}
-					if len(pods.Items) == 0 {
-						return false, nil
-					}
-					pod = &pods.Items[0]
-					return true, nil
-				}); err != nil || pod == nil {
-					t.Fatalf("pod not found: %v", err)
-				}
-
-				// if the feature gate is enabled, then the job should now be unsuspended and
-				// the pod has the node selector.
-				if diff := cmp.Diff(nodeSelector, pod.Spec.NodeSelector); diff != "" {
-					t.Errorf("Unexpected nodeSelector (-want,+got):\n%s", diff)
-				}
-			}
-
-			// (3) Update node selector again. It should fail since the job is unsuspended.
-			_, err = updateJob(ctx, jobClient, jobName, func(j *batchv1.Job) {
-				j.Spec.Template.Spec.NodeSelector = map[string]string{"foo": "baz"}
-			})
-
-			if err == nil || !strings.Contains(err.Error(), "spec.template: Invalid value") {
-				t.Errorf("Expected \"spec.template: Invalid value\" error, got: %v", err)
-			}
-
-		})
 	}
+	jobName := job.Name
+	jobNamespace := job.Namespace
+	jobClient := clientSet.BatchV1().Jobs(jobNamespace)
+
+	// (1) Unsuspend and set node selector in the same update.
+	nodeSelector := map[string]string{"foo": "bar"}
+	if _, err := updateJob(ctx, jobClient, jobName, func(j *batchv1.Job) {
+		j.Spec.Template.Spec.NodeSelector = nodeSelector
+		j.Spec.Suspend = pointer.BoolPtr(false)
+	}); err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	// (2) Check that the pod was created using the expected node selector.
+
+	var pod *v1.Pod
+	if err := wait.PollImmediate(waitInterval, wait.ForeverTestTimeout, func() (bool, error) {
+		pods, err := clientSet.CoreV1().Pods(jobNamespace).List(ctx, metav1.ListOptions{})
+		if err != nil {
+			t.Fatalf("Failed to list Job Pods: %v", err)
+		}
+		if len(pods.Items) == 0 {
+			return false, nil
+		}
+		pod = &pods.Items[0]
+		return true, nil
+	}); err != nil || pod == nil {
+		t.Fatalf("pod not found: %v", err)
+	}
+
+	// if the feature gate is enabled, then the job should now be unsuspended and
+	// the pod has the node selector.
+	if diff := cmp.Diff(nodeSelector, pod.Spec.NodeSelector); diff != "" {
+		t.Errorf("Unexpected nodeSelector (-want,+got):\n%s", diff)
+	}
+
+	// (3) Update node selector again. It should fail since the job is unsuspended.
+	_, err = updateJob(ctx, jobClient, jobName, func(j *batchv1.Job) {
+		j.Spec.Template.Spec.NodeSelector = map[string]string{"foo": "baz"}
+	})
+
+	if err == nil || !strings.Contains(err.Error(), "spec.template: Invalid value") {
+		t.Errorf("Expected \"spec.template: Invalid value\" error, got: %v", err)
+	}
+
 }
 
 type podsByStatus struct {