diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 5c4558bd93e..33163e24e47 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -395,6 +395,7 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po AllowExpandedDNSConfig: utilfeature.DefaultFeatureGate.Enabled(features.ExpandedDNSConfig) || haveSameExpandedDNSConfig(podSpec, oldPodSpec), AllowInvalidLabelValueInSelector: false, AllowInvalidTopologySpreadConstraintLabelSelector: false, + AllowMutableNodeSelectorAndNodeAffinity: utilfeature.DefaultFeatureGate.Enabled(features.PodSchedulingReadiness), } if oldPodSpec != nil { diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 5e1113773b7..61d0fb3af42 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -3665,6 +3665,8 @@ type PodValidationOptions struct { AllowExpandedDNSConfig bool // Allow invalid topologySpreadConstraint labelSelector for backward compatibility AllowInvalidTopologySpreadConstraintLabelSelector bool + // Allow node selector additions for gated pods. + AllowMutableNodeSelectorAndNodeAffinity bool } // validatePodMetadataAndSpec tests if required fields in the pod.metadata and pod.spec are set, @@ -4671,6 +4673,39 @@ func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel mungedPodSpec.TerminationGracePeriodSeconds = oldPod.Spec.TerminationGracePeriodSeconds // +k8s:verify-mutation:reason=clone } + // Handle validations specific to gated pods. + podIsGated := len(oldPod.Spec.SchedulingGates) > 0 + if opts.AllowMutableNodeSelectorAndNodeAffinity && podIsGated { + // Additions to spec.nodeSelector are allowed (no deletions or mutations) for gated pods. + if !apiequality.Semantic.DeepEqual(mungedPodSpec.NodeSelector, oldPod.Spec.NodeSelector) { + allErrs = append(allErrs, validateNodeSelectorMutation(specPath.Child("nodeSelector"), mungedPodSpec.NodeSelector, oldPod.Spec.NodeSelector)...) + mungedPodSpec.NodeSelector = oldPod.Spec.NodeSelector // +k8s:verify-mutation:reason=clone + } + + // Validate node affinity mutations. + var oldNodeAffinity *core.NodeAffinity + if oldPod.Spec.Affinity != nil { + oldNodeAffinity = oldPod.Spec.Affinity.NodeAffinity // +k8s:verify-mutation:reason=clone + } + + var mungedNodeAffinity *core.NodeAffinity + if mungedPodSpec.Affinity != nil { + mungedNodeAffinity = mungedPodSpec.Affinity.NodeAffinity // +k8s:verify-mutation:reason=clone + } + + if !apiequality.Semantic.DeepEqual(oldNodeAffinity, mungedNodeAffinity) { + allErrs = append(allErrs, validateNodeAffinityMutation(specPath.Child("affinity").Child("nodeAffinity"), mungedNodeAffinity, oldNodeAffinity)...) + switch { + case mungedPodSpec.Affinity == nil && oldNodeAffinity == nil: + // already effectively nil, no change needed + case mungedPodSpec.Affinity == nil && oldNodeAffinity != nil: + mungedPodSpec.Affinity = &core.Affinity{NodeAffinity: oldNodeAffinity} // +k8s:verify-mutation:reason=clone + default: + mungedPodSpec.Affinity.NodeAffinity = oldNodeAffinity // +k8s:verify-mutation:reason=clone + } + } + } + if !apiequality.Semantic.DeepEqual(mungedPodSpec, oldPod.Spec) { // This diff isn't perfect, but it's a helluva lot better an "I'm not going to tell you what the difference is". // TODO: Pinpoint the specific field that causes the invalid error after we have strategic merge diff @@ -4681,7 +4716,6 @@ func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel } allErrs = append(allErrs, errs) } - return allErrs } @@ -7297,3 +7331,74 @@ func validatePvNodeAffinity(newPvNodeAffinity, oldPvNodeAffinity *core.VolumeNod } return allErrs } + +func validateNodeSelectorMutation(fldPath *field.Path, newNodeSelector, oldNodeSelector map[string]string) field.ErrorList { + var allErrs field.ErrorList + + // Validate no existing node selectors were deleted or mutated. + for k, v1 := range oldNodeSelector { + if v2, ok := newNodeSelector[k]; !ok || v1 != v2 { + allErrs = append(allErrs, field.Invalid(fldPath, newNodeSelector, "only additions to spec.nodeSelector are allowed (no mutations or deletions)")) + return allErrs + } + } + return allErrs +} + +func validateNodeAffinityMutation(nodeAffinityPath *field.Path, newNodeAffinity, oldNodeAffinity *core.NodeAffinity) field.ErrorList { + var allErrs field.ErrorList + // If old node affinity was nil, anything can be set. + if oldNodeAffinity == nil || oldNodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution == nil { + return allErrs + } + + oldTerms := oldNodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms + var newTerms []core.NodeSelectorTerm + if newNodeAffinity != nil && newNodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil { + newTerms = newNodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms + } + + // If there are no old terms, we can set the new terms to anything. + // If there are old terms, we cannot add any new ones. + if len(oldTerms) > 0 && len(oldTerms) != len(newTerms) { + return append(allErrs, field.Invalid(nodeAffinityPath.Child("requiredDuringSchedulingIgnoredDuringExecution").Child("nodeSelectorTerms"), newTerms, "no additions/deletions to non-empty NodeSelectorTerms list are allowed")) + } + + // For requiredDuringSchedulingIgnoredDuringExecution, if old NodeSelectorTerms + // was empty, anything can be set. If non-empty, only additions of NodeSelectorRequirements + // to matchExpressions or fieldExpressions are allowed. + for i := range oldTerms { + if !validateNodeSelectorTermHasOnlyAdditions(newTerms[i], oldTerms[i]) { + allErrs = append(allErrs, field.Invalid(nodeAffinityPath.Child("requiredDuringSchedulingIgnoredDuringExecution").Child("nodeSelectorTerms").Index(i), newTerms[i], "only additions are allowed (no mutations or deletions)")) + } + } + return allErrs +} + +func validateNodeSelectorTermHasOnlyAdditions(newTerm, oldTerm core.NodeSelectorTerm) bool { + if len(oldTerm.MatchExpressions) == 0 && len(oldTerm.MatchFields) == 0 { + if len(newTerm.MatchExpressions) > 0 || len(newTerm.MatchFields) > 0 { + return false + } + } + + // Validate MatchExpressions only has additions (no deletions or mutations) + if l := len(oldTerm.MatchExpressions); l > 0 { + if len(newTerm.MatchExpressions) < l { + return false + } + if !apiequality.Semantic.DeepEqual(newTerm.MatchExpressions[:l], oldTerm.MatchExpressions) { + return false + } + } + // Validate MatchFields only has additions (no deletions or mutations) + if l := len(oldTerm.MatchFields); l > 0 { + if len(newTerm.MatchFields) < l { + return false + } + if !apiequality.Semantic.DeepEqual(newTerm.MatchFields[:l], oldTerm.MatchFields) { + return false + } + } + return true +} diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 76a9b8801b7..d20f0ffa532 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -11295,6 +11295,7 @@ func TestValidatePodUpdate(t *testing.T) { tests := []struct { new core.Pod old core.Pod + opts PodValidationOptions err string test string }{ @@ -12784,6 +12785,952 @@ func TestValidatePodUpdate(t *testing.T) { err: "", test: "update pod spec schedulingGates: legal deletion", }, + { + old: core.Pod{ + Spec: core.PodSpec{ + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + NodeSelector: map[string]string{ + "foo": "bar", + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + err: "Forbidden: pod updates may not change fields other than `spec.containers[*].image", + test: "node selector is immutable when AllowMutableNodeSelector is false", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + NodeSelector: map[string]string{ + "foo": "bar", + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + test: "adding node selector is allowed for gated pods", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + NodeSelector: map[string]string{ + "foo": "bar", + }, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + NodeSelector: map[string]string{ + "foo": "bar", + "foo2": "bar2", + }, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "Forbidden: pod updates may not change fields other than `spec.containers[*].image", + test: "adding node selector is not allowed for non-gated pods", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + NodeSelector: map[string]string{ + "foo": "bar", + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "spec.nodeSelector: Invalid value:", + test: "removing node selector is not allowed for gated pods", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + NodeSelector: map[string]string{ + "foo": "bar", + }, + }, + }, + new: core.Pod{}, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "Forbidden: pod updates may not change fields other than `spec.containers[*].image", + test: "removing node selector is not allowed for non-gated pods", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + NodeSelector: map[string]string{ + "foo": "bar", + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + NodeSelector: map[string]string{ + "foo": "bar", + "foo2": "bar2", + }, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + test: "old pod spec has scheduling gate, new pod spec does not, and node selector is added", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + NodeSelector: map[string]string{ + "foo": "bar", + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + NodeSelector: map[string]string{ + "foo": "new value", + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "spec.nodeSelector: Invalid value:", + test: "modifying value of existing node selector is not allowed", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + // Add 1 MatchExpression and 1 MatchField. + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + { + Key: "expr2", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo2"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + test: "addition to nodeAffinity is allowed for gated pods", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{}, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms: Invalid value:", + test: "old RequiredDuringSchedulingIgnoredDuringExecution is non-nil, new RequiredDuringSchedulingIgnoredDuringExecution is nil, pod is gated", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + // Add 1 MatchExpression and 1 MatchField. + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + { + Key: "expr2", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo2"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "Forbidden: pod updates may not change fields other than `spec.containers[*].image", + test: "addition to nodeAffinity is not allowed for non-gated pods", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + // Add 1 MatchExpression and 1 MatchField. + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + { + Key: "expr2", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo2"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + test: "old pod spec has scheduling gate, new pod spec does not, and node affinity addition occurs", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0]: Invalid value:", + test: "nodeAffinity deletion from MatchExpressions not allowed", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + // Add 1 MatchExpression and 1 MatchField. + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0]: Invalid value:", + test: "nodeAffinity deletion from MatchFields not allowed", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + // Add 1 MatchExpression and 1 MatchField. + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"bar"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0]: Invalid value:", + test: "nodeAffinity modification of item in MatchExpressions not allowed", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"bar"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0]: Invalid value:", + test: "nodeAffinity modification of item in MatchFields not allowed", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"bar"}, + }, + }, + }, + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo2"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"bar2"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms: Invalid value:", + test: "nodeSelectorTerms addition on gated pod should fail", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []core.PreferredSchedulingTerm{ + { + Weight: 1.0, + Preference: core.NodeSelectorTerm{ + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []core.PreferredSchedulingTerm{ + { + Weight: 1.0, + Preference: core.NodeSelectorTerm{ + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo2"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + test: "preferredDuringSchedulingIgnoredDuringExecution can modified for gated pods", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []core.PreferredSchedulingTerm{ + { + Weight: 1.0, + Preference: core.NodeSelectorTerm{ + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []core.PreferredSchedulingTerm{ + { + Weight: 1.0, + Preference: core.NodeSelectorTerm{ + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + { + Key: "expr2", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo2"}, + }, + }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"bar"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + test: "preferredDuringSchedulingIgnoredDuringExecution can have additions for gated pods", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []core.PreferredSchedulingTerm{ + { + Weight: 1.0, + Preference: core.NodeSelectorTerm{ + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + test: "preferredDuringSchedulingIgnoredDuringExecution can have removals for gated pods", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{}, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms: Invalid value:", + test: "new node affinity is nil", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []core.PreferredSchedulingTerm{ + { + Weight: 1.0, + Preference: core.NodeSelectorTerm{ + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + test: "preferredDuringSchedulingIgnoredDuringExecution can have removals for gated pods", + }, + { + old: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + {}, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + new: core.Pod{ + Spec: core.PodSpec{ + Affinity: &core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchExpressions: []core.NodeSelectorRequirement{ + { + Key: "expr", + Operator: core.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []core.PodSchedulingGate{{Name: "baz"}}, + }, + }, + opts: PodValidationOptions{ + AllowMutableNodeSelectorAndNodeAffinity: true, + }, + err: "spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0]: Invalid value:", + test: "empty NodeSelectorTerm (selects nothing) cannot become populated (selects something)", + }, } for _, test := range tests { test.new.ObjectMeta.ResourceVersion = "1" @@ -12811,7 +13758,7 @@ func TestValidatePodUpdate(t *testing.T) { test.old.Spec.RestartPolicy = "Always" } - errs := ValidatePodUpdate(&test.new, &test.old, PodValidationOptions{}) + errs := ValidatePodUpdate(&test.new, &test.old, test.opts) if test.err == "" { if len(errs) != 0 { t.Errorf("unexpected invalid: %s (%+v)\nA: %+v\nB: %+v", test.test, errs, test.new, test.old) diff --git a/test/integration/pods/pods_test.go b/test/integration/pods/pods_test.go index 225ff46b10e..3f463e7e9f1 100644 --- a/test/integration/pods/pods_test.go +++ b/test/integration/pods/pods_test.go @@ -25,9 +25,12 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" typedv1 "k8s.io/client-go/kubernetes/typed/core/v1" + featuregatetesting "k8s.io/component-base/featuregate/testing" kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/test/integration" "k8s.io/kubernetes/test/integration/framework" ) @@ -674,3 +677,185 @@ func TestPodUpdateEphemeralContainers(t *testing.T) { integration.DeletePodOrErrorf(t, client, ns.Name, pod.Name) } } + +func TestMutablePodSchedulingDirectives(t *testing.T) { + // Disable ServiceAccount admission plugin as we don't have serviceaccount controller running. + server := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount"}, framework.SharedEtcd()) + defer server.TearDownFn() + + client := clientset.NewForConfigOrDie(server.ClientConfig) + + ns := framework.CreateNamespaceOrDie(client, "mutable-pod-scheduling-directives", t) + defer framework.DeleteNamespaceOrDie(client, ns, t) + + cases := []struct { + name string + create *v1.Pod + update *v1.Pod + enableSchedulingGates bool + err string + }{ + { + name: "node selector is immutable when AllowMutableNodeSelector is false", + create: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "fakeimage", + }, + }, + SchedulingGates: []v1.PodSchedulingGate{{Name: "baz"}}, + }, + }, + update: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "fakeimage", + }, + }, + NodeSelector: map[string]string{ + "foo": "bar", + }, + SchedulingGates: []v1.PodSchedulingGate{{Name: "baz"}}, + }, + }, + err: "Forbidden: pod updates may not change fields other than `spec.containers[*].image", + }, + { + name: "adding node selector is allowed for gated pods", + create: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "fakeimage", + }, + }, + SchedulingGates: []v1.PodSchedulingGate{{Name: "baz"}}, + }, + }, + update: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "fakeimage", + }, + }, + NodeSelector: map[string]string{ + "foo": "bar", + }, + SchedulingGates: []v1.PodSchedulingGate{{Name: "baz"}}, + }, + }, + enableSchedulingGates: true, + }, + { + name: "addition to nodeAffinity is allowed for gated pods", + create: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "fakeimage", + }, + }, + Affinity: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "expr", + Operator: v1.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []v1.PodSchedulingGate{{Name: "baz"}}, + }, + }, + update: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "fakeimage", + }, + }, + Affinity: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + // Add 1 MatchExpression and 1 MatchField. + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "expr", + Operator: v1.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + { + Key: "expr2", + Operator: v1.NodeSelectorOpIn, + Values: []string{"foo2"}, + }, + }, + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: v1.NodeSelectorOpIn, + Values: []string{"foo"}, + }, + }, + }, + }, + }, + }, + }, + SchedulingGates: []v1.PodSchedulingGate{{Name: "baz"}}, + }, + }, + enableSchedulingGates: true, + }, + } + for _, tc := range cases { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodSchedulingReadiness, tc.enableSchedulingGates)() + + if _, err := client.CoreV1().Pods(ns.Name).Create(context.TODO(), tc.create, metav1.CreateOptions{}); err != nil { + t.Errorf("Failed to create pod: %v", err) + } + + _, err := client.CoreV1().Pods(ns.Name).Update(context.TODO(), tc.update, metav1.UpdateOptions{}) + if (tc.err == "" && err != nil) || (tc.err != "" && err != nil && !strings.Contains(err.Error(), tc.err)) { + t.Errorf("Unexpected error: got %q, want %q", err.Error(), err) + } + integration.DeletePodOrErrorf(t, client, ns.Name, tc.update.Name) + } +}