diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 0db24c93cba..e926935fc5f 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -12104,7 +12104,7 @@ "type": "string" }, "operator": { - "description": "Operator represents a key's relationship to the value. Valid operators are Exists and Equal. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category.", + "description": "Operator represents a key's relationship to the value. Valid operators are Exists, Equal, Lt, and Gt. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category. Lt and Gt perform numeric comparisons (requires feature gate TaintTolerationComparisonOperators).", "type": "string" }, "tolerationSeconds": { diff --git a/api/openapi-spec/v3/api__v1_openapi.json b/api/openapi-spec/v3/api__v1_openapi.json index f8f22160cbb..74e24d6ed23 100644 --- a/api/openapi-spec/v3/api__v1_openapi.json +++ b/api/openapi-spec/v3/api__v1_openapi.json @@ -8252,7 +8252,7 @@ "type": "string" }, "operator": { - "description": "Operator represents a key's relationship to the value. Valid operators are Exists and Equal. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category.", + "description": "Operator represents a key's relationship to the value. Valid operators are Exists, Equal, Lt, and Gt. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category. Lt and Gt perform numeric comparisons (requires feature gate TaintTolerationComparisonOperators).", "type": "string" }, "tolerationSeconds": { diff --git a/api/openapi-spec/v3/apis__apps__v1_openapi.json b/api/openapi-spec/v3/apis__apps__v1_openapi.json index 04b3a82a46a..15df0b3735e 100644 --- a/api/openapi-spec/v3/apis__apps__v1_openapi.json +++ b/api/openapi-spec/v3/apis__apps__v1_openapi.json @@ -4972,7 +4972,7 @@ "type": "string" }, "operator": { - "description": "Operator represents a key's relationship to the value. Valid operators are Exists and Equal. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category.", + "description": "Operator represents a key's relationship to the value. Valid operators are Exists, Equal, Lt, and Gt. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category. Lt and Gt perform numeric comparisons (requires feature gate TaintTolerationComparisonOperators).", "type": "string" }, "tolerationSeconds": { diff --git a/api/openapi-spec/v3/apis__batch__v1_openapi.json b/api/openapi-spec/v3/apis__batch__v1_openapi.json index d817d279808..2df1e47230b 100644 --- a/api/openapi-spec/v3/apis__batch__v1_openapi.json +++ b/api/openapi-spec/v3/apis__batch__v1_openapi.json @@ -4163,7 +4163,7 @@ "type": "string" }, "operator": { - "description": "Operator represents a key's relationship to the value. Valid operators are Exists and Equal. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category.", + "description": "Operator represents a key's relationship to the value. Valid operators are Exists, Equal, Lt, and Gt. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category. Lt and Gt perform numeric comparisons (requires feature gate TaintTolerationComparisonOperators).", "type": "string" }, "tolerationSeconds": { diff --git a/api/openapi-spec/v3/apis__node.k8s.io__v1_openapi.json b/api/openapi-spec/v3/apis__node.k8s.io__v1_openapi.json index 003198724d2..08fd5417018 100644 --- a/api/openapi-spec/v3/apis__node.k8s.io__v1_openapi.json +++ b/api/openapi-spec/v3/apis__node.k8s.io__v1_openapi.json @@ -13,7 +13,7 @@ "type": "string" }, "operator": { - "description": "Operator represents a key's relationship to the value. Valid operators are Exists and Equal. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category.", + "description": "Operator represents a key's relationship to the value. Valid operators are Exists, Equal, Lt, and Gt. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category. Lt and Gt perform numeric comparisons (requires feature gate TaintTolerationComparisonOperators).", "type": "string" }, "tolerationSeconds": { diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 1a75f25fca8..74331ffbd1b 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -438,6 +438,7 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po opts.AllowUserNamespacesHostNetworkSupport = useAllowUserNamespacesHostNetworkSupport(oldPodSpec) opts.AllowOnlyRecursiveSELinuxChangePolicy = useOnlyRecursiveSELinuxChangePolicy(oldPodSpec) + opts.AllowTaintTolerationComparisonOperators = allowTaintTolerationComparisonOperators(oldPodSpec) if oldPodSpec != nil { // if old spec used non-integer multiple of huge page unit size, we must allow it @@ -1618,6 +1619,28 @@ func useOnlyRecursiveSELinuxChangePolicy(oldPodSpec *api.PodSpec) bool { return true } +func taintTolerationComparisonOperatorsInUse(podSpec *api.PodSpec) bool { + if podSpec == nil { + return false + } + for _, toleration := range podSpec.Tolerations { + if toleration.Operator == api.TolerationOpLt || toleration.Operator == api.TolerationOpGt { + return true + } + } + return false +} + +func allowTaintTolerationComparisonOperators(oldPodSpec *api.PodSpec) bool { + // allow the operators if the feature gate is enabled or the old pod spec uses + // comparison operators + if utilfeature.DefaultFeatureGate.Enabled(features.TaintTolerationComparisonOperators) || + taintTolerationComparisonOperatorsInUse(oldPodSpec) { + return true + } + return false +} + func hasUserNamespacesWithVolumeDevices(podSpec *api.PodSpec) bool { if podSpec.SecurityContext == nil || podSpec.SecurityContext.HostUsers == nil || *podSpec.SecurityContext.HostUsers { return false diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index af967126c57..40b9e3edad7 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -6219,6 +6219,204 @@ func TestHasUserNamespacesWithVolumeDevices(t *testing.T) { } } +func TestTaintTolerationComparisonOperatorsInUse(t *testing.T) { + tests := []struct { + name string + podSpec *api.PodSpec + expected bool + }{ + { + name: "nil pod spec", + podSpec: nil, + expected: false, + }, + { + name: "no tolerations", + podSpec: &api.PodSpec{ + Containers: []api.Container{{Name: "test"}}, + }, + expected: false, + }, + { + name: "only Equal operator", + podSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpEqual, Value: "value1"}, + }, + }, + expected: false, + }, + { + name: "only Exists operator", + podSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpExists}, + }, + }, + expected: false, + }, + { + name: "Lt operator present", + podSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpLt, Value: "100"}, + }, + }, + expected: true, + }, + { + name: "Gt operator present", + podSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpGt, Value: "50"}, + }, + }, + expected: true, + }, + { + name: "mixed operators with Lt", + podSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpEqual, Value: "value1"}, + {Key: "key2", Operator: api.TolerationOpLt, Value: "100"}, + {Key: "key3", Operator: api.TolerationOpExists}, + }, + }, + expected: true, + }, + { + name: "mixed operators with Gt", + podSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpExists}, + {Key: "key2", Operator: api.TolerationOpGt, Value: "200"}, + }, + }, + expected: true, + }, + { + name: "both Lt and Gt operators", + podSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpLt, Value: "100"}, + {Key: "key2", Operator: api.TolerationOpGt, Value: "50"}, + }, + }, + expected: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := taintTolerationComparisonOperatorsInUse(test.podSpec) + if test.expected != actual { + t.Errorf("expected %v, got %v", test.expected, actual) + } + }) + } +} + +func TestAllowTaintTolerationComparisonOperators(t *testing.T) { + tests := []struct { + name string + featureEnabled bool + oldPodSpec *api.PodSpec + expected bool + }{ + { + name: "feature gate enabled, nil old pod spec", + featureEnabled: true, + oldPodSpec: nil, + expected: true, + }, + { + name: "feature gate enabled, old pod spec without comparison operators", + featureEnabled: true, + oldPodSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpEqual, Value: "value1"}, + }, + }, + expected: true, + }, + { + name: "feature gate enabled, old pod spec with Lt operator", + featureEnabled: true, + oldPodSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpLt, Value: "100"}, + }, + }, + expected: true, + }, + { + name: "feature gate disabled, nil old pod spec", + featureEnabled: false, + oldPodSpec: nil, + expected: false, + }, + { + name: "feature gate disabled, old pod spec without comparison operators", + featureEnabled: false, + oldPodSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpEqual, Value: "value1"}, + {Key: "key2", Operator: api.TolerationOpExists}, + }, + }, + expected: false, + }, + { + name: "feature gate disabled, old pod spec with Lt operator", + featureEnabled: false, + oldPodSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpLt, Value: "100"}, + }, + }, + expected: true, + }, + { + name: "feature gate disabled, old pod spec with Gt operator", + featureEnabled: false, + oldPodSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpGt, Value: "50"}, + }, + }, + expected: true, + }, + { + name: "feature gate disabled, old pod spec with mixed operators including Lt", + featureEnabled: false, + oldPodSpec: &api.PodSpec{ + Tolerations: []api.Toleration{ + {Key: "key1", Operator: api.TolerationOpEqual, Value: "value1"}, + {Key: "key2", Operator: api.TolerationOpLt, Value: "100"}, + {Key: "key3", Operator: api.TolerationOpExists}, + }, + }, + expected: true, + }, + { + name: "feature gate disabled, empty old pod spec", + featureEnabled: false, + oldPodSpec: &api.PodSpec{}, + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TaintTolerationComparisonOperators, test.featureEnabled) + actual := allowTaintTolerationComparisonOperators(test.oldPodSpec) + if test.expected != actual { + t.Errorf("expected %v, got %v", test.expected, actual) + } + }) + } +} + func TestDisabledWorkload(t *testing.T) { podWithWorkload := &api.Pod{ Spec: api.PodSpec{ diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index b4304ddd5fd..286646a26b3 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3621,9 +3621,10 @@ type Toleration struct { // +optional Key string // Operator represents a key's relationship to the value. - // Valid operators are Exists and Equal. Defaults to Equal. + // Valid operators are Exists, Equal, Lt, and Gt. Defaults to Equal. // Exists is equivalent to wildcard for value, so that a pod can // tolerate all taints of a particular category. + // Lt and Gt perform numeric comparisons (requires feature gate TaintTolerationComparisonOperators). // +optional Operator TolerationOperator // Value is the taint value the toleration matches to. @@ -3649,6 +3650,8 @@ type TolerationOperator string const ( TolerationOpExists TolerationOperator = "Exists" TolerationOpEqual TolerationOperator = "Equal" + TolerationOpLt TolerationOperator = "Lt" + TolerationOpGt TolerationOperator = "Gt" ) // PodReadinessGate contains the reference to a pod condition diff --git a/pkg/apis/core/v1/helper/helpers.go b/pkg/apis/core/v1/helper/helpers.go index 96accbd3f69..87fc484c8ed 100644 --- a/pkg/apis/core/v1/helper/helpers.go +++ b/pkg/apis/core/v1/helper/helpers.go @@ -18,6 +18,7 @@ package helper import ( "fmt" + "k8s.io/klog/v2" "strings" v1 "k8s.io/api/core/v1" @@ -25,7 +26,9 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/util/validation" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/apis/core/helper" + "k8s.io/kubernetes/pkg/features" ) // IsExtendedResourceName returns true if: @@ -287,18 +290,19 @@ func AddOrUpdateTolerationInPodSpec(spec *v1.PodSpec, toleration *v1.Toleration) } // GetMatchingTolerations returns true and list of Tolerations matching all Taints if all are tolerated, or false otherwise. -func GetMatchingTolerations(taints []v1.Taint, tolerations []v1.Toleration) (bool, []v1.Toleration) { +func GetMatchingTolerations(logger klog.Logger, taints []v1.Taint, tolerations []v1.Toleration) (bool, []v1.Toleration) { if len(taints) == 0 { return true, []v1.Toleration{} } if len(tolerations) == 0 && len(taints) > 0 { return false, []v1.Toleration{} } + enableComparisonOperators := utilfeature.DefaultFeatureGate.Enabled(features.TaintTolerationComparisonOperators) result := []v1.Toleration{} for i := range taints { tolerated := false for j := range tolerations { - if tolerations[j].ToleratesTaint(&taints[i]) { + if tolerations[j].ToleratesTaint(logger, &taints[i], enableComparisonOperators) { result = append(result, tolerations[j]) tolerated = true break diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 009e009586a..8dcfe8a7807 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -27,6 +27,7 @@ import ( "reflect" "regexp" "slices" + "strconv" "strings" "sync" "unicode" @@ -39,6 +40,7 @@ import ( "k8s.io/apimachinery/pkg/api/operation" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/api/validate" + "k8s.io/apimachinery/pkg/api/validate/content" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" unversionedvalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" @@ -198,7 +200,7 @@ func ValidatePodSpecificAnnotations(annotations map[string]string, spec *core.Po } if annotations[core.TolerationsAnnotationKey] != "" { - allErrs = append(allErrs, ValidateTolerationsInPodAnnotations(annotations, fldPath)...) + allErrs = append(allErrs, ValidateTolerationsInPodAnnotations(annotations, fldPath, opts)...) } if !opts.AllowInvalidPodDeletionCost { @@ -214,7 +216,7 @@ func ValidatePodSpecificAnnotations(annotations map[string]string, spec *core.Po } // ValidateTolerationsInPodAnnotations tests that the serialized tolerations in Pod.Annotations has valid data -func ValidateTolerationsInPodAnnotations(annotations map[string]string, fldPath *field.Path) field.ErrorList { +func ValidateTolerationsInPodAnnotations(annotations map[string]string, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { allErrs := field.ErrorList{} tolerations, err := helper.GetTolerationsFromPodAnnotations(annotations) @@ -224,7 +226,7 @@ func ValidateTolerationsInPodAnnotations(annotations map[string]string, fldPath } if len(tolerations) > 0 { - allErrs = append(allErrs, ValidateTolerations(tolerations, fldPath.Child(core.TolerationsAnnotationKey))...) + allErrs = append(allErrs, ValidateTolerations(tolerations, fldPath.Child(core.TolerationsAnnotationKey), opts)...) } return allErrs @@ -4284,7 +4286,7 @@ func validateTaintEffect(effect *core.TaintEffect, allowEmpty bool, fldPath *fie } // validateOnlyAddedTolerations validates updated pod tolerations. -func validateOnlyAddedTolerations(newTolerations []core.Toleration, oldTolerations []core.Toleration, fldPath *field.Path) field.ErrorList { +func validateOnlyAddedTolerations(newTolerations []core.Toleration, oldTolerations []core.Toleration, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { allErrs := field.ErrorList{} for _, old := range oldTolerations { found := false @@ -4303,7 +4305,7 @@ func validateOnlyAddedTolerations(newTolerations []core.Toleration, oldToleratio } } - allErrs = append(allErrs, ValidateTolerations(newTolerations, fldPath)...) + allErrs = append(allErrs, ValidateTolerations(newTolerations, fldPath, opts)...) return allErrs } @@ -4341,7 +4343,7 @@ func ValidateHostAliases(hostAliases []core.HostAlias, fldPath *field.Path) fiel } // ValidateTolerations tests if given tolerations have valid data. -func ValidateTolerations(tolerations []core.Toleration, fldPath *field.Path) field.ErrorList { +func ValidateTolerations(tolerations []core.Toleration, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { allErrors := field.ErrorList{} for i, toleration := range tolerations { idxPath := fldPath.Index(i) @@ -4372,6 +4374,23 @@ func ValidateTolerations(tolerations []core.Toleration, fldPath *field.Path) fie if len(toleration.Value) > 0 { allErrors = append(allErrors, field.Invalid(idxPath.Child("operator"), toleration.Value, "value must be empty when `operator` is 'Exists'")) } + case core.TolerationOpLt, core.TolerationOpGt: + // Numeric comparison operators require validation option + if !opts.AllowTaintTolerationComparisonOperators { + validValues := []core.TolerationOperator{core.TolerationOpEqual, core.TolerationOpExists, core.TolerationOpLt, core.TolerationOpGt} + allErrors = append(allErrors, field.NotSupported(idxPath.Child("operator"), toleration.Operator, validValues)) + break + } + + // validate value is decimal integer + for _, msg := range content.IsDecimalInteger(toleration.Value) { + allErrors = append(allErrors, field.Invalid(idxPath.Child("value"), toleration.Value, msg)) + } + + // validate value is within int64 range + if _, err := strconv.ParseInt(toleration.Value, 10, 64); err != nil { + allErrors = append(allErrors, field.Invalid(idxPath.Child("value"), toleration.Value, err.Error())) + } default: validValues := []core.TolerationOperator{core.TolerationOpEqual, core.TolerationOpExists} allErrors = append(allErrors, field.NotSupported(idxPath.Child("operator"), toleration.Operator, validValues)) @@ -4451,6 +4470,8 @@ type PodValidationOptions struct { AllowContainerRestartPolicyRules bool // Allow user namespaces with volume devices, even though they will not function properly (should only be tolerated in updates of objects which already have this invalid configuration). AllowUserNamespacesWithVolumeDevices bool + // Allow taint toleration comparison operators (Lt, Gt) + AllowTaintTolerationComparisonOperators bool // Allow hostNetwork pods to use user namespaces AllowUserNamespacesHostNetworkSupport bool } @@ -4648,7 +4669,7 @@ func ValidatePodSpec(spec *core.PodSpec, podMeta *metav1.ObjectMeta, fldPath *fi } if len(spec.Tolerations) > 0 { - allErrs = append(allErrs, ValidateTolerations(spec.Tolerations, fldPath.Child("tolerations"))...) + allErrs = append(allErrs, ValidateTolerations(spec.Tolerations, fldPath.Child("tolerations"), opts)...) } if len(spec.HostAliases) > 0 { @@ -5698,7 +5719,7 @@ func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel } // Allow only additions to tolerations updates. - allErrs = append(allErrs, validateOnlyAddedTolerations(newPod.Spec.Tolerations, oldPod.Spec.Tolerations, specPath.Child("tolerations"))...) + allErrs = append(allErrs, validateOnlyAddedTolerations(newPod.Spec.Tolerations, oldPod.Spec.Tolerations, specPath.Child("tolerations"), opts)...) // Allow only deletions to schedulingGates updates. allErrs = append(allErrs, validateOnlyDeletedSchedulingGates(newPod.Spec.SchedulingGates, oldPod.Spec.SchedulingGates, specPath.Child("schedulingGates"))...) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 0012a78c18a..91330415ec8 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -12246,6 +12246,18 @@ func TestValidatePod(t *testing.T) { podtest.SetTolerations(core.Toleration{Key: "node.kubernetes.io/not-ready", Operator: "Exists", Effect: "NoSchedule", TolerationSeconds: &[]int64{20}[0]}), ), }, + "numeric operator Lt requires feature gate (gate disabled)": { + expectedError: "Unsupported value: \"Lt\"", + spec: *podtest.MakePod("123", + podtest.SetTolerations(core.Toleration{Key: "node.kubernetes.io/sla", Operator: "Lt", Value: "950", Effect: "NoSchedule"}), + ), + }, + "numeric operator Gt requires feature gate (gate disabled)": { + expectedError: "Unsupported value: \"Gt\"", + spec: *podtest.MakePod("123", + podtest.SetTolerations(core.Toleration{Key: "node.kubernetes.io/sla", Operator: "Gt", Value: "950", Effect: "NoSchedule"}), + ), + }, "must be a valid pod seccomp profile": { expectedError: "must be a valid seccomp profile", spec: *podtest.MakePod("123", @@ -29554,6 +29566,188 @@ func TestValidateContainerStateTransition(t *testing.T) { } } +func TestNumericTolerationsWithFeatureGate(t *testing.T) { + testCases := []struct { + name string + toleration core.Toleration + featureGateOn bool + errorMsg string + }{ + { + name: "Gt operator with valid numeric value and feature gate enabled", + toleration: core.Toleration{ + Key: "node.kubernetes.io/sla", + Operator: core.TolerationOpGt, + Value: "950", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + }, + { + name: "Lt operator with valid numeric value and feature gate enabled", + toleration: core.Toleration{ + Key: "node.kubernetes.io/sla", + Operator: core.TolerationOpLt, + Value: "800", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + }, + { + name: "Gt operator with negative numeric value and feature gate enabled", + toleration: core.Toleration{ + Key: "test-key", + Operator: core.TolerationOpGt, + Value: "-100", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + }, + { + name: "Gt operator with non-numeric value and feature gate enabled", + toleration: core.Toleration{ + Key: "node.kubernetes.io/sla", + Operator: core.TolerationOpGt, + Value: "high", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + errorMsg: fmt.Sprintf("tolerations[0].value: Invalid value: %q: must be a valid decimal integer in canonical form", "high"), + }, + { + name: "Gt operator with leading zeros and feature gate enabled (invalid - strict validation)", + toleration: core.Toleration{ + Key: "node.kubernetes.io/sla", + Operator: core.TolerationOpGt, + Value: "0950", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + errorMsg: fmt.Sprintf("tolerations[0].value: Invalid value: %q: must be a valid decimal integer in canonical form", "0950"), + }, + { + name: "Gt operator with value '0' and feature gate enabled (valid)", + toleration: core.Toleration{ + Key: "test-key", + Operator: core.TolerationOpGt, + Value: "0", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + }, + { + name: "Gt operator with decimal value and feature gate enabled", + toleration: core.Toleration{ + Key: "node.kubernetes.io/sla", + Operator: core.TolerationOpGt, + Value: "95.5", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + errorMsg: fmt.Sprintf("tolerations[0].value: Invalid value: %q: must be a valid decimal integer in canonical form", "95.5"), + }, + { + name: "Gt operator with just minus sign and feature gate enabled", + toleration: core.Toleration{ + Key: "test-key", + Operator: core.TolerationOpGt, + Value: "-", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + errorMsg: fmt.Sprintf("tolerations[0].value: Invalid value: %q: must be a valid decimal integer in canonical form", "-"), + }, + { + name: "Gt operator with plus sign and feature gate enabled (invalid - strict validation)", + toleration: core.Toleration{ + Key: "test-key", + Operator: core.TolerationOpGt, + Value: "+100", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + errorMsg: fmt.Sprintf("tolerations[0].value: Invalid value: %q: must be a valid decimal integer in canonical form", "+100"), + }, + { + name: "Gt operator with space in value and feature gate enabled", + toleration: core.Toleration{ + Key: "test-key", + Operator: core.TolerationOpGt, + Value: "95 0", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + errorMsg: fmt.Sprintf("tolerations[0].value: Invalid value: %q: must be a valid decimal integer in canonical form", "95 0"), + }, + { + name: "Gt operator with empty value and feature gate enabled", + toleration: core.Toleration{ + Key: "test-key", + Operator: core.TolerationOpGt, + Value: "", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + errorMsg: fmt.Sprintf("tolerations[0].value: Invalid value: %q: must be non-empty", ""), + }, + { + name: "Lt operator with feature gate disabled", + toleration: core.Toleration{ + Key: "node.kubernetes.io/sla", + Operator: core.TolerationOpLt, + Value: "950", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: false, + errorMsg: "Unsupported value", + }, + { + name: "Gt operator with max int64 value and feature gate enabled", + toleration: core.Toleration{ + Key: "test-key", + Operator: core.TolerationOpGt, + Value: "9223372036854775807", + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + }, + { + name: "Gt operator with overflow value and feature gate enabled", + toleration: core.Toleration{ + Key: "test-key", + Operator: core.TolerationOpGt, + Value: "9223372036854775808", // max int64 + 1 + Effect: core.TaintEffectNoSchedule, + }, + featureGateOn: true, + errorMsg: fmt.Sprintf("tolerations[0].value: Invalid value: %q: strconv.ParseInt: parsing %q: value out of range", "9223372036854775808", "9223372036854775808"), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TaintTolerationComparisonOperators, tc.featureGateOn) + + opts := PodValidationOptions{ + AllowTaintTolerationComparisonOperators: tc.featureGateOn, + } + errs := ValidateTolerations([]core.Toleration{tc.toleration}, field.NewPath("tolerations"), opts) + + if tc.errorMsg != "" { + if len(errs) == 0 { + t.Errorf("Expected error but got none") + } else if !strings.Contains(errs.ToAggregate().Error(), tc.errorMsg) { + t.Errorf("Expected error message to contain %q, got %q", tc.errorMsg, errs.ToAggregate().Error()) + } + } else { + if len(errs) > 0 { + t.Errorf("Unexpected error(s): %v", errs) + } + } + }) + } +} + func TestAllRegistedNodeDeclaredFeatures(t *testing.T) { // Test that feature registry is valid. for _, feature := range ndf.AllFeatures { diff --git a/pkg/apis/node/validation/validation.go b/pkg/apis/node/validation/validation.go index d66e82b9382..58b80b55f1c 100644 --- a/pkg/apis/node/validation/validation.go +++ b/pkg/apis/node/validation/validation.go @@ -68,7 +68,7 @@ func validateScheduling(s *node.Scheduling, fldPath *field.Path) field.ErrorList } func validateTolerations(tolerations []core.Toleration, fldPath *field.Path) field.ErrorList { - allErrs := corevalidation.ValidateTolerations(tolerations, fldPath.Child("tolerations")) + allErrs := corevalidation.ValidateTolerations(tolerations, fldPath, corevalidation.PodValidationOptions{}) // Ensure uniquenes of tolerations. tolerationSet := map[core.Toleration]bool{} for i, t := range tolerations { diff --git a/pkg/controller/daemon/daemon_controller.go b/pkg/controller/daemon/daemon_controller.go index 3b1191ab2c0..91c9dcdc3b6 100644 --- a/pkg/controller/daemon/daemon_controller.go +++ b/pkg/controller/daemon/daemon_controller.go @@ -33,6 +33,7 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" + utilfeature "k8s.io/apiserver/pkg/util/feature" appsinformers "k8s.io/client-go/informers/apps/v1" coreinformers "k8s.io/client-go/informers/core/v1" clientset "k8s.io/client-go/kubernetes" @@ -51,6 +52,7 @@ import ( podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller/daemon/util" + "k8s.io/kubernetes/pkg/features" ) const ( @@ -794,7 +796,7 @@ func (dsc *DaemonSetsController) podsShouldBeOnNode( hash string, ) (nodesNeedingDaemonPods, podsToDelete []string) { - shouldRun, shouldContinueRunning := NodeShouldRunDaemonPod(node, ds) + shouldRun, shouldContinueRunning := NodeShouldRunDaemonPod(logger, node, ds) daemonPods, exists := nodeToDaemonPods[node.Name] switch { @@ -1149,7 +1151,7 @@ func (dsc *DaemonSetsController) updateDaemonSetStatus(ctx context.Context, ds * var desiredNumberScheduled, currentNumberScheduled, numberMisscheduled, numberReady, updatedNumberScheduled, numberAvailable int now := dsc.failedPodsBackoff.Clock.Now() for _, node := range nodeList { - shouldRun, _ := NodeShouldRunDaemonPod(node, ds) + shouldRun, _ := NodeShouldRunDaemonPod(logger, node, ds) scheduled := len(nodeToDaemonPods[node.Name]) > 0 if shouldRun { @@ -1288,7 +1290,7 @@ func (dsc *DaemonSetsController) syncDaemonSet(ctx context.Context, key string) // - shouldContinueRunning: // Returns true when a daemonset should continue running on a node if a daemonset pod is already // running on that node. -func NodeShouldRunDaemonPod(node *v1.Node, ds *apps.DaemonSet) (bool, bool) { +func NodeShouldRunDaemonPod(logger klog.Logger, node *v1.Node, ds *apps.DaemonSet) (bool, bool) { pod := NewPod(ds, node.Name) // If the daemon set specifies a node name, check that it matches with node.Name. @@ -1297,16 +1299,16 @@ func NodeShouldRunDaemonPod(node *v1.Node, ds *apps.DaemonSet) (bool, bool) { } taints := node.Spec.Taints - fitsNodeName, fitsNodeAffinity, fitsTaints := predicates(pod, node, taints) + fitsNodeName, fitsNodeAffinity, fitsTaints := predicates(logger, pod, node, taints) if !fitsNodeName || !fitsNodeAffinity { return false, false } if !fitsTaints { // Scheduled daemon pods should continue running if they tolerate NoExecute taint. - _, hasUntoleratedTaint := v1helper.FindMatchingUntoleratedTaint(taints, pod.Spec.Tolerations, func(t *v1.Taint) bool { + _, hasUntoleratedTaint := v1helper.FindMatchingUntoleratedTaint(logger, taints, pod.Spec.Tolerations, func(t *v1.Taint) bool { return t.Effect == v1.TaintEffectNoExecute - }) + }, utilfeature.DefaultFeatureGate.Enabled(features.TaintTolerationComparisonOperators)) return false, !hasUntoleratedTaint } @@ -1314,13 +1316,13 @@ func NodeShouldRunDaemonPod(node *v1.Node, ds *apps.DaemonSet) (bool, bool) { } // predicates checks if a DaemonSet's pod can run on a node. -func predicates(pod *v1.Pod, node *v1.Node, taints []v1.Taint) (fitsNodeName, fitsNodeAffinity, fitsTaints bool) { +func predicates(logger klog.Logger, pod *v1.Pod, node *v1.Node, taints []v1.Taint) (fitsNodeName, fitsNodeAffinity, fitsTaints bool) { fitsNodeName = len(pod.Spec.NodeName) == 0 || pod.Spec.NodeName == node.Name // Ignore parsing errors for backwards compatibility. fitsNodeAffinity, _ = nodeaffinity.GetRequiredNodeAffinity(pod).Match(node) - _, hasUntoleratedTaint := v1helper.FindMatchingUntoleratedTaint(taints, pod.Spec.Tolerations, func(t *v1.Taint) bool { + _, hasUntoleratedTaint := v1helper.FindMatchingUntoleratedTaint(logger, taints, pod.Spec.Tolerations, func(t *v1.Taint) bool { return t.Effect == v1.TaintEffectNoExecute || t.Effect == v1.TaintEffectNoSchedule - }) + }, utilfeature.DefaultFeatureGate.Enabled(features.TaintTolerationComparisonOperators)) fitsTaints = !hasUntoleratedTaint return } @@ -1444,7 +1446,7 @@ func (dsc *DaemonSetsController) syncNodeUpdate(ctx context.Context, nodeName st } for _, ds := range dsList { - shouldRun, shouldContinueRunning := NodeShouldRunDaemonPod(node, ds) + shouldRun, shouldContinueRunning := NodeShouldRunDaemonPod(logger, node, ds) dsKey, err := controller.KeyFunc(ds) if err != nil { diff --git a/pkg/controller/daemon/daemon_controller_test.go b/pkg/controller/daemon/daemon_controller_test.go index 1f26d2a416b..221ee3b9acb 100644 --- a/pkg/controller/daemon/daemon_controller_test.go +++ b/pkg/controller/daemon/daemon_controller_test.go @@ -2376,7 +2376,7 @@ func TestNodeShouldRunDaemonPod(t *testing.T) { node.Status.Conditions = append(node.Status.Conditions, c.nodeCondition...) node.Status.Allocatable = allocatableResources("100M", "1") node.Spec.Unschedulable = c.nodeUnschedulable - _, ctx := ktesting.NewTestContext(t) + logger, ctx := ktesting.NewTestContext(t) manager, _, _, err := newTestController(ctx) if err != nil { t.Fatalf("error creating DaemonSets controller: %v", err) @@ -2387,7 +2387,7 @@ func TestNodeShouldRunDaemonPod(t *testing.T) { manager.podStore.Add(p) } c.ds.Spec.UpdateStrategy = *strategy - shouldRun, shouldContinueRunning := NodeShouldRunDaemonPod(node, c.ds) + shouldRun, shouldContinueRunning := NodeShouldRunDaemonPod(logger, node, c.ds) if shouldRun != c.shouldRun { t.Errorf("[%v] strategy: %v, predicateName: %v expected shouldRun: %v, got: %v", i, c.ds.Spec.UpdateStrategy.Type, c.predicateName, c.shouldRun, shouldRun) diff --git a/pkg/controller/daemon/update.go b/pkg/controller/daemon/update.go index 6add594a817..8aa6249d919 100644 --- a/pkg/controller/daemon/update.go +++ b/pkg/controller/daemon/update.go @@ -179,7 +179,7 @@ func (dsc *DaemonSetsController) rollingUpdate(ctx context.Context, ds *apps.Dae if err != nil { return fmt.Errorf("couldn't get node for nodeName %q: %v", nodeName, err) } - if shouldRun, _ := NodeShouldRunDaemonPod(node, ds); !shouldRun { + if shouldRun, _ := NodeShouldRunDaemonPod(logger, node, ds); !shouldRun { logger.V(5).Info("DaemonSet pod on node is not available and does not match scheduling constraints, remove old pod", "daemonset", klog.KObj(ds), "node", nodeName, "oldPod", klog.KObj(oldPod)) oldPodsToDelete = append(oldPodsToDelete, oldPod.Name) continue @@ -196,7 +196,7 @@ func (dsc *DaemonSetsController) rollingUpdate(ctx context.Context, ds *apps.Dae if err != nil { return fmt.Errorf("couldn't get node for nodeName %q: %v", nodeName, err) } - if shouldRun, _ := NodeShouldRunDaemonPod(node, ds); !shouldRun { + if shouldRun, _ := NodeShouldRunDaemonPod(logger, node, ds); !shouldRun { shouldNotRunPodsToDelete = append(shouldNotRunPodsToDelete, oldPod.Name) continue } @@ -586,7 +586,7 @@ func (dsc *DaemonSetsController) updatedDesiredNodeCounts(ctx context.Context, d logger := klog.FromContext(ctx) for i := range nodeList { node := nodeList[i] - wantToRun, _ := NodeShouldRunDaemonPod(node, ds) + wantToRun, _ := NodeShouldRunDaemonPod(logger, node, ds) if !wantToRun { continue } diff --git a/pkg/controller/tainteviction/taint_eviction.go b/pkg/controller/tainteviction/taint_eviction.go index f2f098900e9..2f0a291cacc 100644 --- a/pkg/controller/tainteviction/taint_eviction.go +++ b/pkg/controller/tainteviction/taint_eviction.go @@ -460,7 +460,7 @@ func (tc *Controller) processPodOnNode( if len(taints) == 0 { tc.cancelWorkWithEvent(logger, podNamespacedName) } - allTolerated, usedTolerations := v1helper.GetMatchingTolerations(taints, tolerations) + allTolerated, usedTolerations := v1helper.GetMatchingTolerations(logger, taints, tolerations) if !allTolerated { logger.V(2).Info("Not all taints are tolerated after update for pod on node", "pod", podNamespacedName.String(), "node", klog.KRef("", nodeName)) // We're canceling scheduled work (if any), as we're going to delete the Pod right away. diff --git a/pkg/controller/tainteviction/taint_eviction_test.go b/pkg/controller/tainteviction/taint_eviction_test.go index f10c5a630f7..38773a156ca 100644 --- a/pkg/controller/tainteviction/taint_eviction_test.go +++ b/pkg/controller/tainteviction/taint_eviction_test.go @@ -211,6 +211,8 @@ func TestCreatePod(t *testing.T) { controller.PodUpdated(nil, item.pod) verifyPodActions(t, item.description, fakeClientset, item.expectPatch, item.expectDelete) + + cancel() }) } } @@ -329,6 +331,7 @@ func TestUpdatePod(t *testing.T) { controller.PodUpdated(item.prevPod, item.newPod) verifyPodActions(t, item.description, fakeClientset, item.expectPatch, item.expectDelete) + cancel() }) } } @@ -391,6 +394,8 @@ func TestCreateNode(t *testing.T) { controller.NodeUpdated(nil, item.node) verifyPodActions(t, item.description, fakeClientset, item.expectPatch, item.expectDelete) + + cancel() }) } } diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index b7c2435c7fc..6d7e0adc190 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -994,6 +994,12 @@ const ( // if the system supports the systemd watchdog feature and has it configured properly. SystemdWatchdog = featuregate.Feature("SystemdWatchdog") + // owner: @helayoty + // kep: https://kep.k8s.io/5471 + // + // Enables numeric comparison operators (Lt, Gt) for tolerations to match taints with threshold-based values. + TaintTolerationComparisonOperators featuregate.Feature = "TaintTolerationComparisonOperators" + // owner: @robscott // kep: https://kep.k8s.io/2433 // @@ -1821,6 +1827,10 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate {Version: version.MustParse("1.35"), Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remov in 1.37 }, + TaintTolerationComparisonOperators: { + {Version: version.MustParse("1.35"), Default: false, PreRelease: featuregate.Alpha}, + }, + TopologyAwareHints: { {Version: version.MustParse("1.21"), Default: false, PreRelease: featuregate.Alpha}, {Version: version.MustParse("1.23"), Default: false, PreRelease: featuregate.Beta}, @@ -2413,6 +2423,8 @@ var defaultKubernetesFeatureGateDependencies = map[featuregate.Feature][]feature SystemdWatchdog: {}, + TaintTolerationComparisonOperators: {}, + TopologyAwareHints: {}, TopologyManagerPolicyAlphaOptions: {}, diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index 5a40bf4fd5f..5f81ede91b1 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -33674,10 +33674,10 @@ func schema_k8sio_api_core_v1_Toleration(ref common.ReferenceCallback) common.Op }, "operator": { SchemaProps: spec.SchemaProps{ - Description: "Operator represents a key's relationship to the value. Valid operators are Exists and Equal. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category.\n\nPossible enum values:\n - `\"Equal\"`\n - `\"Exists\"`", + Description: "Operator represents a key's relationship to the value. Valid operators are Exists, Equal, Lt, and Gt. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category. Lt and Gt perform numeric comparisons (requires feature gate TaintTolerationComparisonOperators).\n\nPossible enum values:\n - `\"Equal\"`\n - `\"Exists\"`\n - `\"Gt\"`\n - `\"Lt\"`", Type: []string{"string"}, Format: "", - Enum: []interface{}{"Equal", "Exists"}, + Enum: []interface{}{"Equal", "Exists", "Gt", "Lt"}, }, }, "value": { diff --git a/pkg/kubelet/eviction/eviction_manager.go b/pkg/kubelet/eviction/eviction_manager.go index 82d974e4bef..556c0d2a7c4 100644 --- a/pkg/kubelet/eviction/eviction_manager.go +++ b/pkg/kubelet/eviction/eviction_manager.go @@ -146,6 +146,10 @@ func NewManager( func (m *managerImpl) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitResult { m.RLock() defer m.RUnlock() + + ctx := context.Background() + logger := klog.FromContext(ctx) + if len(m.nodeConditions) == 0 { return lifecycle.PodAdmitResult{Admit: true} } @@ -165,10 +169,10 @@ func (m *managerImpl) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAd // When node has memory pressure, check BestEffort Pod's toleration: // admit it if tolerates memory pressure taint, fail for other tolerations, e.g. DiskPressure. - if corev1helpers.TolerationsTolerateTaint(attrs.Pod.Spec.Tolerations, &v1.Taint{ + if corev1helpers.TolerationsTolerateTaint(logger, attrs.Pod.Spec.Tolerations, &v1.Taint{ Key: v1.TaintNodeMemoryPressure, Effect: v1.TaintEffectNoSchedule, - }) { + }, utilfeature.DefaultFeatureGate.Enabled(features.TaintTolerationComparisonOperators)) { return lifecycle.PodAdmitResult{Admit: true} } } diff --git a/pkg/kubelet/lifecycle/predicate.go b/pkg/kubelet/lifecycle/predicate.go index 9df3081c4bd..62c9c300080 100644 --- a/pkg/kubelet/lifecycle/predicate.go +++ b/pkg/kubelet/lifecycle/predicate.go @@ -250,7 +250,9 @@ func (w *predicateAdmitHandler) Admit(attrs *PodAdmitAttributes) PodAdmitResult // generalFilter checks a group of filterings that the kubelet cares about. func (w *predicateAdmitHandler) generalFilter(ctx context.Context, pod *v1.Pod, nodeInfo *schedulerframework.NodeInfo) []PredicateFailureReason { - reasons := generalFilter(pod, nodeInfo) + logger := klog.FromContext(ctx) + + reasons := generalFilter(logger, pod, nodeInfo) for _, r := range reasons { if r.GetReason() != nodeaffinity.ErrReasonPod { return reasons @@ -259,14 +261,14 @@ func (w *predicateAdmitHandler) generalFilter(ctx context.Context, pod *v1.Pod, if len(reasons) > 0 { // If the only reason for failure is the node affinity labels, fetch the node synchronously // and try again. - logger := klog.FromContext(ctx) + node, err := w.getNodeAnyWayFunc(ctx, false) if err != nil { logger.Error(err, "Failed to synchronously fetch node info") return reasons } nodeInfo.SetNode(node) - reasons = generalFilter(pod, nodeInfo) + reasons = generalFilter(logger, pod, nodeInfo) } return reasons @@ -412,7 +414,7 @@ func (e *PredicateFailureError) GetReason() string { } // generalFilter checks a group of filterings that the kubelet cares about. -func generalFilter(pod *v1.Pod, nodeInfo *schedulerframework.NodeInfo) []PredicateFailureReason { +func generalFilter(logger klog.Logger, pod *v1.Pod, nodeInfo *schedulerframework.NodeInfo) []PredicateFailureReason { admissionResults := scheduler.AdmissionCheck(pod, nodeInfo, true) var reasons []PredicateFailureReason for _, r := range admissionResults { @@ -430,10 +432,10 @@ func generalFilter(pod *v1.Pod, nodeInfo *schedulerframework.NodeInfo) []Predica // Check taint/toleration except for static pods if !types.IsStaticPod(pod) { - _, isUntolerated := corev1.FindMatchingUntoleratedTaint(nodeInfo.Node().Spec.Taints, pod.Spec.Tolerations, func(t *v1.Taint) bool { + _, isUntolerated := corev1.FindMatchingUntoleratedTaint(logger, nodeInfo.Node().Spec.Taints, pod.Spec.Tolerations, func(t *v1.Taint) bool { // Kubelet is only interested in the NoExecute taint. return t.Effect == v1.TaintEffectNoExecute - }) + }, utilfeature.DefaultFeatureGate.Enabled(features.TaintTolerationComparisonOperators)) if isUntolerated { reasons = append(reasons, &PredicateFailureError{tainttoleration.Name, tainttoleration.ErrReasonNotMatch}) } diff --git a/pkg/scheduler/eventhandlers.go b/pkg/scheduler/eventhandlers.go index babdbb3ff05..2febf790f53 100644 --- a/pkg/scheduler/eventhandlers.go +++ b/pkg/scheduler/eventhandlers.go @@ -63,7 +63,7 @@ func (sched *Scheduler) addNodeToCache(obj interface{}) { logger.V(3).Info("Add event for node", "node", klog.KObj(node)) nodeInfo := sched.Cache.AddNode(logger, node) - sched.SchedulingQueue.MoveAllToActiveOrBackoffQueue(logger, evt, nil, node, preCheckForNode(nodeInfo)) + sched.SchedulingQueue.MoveAllToActiveOrBackoffQueue(logger, evt, nil, node, preCheckForNode(logger, nodeInfo)) } func (sched *Scheduler) updateNodeInCache(oldObj, newObj interface{}) { @@ -90,7 +90,7 @@ func (sched *Scheduler) updateNodeInCache(oldObj, newObj interface{}) { // Only requeue unschedulable pods if the node became more schedulable. for _, evt := range events { startMoving := time.Now() - sched.SchedulingQueue.MoveAllToActiveOrBackoffQueue(logger, evt, oldNode, newNode, preCheckForNode(nodeInfo)) + sched.SchedulingQueue.MoveAllToActiveOrBackoffQueue(logger, evt, oldNode, newNode, preCheckForNode(logger, nodeInfo)) movingDuration := metrics.SinceInSeconds(startMoving) metrics.EventHandlingLatency.WithLabelValues(evt.Label()).Observe(updatingDuration + movingDuration) @@ -724,7 +724,7 @@ func addAllEventHandlers( return nil } -func preCheckForNode(nodeInfo *framework.NodeInfo) queue.PreEnqueueCheck { +func preCheckForNode(logger klog.Logger, nodeInfo *framework.NodeInfo) queue.PreEnqueueCheck { if utilfeature.DefaultFeatureGate.Enabled(features.SchedulerQueueingHints) { // QHint is initially created from the motivation of replacing this preCheck. // It assumes that the scheduler only has in-tree plugins, which is problematic for our extensibility. @@ -740,7 +740,9 @@ func preCheckForNode(nodeInfo *framework.NodeInfo) queue.PreEnqueueCheck { if len(admissionResults) != 0 { return false } - _, isUntolerated := corev1helpers.FindMatchingUntoleratedTaint(nodeInfo.Node().Spec.Taints, pod.Spec.Tolerations, helper.DoNotScheduleTaintsFilterFunc()) + _, isUntolerated := corev1helpers.FindMatchingUntoleratedTaint(logger, nodeInfo.Node().Spec.Taints, pod.Spec.Tolerations, + helper.DoNotScheduleTaintsFilterFunc(), + utilfeature.DefaultFeatureGate.Enabled(features.TaintTolerationComparisonOperators)) return !isUntolerated } } diff --git a/pkg/scheduler/eventhandlers_test.go b/pkg/scheduler/eventhandlers_test.go index f4aed3abfbb..2684074b6ec 100644 --- a/pkg/scheduler/eventhandlers_test.go +++ b/pkg/scheduler/eventhandlers_test.go @@ -284,6 +284,8 @@ func withPodName(pod *v1.Pod, name string) *v1.Pod { } func TestPreCheckForNode(t *testing.T) { + logger, _ := ktesting.NewTestContext(t) + cpu4 := map[v1.ResourceName]string{v1.ResourceCPU: "4"} cpu8 := map[v1.ResourceName]string{v1.ResourceCPU: "8"} cpu16 := map[v1.ResourceName]string{v1.ResourceCPU: "16"} @@ -421,7 +423,7 @@ func TestPreCheckForNode(t *testing.T) { nodeInfo := framework.NewNodeInfo(tt.existingPods...) nodeInfo.SetNode(tt.nodeFn()) - preCheckFn := preCheckForNode(nodeInfo) + preCheckFn := preCheckForNode(logger, nodeInfo) got := make([]bool, 0, len(tt.pods)) for _, pod := range tt.pods { diff --git a/pkg/scheduler/framework/plugins/feature/feature.go b/pkg/scheduler/framework/plugins/feature/feature.go index 11618359c03..b0871976a70 100644 --- a/pkg/scheduler/framework/plugins/feature/feature.go +++ b/pkg/scheduler/framework/plugins/feature/feature.go @@ -48,6 +48,7 @@ type Features struct { EnableStorageCapacityScoring bool EnableNodeDeclaredFeatures bool EnableGangScheduling bool + EnableTaintTolerationComparisonOperators bool } // NewSchedulerFeaturesFromGates copies the current state of the feature gates into the struct. @@ -76,5 +77,6 @@ func NewSchedulerFeaturesFromGates(featureGate featuregate.FeatureGate) Features EnableStorageCapacityScoring: featureGate.Enabled(features.StorageCapacityScoring), EnableNodeDeclaredFeatures: featureGate.Enabled(features.NodeDeclaredFeatures), EnableGangScheduling: featureGate.Enabled(features.GangScheduling), + EnableTaintTolerationComparisonOperators: featureGate.Enabled(features.TaintTolerationComparisonOperators), } } diff --git a/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go b/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go index faa6c01c497..741d6cfeb58 100644 --- a/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go +++ b/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go @@ -21,9 +21,11 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" v1helper "k8s.io/component-helpers/scheduling/corev1" "k8s.io/klog/v2" fwk "k8s.io/kube-scheduler/framework" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/names" "k8s.io/kubernetes/pkg/scheduler/util" @@ -84,10 +86,10 @@ func (pl *NodeUnschedulable) isSchedulableAfterPodTolerationChange(logger klog.L // - Taint can be added, but can't be modified nor removed. // - If the Pod already has the toleration, it shouldn't have rejected by this plugin in the first place. // Meaning, here this Pod has been rejected by this plugin, and hence it shouldn't have the toleration yet. - if v1helper.TolerationsTolerateTaint(modifiedPod.Spec.Tolerations, &v1.Taint{ + if v1helper.TolerationsTolerateTaint(logger, modifiedPod.Spec.Tolerations, &v1.Taint{ Key: v1.TaintNodeUnschedulable, Effect: v1.TaintEffectNoSchedule, - }) { + }, utilfeature.DefaultFeatureGate.Enabled(features.TaintTolerationComparisonOperators)) { // This update makes the pod tolerate the unschedulable taint. logger.V(5).Info("a new toleration is added for the unschedulable Pod, and it may make it schedulable", "pod", klog.KObj(modifiedPod)) return fwk.Queue, nil @@ -136,11 +138,12 @@ func (pl *NodeUnschedulable) Filter(ctx context.Context, _ fwk.CycleState, pod * return nil } + logger := klog.FromContext(ctx) // If pod tolerate unschedulable taint, it's also tolerate `node.Spec.Unschedulable`. - podToleratesUnschedulable := v1helper.TolerationsTolerateTaint(pod.Spec.Tolerations, &v1.Taint{ + podToleratesUnschedulable := v1helper.TolerationsTolerateTaint(logger, pod.Spec.Tolerations, &v1.Taint{ Key: v1.TaintNodeUnschedulable, Effect: v1.TaintEffectNoSchedule, - }) + }, utilfeature.DefaultFeatureGate.Enabled(features.TaintTolerationComparisonOperators)) if !podToleratesUnschedulable { return fwk.NewStatus(fwk.UnschedulableAndUnresolvable, ErrReasonUnschedulable) } diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/common.go b/pkg/scheduler/framework/plugins/podtopologyspread/common.go index 1a302dd3ab1..94e0f4d0f9b 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/common.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/common.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/labels" v1helper "k8s.io/component-helpers/scheduling/corev1" "k8s.io/component-helpers/scheduling/corev1/nodeaffinity" + "k8s.io/klog/v2" fwk "k8s.io/kube-scheduler/framework" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/helper" "k8s.io/utils/ptr" @@ -39,7 +40,7 @@ type topologySpreadConstraint struct { NodeTaintsPolicy v1.NodeInclusionPolicy } -func (tsc *topologySpreadConstraint) matchNodeInclusionPolicies(pod *v1.Pod, node *v1.Node, require nodeaffinity.RequiredNodeAffinity) bool { +func (tsc *topologySpreadConstraint) matchNodeInclusionPolicies(logger klog.Logger, pod *v1.Pod, node *v1.Node, require nodeaffinity.RequiredNodeAffinity, enableComparisonOperators bool) bool { if tsc.NodeAffinityPolicy == v1.NodeInclusionPolicyHonor { // We ignore parsing errors here for backwards compatibility. if match, _ := require.Match(node); !match { @@ -48,7 +49,7 @@ func (tsc *topologySpreadConstraint) matchNodeInclusionPolicies(pod *v1.Pod, nod } if tsc.NodeTaintsPolicy == v1.NodeInclusionPolicyHonor { - if _, untolerated := v1helper.FindMatchingUntoleratedTaint(node.Spec.Taints, pod.Spec.Tolerations, helper.DoNotScheduleTaintsFilterFunc()); untolerated { + if _, untolerated := v1helper.FindMatchingUntoleratedTaint(logger, node.Spec.Taints, pod.Spec.Tolerations, helper.DoNotScheduleTaintsFilterFunc(), enableComparisonOperators); untolerated { return false } } diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/filtering.go b/pkg/scheduler/framework/plugins/podtopologyspread/filtering.go index d27211b617d..cbc1baf397c 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/filtering.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/filtering.go @@ -155,27 +155,30 @@ func (pl *PodTopologySpread) PreFilterExtensions() fwk.PreFilterExtensions { // AddPod from pre-computed data in cycleState. func (pl *PodTopologySpread) AddPod(ctx context.Context, cycleState fwk.CycleState, podToSchedule *v1.Pod, podInfoToAdd fwk.PodInfo, nodeInfo fwk.NodeInfo) *fwk.Status { + logger := klog.FromContext(ctx) + s, err := getPreFilterState(cycleState) if err != nil { return fwk.AsStatus(err) } - pl.updateWithPod(s, podInfoToAdd.GetPod(), podToSchedule, nodeInfo.Node(), 1) + pl.updateWithPod(logger, s, podInfoToAdd.GetPod(), podToSchedule, nodeInfo.Node(), 1) return nil } // RemovePod from pre-computed data in cycleState. func (pl *PodTopologySpread) RemovePod(ctx context.Context, cycleState fwk.CycleState, podToSchedule *v1.Pod, podInfoToRemove fwk.PodInfo, nodeInfo fwk.NodeInfo) *fwk.Status { + logger := klog.FromContext(ctx) s, err := getPreFilterState(cycleState) if err != nil { return fwk.AsStatus(err) } - pl.updateWithPod(s, podInfoToRemove.GetPod(), podToSchedule, nodeInfo.Node(), -1) + pl.updateWithPod(logger, s, podInfoToRemove.GetPod(), podToSchedule, nodeInfo.Node(), -1) return nil } -func (pl *PodTopologySpread) updateWithPod(s *preFilterState, updatedPod, preemptorPod *v1.Pod, node *v1.Node, delta int) { +func (pl *PodTopologySpread) updateWithPod(logger klog.Logger, s *preFilterState, updatedPod, preemptorPod *v1.Pod, node *v1.Node, delta int) { if s == nil || updatedPod.Namespace != preemptorPod.Namespace || node == nil { return } @@ -199,7 +202,7 @@ func (pl *PodTopologySpread) updateWithPod(s *preFilterState, updatedPod, preemp } if pl.enableNodeInclusionPolicyInPodTopologySpread && - !constraint.matchNodeInclusionPolicies(preemptorPod, node, requiredSchedulingTerm) { + !constraint.matchNodeInclusionPolicies(logger, preemptorPod, node, requiredSchedulingTerm, pl.enableTaintTolerationComparisonOperators) { continue } @@ -240,6 +243,7 @@ func (pl *PodTopologySpread) calPreFilterState(ctx context.Context, pod *v1.Pod, return &preFilterState{}, nil } + logger := klog.FromContext(ctx) s := preFilterState{ Constraints: constraints, CriticalPaths: make([]*criticalPaths, len(constraints)), @@ -271,7 +275,7 @@ func (pl *PodTopologySpread) calPreFilterState(ctx context.Context, pod *v1.Pod, tpCounts := make([]topologyCount, 0, len(constraints)) for i, c := range constraints { if pl.enableNodeInclusionPolicyInPodTopologySpread && - !c.matchNodeInclusionPolicies(pod, node, requiredNodeAffinity) { + !c.matchNodeInclusionPolicies(logger, pod, node, requiredNodeAffinity, pl.enableTaintTolerationComparisonOperators) { continue } diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go index b20f017530c..5f1cdf70d32 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go @@ -69,6 +69,7 @@ type PodTopologySpread struct { enableNodeInclusionPolicyInPodTopologySpread bool enableMatchLabelKeysInPodTopologySpread bool enableSchedulingQueueHint bool + enableTaintTolerationComparisonOperators bool } var _ fwk.PreFilterPlugin = &PodTopologySpread{} @@ -104,6 +105,7 @@ func New(_ context.Context, plArgs runtime.Object, h fwk.Handle, fts feature.Fea enableNodeInclusionPolicyInPodTopologySpread: fts.EnableNodeInclusionPolicyInPodTopologySpread, enableMatchLabelKeysInPodTopologySpread: fts.EnableMatchLabelKeysInPodTopologySpread, enableSchedulingQueueHint: fts.EnableSchedulingQueueHint, + enableTaintTolerationComparisonOperators: fts.EnableTaintTolerationComparisonOperators, } if args.DefaultingType == config.SystemDefaulting { pl.defaultConstraints = systemDefaultConstraints diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/scoring.go b/pkg/scheduler/framework/plugins/podtopologyspread/scoring.go index 020e94ee12a..5f2631ace86 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/scoring.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/scoring.go @@ -19,6 +19,7 @@ package podtopologyspread import ( "context" "fmt" + "k8s.io/klog/v2" "math" "sync/atomic" @@ -120,6 +121,7 @@ func (pl *PodTopologySpread) PreScore( pod *v1.Pod, filteredNodes []fwk.NodeInfo, ) *fwk.Status { + allNodes, err := pl.sharedLister.NodeInfos().List() if err != nil { return fwk.AsStatus(fmt.Errorf("getting all nodes: %w", err)) @@ -130,6 +132,7 @@ func (pl *PodTopologySpread) PreScore( return fwk.NewStatus(fwk.Skip) } + logger := klog.FromContext(ctx) state := &preScoreState{ IgnoredNodes: sets.New[string](), } @@ -167,7 +170,8 @@ func (pl *PodTopologySpread) PreScore( for i, c := range state.Constraints { if pl.enableNodeInclusionPolicyInPodTopologySpread && - !c.matchNodeInclusionPolicies(pod, node, requiredNodeAffinity) { + !c.matchNodeInclusionPolicies(logger, pod, node, requiredNodeAffinity, + pl.enableTaintTolerationComparisonOperators) { continue } diff --git a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go index e34311188b2..3d7a3e57195 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go +++ b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go @@ -33,8 +33,9 @@ import ( // TaintToleration is a plugin that checks if a pod tolerates a node's taints. type TaintToleration struct { - handle fwk.Handle - enableSchedulingQueueHint bool + handle fwk.Handle + enableSchedulingQueueHint bool + enableTaintTolerationComparisonOperators bool } var _ fwk.FilterPlugin = &TaintToleration{} @@ -92,10 +93,10 @@ func (pl *TaintToleration) isSchedulableAfterNodeChange(logger klog.Logger, pod wasUntolerated := true if originalNode != nil { - _, wasUntolerated = v1helper.FindMatchingUntoleratedTaint(originalNode.Spec.Taints, pod.Spec.Tolerations, helper.DoNotScheduleTaintsFilterFunc()) + _, wasUntolerated = v1helper.FindMatchingUntoleratedTaint(logger, originalNode.Spec.Taints, pod.Spec.Tolerations, helper.DoNotScheduleTaintsFilterFunc(), pl.enableTaintTolerationComparisonOperators) } - _, isUntolerated := v1helper.FindMatchingUntoleratedTaint(modifiedNode.Spec.Taints, pod.Spec.Tolerations, helper.DoNotScheduleTaintsFilterFunc()) + _, isUntolerated := v1helper.FindMatchingUntoleratedTaint(logger, modifiedNode.Spec.Taints, pod.Spec.Tolerations, helper.DoNotScheduleTaintsFilterFunc(), pl.enableTaintTolerationComparisonOperators) if wasUntolerated && !isUntolerated { logger.V(5).Info("node was created or updated, and this may make the Pod rejected by TaintToleration plugin in the previous scheduling cycle schedulable", "pod", klog.KObj(pod), "node", klog.KObj(modifiedNode)) @@ -108,9 +109,12 @@ func (pl *TaintToleration) isSchedulableAfterNodeChange(logger klog.Logger, pod // Filter invoked at the filter extension point. func (pl *TaintToleration) Filter(ctx context.Context, state fwk.CycleState, pod *v1.Pod, nodeInfo fwk.NodeInfo) *fwk.Status { + logger := klog.FromContext(ctx) node := nodeInfo.Node() - taint, isUntolerated := v1helper.FindMatchingUntoleratedTaint(node.Spec.Taints, pod.Spec.Tolerations, helper.DoNotScheduleTaintsFilterFunc()) + taint, isUntolerated := v1helper.FindMatchingUntoleratedTaint(logger, node.Spec.Taints, pod.Spec.Tolerations, + helper.DoNotScheduleTaintsFilterFunc(), + pl.enableTaintTolerationComparisonOperators) if !isUntolerated { return nil } @@ -165,14 +169,14 @@ func getPreScoreState(cycleState fwk.CycleState) (*preScoreState, error) { } // CountIntolerableTaintsPreferNoSchedule gives the count of intolerable taints of a pod with effect PreferNoSchedule -func countIntolerableTaintsPreferNoSchedule(taints []v1.Taint, tolerations []v1.Toleration) (intolerableTaints int) { +func (pl *TaintToleration) countIntolerableTaintsPreferNoSchedule(logger klog.Logger, taints []v1.Taint, tolerations []v1.Toleration) (intolerableTaints int) { for _, taint := range taints { // check only on taints that have effect PreferNoSchedule if taint.Effect != v1.TaintEffectPreferNoSchedule { continue } - if !v1helper.TolerationsTolerateTaint(tolerations, &taint) { + if !v1helper.TolerationsTolerateTaint(logger, tolerations, &taint, pl.enableTaintTolerationComparisonOperators) { intolerableTaints++ } } @@ -181,6 +185,8 @@ func countIntolerableTaintsPreferNoSchedule(taints []v1.Taint, tolerations []v1. // Score invoked at the Score extension point. func (pl *TaintToleration) Score(ctx context.Context, state fwk.CycleState, pod *v1.Pod, nodeInfo fwk.NodeInfo) (int64, *fwk.Status) { + logger := klog.FromContext(ctx) + node := nodeInfo.Node() s, err := getPreScoreState(state) @@ -188,7 +194,7 @@ func (pl *TaintToleration) Score(ctx context.Context, state fwk.CycleState, pod return 0, fwk.AsStatus(err) } - score := int64(countIntolerableTaintsPreferNoSchedule(node.Spec.Taints, s.tolerationsPreferNoSchedule)) + score := int64(pl.countIntolerableTaintsPreferNoSchedule(logger, node.Spec.Taints, s.tolerationsPreferNoSchedule)) return score, nil } @@ -205,8 +211,9 @@ func (pl *TaintToleration) ScoreExtensions() fwk.ScoreExtensions { // New initializes a new plugin and returns it. func New(_ context.Context, _ runtime.Object, h fwk.Handle, fts feature.Features) (fwk.Plugin, error) { return &TaintToleration{ - handle: h, - enableSchedulingQueueHint: fts.EnableSchedulingQueueHint, + handle: h, + enableSchedulingQueueHint: fts.EnableSchedulingQueueHint, + enableTaintTolerationComparisonOperators: fts.EnableTaintTolerationComparisonOperators, }, nil } diff --git a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go index 3478ae79e43..5c44e3dbe0a 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go +++ b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go @@ -56,10 +56,11 @@ func podWithTolerations(podName string, tolerations []v1.Toleration) *v1.Pod { func TestTaintTolerationScore(t *testing.T) { tests := []struct { - name string - pod *v1.Pod - nodes []*v1.Node - expectedList fwk.NodeScoreList + name string + pod *v1.Pod + nodes []*v1.Node + expectedList fwk.NodeScoreList + enableTaintTolerationComparisonOps bool }{ // basic test case { @@ -229,6 +230,58 @@ func TestTaintTolerationScore(t *testing.T) { {Name: "nodeB", Score: 0}, }, }, + { + name: "Numeric Gt operator: pod with Gt toleration prefers nodes with matching SLA taint", + pod: podWithTolerations("pod1", []v1.Toleration{{ + Key: "node.kubernetes.io/sla", + Operator: "Gt", + Value: "950", + Effect: v1.TaintEffectPreferNoSchedule, + }}), + nodes: []*v1.Node{ + nodeWithTaints("nodeA", []v1.Taint{{ + Key: "node.kubernetes.io/sla", + Value: "800", + Effect: v1.TaintEffectPreferNoSchedule, + }}), + nodeWithTaints("nodeB", []v1.Taint{{ + Key: "node.kubernetes.io/sla", + Value: "999", + Effect: v1.TaintEffectPreferNoSchedule, + }}), + }, + expectedList: []fwk.NodeScore{ + {Name: "nodeA", Score: 0}, + {Name: "nodeB", Score: fwk.MaxNodeScore}, + }, + enableTaintTolerationComparisonOps: true, + }, + { + name: "Numeric Lt operator: pod with Lt toleration prefers nodes with matching SLA taint", + pod: podWithTolerations("pod1", []v1.Toleration{{ + Key: "node.kubernetes.io/sla", + Operator: "Lt", + Value: "800", + Effect: v1.TaintEffectPreferNoSchedule, + }}), + nodes: []*v1.Node{ + nodeWithTaints("nodeA", []v1.Taint{{ + Key: "node.kubernetes.io/sla", + Value: "950", + Effect: v1.TaintEffectPreferNoSchedule, + }}), + nodeWithTaints("nodeB", []v1.Taint{{ + Key: "node.kubernetes.io/sla", + Value: "700", + Effect: v1.TaintEffectPreferNoSchedule, + }}), + }, + expectedList: []fwk.NodeScore{ + {Name: "nodeA", Score: 0}, + {Name: "nodeB", Score: fwk.MaxNodeScore}, + }, + enableTaintTolerationComparisonOps: true, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -239,8 +292,10 @@ func TestTaintTolerationScore(t *testing.T) { state := framework.NewCycleState() snapshot := cache.NewSnapshot(nil, test.nodes) fh, _ := runtime.NewFramework(ctx, nil, nil, runtime.WithSnapshotSharedLister(snapshot)) + p, err := New(ctx, nil, fh, feature.Features{ + EnableTaintTolerationComparisonOperators: test.enableTaintTolerationComparisonOps, + }) - p, err := New(ctx, nil, fh, feature.Features{}) if err != nil { t.Fatalf("creating plugin: %v", err) } @@ -273,17 +328,17 @@ func TestTaintTolerationScore(t *testing.T) { func TestTaintTolerationFilter(t *testing.T) { tests := []struct { - name string - pod *v1.Pod - node *v1.Node - wantStatus *fwk.Status + name string + pod *v1.Pod + node *v1.Node + wantStatus *fwk.Status + enableTaintTolerationComparisonOps bool }{ { - name: "A pod having no tolerations can't be scheduled onto a node with nonempty taints", - pod: podWithTolerations("pod1", []v1.Toleration{}), - node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), - wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, - "node(s) had untolerated taint(s)"), + name: "A pod having no tolerations can't be scheduled onto a node with nonempty taints", + pod: podWithTolerations("pod1", []v1.Toleration{}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), + wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, "node(s) had untolerated taint(s)"), }, { name: "A pod which can be scheduled on a dedicated node assigned to user1 with effect NoSchedule", @@ -291,11 +346,10 @@ func TestTaintTolerationFilter(t *testing.T) { node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), }, { - name: "A pod which can't be scheduled on a dedicated node assigned to user2 with effect NoSchedule", - pod: podWithTolerations("pod1", []v1.Toleration{{Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}}), - node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), - wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, - "node(s) had untolerated taint(s)"), + name: "A pod which can't be scheduled on a dedicated node assigned to user2 with effect NoSchedule", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "dedicated", Operator: "Equal", Value: "user2", Effect: "NoSchedule"}}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}}), + wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, "node(s) had untolerated taint(s)"), }, { name: "A pod can be scheduled onto the node, with a toleration uses operator Exists that tolerates the taints on the node", @@ -316,10 +370,9 @@ func TestTaintTolerationFilter(t *testing.T) { { name: "A pod has a toleration that keys and values match the taint on the node, but (non-empty) effect doesn't match, " + "can't be scheduled onto the node", - pod: podWithTolerations("pod1", []v1.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "PreferNoSchedule"}}), - node: nodeWithTaints("nodeA", []v1.Taint{{Key: "foo", Value: "bar", Effect: "NoSchedule"}}), - wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, - "node(s) had untolerated taint(s)"), + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "PreferNoSchedule"}}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "foo", Value: "bar", Effect: "NoSchedule"}}), + wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, "node(s) had untolerated taint(s)"), }, { name: "The pod has a toleration that keys and values match the taint on the node, the effect of toleration is empty, " + @@ -339,13 +392,135 @@ func TestTaintTolerationFilter(t *testing.T) { pod: podWithTolerations("pod1", []v1.Toleration{}), node: nodeWithTaints("nodeA", []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "PreferNoSchedule"}}), }, + { + name: "Pod with Gt toleration cannot be scheduled on node when taint value is lower than threshold", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "node.example.com/priority-level", Operator: "Gt", Value: "950", Effect: "NoSchedule"}}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "node.example.com/priority-level", Value: "800", Effect: "NoSchedule"}}), + wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, "node(s) had untolerated taint(s)"), + enableTaintTolerationComparisonOps: true, + }, + { + name: "Pod with Gt toleration can be scheduled on node when taint value is higher than threshold", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "node.kubernetes.io/sla", Operator: "Gt", Value: "750", Effect: "NoSchedule"}}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "node.kubernetes.io/sla", Value: "950", Effect: "NoSchedule"}}), + enableTaintTolerationComparisonOps: true, + }, + { + name: "Pod with Lt toleration cannot be scheduled on node when taint value is higher than threshold", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "node.example.com/priority-level", Operator: "Lt", Value: "800", Effect: "NoSchedule"}}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "node.example.com/priority-level", Value: "950", Effect: "NoSchedule"}}), + wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, "node(s) had untolerated taint(s)"), + enableTaintTolerationComparisonOps: true, + }, + { + name: "Pod with Lt toleration can be scheduled on node when taint value is lower than threshold", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "node.kubernetes.io/sla", Operator: "Lt", Value: "950", Effect: "NoSchedule"}}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "node.kubernetes.io/sla", Value: "800", Effect: "NoSchedule"}}), + enableTaintTolerationComparisonOps: true, + }, + { + name: "Pod with Gt toleration cannot be scheduled on node with non-numeric taint value", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "node.kubernetes.io/sla", Operator: "Gt", Value: "950", Effect: "NoSchedule"}}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "node.kubernetes.io/sla", Value: "high", Effect: "NoSchedule"}}), + wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, + "node(s) had untolerated taint(s)"), + enableTaintTolerationComparisonOps: true, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { _, ctx := ktesting.NewTestContext(t) nodeInfo := framework.NewNodeInfo() nodeInfo.SetNode(test.node) - p, err := New(ctx, nil, nil, feature.Features{}) + p, err := New(ctx, nil, nil, feature.Features{ + EnableTaintTolerationComparisonOperators: test.enableTaintTolerationComparisonOps, + }) + if err != nil { + t.Fatalf("creating plugin: %v", err) + } + gotStatus := p.(fwk.FilterPlugin).Filter(ctx, nil, test.pod, nodeInfo) + if diff := cmp.Diff(test.wantStatus, gotStatus); diff != "" { + t.Errorf("Unexpected status (-want,+got):\n%s", diff) + } + }) + } +} + +func TestTaintTolerationFilterWithFeatureGate(t *testing.T) { + tests := []struct { + name string + pod *v1.Pod + node *v1.Node + enableTaintTolerationComparisonOps bool + wantStatus *fwk.Status + }{ + { + name: "Pod with Gt toleration can be scheduled when feature gate is enabled and taint value is higher", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "node.kubernetes.io/sla", Operator: "Gt", Value: "750", Effect: "NoSchedule"}}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "node.kubernetes.io/sla", Value: "950", Effect: "NoSchedule"}}), + enableTaintTolerationComparisonOps: true, + wantStatus: nil, + }, + { + name: "Pod with Gt toleration cannot be scheduled when feature gate is disabled", + pod: podWithTolerations("pod1", []v1.Toleration{ + {Key: "node.kubernetes.io/sla", Operator: "Gt", Value: "750", Effect: "NoSchedule"}, + }), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "node.kubernetes.io/sla", Value: "950", Effect: "NoSchedule"}}), + enableTaintTolerationComparisonOps: false, + wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, "node(s) had untolerated taint(s)"), + }, + { + name: "Pod with Lt toleration can be scheduled when feature gate is enabled and taint value is lower", + pod: podWithTolerations("pod1", []v1.Toleration{{Key: "node.kubernetes.io/sla", Operator: "Lt", Value: "950", Effect: "NoSchedule"}}), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "node.kubernetes.io/sla", Value: "800", Effect: "NoSchedule"}}), + enableTaintTolerationComparisonOps: true, + wantStatus: nil, + }, + { + name: "Pod with Lt toleration cannot be scheduled when feature gate is disabled", + pod: podWithTolerations("pod1", []v1.Toleration{ + {Key: "node.kubernetes.io/sla", Operator: "Lt", Value: "950", Effect: "NoSchedule"}, + }), + node: nodeWithTaints("nodeA", []v1.Taint{{Key: "node.kubernetes.io/sla", Value: "800", Effect: "NoSchedule"}}), + enableTaintTolerationComparisonOps: false, + wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, "node(s) had untolerated taint(s)"), + }, + { + name: "Pod with mixed tolerations (Equal and Gt) when feature gate is disabled - only Equal is honored", + pod: podWithTolerations("pod1", []v1.Toleration{ + {Key: "dedicated", Operator: "Equal", Value: "user1", Effect: "NoSchedule"}, + {Key: "node.kubernetes.io/sla", Operator: "Gt", Value: "750", Effect: "NoSchedule"}, + }), + node: nodeWithTaints("nodeA", []v1.Taint{ + {Key: "dedicated", Value: "user1", Effect: "NoSchedule"}, + }), + enableTaintTolerationComparisonOps: false, + wantStatus: nil, + }, + { + name: "Pod with mixed tolerations, Gt toleration needed but filtered out when feature gate is disabled", + pod: podWithTolerations("pod1", []v1.Toleration{ + {Key: "dedicated", Operator: "Equal", Value: "user1", Effect: "NoSchedule"}, + {Key: "node.kubernetes.io/sla", Operator: "Gt", Value: "750", Effect: "NoSchedule"}, + }), + node: nodeWithTaints("nodeA", []v1.Taint{ + {Key: "dedicated", Value: "user1", Effect: "NoSchedule"}, + {Key: "node.kubernetes.io/sla", Value: "950", Effect: "NoSchedule"}, + }), + enableTaintTolerationComparisonOps: false, + wantStatus: fwk.NewStatus(fwk.UnschedulableAndUnresolvable, "node(s) had untolerated taint(s)"), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, ctx := ktesting.NewTestContext(t) + nodeInfo := framework.NewNodeInfo() + nodeInfo.SetNode(test.node) + p, err := New(ctx, nil, nil, feature.Features{ + EnableTaintTolerationComparisonOperators: test.enableTaintTolerationComparisonOps, + }) if err != nil { t.Fatalf("creating plugin: %v", err) } diff --git a/pkg/util/tolerations/tolerations_test.go b/pkg/util/tolerations/tolerations_test.go index ea072ebde3b..bbfd5b29f43 100644 --- a/pkg/util/tolerations/tolerations_test.go +++ b/pkg/util/tolerations/tolerations_test.go @@ -341,7 +341,7 @@ func TestFuzzed(t *testing.T) { gen.TolerationSeconds = ptr.To[int64](r.Int63n(10)) } // Ensure only valid tolerations are generated. - require.NoError(t, validation.ValidateTolerations([]api.Toleration{gen}, field.NewPath("")).ToAggregate(), "%#v", gen) + require.NoError(t, validation.ValidateTolerations([]api.Toleration{gen}, field.NewPath(""), validation.PodValidationOptions{}).ToAggregate(), "%#v", gen) return gen } genTolerations := func() []api.Toleration { diff --git a/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/validation/validation.go b/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/validation/validation.go index 99a36ed3aaf..42de6a7c966 100644 --- a/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/validation/validation.go +++ b/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/validation/validation.go @@ -28,8 +28,8 @@ import ( func ValidateConfiguration(config *internalapi.Configuration) error { allErrs := field.ErrorList{} fldpath := field.NewPath("podtolerationrestriction") - allErrs = append(allErrs, validation.ValidateTolerations(config.Default, fldpath.Child("default"))...) - allErrs = append(allErrs, validation.ValidateTolerations(config.Whitelist, fldpath.Child("whitelist"))...) + allErrs = append(allErrs, validation.ValidateTolerations(config.Default, fldpath.Child("default"), validation.PodValidationOptions{})...) + allErrs = append(allErrs, validation.ValidateTolerations(config.Whitelist, fldpath.Child("whitelist"), validation.PodValidationOptions{})...) if len(allErrs) > 0 { return fmt.Errorf("invalid config: %v", allErrs) } diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index c75185861e1..cfb5789ea69 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -6550,9 +6550,10 @@ message Toleration { optional string key = 1; // Operator represents a key's relationship to the value. - // Valid operators are Exists and Equal. Defaults to Equal. + // Valid operators are Exists, Equal, Lt, and Gt. Defaults to Equal. // Exists is equivalent to wildcard for value, so that a pod can // tolerate all taints of a particular category. + // Lt and Gt perform numeric comparisons (requires feature gate TaintTolerationComparisonOperators). // +optional optional string operator = 2; diff --git a/staging/src/k8s.io/api/core/v1/toleration.go b/staging/src/k8s.io/api/core/v1/toleration.go index e803d518b5c..080c228d2d9 100644 --- a/staging/src/k8s.io/api/core/v1/toleration.go +++ b/staging/src/k8s.io/api/core/v1/toleration.go @@ -16,6 +16,16 @@ limitations under the License. package v1 +import ( + "errors" + "strconv" + "strings" + + "k8s.io/apimachinery/pkg/api/validate/content" + + "k8s.io/klog/v2" +) + // MatchToleration checks if the toleration matches tolerationToMatch. Tolerations are unique by , // if the two tolerations have same combination, regard as they match. // TODO: uniqueness check for tolerations in api validations. @@ -35,7 +45,11 @@ func (t *Toleration) MatchToleration(tolerationToMatch *Toleration) bool { // 3. Empty toleration.key means to match all taint keys. // If toleration.key is empty, toleration.operator must be 'Exists'; // this combination means to match all taint values and all taint keys. -func (t *Toleration) ToleratesTaint(taint *Taint) bool { +// 4. If toleration.operator is 'Lt' or 'Gt', numeric comparison is performed +// between toleration.value and taint.value. +// 5. If enableComparisonOperators is false and the toleration uses 'Lt' or 'Gt' +// operators, the toleration does not match (returns false). +func (t *Toleration) ToleratesTaint(logger klog.Logger, taint *Taint, enableComparisonOperators bool) bool { if len(t.Effect) > 0 && t.Effect != taint.Effect { return false } @@ -51,6 +65,47 @@ func (t *Toleration) ToleratesTaint(taint *Taint) bool { return t.Value == taint.Value case TolerationOpExists: return true + case TolerationOpLt, TolerationOpGt: + // If comparison operators are disabled, this toleration doesn't match + if !enableComparisonOperators { + return false + } + return compareNumericValues(logger, t.Value, taint.Value, t.Operator) + default: + return false + } +} + +// compareNumericValues performs numeric comparison between toleration and taint values +func compareNumericValues(logger klog.Logger, tolerationVal, taintVal string, op TolerationOperator) bool { + + errorMsgs := content.IsDecimalInteger(tolerationVal) + if len(errorMsgs) > 0 { + logger.Error(errors.New(strings.Join(errorMsgs, ",")), "failed to parse toleration value as int64", "toleration", tolerationVal) + return false + } + tVal, err := strconv.ParseInt(tolerationVal, 10, 64) + if err != nil { + logger.Error(err, "failed to parse toleration value as int64", "toleration", tolerationVal) + return false + } + + errorMsgs = content.IsDecimalInteger(taintVal) + if len(errorMsgs) > 0 { + logger.Error(errors.New(strings.Join(errorMsgs, ",")), "failed to parse taint value as int64", "taint", taintVal) + return false + } + tntVal, err := strconv.ParseInt(taintVal, 10, 64) + if err != nil { + logger.Error(err, "failed to parse taint value as int64", "taint", taintVal) + return false + } + + switch op { + case TolerationOpLt: + return tntVal < tVal + case TolerationOpGt: + return tntVal > tVal default: return false } diff --git a/staging/src/k8s.io/api/core/v1/toleration_test.go b/staging/src/k8s.io/api/core/v1/toleration_test.go index 3dd48f08778..93fc2839dc0 100644 --- a/staging/src/k8s.io/api/core/v1/toleration_test.go +++ b/staging/src/k8s.io/api/core/v1/toleration_test.go @@ -17,16 +17,19 @@ limitations under the License. package v1 import ( + "k8s.io/klog/v2/ktesting" "testing" ) func TestTolerationToleratesTaint(t *testing.T) { - + logger, _ := ktesting.NewTestContext(t) testCases := []struct { - description string - toleration Toleration - taint Taint - expectTolerated bool + description string + toleration Toleration + taint Taint + expectTolerated bool + expectError bool + enableTaintTolerationComparisonOperatorsFG bool }{ { description: "toleration and taint have the same key and effect, and operator is Exists, and taint has no value, expect tolerated", @@ -114,10 +117,384 @@ func TestTolerationToleratesTaint(t *testing.T) { }, expectTolerated: false, }, + { + description: "toleration with Gt operator - taint value less than toleration value, expect not tolerated", + toleration: Toleration{ + Key: "node.kubernetes.io/sla", + Operator: TolerationOpGt, + Value: "950", + Effect: TaintEffectNoSchedule, + }, + taint: Taint{ + Key: "node.kubernetes.io/sla", + Value: "800", + Effect: TaintEffectNoSchedule, + }, + expectTolerated: false, + enableTaintTolerationComparisonOperatorsFG: true, + }, + { + description: "toleration with Gt operator - taint value greater than toleration value, expect tolerated", + toleration: Toleration{ + Key: "node.kubernetes.io/sla", + Operator: TolerationOpGt, + Value: "750", + Effect: TaintEffectNoSchedule, + }, + taint: Taint{ + Key: "node.kubernetes.io/sla", + Value: "950", + Effect: TaintEffectNoSchedule, + }, + expectTolerated: true, + enableTaintTolerationComparisonOperatorsFG: true, + }, + { + description: "toleration with Lt operator - taint value greater than toleration value, expect not tolerated", + toleration: Toleration{ + Key: "node.kubernetes.io/sla", + Operator: TolerationOpLt, + Value: "800", + Effect: TaintEffectNoSchedule, + }, + taint: Taint{ + Key: "node.kubernetes.io/sla", + Value: "950", + Effect: TaintEffectNoSchedule, + }, + expectTolerated: false, + enableTaintTolerationComparisonOperatorsFG: true, + }, + { + description: "toleration with Lt operator - taint value less than toleration value, expect tolerated", + toleration: Toleration{ + Key: "node.kubernetes.io/sla", + Operator: TolerationOpLt, + Value: "950", + Effect: TaintEffectNoSchedule, + }, + taint: Taint{ + Key: "node.kubernetes.io/sla", + Value: "800", + Effect: TaintEffectNoSchedule, + }, + expectTolerated: true, + enableTaintTolerationComparisonOperatorsFG: true, + }, + { + description: "toleration with Gt operator and taint with equal numeric value, expect not tolerated", + toleration: Toleration{ + Key: "node.kubernetes.io/sla", + Operator: TolerationOpGt, + Value: "950", + Effect: TaintEffectNoSchedule, + }, + taint: Taint{ + Key: "node.kubernetes.io/sla", + Value: "950", + Effect: TaintEffectNoSchedule, + }, + expectTolerated: false, + enableTaintTolerationComparisonOperatorsFG: true, + }, + { + description: "toleration with Gt operator and taint with non-numeric value, expect not tolerated", + toleration: Toleration{ + Key: "node.kubernetes.io/sla", + Operator: TolerationOpGt, + Value: "950", + Effect: TaintEffectNoSchedule, + }, + taint: Taint{ + Key: "node.kubernetes.io/sla", + Value: "high", + Effect: TaintEffectNoSchedule, + }, + expectTolerated: false, + expectError: true, + enableTaintTolerationComparisonOperatorsFG: true, + }, + { + description: "toleration with Gt operator and negative numeric values - taint value less than threshold, expect not tolerated", + toleration: Toleration{ + Key: "test-key", + Operator: TolerationOpGt, + Value: "-100", + Effect: TaintEffectNoSchedule, + }, + taint: Taint{ + Key: "test-key", + Value: "-200", + Effect: TaintEffectNoSchedule, + }, + expectTolerated: false, + enableTaintTolerationComparisonOperatorsFG: true, + }, + { + description: "toleration with Gt operator and large int64 values - taint value less than threshold, expect not tolerated", + toleration: Toleration{ + Key: "test-key", + Operator: TolerationOpGt, + Value: "9223372036854775806", + Effect: TaintEffectNoSchedule, + }, + taint: Taint{ + Key: "test-key", + Value: "100", + Effect: TaintEffectNoSchedule, + }, + expectTolerated: false, + enableTaintTolerationComparisonOperatorsFG: true, + }, } for _, tc := range testCases { - if tolerated := tc.toleration.ToleratesTaint(&tc.taint); tc.expectTolerated != tolerated { + if tolerated := tc.toleration.ToleratesTaint(logger, &tc.taint, tc.enableTaintTolerationComparisonOperatorsFG); tc.expectTolerated != tolerated { t.Errorf("[%s] expect %v, got %v: toleration %+v, taint %s", tc.description, tc.expectTolerated, tolerated, tc.toleration, tc.taint.ToString()) } } } + +func TestCompareNumericValues(t *testing.T) { + logger, _ := ktesting.NewTestContext(t) + testCases := []struct { + description string + tolerationVal string + taintVal string + operator TolerationOperator + expectedResult bool + }{ + // Valid Gt operator cases + { + description: "Gt operator - taint value greater than toleration value, expect true", + tolerationVal: "100", + taintVal: "200", + operator: TolerationOpGt, + expectedResult: true, + }, + { + description: "Gt operator - taint value less than toleration value, expect false", + tolerationVal: "200", + taintVal: "100", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Gt operator - taint value equal to toleration value, expect false", + tolerationVal: "100", + taintVal: "100", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Gt operator - negative numbers, taint greater, expect true", + tolerationVal: "-100", + taintVal: "-50", + operator: TolerationOpGt, + expectedResult: true, + }, + { + description: "Gt operator - negative numbers, taint less, expect false", + tolerationVal: "-50", + taintVal: "-100", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Gt operator - zero and positive, expect true", + tolerationVal: "0", + taintVal: "1", + operator: TolerationOpGt, + expectedResult: true, + }, + { + description: "Gt operator - large int64 values, taint greater, expect true", + tolerationVal: "9223372036854775806", + taintVal: "9223372036854775807", + operator: TolerationOpGt, + expectedResult: true, + }, + + // Valid Lt operator cases + { + description: "Lt operator - taint value less than toleration value, expect true", + tolerationVal: "200", + taintVal: "100", + operator: TolerationOpLt, + expectedResult: true, + }, + { + description: "Lt operator - taint value greater than toleration value, expect false", + tolerationVal: "100", + taintVal: "200", + operator: TolerationOpLt, + expectedResult: false, + }, + { + description: "Lt operator - taint value equal to toleration value, expect false", + tolerationVal: "100", + taintVal: "100", + operator: TolerationOpLt, + expectedResult: false, + }, + { + description: "Lt operator - negative numbers, taint less, expect true", + tolerationVal: "-50", + taintVal: "-100", + operator: TolerationOpLt, + expectedResult: true, + }, + { + description: "Lt operator - negative numbers, taint greater, expect false", + tolerationVal: "-100", + taintVal: "-50", + operator: TolerationOpLt, + expectedResult: false, + }, + { + description: "Lt operator - zero and negative, expect true", + tolerationVal: "0", + taintVal: "-1", + operator: TolerationOpLt, + expectedResult: true, + }, + + // Invalid toleration values - should return false + { + description: "Gt operator - invalid toleration value (non-numeric), expect false", + tolerationVal: "abc", + taintVal: "100", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Gt operator - invalid toleration value (empty string), expect false", + tolerationVal: "", + taintVal: "100", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Gt operator - invalid toleration value (leading zero), expect false", + tolerationVal: "0100", + taintVal: "200", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Gt operator - invalid toleration value (plus sign), expect false", + tolerationVal: "+100", + taintVal: "200", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Gt operator - invalid toleration value (floating point), expect false", + tolerationVal: "100.5", + taintVal: "200", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Gt operator - invalid toleration value (just minus sign), expect false", + tolerationVal: "-", + taintVal: "100", + operator: TolerationOpGt, + expectedResult: false, + }, + + // Invalid taint values - should return false + { + description: "Gt operator - invalid taint value (non-numeric), expect false", + tolerationVal: "100", + taintVal: "xyz", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Gt operator - invalid taint value (empty string), expect false", + tolerationVal: "100", + taintVal: "", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Gt operator - invalid taint value (leading zero), expect false", + tolerationVal: "100", + taintVal: "0200", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Lt operator - invalid taint value (plus sign), expect false", + tolerationVal: "100", + taintVal: "+200", + operator: TolerationOpLt, + expectedResult: false, + }, + { + description: "Lt operator - invalid taint value (spaces), expect false", + tolerationVal: "100", + taintVal: " 200 ", + operator: TolerationOpLt, + expectedResult: false, + }, + + // Invalid operator - should return false + { + description: "Equal operator (unsupported for numeric comparison), expect false", + tolerationVal: "100", + taintVal: "100", + operator: TolerationOpEqual, + expectedResult: false, + }, + { + description: "Exists operator (unsupported for numeric comparison), expect false", + tolerationVal: "100", + taintVal: "100", + operator: TolerationOpExists, + expectedResult: false, + }, + + // Edge cases with zero + { + description: "Gt operator - both zero, expect false", + tolerationVal: "0", + taintVal: "0", + operator: TolerationOpGt, + expectedResult: false, + }, + { + description: "Lt operator - both zero, expect false", + tolerationVal: "0", + taintVal: "0", + operator: TolerationOpLt, + expectedResult: false, + }, + + // Int64 boundary cases + { + description: "Gt operator - max int64 as taint, expect true", + tolerationVal: "0", + taintVal: "9223372036854775807", + operator: TolerationOpGt, + expectedResult: true, + }, + { + description: "Lt operator - min int64 as taint, expect true", + tolerationVal: "0", + taintVal: "-9223372036854775808", + operator: TolerationOpLt, + expectedResult: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + result := compareNumericValues(logger, tc.tolerationVal, tc.taintVal, tc.operator) + if result != tc.expectedResult { + t.Errorf("[%s] expected %v, got %v: tolerationVal=%q, taintVal=%q, operator=%v", + tc.description, tc.expectedResult, result, tc.tolerationVal, tc.taintVal, tc.operator) + } + }) + } +} diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index c8ea9cbb903..2ce22b972a1 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -4054,9 +4054,10 @@ type Toleration struct { // +optional Key string `json:"key,omitempty" protobuf:"bytes,1,opt,name=key"` // Operator represents a key's relationship to the value. - // Valid operators are Exists and Equal. Defaults to Equal. + // Valid operators are Exists, Equal, Lt, and Gt. Defaults to Equal. // Exists is equivalent to wildcard for value, so that a pod can // tolerate all taints of a particular category. + // Lt and Gt perform numeric comparisons (requires feature gate TaintTolerationComparisonOperators). // +optional Operator TolerationOperator `json:"operator,omitempty" protobuf:"bytes,2,opt,name=operator,casttype=TolerationOperator"` // Value is the taint value the toleration matches to. @@ -4082,6 +4083,8 @@ type TolerationOperator string const ( TolerationOpExists TolerationOperator = "Exists" TolerationOpEqual TolerationOperator = "Equal" + TolerationOpLt TolerationOperator = "Lt" + TolerationOpGt TolerationOperator = "Gt" ) // PodReadinessGate contains the reference to a pod condition diff --git a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go index f5f6eb71bc4..a5db088c322 100644 --- a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go @@ -2672,7 +2672,7 @@ func (Taint) SwaggerDoc() map[string]string { var map_Toleration = map[string]string{ "": "The pod this Toleration is attached to tolerates any taint that matches the triple using the matching operator .", "key": "Key is the taint key that the toleration applies to. Empty means match all taint keys. If the key is empty, operator must be Exists; this combination means to match all values and all keys.", - "operator": "Operator represents a key's relationship to the value. Valid operators are Exists and Equal. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category.", + "operator": "Operator represents a key's relationship to the value. Valid operators are Exists, Equal, Lt, and Gt. Defaults to Equal. Exists is equivalent to wildcard for value, so that a pod can tolerate all taints of a particular category. Lt and Gt perform numeric comparisons (requires feature gate TaintTolerationComparisonOperators).", "value": "Value is the taint value the toleration matches to. If the operator is Exists, the value should be empty, otherwise just a regular string.", "effect": "Effect indicates the taint effect to match. Empty means match all taint effects. When specified, allowed values are NoSchedule, PreferNoSchedule and NoExecute.", "tolerationSeconds": "TolerationSeconds represents the period of time the toleration (which must be of effect NoExecute, otherwise this field is ignored) tolerates the taint. By default, it is not set, which means tolerate the taint forever (do not evict). Zero and negative values will be treated as 0 (evict immediately) by the system.", diff --git a/staging/src/k8s.io/api/go.mod b/staging/src/k8s.io/api/go.mod index 9de3b9a888c..95cfab8b4c5 100644 --- a/staging/src/k8s.io/api/go.mod +++ b/staging/src/k8s.io/api/go.mod @@ -6,7 +6,10 @@ go 1.25.0 godebug default=go1.25 -require k8s.io/apimachinery v0.0.0 +require ( + k8s.io/apimachinery v0.0.0 + k8s.io/klog/v2 v2.130.1 +) require ( github.com/davecgh/go-spew v1.1.1 // indirect @@ -24,7 +27,6 @@ require ( golang.org/x/text v0.29.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/inf.v0 v0.9.1 // indirect - k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20250910181357-589584f1c912 // indirect k8s.io/utils v0.0.0-20251002143259-bc988d571ff4 // indirect sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validate/content/decimal_int.go b/staging/src/k8s.io/apimachinery/pkg/api/validate/content/decimal_int.go new file mode 100644 index 00000000000..5622ca15a87 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/api/validate/content/decimal_int.go @@ -0,0 +1,62 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package content + +const decimalIntegerErrMsg string = "must be a valid decimal integer in canonical form" + +// IsDecimalInteger validates that a string represents a decimal integer in strict canonical form. +// This means the string must be formatted exactly as a human would naturally write an integer, +// without any programming language conventions like leading zeros, plus signs, or alternate bases. +// +// valid values:"0" or Non-zero integers (i.e., "123", "-456") where the first digit is 1-9, +// followed by any digits 0-9. +// +// This validator is stricter than strconv.ParseInt, which accepts leading zeros values (i.e, "0700") +// and interprets them as decimal 700, potentially causing confusion with octal notation. +func IsDecimalInteger(value string) []string { + n := len(value) + if n == 0 { + return []string{EmptyError()} + } + + i := 0 + if value[0] == '-' { + if n == 1 { + return []string{decimalIntegerErrMsg} + } + i = 1 + } + + if value[i] == '0' { + if n == 1 && i == 0 { + return nil + } + return []string{decimalIntegerErrMsg} + } + + if value[i] < '1' || value[i] > '9' { + return []string{decimalIntegerErrMsg} + } + + for i++; i < n; i++ { + if value[i] < '0' || value[i] > '9' { + return []string{decimalIntegerErrMsg} + } + } + + return nil +} diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validate/content/decimal_int_test.go b/staging/src/k8s.io/apimachinery/pkg/api/validate/content/decimal_int_test.go new file mode 100644 index 00000000000..369417d26a8 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/api/validate/content/decimal_int_test.go @@ -0,0 +1,234 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package content + +import ( + "strconv" + "strings" + "testing" +) + +func TestIsDecimalInteger(t *testing.T) { + testCases := []struct { + name string + input string + shouldPass bool + errContains string + }{ + // valid + {name: "zero", input: "0", shouldPass: true}, + {name: "positive single digit 1", input: "1", shouldPass: true}, + {name: "positive single digit 2", input: "2", shouldPass: true}, + {name: "positive single digit 5", input: "5", shouldPass: true}, + {name: "positive single digit 9", input: "9", shouldPass: true}, + {name: "negative single digit", input: "-5", shouldPass: true}, + {name: "negative single digit 1", input: "-1", shouldPass: true}, + {name: "negative single digit 9", input: "-9", shouldPass: true}, + + {name: "number starting with 1", input: "100", shouldPass: true}, + {name: "number starting with 2", input: "234", shouldPass: true}, + {name: "number starting with 3", input: "345", shouldPass: true}, + {name: "number starting with 4", input: "456", shouldPass: true}, + {name: "number starting with 5", input: "567", shouldPass: true}, + {name: "number starting with 6", input: "678", shouldPass: true}, + {name: "number starting with 7", input: "789", shouldPass: true}, + {name: "number starting with 8", input: "890", shouldPass: true}, + {name: "number starting with 9", input: "999", shouldPass: true}, + + {name: "positive multi-digit", input: "123", shouldPass: true}, + {name: "negative multi-digit", input: "-456", shouldPass: true}, + {name: "negative starting with 1", input: "-100", shouldPass: true}, + {name: "negative starting with 9", input: "-987", shouldPass: true}, + {name: "large positive number", input: "9223372036854775807", shouldPass: true}, // max int64 + {name: "large negative number", input: "-9223372036854775808", shouldPass: true}, // min int64 + {name: "very long valid number", input: "12345678901234567890", shouldPass: true}, + {name: "all nines", input: "999999999999", shouldPass: true}, + + // invalid + {name: "negative zero", input: "-0", shouldPass: false}, + {name: "double zero", input: "00", shouldPass: false}, + {name: "triple zero", input: "000", shouldPass: false}, + {name: "many zeros", input: "0000000", shouldPass: false}, + {name: "leading zero single digit", input: "01", shouldPass: false}, + {name: "leading zero digit 2", input: "02", shouldPass: false}, + {name: "leading zero digit 9", input: "09", shouldPass: false}, + {name: "leading zero multi-digit", input: "0123", shouldPass: false}, + {name: "octal-like format", input: "0700", shouldPass: false}, + {name: "octal-like format 2", input: "0950", shouldPass: false}, + {name: "multiple leading zeros", input: "00123", shouldPass: false}, + {name: "negative with leading zero", input: "-01", shouldPass: false}, + {name: "negative with leading zeros", input: "-0123", shouldPass: false}, + {name: "negative double zero", input: "-00", shouldPass: false}, + {name: "plus sign", input: "+123", shouldPass: false}, + {name: "positive plus sign", input: "+5", shouldPass: false}, + {name: "plus zero", input: "+0", shouldPass: false}, + + // Invalid cases - empty and whitespace + {name: "empty string", input: "", shouldPass: false, errContains: "non-empty"}, + {name: "just minus sign", input: "-", shouldPass: false}, + {name: "just plus sign", input: "+", shouldPass: false}, + {name: "single space", input: " ", shouldPass: false}, + {name: "multiple spaces", input: " ", shouldPass: false}, + {name: "leading space", input: " 123", shouldPass: false}, + {name: "trailing space", input: "123 ", shouldPass: false}, + {name: "space in middle", input: "12 3", shouldPass: false}, + {name: "spaces around", input: " 123 ", shouldPass: false}, + + {name: "decimal number", input: "12.3", shouldPass: false}, + {name: "decimal zero", input: "0.0", shouldPass: false}, + {name: "negative decimal", input: "-12.5", shouldPass: false}, + {name: "trailing dot", input: "123.", shouldPass: false}, + {name: "leading dot", input: ".123", shouldPass: false}, + + {name: "alphabetic", input: "abc", shouldPass: false}, + {name: "alphanumeric", input: "12a3", shouldPass: false}, + {name: "letter at start", input: "a123", shouldPass: false}, + {name: "letter at end", input: "123a", shouldPass: false}, + {name: "uppercase letters", input: "ABC", shouldPass: false}, + {name: "mixed case", input: "12A3", shouldPass: false}, + + {name: "hexadecimal", input: "0x123", shouldPass: false}, + {name: "hex uppercase", input: "0X123", shouldPass: false}, + {name: "octal prefix", input: "0o777", shouldPass: false}, + {name: "binary prefix", input: "0b101", shouldPass: false}, + {name: "scientific notation", input: "1e5", shouldPass: false}, + {name: "scientific negative exp", input: "1e-5", shouldPass: false}, + {name: "scientific uppercase", input: "1E5", shouldPass: false}, + + {name: "underscore separator", input: "1_000", shouldPass: false}, + {name: "comma separator", input: "1,000", shouldPass: false}, + {name: "period separator", input: "1.000", shouldPass: false}, + {name: "apostrophe separator", input: "1'000", shouldPass: false}, + + {name: "double minus", input: "--123", shouldPass: false}, + {name: "double plus", input: "++123", shouldPass: false}, + {name: "plus minus", input: "+-123", shouldPass: false}, + {name: "minus plus", input: "-+123", shouldPass: false}, + {name: "minus at end", input: "123-", shouldPass: false}, + {name: "minus in middle", input: "12-3", shouldPass: false}, + {name: "plus at end", input: "123+", shouldPass: false}, + {name: "plus in middle", input: "12+3", shouldPass: false}, + + {name: "tab character at start", input: "\t123", shouldPass: false}, + {name: "tab character at end", input: "123\t", shouldPass: false}, + {name: "newline character", input: "123\n", shouldPass: false}, + {name: "carriage return", input: "123\r", shouldPass: false}, + {name: "null character", input: "123\x00", shouldPass: false}, + {name: "vertical tab", input: "123\v", shouldPass: false}, + {name: "form feed", input: "123\f", shouldPass: false}, + + {name: "parentheses", input: "(123)", shouldPass: false}, + {name: "brackets", input: "[123]", shouldPass: false}, + {name: "braces", input: "{123}", shouldPass: false}, + {name: "dollar sign", input: "$123", shouldPass: false}, + {name: "percent sign", input: "123%", shouldPass: false}, + {name: "hash", input: "#123", shouldPass: false}, + {name: "at sign", input: "@123", shouldPass: false}, + {name: "ampersand", input: "&123", shouldPass: false}, + {name: "asterisk", input: "*123", shouldPass: false}, + {name: "slash", input: "12/3", shouldPass: false}, + {name: "backslash", input: "12\\3", shouldPass: false}, + {name: "pipe", input: "12|3", shouldPass: false}, + {name: "semicolon", input: "12;3", shouldPass: false}, + {name: "colon", input: "12:3", shouldPass: false}, + {name: "question mark", input: "12?3", shouldPass: false}, + {name: "exclamation", input: "12!3", shouldPass: false}, + {name: "tilde", input: "~123", shouldPass: false}, + {name: "backtick", input: "`123", shouldPass: false}, + {name: "single quote", input: "'123'", shouldPass: false}, + {name: "double quote", input: "\"123\"", shouldPass: false}, + + {name: "unicode minus", input: "−123", shouldPass: false}, // U+2212 minus sign + {name: "unicode digit", input: "１２３", shouldPass: false}, // fullwidth digits + {name: "arabic digits", input: "١٢٣", shouldPass: false}, // Arabic-Indic digits + {name: "chinese characters", input: "一二三", shouldPass: false}, + {name: "superscript", input: "123⁴", shouldPass: false}, + {name: "subscript", input: "123₄", shouldPass: false}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + errs := IsDecimalInteger(tc.input) + if tc.shouldPass { + if len(errs) != 0 { + t.Errorf("IsDecimalInteger(%q) = %v, want no errors", tc.input, errs) + } + } else { + if len(errs) == 0 { + t.Errorf("IsDecimalInteger(%q) = no errors, want errors", tc.input) + } else if tc.errContains != "" { + found := false + for _, err := range errs { + if strings.Contains(err, tc.errContains) { + found = true + break + } + } + if !found { + t.Errorf("IsDecimalInteger(%q) errors %v should contain %q", tc.input, errs, tc.errContains) + } + } + } + }) + } + + // Additional verification: valid strings should parse with strconv.ParseInt + validCases := []string{ + "0", "1", "2", "5", "9", + "-1", "-5", "-9", + "123", "-456", "100", "999", + "9223372036854775807", "-9223372036854775808", + "12345678901234567890", + } + for _, validCase := range validCases { + if errs := IsDecimalInteger(validCase); len(errs) != 0 { + t.Errorf("Valid case %q should return no errors, got: %v", validCase, errs) + } + // Verify it can also be parsed by strconv.ParseInt (within range) + if len(validCase) <= 19 { // Only test cases that fit in int64 + if _, err := strconv.ParseInt(validCase, 10, 64); err != nil { + t.Errorf("Valid case %q should be parseable by strconv.ParseInt: %v", validCase, err) + } + } + } + + // Verify that our function rejects what we intend to reject (even if strconv.ParseInt accepts it) + rejectedCases := []string{ + "0700", "0950", "01", "02", "09", + "+123", "+5", "+0", + "-0", "00", "000", + "-01", "-00", + } + for _, rejectedCase := range rejectedCases { + if errs := IsDecimalInteger(rejectedCase); len(errs) == 0 { + t.Errorf("Case %q should be rejected by strict validation", rejectedCase) + } + } + + // Edge case: verify strconv.ParseInt accepts things we reject (proving we're stricter) + strconvAcceptsButWeReject := []string{"+123", "0700", "01"} + for _, case_ := range strconvAcceptsButWeReject { + // strconv.ParseInt should accept it + if _, err := strconv.ParseInt(case_, 10, 64); err != nil { + t.Errorf("strconv.ParseInt should accept %q but got error: %v", case_, err) + } + // But our function should reject it + if errs := IsDecimalInteger(case_); len(errs) == 0 { + t.Errorf("IsDecimalInteger should reject %q (stricter than strconv)", case_) + } + } +} diff --git a/staging/src/k8s.io/client-go/applyconfigurations/core/v1/toleration.go b/staging/src/k8s.io/client-go/applyconfigurations/core/v1/toleration.go index 609051a4000..1870cbf1865 100644 --- a/staging/src/k8s.io/client-go/applyconfigurations/core/v1/toleration.go +++ b/staging/src/k8s.io/client-go/applyconfigurations/core/v1/toleration.go @@ -32,9 +32,10 @@ type TolerationApplyConfiguration struct { // If the key is empty, operator must be Exists; this combination means to match all values and all keys. Key *string `json:"key,omitempty"` // Operator represents a key's relationship to the value. - // Valid operators are Exists and Equal. Defaults to Equal. + // Valid operators are Exists, Equal, Lt, and Gt. Defaults to Equal. // Exists is equivalent to wildcard for value, so that a pod can // tolerate all taints of a particular category. + // Lt and Gt perform numeric comparisons (requires feature gate TaintTolerationComparisonOperators). Operator *corev1.TolerationOperator `json:"operator,omitempty"` // Value is the taint value the toleration matches to. // If the operator is Exists, the value should be empty, otherwise just a regular string. diff --git a/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers.go b/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers.go index 23a94bd7bcd..5e311df9843 100644 --- a/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers.go +++ b/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers.go @@ -21,6 +21,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/component-helpers/scheduling/corev1/nodeaffinity" + "k8s.io/klog/v2" ) // PodPriority returns priority of the given pod. @@ -60,9 +61,9 @@ func GetAvoidPodsFromNodeAnnotations(annotations map[string]string) (v1.AvoidPod } // TolerationsTolerateTaint checks if taint is tolerated by any of the tolerations. -func TolerationsTolerateTaint(tolerations []v1.Toleration, taint *v1.Taint) bool { +func TolerationsTolerateTaint(logger klog.Logger, tolerations []v1.Toleration, taint *v1.Taint, enableComparisonOperators bool) bool { for i := range tolerations { - if tolerations[i].ToleratesTaint(taint) { + if tolerations[i].ToleratesTaint(logger, taint, enableComparisonOperators) { return true } } @@ -75,10 +76,10 @@ type taintsFilterFunc func(*v1.Taint) bool // all the filtered taints, and returns the first taint without a toleration // Returns true if there is an untolerated taint // Returns false if all taints are tolerated -func FindMatchingUntoleratedTaint(taints []v1.Taint, tolerations []v1.Toleration, inclusionFilter taintsFilterFunc) (v1.Taint, bool) { +func FindMatchingUntoleratedTaint(logger klog.Logger, taints []v1.Taint, tolerations []v1.Toleration, inclusionFilter taintsFilterFunc, enableComparisonOperators bool) (v1.Taint, bool) { filteredTaints := getFilteredTaints(taints, inclusionFilter) for _, taint := range filteredTaints { - if !TolerationsTolerateTaint(tolerations, &taint) { + if !TolerationsTolerateTaint(logger, tolerations, &taint, enableComparisonOperators) { return taint, true } } diff --git a/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers_test.go b/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers_test.go index 54679d17e94..6afc5e02ede 100644 --- a/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers_test.go +++ b/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers_test.go @@ -23,6 +23,7 @@ import ( v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/klog/v2/ktesting" ) // TestPodPriority tests PodPriority function. @@ -644,12 +645,15 @@ func TestGetAvoidPodsFromNode(t *testing.T) { } func TestFindMatchingUntoleratedTaint(t *testing.T) { + logger, _ := ktesting.NewTestContext(t) testCases := []struct { - description string - tolerations []v1.Toleration - taints []v1.Taint - applyFilter taintsFilterFunc - expectTolerated bool + description string + tolerations []v1.Toleration + taints []v1.Taint + applyFilter taintsFilterFunc + expectTolerated bool + expectError bool + enableComparisonOperatorsFG bool }{ { description: "empty tolerations tolerate empty taints", @@ -747,10 +751,95 @@ func TestFindMatchingUntoleratedTaint(t *testing.T) { applyFilter: func(t *v1.Taint) bool { return t.Effect == v1.TaintEffectNoExecute }, expectTolerated: true, }, + { + description: "numeric Gt operator with taint value below threshold, expect not tolerated", + tolerations: []v1.Toleration{ + { + Key: "node.kubernetes.io/sla", + Operator: "Gt", + Value: "950", + Effect: v1.TaintEffectNoSchedule, + }, + }, + taints: []v1.Taint{ + { + Key: "node.kubernetes.io/sla", + Value: "800", + Effect: v1.TaintEffectNoSchedule, + }, + }, + applyFilter: func(t *v1.Taint) bool { return true }, + expectTolerated: false, + enableComparisonOperatorsFG: true, + }, + { + description: "numeric Gt operator with taint value above threshold, expect tolerated", + tolerations: []v1.Toleration{ + { + Key: "node.kubernetes.io/sla", + Operator: "Gt", + Value: "750", + Effect: v1.TaintEffectNoSchedule, + }, + }, + taints: []v1.Taint{ + { + Key: "node.kubernetes.io/sla", + Value: "950", + Effect: v1.TaintEffectNoSchedule, + }, + }, + applyFilter: func(t *v1.Taint) bool { return true }, + expectTolerated: true, + enableComparisonOperatorsFG: true, + }, + { + description: "numeric Lt operator with taint value above threshold, expect not tolerated", + tolerations: []v1.Toleration{ + { + Key: "node.kubernetes.io/sla", + Operator: "Lt", + Value: "800", + Effect: v1.TaintEffectNoSchedule, + }, + }, + taints: []v1.Taint{ + { + Key: "node.kubernetes.io/sla", + Value: "950", + Effect: v1.TaintEffectNoSchedule, + }, + }, + applyFilter: func(t *v1.Taint) bool { return true }, + expectTolerated: false, + enableComparisonOperatorsFG: true, + }, + { + description: "numeric Gt operator with non-numeric taint value, expect not tolerated", + tolerations: []v1.Toleration{ + { + Key: "node.kubernetes.io/sla", + Operator: "Gt", + Value: "950", + Effect: v1.TaintEffectNoSchedule, + }, + }, + taints: []v1.Taint{ + { + Key: "node.kubernetes.io/sla", + Value: "high", + Effect: v1.TaintEffectNoSchedule, + }, + }, + applyFilter: func(t *v1.Taint) bool { return true }, + expectTolerated: false, + expectError: true, + enableComparisonOperatorsFG: true, + }, } for _, tc := range testCases { - _, untolerated := FindMatchingUntoleratedTaint(tc.taints, tc.tolerations, tc.applyFilter) + _, untolerated := FindMatchingUntoleratedTaint(logger, tc.taints, tc.tolerations, tc.applyFilter, tc.enableComparisonOperatorsFG) if tc.expectTolerated != !untolerated { filteredTaints := []v1.Taint{} for _, taint := range tc.taints { diff --git a/test/compatibility_lifecycle/reference/versioned_feature_list.yaml b/test/compatibility_lifecycle/reference/versioned_feature_list.yaml index 3050ab2f281..ee25712dd45 100644 --- a/test/compatibility_lifecycle/reference/versioned_feature_list.yaml +++ b/test/compatibility_lifecycle/reference/versioned_feature_list.yaml @@ -1857,6 +1857,12 @@ lockToDefault: true preRelease: GA version: "1.35" +- name: TaintTolerationComparisonOperators + versionedSpecs: + - default: false + lockToDefault: false + preRelease: Alpha + version: "1.35" - name: TokenRequestServiceAccountUIDValidation versionedSpecs: - default: true diff --git a/test/e2e/common/node/pod_admission.go b/test/e2e/common/node/pod_admission.go index 6a411522edf..96b273dae90 100644 --- a/test/e2e/common/node/pod_admission.go +++ b/test/e2e/common/node/pod_admission.go @@ -18,6 +18,7 @@ package node import ( "context" + "k8s.io/klog/v2" "github.com/onsi/ginkgo/v2" @@ -67,6 +68,7 @@ var _ = SIGDescribe("PodOSRejection", framework.WithNodeConformance(), func() { // findLinuxNode finds a Linux node that is Ready and Schedulable func findLinuxNode(ctx context.Context, f *framework.Framework) (v1.Node, error) { + logger := klog.FromContext(ctx) selector := labels.Set{"kubernetes.io/os": "linux"}.AsSelector() nodeList, err := f.ClientSet.CoreV1().Nodes().List(ctx, metav1.ListOptions{LabelSelector: selector.String()}) @@ -77,7 +79,7 @@ func findLinuxNode(ctx context.Context, f *framework.Framework) (v1.Node, error) var targetNode v1.Node foundNode := false for _, n := range nodeList.Items { - if e2enode.IsNodeReady(&n) && e2enode.IsNodeSchedulable(&n) { + if e2enode.IsNodeReady(logger, &n) && e2enode.IsNodeSchedulable(logger, &n) { targetNode = n foundNode = true break diff --git a/test/e2e/framework/daemonset/fixtures.go b/test/e2e/framework/daemonset/fixtures.go index 5c1a5b54fc7..5dda5cb63d2 100644 --- a/test/e2e/framework/daemonset/fixtures.go +++ b/test/e2e/framework/daemonset/fixtures.go @@ -19,6 +19,7 @@ package daemonset import ( "context" "fmt" + "k8s.io/klog/v2" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" @@ -81,11 +82,12 @@ func CheckPresentOnNodes(ctx context.Context, c clientset.Interface, ds *appsv1. } func SchedulableNodes(ctx context.Context, c clientset.Interface, ds *appsv1.DaemonSet) []string { + logger := klog.FromContext(ctx) nodeList, err := c.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) framework.ExpectNoError(err) nodeNames := make([]string, 0) for _, node := range nodeList.Items { - shouldRun, _ := daemon.NodeShouldRunDaemonPod(&node, ds) + shouldRun, _ := daemon.NodeShouldRunDaemonPod(logger, &node, ds) if !shouldRun { framework.Logf("DaemonSet pods can't tolerate node %s with taints %+v, skip checking this node", node.Name, node.Spec.Taints) continue diff --git a/test/e2e/framework/node/resource.go b/test/e2e/framework/node/resource.go index 819275b9d22..5c154d68992 100644 --- a/test/e2e/framework/node/resource.go +++ b/test/e2e/framework/node/resource.go @@ -20,6 +20,7 @@ import ( "context" "encoding/json" "fmt" + "k8s.io/klog/v2" "net" "strings" "time" @@ -325,12 +326,13 @@ func GetPublicIps(ctx context.Context, c clientset.Interface) ([]string, error) // If EITHER 1 or 2 is not true, most tests will want to ignore the node entirely. // If there are no nodes that are both ready and schedulable, this will return an error. func GetReadySchedulableNodes(ctx context.Context, c clientset.Interface) (nodes *v1.NodeList, err error) { + logger := klog.FromContext(ctx) nodes, err = checkWaitListSchedulableNodes(ctx, c) if err != nil { return nil, fmt.Errorf("listing schedulable nodes error: %w", err) } Filter(nodes, func(node v1.Node) bool { - return IsNodeSchedulable(&node) && isNodeUntainted(&node) + return IsNodeSchedulable(logger, &node) && isNodeUntainted(logger, &node) }) if len(nodes.Items) == 0 { return nil, fmt.Errorf("there are currently no ready, schedulable nodes in the cluster") @@ -374,25 +376,26 @@ func GetRandomReadySchedulableNode(ctx context.Context, c clientset.Interface) ( // E.g. in tests related to nodes with gpu we care about nodes despite // presence of nvidia.com/gpu=present:NoSchedule taint func GetReadyNodesIncludingTainted(ctx context.Context, c clientset.Interface) (nodes *v1.NodeList, err error) { + logger := klog.FromContext(ctx) nodes, err = checkWaitListSchedulableNodes(ctx, c) if err != nil { return nil, fmt.Errorf("listing schedulable nodes error: %w", err) } Filter(nodes, func(node v1.Node) bool { - return IsNodeSchedulable(&node) + return IsNodeSchedulable(logger, &node) }) return nodes, nil } // isNodeUntainted tests whether a fake pod can be scheduled on "node", given its current taints. // TODO: need to discuss wether to return bool and error type -func isNodeUntainted(node *v1.Node) bool { - return isNodeUntaintedWithNonblocking(node, "") +func isNodeUntainted(logger klog.Logger, node *v1.Node) bool { + return isNodeUntaintedWithNonblocking(logger, node, "") } // isNodeUntaintedWithNonblocking tests whether a fake pod can be scheduled on "node" // but allows for taints in the list of non-blocking taints. -func isNodeUntaintedWithNonblocking(node *v1.Node, nonblockingTaints string) bool { +func isNodeUntaintedWithNonblocking(logger klog.Logger, node *v1.Node, nonblockingTaints string) bool { // Simple lookup for nonblocking taints based on comma-delimited list. nonblockingTaintsMap := map[string]struct{}{} for _, t := range strings.Split(nonblockingTaints, ",") { @@ -413,10 +416,10 @@ func isNodeUntaintedWithNonblocking(node *v1.Node, nonblockingTaints string) boo n = nodeCopy } - return toleratesTaintsWithNoScheduleNoExecuteEffects(n.Spec.Taints, nil) + return toleratesTaintsWithNoScheduleNoExecuteEffects(logger, n.Spec.Taints, nil) } -func toleratesTaintsWithNoScheduleNoExecuteEffects(taints []v1.Taint, tolerations []v1.Toleration) bool { +func toleratesTaintsWithNoScheduleNoExecuteEffects(logger klog.Logger, taints []v1.Taint, tolerations []v1.Toleration) bool { filteredTaints := []v1.Taint{} for _, taint := range taints { if taint.Effect == v1.TaintEffectNoExecute || taint.Effect == v1.TaintEffectNoSchedule { @@ -426,7 +429,8 @@ func toleratesTaintsWithNoScheduleNoExecuteEffects(taints []v1.Taint, toleration toleratesTaint := func(taint v1.Taint) bool { for _, toleration := range tolerations { - if toleration.ToleratesTaint(&taint) { + // TaintTolerationComparisonOperators feature gate will be false for e2e since the feature is in Alpha. + if toleration.ToleratesTaint(logger, &taint, false) { return true } } @@ -446,17 +450,18 @@ func toleratesTaintsWithNoScheduleNoExecuteEffects(taints []v1.Taint, toleration // IsNodeSchedulable returns true if: // 1) doesn't have "unschedulable" field set // 2) it also returns true from IsNodeReady -func IsNodeSchedulable(node *v1.Node) bool { +func IsNodeSchedulable(logger klog.Logger, node *v1.Node) bool { if node == nil { return false } - return !node.Spec.Unschedulable && IsNodeReady(node) + + return !node.Spec.Unschedulable && IsNodeReady(logger, node) } // IsNodeReady returns true if: // 1) it's Ready condition is set to true // 2) doesn't have NetworkUnavailable condition set to true -func IsNodeReady(node *v1.Node) bool { +func IsNodeReady(logger klog.Logger, node *v1.Node) bool { nodeReady := IsConditionSetAsExpected(node, v1.NodeReady, true) networkReady := isConditionUnset(node, v1.NodeNetworkUnavailable) || IsConditionSetAsExpectedSilent(node, v1.NodeNetworkUnavailable, false) @@ -467,8 +472,8 @@ func IsNodeReady(node *v1.Node) bool { // 1) doesn't have "unschedulable" field set // 2) it also returns true from IsNodeReady // 3) it also returns true from isNodeUntainted -func isNodeSchedulableWithoutTaints(node *v1.Node) bool { - return IsNodeSchedulable(node) && isNodeUntainted(node) +func isNodeSchedulableWithoutTaints(logger klog.Logger, node *v1.Node) bool { + return IsNodeSchedulable(logger, node) && isNodeUntainted(logger, node) } // hasNonblockingTaint returns true if the node contains at least diff --git a/test/e2e/framework/node/wait.go b/test/e2e/framework/node/wait.go index cf8b3d388c9..cde16d79fdf 100644 --- a/test/e2e/framework/node/wait.go +++ b/test/e2e/framework/node/wait.go @@ -19,6 +19,7 @@ package node import ( "context" "fmt" + "k8s.io/klog/v2" "regexp" "time" @@ -48,6 +49,7 @@ func WaitForReadyNodes(ctx context.Context, c clientset.Interface, size int, tim // WaitForTotalHealthy checks whether all registered nodes are ready and all required Pods are running on them. func WaitForTotalHealthy(ctx context.Context, c clientset.Interface, timeout time.Duration) error { + logger := klog.FromContext(ctx) framework.Logf("Waiting up to %v for all nodes to be ready", timeout) var notReady []v1.Node @@ -79,7 +81,7 @@ func WaitForTotalHealthy(ctx context.Context, c clientset.Interface, timeout tim } missingPodsPerNode = make(map[string][]string) for _, node := range nodes.Items { - if isNodeSchedulableWithoutTaints(&node) { + if isNodeSchedulableWithoutTaints(logger, &node) { for _, requiredPod := range requiredPerNodePods { foundRequired := false for _, presentPod := range systemPodsPerNode[node.Name] { @@ -145,6 +147,7 @@ func WaitForNodeToBeReady(ctx context.Context, c clientset.Interface, name strin } func WaitForNodeSchedulable(ctx context.Context, c clientset.Interface, name string, timeout time.Duration, wantSchedulable bool) bool { + logger := klog.FromContext(ctx) framework.Logf("Waiting up to %v for node %s to be schedulable: %t", timeout, name, wantSchedulable) for start := time.Now(); time.Since(start) < timeout; time.Sleep(poll) { node, err := c.CoreV1().Nodes().Get(ctx, name, metav1.GetOptions{}) @@ -153,7 +156,7 @@ func WaitForNodeSchedulable(ctx context.Context, c clientset.Interface, name str continue } - if IsNodeSchedulable(node) == wantSchedulable { + if IsNodeSchedulable(logger, node) == wantSchedulable { return true } } @@ -235,6 +238,7 @@ func checkWaitListSchedulableNodes(ctx context.Context, c clientset.Interface) ( // CheckReadyForTests returns a function which will return 'true' once the number of ready nodes is above the allowedNotReadyNodes threshold (i.e. to be used as a global gate for starting the tests). func CheckReadyForTests(ctx context.Context, c clientset.Interface, nonblockingTaints string, allowedNotReadyNodes, largeClusterThreshold int) func(ctx context.Context) (bool, error) { + logger := klog.FromContext(ctx) attempt := 0 return func(ctx context.Context) (bool, error) { if allowedNotReadyNodes == -1 { @@ -257,7 +261,7 @@ func CheckReadyForTests(ctx context.Context, c clientset.Interface, nonblockingT return false, terminalListNodesErr } for _, node := range allNodes.Items { - if !readyForTests(&node, nonblockingTaints) { + if !readyForTests(logger, &node, nonblockingTaints) { nodesNotReadyYet = append(nodesNotReadyYet, node) } } @@ -297,15 +301,15 @@ func CheckReadyForTests(ctx context.Context, c clientset.Interface, nonblockingT // to enter a testable state. By default this means it is schedulable, NodeReady, and untainted. // Nodes with taints nonblocking taints are permitted to have that taint and // also have their node.Spec.Unschedulable field ignored for the purposes of this function. -func readyForTests(node *v1.Node, nonblockingTaints string) bool { +func readyForTests(logger klog.Logger, node *v1.Node, nonblockingTaints string) bool { if hasNonblockingTaint(node, nonblockingTaints) { // If the node has one of the nonblockingTaints taints; just check that it is ready // and don't require node.Spec.Unschedulable to be set either way. - if !IsNodeReady(node) || !isNodeUntaintedWithNonblocking(node, nonblockingTaints) { + if !IsNodeReady(logger, node) || !isNodeUntaintedWithNonblocking(logger, node, nonblockingTaints) { return false } } else { - if !IsNodeSchedulable(node) || !isNodeUntainted(node) { + if !IsNodeSchedulable(logger, node) || !isNodeUntainted(logger, node) { return false } } diff --git a/test/e2e/framework/node/wait_test.go b/test/e2e/framework/node/wait_test.go index f917d40d6d2..062ab505412 100644 --- a/test/e2e/framework/node/wait_test.go +++ b/test/e2e/framework/node/wait_test.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" + "k8s.io/klog/v2/ktesting" ) // TestCheckReadyForTests specifically is concerned about the multi-node logic @@ -195,6 +196,7 @@ func TestCheckReadyForTests(t *testing.T) { } func TestReadyForTests(t *testing.T) { + logger, _ := ktesting.NewTestContext(t) fromVanillaNode := func(f func(*v1.Node)) *v1.Node { vanillaNode := &v1.Node{ ObjectMeta: metav1.ObjectMeta{Name: "test-node"}, @@ -269,7 +271,7 @@ func TestReadyForTests(t *testing.T) { for _, tc := range tcs { t.Run(tc.desc, func(t *testing.T) { - out := readyForTests(tc.node, tc.nonblockingTaints) + out := readyForTests(logger, tc.node, tc.nonblockingTaints) if out != tc.expected { t.Errorf("Expected %v but got %v", tc.expected, out) } diff --git a/test/e2e/windows/kubelet_stats.go b/test/e2e/windows/kubelet_stats.go index 0ce15712503..e2fc5a5c10c 100644 --- a/test/e2e/windows/kubelet_stats.go +++ b/test/e2e/windows/kubelet_stats.go @@ -19,6 +19,7 @@ package windows import ( "context" "fmt" + "k8s.io/klog/v2" "time" v1 "k8s.io/api/core/v1" @@ -210,6 +211,7 @@ var _ = sigDescribe(feature.Windows, "Kubelet-Stats", skipUnlessWindows(func() { // findWindowsNode finds a Windows node that is Ready and Schedulable func findWindowsNode(ctx context.Context, f *framework.Framework) (v1.Node, error) { + logger := klog.FromContext(ctx) selector := labels.Set{"kubernetes.io/os": "windows"}.AsSelector() nodeList, err := f.ClientSet.CoreV1().Nodes().List(ctx, metav1.ListOptions{LabelSelector: selector.String()}) @@ -220,7 +222,7 @@ func findWindowsNode(ctx context.Context, f *framework.Framework) (v1.Node, erro var targetNode v1.Node foundNode := false for _, n := range nodeList.Items { - if e2enode.IsNodeReady(&n) && e2enode.IsNodeSchedulable(&n) { + if e2enode.IsNodeReady(logger, &n) && e2enode.IsNodeSchedulable(logger, &n) { targetNode = n foundNode = true break @@ -236,6 +238,7 @@ func findWindowsNode(ctx context.Context, f *framework.Framework) (v1.Node, erro // findWindowsNodes finds all Windows nodes that are Ready and Schedulable func findWindowsNodes(ctx context.Context, f *framework.Framework) ([]v1.Node, error) { + logger := klog.FromContext(ctx) selector := labels.Set{"kubernetes.io/os": "windows"}.AsSelector() nodeList, err := f.ClientSet.CoreV1().Nodes().List(ctx, metav1.ListOptions{LabelSelector: selector.String()}) @@ -245,7 +248,7 @@ func findWindowsNodes(ctx context.Context, f *framework.Framework) ([]v1.Node, e var targetNodes []v1.Node for _, n := range nodeList.Items { - if e2enode.IsNodeReady(&n) && e2enode.IsNodeSchedulable(&n) { + if e2enode.IsNodeReady(logger, &n) && e2enode.IsNodeSchedulable(logger, &n) { targetNodes = append(targetNodes, n) } } diff --git a/test/e2e_node/util.go b/test/e2e_node/util.go index f2ce02d6dc0..77d03f17560 100644 --- a/test/e2e_node/util.go +++ b/test/e2e_node/util.go @@ -239,10 +239,11 @@ func getLocalNode(ctx context.Context, f *framework.Framework) *v1.Node { // the caller decide. The check is intentionally done like `getLocalNode` does. // Note `getLocalNode` aborts (as in ginkgo.Expect) the test implicitly if the worker node is not ready. func getLocalTestNode(ctx context.Context, f *framework.Framework) (*v1.Node, bool) { + logger := klog.FromContext(ctx) node, err := f.ClientSet.CoreV1().Nodes().Get(ctx, framework.TestContext.NodeName, metav1.GetOptions{}) framework.ExpectNoError(err) - ready := e2enode.IsNodeReady(node) - schedulable := e2enode.IsNodeSchedulable(node) + ready := e2enode.IsNodeReady(logger, node) + schedulable := e2enode.IsNodeSchedulable(logger, node) framework.Logf("node %q ready=%v schedulable=%v", node.Name, ready, schedulable) return node, ready && schedulable } diff --git a/test/integration/scheduler/eventhandler/eventhandler_test.go b/test/integration/scheduler/eventhandler/eventhandler_test.go index 08f5a90d398..3a38f31dbf7 100644 --- a/test/integration/scheduler/eventhandler/eventhandler_test.go +++ b/test/integration/scheduler/eventhandler/eventhandler_test.go @@ -30,6 +30,7 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/component-helpers/scheduling/corev1" + "k8s.io/klog/v2" configv1 "k8s.io/kube-scheduler/config/v1" fwk "k8s.io/kube-scheduler/framework" "k8s.io/kubernetes/pkg/features" @@ -55,12 +56,13 @@ func (pl *fooPlugin) Name() string { } func (pl *fooPlugin) Filter(ctx context.Context, state fwk.CycleState, pod *v1.Pod, nodeInfo fwk.NodeInfo) *fwk.Status { + logger := klog.FromContext(ctx) taints := nodeInfo.Node().Spec.Taints if len(taints) == 0 { return nil } - if corev1.TolerationsTolerateTaint(pod.Spec.Tolerations, &nodeInfo.Node().Spec.Taints[0]) { + if corev1.TolerationsTolerateTaint(logger, pod.Spec.Tolerations, &nodeInfo.Node().Spec.Taints[0], utilfeature.DefaultFeatureGate.Enabled(features.TaintTolerationComparisonOperators)) { return nil } return fwk.NewStatus(fwk.Unschedulable) diff --git a/test/integration/scheduler/filters/filters_test.go b/test/integration/scheduler/filters/filters_test.go index 00c845ad061..8e142ecc452 100644 --- a/test/integration/scheduler/filters/filters_test.go +++ b/test/integration/scheduler/filters/filters_test.go @@ -1495,9 +1495,238 @@ func TestTaintTolerationFilter(t *testing.T) { }).Container(pause).Obj(), fit: true, }, + { + name: "Pod with Gt toleration matches node taint with greater value", + nodes: []*v1.Node{ + st.MakeNode().Name("node-gt-1"). + Taints([]v1.Taint{ + { + Key: "node.example.com/priority-level", + Value: "999", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj(), + }, + incomingPod: st.MakePod().Name("pod-gt-1"). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/priority-level", + Operator: v1.TolerationOpGt, + Value: "900", + Effect: v1.TaintEffectNoSchedule, + }, + }).Container(pause). + Obj(), + fit: true, + }, + { + name: "Pod with Gt toleration does not match node taint with lesser value", + nodes: []*v1.Node{ + st.MakeNode().Name("node-gt-2"). + Taints([]v1.Taint{ + { + Key: "node.example.com/priority-level", + Value: "850", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj(), + }, + incomingPod: st.MakePod().Name("pod-gt-2"). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/priority-level", + Operator: v1.TolerationOpGt, + Value: "900", + Effect: v1.TaintEffectNoSchedule, + }, + }).Container(pause). + Obj(), + fit: false, + }, + { + name: "Pod with Lt toleration matches node taint with lesser value", + nodes: []*v1.Node{ + st.MakeNode().Name("node-lt-1"). + Taints([]v1.Taint{ + { + Key: "node.example.com/error-rate", + Value: "5", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj(), + }, + incomingPod: st.MakePod().Name("pod-lt-1"). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/error-rate", + Operator: v1.TolerationOpLt, + Value: "10", + Effect: v1.TaintEffectNoSchedule, + }, + }).Container(pause). + Obj(), + fit: true, + }, + { + name: "Pod with Lt toleration does not match node taint with greater value", + nodes: []*v1.Node{ + st.MakeNode().Name("node-lt-2"). + Taints([]v1.Taint{ + { + Key: "node.example.com/error-rate", + Value: "15", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj(), + }, + incomingPod: st.MakePod().Name("pod-lt-2"). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/error-rate", + Operator: v1.TolerationOpLt, + Value: "10", + Effect: v1.TaintEffectNoSchedule, + }, + }).Container(pause). + Obj(), + fit: false, + }, + { + name: "Pod with mixed tolerations including Gt operator", + nodes: []*v1.Node{ + st.MakeNode().Name("node-mixed"). + Taints([]v1.Taint{ + { + Key: "node.example.com/priority-level", + Value: "950", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "node.example.com/zone", + Value: "west", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj(), + }, + incomingPod: st.MakePod().Name("pod-mixed"). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/priority-level", + Operator: v1.TolerationOpGt, + Value: "900", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "node.example.com/zone", + Operator: v1.TolerationOpEqual, + Value: "west", + Effect: v1.TaintEffectNoSchedule, + }, + }).Container(pause). + Obj(), + fit: true, + }, + { + name: "Pod with Gt toleration with negative values", + nodes: []*v1.Node{ + st.MakeNode().Name("node-negative"). + Taints([]v1.Taint{ + { + Key: "node.example.com/temperature", + Value: "10", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj(), + }, + incomingPod: st.MakePod().Name("pod-negative"). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/temperature", + Operator: v1.TolerationOpGt, + Value: "-20", + Effect: v1.TaintEffectNoSchedule, + }, + }).Container(pause). + Obj(), + fit: true, + }, + { + name: "Pod with Lt toleration with zero values", + nodes: []*v1.Node{ + st.MakeNode().Name("node-zero"). + Taints([]v1.Taint{ + { + Key: "node.example.com/latency", + Value: "0", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj(), + }, + incomingPod: st.MakePod().Name("pod-zero"). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/latency", + Operator: v1.TolerationOpLt, + Value: "5", + Effect: v1.TaintEffectNoSchedule, + }, + }).Container(pause). + Obj(), + fit: true, // 0 < 5 + }, + { + name: "Pod with Gt toleration and empty Effect matches any taint effect", + nodes: []*v1.Node{ + st.MakeNode().Name("node-any-effect"). + Taints([]v1.Taint{ + { + Key: "node.example.com/priority-level", + Value: "999", + Effect: v1.TaintEffectPreferNoSchedule, + }, + }).Obj(), + }, + incomingPod: st.MakePod().Name("pod-any-effect"). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/priority-level", + Operator: v1.TolerationOpGt, + Value: "900", + }, + }).Container(pause). + Obj(), + fit: true, + }, + { + name: "Pod with Lt toleration with large numbers", + nodes: []*v1.Node{ + st.MakeNode().Name("node-large"). + Taints([]v1.Taint{ + { + Key: "node.example.com/memory", + Value: "999999999", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj(), + }, + incomingPod: st.MakePod().Name("pod-large"). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/memory", + Operator: v1.TolerationOpLt, + Value: "1000000000", + Effect: v1.TaintEffectNoSchedule, + }, + }).Container(pause). + Obj(), + fit: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Enable the TaintTolerationComparisonOperators feature gate for Gt/Lt tests + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TaintTolerationComparisonOperators, true) + testCtx := initTest(t, "taint-toleration-filter") cs := testCtx.ClientSet ns := testCtx.NS.Name diff --git a/test/integration/scheduler/scheduler_test.go b/test/integration/scheduler/scheduler_test.go index 2b07119c2a2..f862dbe1dfb 100644 --- a/test/integration/scheduler/scheduler_test.go +++ b/test/integration/scheduler/scheduler_test.go @@ -32,10 +32,13 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" + utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" + featuregatetesting "k8s.io/component-base/featuregate/testing" configv1 "k8s.io/kube-scheduler/config/v1" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/scheduler" configtesting "k8s.io/kubernetes/pkg/scheduler/apis/config/testing" st "k8s.io/kubernetes/pkg/scheduler/testing" @@ -1024,3 +1027,222 @@ func TestHostPorts(t *testing.T) { }) } } + +// TestTaintTolerationGtLtIntegration tests integration scenarios for Gt/Lt toleration operators +// The test verifies that unschedulable pods are re-queued when taint values change. +// 1. Create node1 with dedicated taint and node2 with low priority taint +// 2. Wait for scheduler to observe both nodes +// 3. Create pod1 that tolerates node1's taint; it schedules on node1 +// 4. Create pod2 with Gt toleration for priority; it's unschedulable (doesn't tolerate node1, node2's priority too low) +// 5. Update node2's taint to acceptable priority; pod2 schedules on node2 +// 6. Create node3 with high error-rate taint +// 7. Create pod3 with Lt toleration for error-rate; it's unschedulable (node3's error-rate too high) +// 8. Update node3's taint to acceptable error-rate; pod3 schedules on node3 +func TestTaintTolerationGtLtIntegration(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TaintTolerationComparisonOperators, true) + + testCtx := testutils.InitTestSchedulerWithNS(t, "gt-lt-integration") + + goodCondition := v1.NodeCondition{ + Type: v1.NodeReady, + Status: v1.ConditionTrue, + Reason: "schedulable condition", + LastHeartbeatTime: metav1.Time{Time: time.Now()}, + } + + // 1. Create node1 with dedicated taint + node1 := st.MakeNode().Name("node1"). + Capacity(map[v1.ResourceName]string{v1.ResourceCPU: "1"}). + Taints([]v1.Taint{ + { + Key: "node.example.com/dedicated", + Value: "special", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj() + node1.Status.Conditions = []v1.NodeCondition{goodCondition} + _, err := testutils.CreateNode(testCtx.ClientSet, node1) + if err != nil { + t.Fatalf("Failed to create node1: %v", err) + } + + // Create node2 with a taint that pod2 can't tolerate (priority too low) + node2 := st.MakeNode().Name("node2"). + Capacity(map[v1.ResourceName]string{v1.ResourceCPU: "1"}). + Taints([]v1.Taint{ + { + Key: "node.example.com/priority-level", + Value: "850", // Too low for pod2's Gt 900 requirement + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj() + node2.Status.Conditions = []v1.NodeCondition{goodCondition} + node2, err = testutils.CreateNode(testCtx.ClientSet, node2) + if err != nil { + t.Fatalf("Failed to create node2: %v", err) + } + + // 2. Wait for scheduler to observe both nodes + if err := testutils.WaitForNodesInCache(testCtx.Ctx, testCtx.Scheduler, 2); err != nil { + t.Fatalf("Failed to wait for nodes in cache: %v", err) + } + + // 3. Create pod1 that tolerates node1's taint and should schedule on node1 + pod1 := st.MakePod().Name("pod1").Namespace(testCtx.NS.Name). + Container("busybox"). + Req(map[v1.ResourceName]string{v1.ResourceCPU: "900m"}). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/dedicated", + Operator: v1.TolerationOpEqual, + Value: "special", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj() + pod1, err = testutils.CreatePausePod(testCtx.ClientSet, pod1) + if err != nil { + t.Fatalf("Failed to create pod1: %v", err) + } + + err = testutils.WaitForPodToSchedule(testCtx.Ctx, testCtx.ClientSet, pod1) + if err != nil { + t.Fatalf("Failed to schedule pod1: %v", err) + } + + // 4. Create pod2 with Gt toleration, it should be unschedulable as it can't tolerate node1's taint, and node2's taint value is too low + pod2 := st.MakePod().Name("pod2").Namespace(testCtx.NS.Name). + Container("busybox"). + Req(map[v1.ResourceName]string{v1.ResourceCPU: "200m"}). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/priority-level", + Operator: v1.TolerationOpGt, + Value: "900", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj() + pod2, err = testutils.CreatePausePod(testCtx.ClientSet, pod2) + if err != nil { + t.Fatalf("Failed to create pod2: %v", err) + } + + err = testutils.WaitForPodUnschedulable(testCtx.Ctx, testCtx.ClientSet, pod2) + if err != nil { + t.Fatalf("Failed to verify pod2 is unschedulable: %v", err) + } + + // 5. Update the taint value on node2 to acceptable priority; pod2 should now schedule on node2 + node2, err = testCtx.ClientSet.CoreV1().Nodes().Get(testCtx.Ctx, node2.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get node2: %v", err) + } + + // Update taint to have acceptable priority value + for i := range node2.Spec.Taints { + if node2.Spec.Taints[i].Key == "node.example.com/priority-level" { + node2.Spec.Taints[i].Value = "950" + break + } + } + + _, err = testCtx.ClientSet.CoreV1().Nodes().Update(testCtx.Ctx, node2, metav1.UpdateOptions{}) + if err != nil { + t.Fatalf("Failed to update node2 taint: %v", err) + } + + // Verify pod2 now schedules on node2 + err = testutils.WaitForPodToSchedule(testCtx.Ctx, testCtx.ClientSet, pod2) + if err != nil { + t.Fatalf("Failed to schedule pod2: %v", err) + } + + scheduledPod2, err := testCtx.ClientSet.CoreV1().Pods(testCtx.NS.Name).Get(testCtx.Ctx, pod2.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get scheduled pod2: %v", err) + } + + if scheduledPod2.Spec.NodeName != node2.Name { + t.Errorf("Pod2 scheduled on unexpected node: got %s, expected %s", scheduledPod2.Spec.NodeName, node2.Name) + } + + // 6. Test Lt operator scenario - create node3 with high error rate + node3 := st.MakeNode().Name("node3"). + Capacity(map[v1.ResourceName]string{v1.ResourceCPU: "1"}). + Taints([]v1.Taint{ + { + Key: "node.example.com/error-rate", + Value: "15", // Too high for pod3's Lt 10 requirement + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj() + node3.Status.Conditions = []v1.NodeCondition{goodCondition} + node3, err = testutils.CreateNode(testCtx.ClientSet, node3) + if err != nil { + t.Fatalf("Failed to create node3: %v", err) + } + + // Wait for scheduler to observe node3 + if err := testutils.WaitForNodesInCache(testCtx.Ctx, testCtx.Scheduler, 3); err != nil { + t.Fatalf("Failed to wait for nodes in cache: %v", err) + } + + // 7. Create pod3 with Lt toleration, it should be unschedulable as node3's error rate is too high + pod3 := st.MakePod().Name("pod3").Namespace(testCtx.NS.Name). + Container("busybox"). + Req(map[v1.ResourceName]string{v1.ResourceCPU: "100m"}). + Tolerations([]v1.Toleration{ + { + Key: "node.example.com/error-rate", + Operator: v1.TolerationOpLt, + Value: "10", + Effect: v1.TaintEffectNoSchedule, + }, + }).Obj() + pod3, err = testutils.CreatePausePod(testCtx.ClientSet, pod3) + if err != nil { + t.Fatalf("Failed to create pod3: %v", err) + } + + err = testutils.WaitForPodUnschedulable(testCtx.Ctx, testCtx.ClientSet, pod3) + if err != nil { + t.Fatalf("Failed to verify pod3 is unschedulable: %v", err) + } + + // 8. Update node3 taint to acceptable error rate; pod3 should now schedule + node3, err = testCtx.ClientSet.CoreV1().Nodes().Get(testCtx.Ctx, node3.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get node3: %v", err) + } + + for i := range node3.Spec.Taints { + if node3.Spec.Taints[i].Key == "node.example.com/error-rate" { + node3.Spec.Taints[i].Value = "5" + break + } + } + + _, err = testCtx.ClientSet.CoreV1().Nodes().Update(testCtx.Ctx, node3, metav1.UpdateOptions{}) + if err != nil { + t.Fatalf("Failed to update node3 taint: %v", err) + } + + err = testutils.WaitForPodToSchedule(testCtx.Ctx, testCtx.ClientSet, pod3) + if err != nil { + t.Fatalf("Failed to schedule pod3: %v", err) + } + + scheduledPod3, err := testCtx.ClientSet.CoreV1().Pods(testCtx.NS.Name).Get(testCtx.Ctx, pod3.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get scheduled pod3: %v", err) + } + + if scheduledPod3.Spec.NodeName != node3.Name { + t.Errorf("Pod3 scheduled on unexpected node: got %s, expected %s", scheduledPod3.Spec.NodeName, node3.Name) + } + + // Cleanup pods + defer testutils.CleanupPods(testCtx.Ctx, testCtx.ClientSet, t, []*v1.Pod{pod1, pod2, pod3}) + if err := testCtx.ClientSet.CoreV1().Nodes().DeleteCollection(testCtx.Ctx, metav1.DeleteOptions{}, metav1.ListOptions{}); err != nil { + t.Errorf("error whiling deleting nodes, error: %v", err) + } +} diff --git a/test/integration/scheduler/scoring/priorities_test.go b/test/integration/scheduler/scoring/priorities_test.go index 6cfe12cf14e..572a7748c10 100644 --- a/test/integration/scheduler/scoring/priorities_test.go +++ b/test/integration/scheduler/scoring/priorities_test.go @@ -67,6 +67,9 @@ var ( ignorePolicy = v1.NodeInclusionPolicyIgnore honorPolicy = v1.NodeInclusionPolicyHonor taints = []v1.Taint{{Key: v1.TaintNodeUnschedulable, Value: "", Effect: v1.TaintEffectPreferNoSchedule}} + + priorityLowTaint = v1.Taint{Key: "node.example.com/priority-class", Value: "800", Effect: v1.TaintEffectNoSchedule} + priorityHighTaint = v1.Taint{Key: "node.example.com/priority-class", Value: "999", Effect: v1.TaintEffectPreferNoSchedule} ) const ( @@ -834,9 +837,29 @@ func TestTaintTolerationScoring(t *testing.T) { }, expectedNodesName: sets.New("node-2"), }, + { + name: "pod with Gt toleration prefers nodes with matching numeric taints", + podTolerations: []v1.Toleration{ + { + Key: "node.example.com/priority-class", + Operator: v1.TolerationOpGt, + Value: "900", + Effect: v1.TaintEffectPreferNoSchedule, + }, + }, + nodes: []*v1.Node{ + st.MakeNode().Name("node-gt-low"). + Taints([]v1.Taint{priorityLowTaint}).Obj(), + st.MakeNode().Name("node-gt-high"). + Taints([]v1.Taint{priorityHighTaint}).Obj(), + }, + expectedNodesName: sets.New("node-gt-high"), + }, } for i, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Enable the TaintTolerationComparisonOperators feature gate for Gt/Lt tests + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TaintTolerationComparisonOperators, true) testCtx := initTestSchedulerForScoringTests(t, tainttoleration.Name, tainttoleration.Name) for _, n := range tt.nodes { diff --git a/test/integration/scheduler/taint/taint_test.go b/test/integration/scheduler/taint/taint_test.go index 0777a7e140a..a2fe4f288b2 100644 --- a/test/integration/scheduler/taint/taint_test.go +++ b/test/integration/scheduler/taint/taint_test.go @@ -26,10 +26,13 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/controller/nodelifecycle" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction" pluginapi "k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction" testutils "k8s.io/kubernetes/test/integration/util" @@ -72,8 +75,6 @@ func TestTaintNodeByCondition(t *testing.T) { admission.SetExternalKubeClientSet(externalClientset) admission.SetExternalKubeInformerFactory(externalInformers) - testCtx = testutils.InitTestScheduler(t, testCtx) - cs := testCtx.ClientSet nsName := testCtx.NS.Name @@ -101,11 +102,9 @@ func TestTaintNodeByCondition(t *testing.T) { // Waiting for all controllers to sync externalInformers.Start(testCtx.Ctx.Done()) externalInformers.WaitForCacheSync(testCtx.Ctx.Done()) - testutils.SyncSchedulerInformerFactory(testCtx) - // Run all controllers + // Run node lifecycle controller go nc.Run(testCtx.Ctx) - go testCtx.Scheduler.Run(testCtx.Ctx) // ------------------------------------------- // Test TaintNodeByCondition feature. @@ -157,6 +156,53 @@ func TestTaintNodeByCondition(t *testing.T) { Effect: v1.TaintEffectNoSchedule, } + priorityClassTaint := v1.Taint{Key: "node.example.com/priority-class", Value: "950", Effect: v1.TaintEffectNoSchedule} + priorityClassPreferTaint := v1.Taint{Key: "node.example.com/priority-class", Value: "950", Effect: v1.TaintEffectPreferNoSchedule} + errorRateTaint := v1.Taint{Key: "node.example.com/error-rate", Value: "5", Effect: v1.TaintEffectNoSchedule} + cpuUtilizationTaint := v1.Taint{Key: "node.example.com/cpu-utilization", Value: "75", Effect: v1.TaintEffectNoExecute} + + priorityClassGtToleration := v1.Toleration{ + Key: "node.example.com/priority-class", + Operator: v1.TolerationOpGt, + Value: "900", + Effect: v1.TaintEffectNoSchedule, + } + + errorRateLtToleration := v1.Toleration{ + Key: "node.example.com/error-rate", + Operator: v1.TolerationOpLt, + Value: "10", + Effect: v1.TaintEffectNoSchedule, + } + + cpuUtilizationGtToleration := v1.Toleration{ + Key: "node.example.com/cpu-utilization", + Operator: v1.TolerationOpGt, + Value: "50", + Effect: v1.TaintEffectNoExecute, + } + + cpuUtilizationLtToleration := v1.Toleration{ + Key: "node.example.com/cpu-utilization", + Operator: v1.TolerationOpLt, + Value: "90", + Effect: v1.TaintEffectNoExecute, + } + + priorityClassGtPreferToleration := v1.Toleration{ + Key: "node.example.com/priority-class", + Operator: v1.TolerationOpGt, + Value: "900", + Effect: v1.TaintEffectPreferNoSchedule, + } + + errorRateLtPreferToleration := v1.Toleration{ + Key: "node.example.com/error-rate", + Operator: v1.TolerationOpLt, + Value: "10", + Effect: v1.TaintEffectPreferNoSchedule, + } + bestEffortPod := newPod(nsName, "besteffort-pod", nil, nil) burstablePod := newPod(nsName, "burstable-pod", podRes, nil) guaranteePod := newPod(nsName, "guarantee-pod", podRes, podRes) @@ -169,12 +215,13 @@ func TestTaintNodeByCondition(t *testing.T) { // switch to table driven testings tests := []struct { - name string - existingTaints []v1.Taint - nodeConditions []v1.NodeCondition - unschedulable bool - expectedTaints []v1.Taint - pods []podCase + name string + existingTaints []v1.Taint + nodeConditions []v1.NodeCondition + unschedulable bool + expectedTaints []v1.Taint + pods []podCase + enableTaintTolerationComparisonOperatorsFG []bool }{ { name: "not-ready node", @@ -499,67 +546,219 @@ func TestTaintNodeByCondition(t *testing.T) { }, }, }, + { + name: "node with numeric priority-class taint - pods with Gt toleration", + existingTaints: []v1.Taint{priorityClassTaint}, + nodeConditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + expectedTaints: []v1.Taint{priorityClassTaint}, + pods: []podCase{ + { + pod: bestEffortPod, + fits: false, + }, + { + pod: bestEffortPod, + tolerations: []v1.Toleration{priorityClassGtToleration}, + fits: true, + }, + }, + enableTaintTolerationComparisonOperatorsFG: []bool{true}, + }, + { + name: "node with numeric error-rate taint - pods with Lt toleration", + existingTaints: []v1.Taint{errorRateTaint}, + nodeConditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + expectedTaints: []v1.Taint{errorRateTaint}, + pods: []podCase{ + { + pod: burstablePod, + fits: false, + }, + { + pod: burstablePod, + tolerations: []v1.Toleration{errorRateLtToleration}, + fits: true, + }, + }, + enableTaintTolerationComparisonOperatorsFG: []bool{true}, + }, + { + name: "node with multiple numeric taints - mixed tolerations", + existingTaints: []v1.Taint{priorityClassTaint, errorRateTaint}, + nodeConditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + expectedTaints: []v1.Taint{priorityClassTaint, errorRateTaint}, + pods: []podCase{ + { + pod: guaranteePod, + fits: false, + }, + { + pod: guaranteePod, + tolerations: []v1.Toleration{priorityClassGtToleration}, + fits: false, + }, + { + pod: guaranteePod, + tolerations: []v1.Toleration{ + priorityClassGtToleration, + errorRateLtToleration, + }, + fits: true, + }, + }, + enableTaintTolerationComparisonOperatorsFG: []bool{true}, + }, + { + name: "node with numeric taint - pods with Lt or Gt tolerations, and NoExecute effect", + existingTaints: []v1.Taint{cpuUtilizationTaint}, + nodeConditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + expectedTaints: []v1.Taint{cpuUtilizationTaint}, + pods: []podCase{ + { + pod: guaranteePod, + fits: false, + }, + { + pod: guaranteePod, + tolerations: []v1.Toleration{cpuUtilizationLtToleration}, + fits: true, + }, + { + pod: guaranteePod, + tolerations: []v1.Toleration{cpuUtilizationGtToleration}, + fits: true, + }, + }, + enableTaintTolerationComparisonOperatorsFG: []bool{true}, + }, + { + name: "node with numeric taint - pods with PreferNoSchedule effect and Gt toleration", + existingTaints: []v1.Taint{priorityClassPreferTaint}, + nodeConditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + expectedTaints: []v1.Taint{priorityClassPreferTaint}, + pods: []podCase{ + { + pod: bestEffortPod, + fits: true, + }, + { + pod: bestEffortPod, + tolerations: []v1.Toleration{priorityClassGtPreferToleration}, + fits: true, + }, + { + pod: bestEffortPod, + tolerations: []v1.Toleration{errorRateLtPreferToleration}, + fits: true, + }, + }, + enableTaintTolerationComparisonOperatorsFG: []bool{true}, + }, } for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - node := &v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - Spec: v1.NodeSpec{ - Unschedulable: test.unschedulable, - Taints: test.existingTaints, - }, - Status: v1.NodeStatus{ - Capacity: nodeRes, - Allocatable: nodeRes, - Conditions: test.nodeConditions, - }, - } + featureGateEnabled := []bool{true, false} + if len(test.enableTaintTolerationComparisonOperatorsFG) != 0 { + featureGateEnabled = test.enableTaintTolerationComparisonOperatorsFG + } + for _, enabled := range featureGateEnabled { + t.Run(fmt.Sprintf("%s (TaintToleration Comparison Operators enabled: %v)", test.name, enabled), func(t *testing.T) { + featuregatetesting.SetFeatureGatesDuringTest(t, utilfeature.DefaultFeatureGate, featuregatetesting.FeatureOverrides{ + features.TaintTolerationComparisonOperators: enabled, + }) - if _, err := cs.CoreV1().Nodes().Create(testCtx.Ctx, node, metav1.CreateOptions{}); err != nil { - t.Errorf("Failed to create node, err: %v", err) - } - if err := testutils.WaitForNodeTaints(testCtx.Ctx, cs, node, test.expectedTaints); err != nil { - node, err = cs.CoreV1().Nodes().Get(testCtx.Ctx, node.Name, metav1.GetOptions{}) - if err != nil { - t.Errorf("Failed to get node <%s>", node.Name) + testCtx := testutils.InitTestScheduler(t, testCtx) + testutils.SyncSchedulerInformerFactory(testCtx) + go testCtx.Scheduler.Run(testCtx.SchedulerCtx) + + node := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node-1", + }, + Spec: v1.NodeSpec{ + Unschedulable: test.unschedulable, + Taints: test.existingTaints, + }, + Status: v1.NodeStatus{ + Capacity: nodeRes, + Allocatable: nodeRes, + Conditions: test.nodeConditions, + }, } - t.Errorf("Failed to taint node <%s>, expected: %v, got: %v, err: %v", node.Name, test.expectedTaints, node.Spec.Taints, err) - } + if _, err := cs.CoreV1().Nodes().Create(testCtx.Ctx, node, metav1.CreateOptions{}); err != nil { + t.Errorf("Failed to create node, err: %v", err) + } + if err := testutils.WaitForNodeTaints(testCtx.Ctx, cs, node, test.expectedTaints); err != nil { + node, err = cs.CoreV1().Nodes().Get(testCtx.Ctx, node.Name, metav1.GetOptions{}) + if err != nil { + t.Errorf("Failed to get node <%s>", node.Name) + } - var pods []*v1.Pod - for i, p := range test.pods { - pod := p.pod.DeepCopy() - pod.Name = fmt.Sprintf("%s-%d", pod.Name, i) - pod.Spec.Tolerations = p.tolerations - - createdPod, err := cs.CoreV1().Pods(pod.Namespace).Create(testCtx.Ctx, pod, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("Failed to create pod %s/%s, error: %v", - pod.Namespace, pod.Name, err) + t.Errorf("Failed to taint node <%s>, expected: %v, got: %v, err: %v", node.Name, test.expectedTaints, node.Spec.Taints, err) } - pods = append(pods, createdPod) + var pods []*v1.Pod + for i, p := range test.pods { + pod := p.pod.DeepCopy() + pod.Name = fmt.Sprintf("%s-%d", pod.Name, i) + pod.Spec.Tolerations = p.tolerations - if p.fits { - if err := testutils.WaitForPodToSchedule(testCtx.Ctx, cs, createdPod); err != nil { - t.Errorf("Failed to schedule pod %s/%s on the node, err: %v", + createdPod, err := cs.CoreV1().Pods(pod.Namespace).Create(testCtx.Ctx, pod, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to create pod %s/%s, error: %v", pod.Namespace, pod.Name, err) } - } else { - if err := testutils.WaitForPodUnschedulable(testCtx.Ctx, cs, createdPod); err != nil { - t.Errorf("Unschedulable pod %s/%s gets scheduled on the node, err: %v", - pod.Namespace, pod.Name, err) + + pods = append(pods, createdPod) + + if p.fits { + if err := testutils.WaitForPodToSchedule(testCtx.Ctx, cs, createdPod); err != nil { + t.Errorf("Failed to schedule pod %s/%s on the node, err: %v", + pod.Namespace, pod.Name, err) + } + } else { + if err := testutils.WaitForPodUnschedulable(testCtx.Ctx, cs, createdPod); err != nil { + t.Errorf("Unschedulable pod %s/%s gets scheduled on the node, err: %v", + pod.Namespace, pod.Name, err) + } } } - } - testutils.CleanupPods(testCtx.Ctx, cs, t, pods) - testutils.CleanupNodes(cs, t) - testutils.WaitForSchedulerCacheCleanup(testCtx.Ctx, testCtx.Scheduler, t) - }) + testutils.CleanupPods(testCtx.Ctx, cs, t, pods) + testutils.CleanupNodes(cs, t) + testutils.WaitForSchedulerCacheCleanup(testCtx.Ctx, testCtx.Scheduler, t) + + // Clean up scheduler context + if testCtx.SchedulerCloseFn != nil { + testCtx.SchedulerCloseFn() + } + }) + } } }