Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow updates to pod tolerations. #41818

Merged
merged 1 commit into from
Feb 26, 2017
Merged

Allow updates to pod tolerations. #41818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 29 additions & 1 deletion pkg/api/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say "modified or deleted" but you can fix in a followup PR

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{}
Expand Down Expand Up @@ -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
Expand Down
152 changes: 151 additions & 1 deletion pkg/api/validation/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4506,7 +4506,6 @@ func TestValidatePodUpdate(t *testing.T) {
false,
"activeDeadlineSeconds change to nil from positive",
},

{
api.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion plugin/pkg/scheduler/algorithm/predicates/predicates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down