diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 880f15fb4c8..872f524741d 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -1848,6 +1848,29 @@ func validateTaintEffect(effect *api.TaintEffect, allowEmpty bool, fldPath *fiel return allErrors } +// validateOnlyAddedTolerations validates updated pod tolerations. +func validateOnlyAddedTolerations(newTolerations []api.Toleration, oldTolerations []api.Toleration, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + for _, old := range oldTolerations { + found := false + old.TolerationSeconds = nil + for _, new := range newTolerations { + new.TolerationSeconds = nil + if reflect.DeepEqual(old, new) { + found = true + break + } + } + if !found { + allErrs = append(allErrs, field.Forbidden(fldPath, "existing toleration can not be modified except its tolerationSeconds")) + return allErrs + } + } + + allErrs = append(allErrs, validateTolerations(newTolerations, fldPath)...) + return allErrs +} + // validateTolerations tests if given tolerations have valid data. func validateTolerations(tolerations []api.Toleration, fldPath *field.Path) field.ErrorList { allErrors := field.ErrorList{} @@ -2348,9 +2371,14 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList { activeDeadlineSeconds := *oldPod.Spec.ActiveDeadlineSeconds mungedPod.Spec.ActiveDeadlineSeconds = &activeDeadlineSeconds } + + // Allow only additions to tolerations updates. + mungedPod.Spec.Tolerations = oldPod.Spec.Tolerations + allErrs = append(allErrs, validateOnlyAddedTolerations(newPod.Spec.Tolerations, oldPod.Spec.Tolerations, specPath.Child("tolerations"))...) + if !apiequality.Semantic.DeepEqual(mungedPod.Spec, oldPod.Spec) { //TODO: Pinpoint the specific field that causes the invalid error after we have strategic merge diff - allErrs = append(allErrs, field.Forbidden(specPath, "pod updates may not change fields other than `containers[*].image` or `spec.activeDeadlineSeconds`")) + allErrs = append(allErrs, field.Forbidden(specPath, "pod updates may not change fields other than `containers[*].image` or `spec.activeDeadlineSeconds` or `spec.tolerations` (only additions to existing tolerations)")) } return allErrs diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 92f3b0ef0f1..f7fac9351a4 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -4506,7 +4506,6 @@ func TestValidatePodUpdate(t *testing.T) { false, "activeDeadlineSeconds change to nil from positive", }, - { api.Pod{ ObjectMeta: metav1.ObjectMeta{Name: "foo"}, @@ -4587,6 +4586,157 @@ func TestValidatePodUpdate(t *testing.T) { true, "bad label change", }, + { + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "node1", + Tolerations: []api.Toleration{{Key: "key1", Value: "value2"}}, + }, + }, + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "node1", + Tolerations: []api.Toleration{{Key: "key1", Value: "value1"}}, + }, + }, + false, + "existing toleration value modified in pod spec updates", + }, + { + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "node1", + Tolerations: []api.Toleration{{Key: "key1", Value: "value2", Operator: "Equal", Effect: "NoExecute", TolerationSeconds: nil}}, + }, + }, + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "node1", + Tolerations: []api.Toleration{{Key: "key1", Value: "value1", Operator: "Equal", Effect: "NoExecute", TolerationSeconds: &[]int64{10}[0]}}, + }, + }, + false, + "existing toleration value modified in pod spec updates with modified tolerationSeconds", + }, + { + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "node1", + Tolerations: []api.Toleration{{Key: "key1", Value: "value1", Operator: "Equal", Effect: "NoExecute", TolerationSeconds: &[]int64{10}[0]}}, + }, + }, + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "node1", + Tolerations: []api.Toleration{{Key: "key1", Value: "value1", Operator: "Equal", Effect: "NoExecute", TolerationSeconds: &[]int64{20}[0]}}, + }}, + true, + "modified tolerationSeconds in existing toleration value in pod spec updates", + }, + { + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + Tolerations: []api.Toleration{{Key: "key1", Value: "value2"}}, + }, + }, + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "", + Tolerations: []api.Toleration{{Key: "key1", Value: "value1"}}, + }, + }, + false, + "toleration modified in updates to an unscheduled pod", + }, + { + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "node1", + Tolerations: []api.Toleration{{Key: "key1", Value: "value1"}}, + }, + }, + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "node1", + Tolerations: []api.Toleration{{Key: "key1", Value: "value1"}}, + }, + }, + true, + "tolerations unmodified in updates to a scheduled pod", + }, + { + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "node1", + Tolerations: []api.Toleration{ + {Key: "key1", Value: "value1", Operator: "Equal", Effect: "NoExecute", TolerationSeconds: &[]int64{20}[0]}, + {Key: "key2", Value: "value2", Operator: "Equal", Effect: "NoExecute", TolerationSeconds: &[]int64{30}[0]}, + }, + }}, + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "node1", + Tolerations: []api.Toleration{{Key: "key1", Value: "value1", Operator: "Equal", Effect: "NoExecute", TolerationSeconds: &[]int64{10}[0]}}, + }, + }, + true, + "added valid new toleration to existing tolerations in pod spec updates", + }, + { + api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: api.PodSpec{ + NodeName: "node1", + Tolerations: []api.Toleration{ + {Key: "key1", Value: "value1", Operator: "Equal", Effect: "NoExecute", TolerationSeconds: &[]int64{20}[0]}, + {Key: "key2", Value: "value2", Operator: "Equal", Effect: "NoSchedule", TolerationSeconds: &[]int64{30}[0]}, + }, + }}, + api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + NodeName: "node1", Tolerations: []api.Toleration{{Key: "key1", Value: "value1", Operator: "Equal", Effect: "NoExecute", TolerationSeconds: &[]int64{10}[0]}}, + }}, + false, + "added invalid new toleration to existing tolerations in pod spec updates", + }, } for _, test := range tests { diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates.go b/plugin/pkg/scheduler/algorithm/predicates/predicates.go index 4be7fe0ca86..f7a69cca853 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates.go @@ -1161,7 +1161,7 @@ func PodToleratesNodeTaints(pod *v1.Pod, meta interface{}, nodeInfo *schedulerca } if v1.TolerationsTolerateTaintsWithFilter(pod.Spec.Tolerations, taints, func(t *v1.Taint) bool { - // PodToleratesNodeTaints is only interested in NoSchedule taints. + // PodToleratesNodeTaints is only interested in NoSchedule and NoExecute taints. return t.Effect == v1.TaintEffectNoSchedule || t.Effect == v1.TaintEffectNoExecute }) { return true, nil, nil