Merge pull request kubernetes#115636 from Huang-Wei/automated-cherry-…

…pick-of-#115569-upstream-release-1.26

Automated cherry pick of kubernetes#115569: Enforce nodeName cannot be set along with non-empty schedulingGates

pkg/apis/core/validation/validation.go

@@ -4432,8 +4432,7 @@ func ValidatePodCreate(pod *core.Pod, opts PodValidationOptions) field.ErrorList
 		allErrs = append(allErrs, field.Forbidden(fldPath.Child("ephemeralContainers"), "cannot be set on create"))
 	}
 	// A Pod cannot be assigned a Node if there are remaining scheduling gates.
-	if utilfeature.DefaultFeatureGate.Enabled(features.PodSchedulingReadiness) &&
-		pod.Spec.NodeName != "" && len(pod.Spec.SchedulingGates) != 0 {
+	if pod.Spec.NodeName != "" && len(pod.Spec.SchedulingGates) != 0 {
 		allErrs = append(allErrs, field.Forbidden(fldPath.Child("nodeName"), "cannot be set until all schedulingGates have been cleared"))
 	}
 	allErrs = append(allErrs, validateSeccompAnnotationsAndFields(pod.ObjectMeta, &pod.Spec, fldPath)...)

pkg/apis/core/validation/validation_test.go

@@ -10836,7 +10836,7 @@ func TestValidatePodCreateWithSchedulingGates(t *testing.T) {
 				},
 			},
 			featureEnabled:  false,
-			wantFieldErrors: nil,
+			wantFieldErrors: []*field.Error{field.Forbidden(fldPath.Child("nodeName"), "cannot be set until all schedulingGates have been cleared")},
 		},
 		{
 			name: "create a Pod with nodeName and schedulingGates, feature enabled",

pkg/registry/core/pod/storage/storage.go

@@ -33,12 +33,10 @@ import (
 	"k8s.io/apiserver/pkg/storage"
 	storeerr "k8s.io/apiserver/pkg/storage/errors"
 	"k8s.io/apiserver/pkg/util/dryrun"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	policyclient "k8s.io/client-go/kubernetes/typed/policy/v1"
 	podutil "k8s.io/kubernetes/pkg/api/pod"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/core/validation"
-	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/kubelet/client"
 	"k8s.io/kubernetes/pkg/printers"
 	printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
@@ -224,7 +222,7 @@ func (r *BindingREST) setPodHostAndAnnotations(ctx context.Context, podUID types
 			return nil, fmt.Errorf("pod %v is already assigned to node %q", pod.Name, pod.Spec.NodeName)
 		}
 		// Reject binding to a scheduling un-ready Pod.
-		if utilfeature.DefaultFeatureGate.Enabled(features.PodSchedulingReadiness) && len(pod.Spec.SchedulingGates) != 0 {
+		if len(pod.Spec.SchedulingGates) != 0 {
 			return nil, fmt.Errorf("pod %v has non-empty .spec.schedulingGates", pod.Name)
 		}
 		pod.Spec.NodeName = machine

pkg/registry/core/pod/storage/storage_test.go

@@ -789,34 +789,42 @@ func TestEtcdCreateWithSchedulingGates(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodSchedulingReadiness, tt.featureEnabled)()
-			storage, bindingStorage, _, server := newStorage(t)
-			defer server.Terminate(t)
-			defer storage.Store.DestroyFunc()
-			ctx := genericapirequest.NewDefaultContext()
-
-			pod := validNewPod()
-			pod.Spec.SchedulingGates = tt.schedulingGates
-			if _, err := storage.Create(ctx, pod, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}); err != nil {
-				t.Fatalf("Unexpected error: %v", err)
+		for _, flipFeatureGateBeforeBinding := range []bool{false, true} {
+			if flipFeatureGateBeforeBinding {
+				tt.name = fmt.Sprintf("%v and flipped before binding", tt.name)
 			}
-			_, err := bindingStorage.Create(ctx, "foo", &api.Binding{
-				ObjectMeta: metav1.ObjectMeta{Namespace: metav1.NamespaceDefault, Name: "foo"},
-				Target:     api.ObjectReference{Name: "machine"},
-			}, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
-			if tt.wantErr == nil {
-				if err != nil {
-					t.Errorf("Want nil err, but got %v", err)
+			t.Run(tt.name, func(t *testing.T) {
+				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodSchedulingReadiness, tt.featureEnabled)()
+				storage, bindingStorage, _, server := newStorage(t)
+				defer server.Terminate(t)
+				defer storage.Store.DestroyFunc()
+				ctx := genericapirequest.NewDefaultContext()
+
+				pod := validNewPod()
+				pod.Spec.SchedulingGates = tt.schedulingGates
+				if _, err := storage.Create(ctx, pod, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}); err != nil {
+					t.Fatalf("Unexpected error: %v", err)
 				}
-			} else {
-				if err == nil {
-					t.Errorf("Want %v, but got nil err", tt.wantErr)
-				} else if tt.wantErr.Error() != err.Error() {
-					t.Errorf("Want %v, but got %v", tt.wantErr, err)
+				if flipFeatureGateBeforeBinding {
+					defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodSchedulingReadiness, !tt.featureEnabled)()
 				}
-			}
-		})
+				_, err := bindingStorage.Create(ctx, "foo", &api.Binding{
+					ObjectMeta: metav1.ObjectMeta{Namespace: metav1.NamespaceDefault, Name: "foo"},
+					Target:     api.ObjectReference{Name: "machine"},
+				}, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
+				if tt.wantErr == nil {
+					if err != nil {
+						t.Errorf("Want nil err, but got %v", err)
+					}
+				} else {
+					if err == nil {
+						t.Errorf("Want %v, but got nil err", tt.wantErr)
+					} else if tt.wantErr.Error() != err.Error() {
+						t.Errorf("Want %v, but got %v", tt.wantErr, err)
+					}
+				}
+			})
+		}
 	}
 }