github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-24 12:15:52 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #42755 from aveshagarwal/master-fix-default-toleration-seconds

Browse Source 
Automatic merge from submit-queue (batch tested with PRs 41794, 42349, 42755, 42901, 42933)

Fix DefaultTolerationSeconds admission plugin

DefaultTolerationSeconds is not working as expected. It is supposed to add default tolerations (for unreachable and notready conditions). but no pod was getting these toleration. And api server was throwing this error:

```
Mar 08 13:43:57 fedora25 hyperkube[32070]: E0308 13:43:57.769212   32070 admission.go:71] expected pod but got Pod
Mar 08 13:43:57 fedora25 hyperkube[32070]: E0308 13:43:57.789055   32070 admission.go:71] expected pod but got Pod
Mar 08 13:44:02 fedora25 hyperkube[32070]: E0308 13:44:02.006784   32070 admission.go:71] expected pod but got Pod
Mar 08 13:45:39 fedora25 hyperkube[32070]: E0308 13:45:39.754669   32070 admission.go:71] expected pod but got Pod
Mar 08 14:48:16 fedora25 hyperkube[32070]: E0308 14:48:16.673181   32070 admission.go:71] expected pod but got Pod
```

The reason for this error is that the input to admission plugins is internal api objects not versioned objects so expecting versioned object is incorrect. Due to this, no pod got desired tolerations and it always showed:

```
Tolerations: <none>
```

After this fix, the correct  tolerations are being assigned to pods as follows:

```
Tolerations:	node.alpha.kubernetes.io/notReady=:Exists:NoExecute for 300s
		node.alpha.kubernetes.io/unreachable=:Exists:NoExecute for 300s
```

@davidopp @kevin-wangzefeng @kubernetes/sig-scheduling-pr-reviews @kubernetes/sig-scheduling-bugs @derekwaynecarr 

Fixes https://github.com/kubernetes/kubernetes/issues/42716
This commit is contained in:
Kubernetes Submit Queue 2017-03-10 22:02:18 -08:00 committed by GitHub
commit 8cb14a4f7f
6 changed files with 282 additions and 101 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
pkg/api/helpers.go
View File

@ -465,6 +465,44 @@ const (
ObjectTTLAnnotationKey string = "node.alpha.kubernetes.io/ttl"
)
// AddOrUpdateTolerationInPod tries to add a toleration to the pod's toleration list.
// Returns true if something was updated, false otherwise.
func AddOrUpdateTolerationInPod(pod *Pod, toleration *Toleration) (bool, error) {
podTolerations := pod.Spec.Tolerations
var newTolerations []Toleration
updated := false
for i := range podTolerations {
if toleration.MatchToleration(&podTolerations[i]) {
if Semantic.DeepEqual(toleration, podTolerations[i]) {
return false, nil
}
newTolerations = append(newTolerations, *toleration)
updated = true
continue
}
newTolerations = append(newTolerations, podTolerations[i])
}
if !updated {
newTolerations = append(newTolerations, *toleration)
}
pod.Spec.Tolerations = newTolerations
return true, nil
}
// MatchToleration checks if the toleration matches tolerationToMatch. Tolerations are unique by <key,effect,operator,value>,
// if the two tolerations have same <key,effect,operator,value> combination, regard as they match.
// TODO: uniqueness check for tolerations in api validations.
func (t *Toleration) MatchToleration(tolerationToMatch *Toleration) bool {
return t.Key == tolerationToMatch.Key &&
t.Effect == tolerationToMatch.Effect &&
t.Operator == tolerationToMatch.Operator &&
t.Value == tolerationToMatch.Value
}
// TolerationToleratesTaint checks if the toleration tolerates the taint.
func TolerationToleratesTaint(toleration *Toleration, taint *Taint) bool {
if len(toleration.Effect) != 0 && toleration.Effect != taint.Effect {

5
plugin/pkg/admission/defaulttolerationseconds/BUILD
View File

@ -15,7 +15,6 @@ go_test(
tags = ["automanaged"],
deps = [
"//pkg/api:go_default_library",
"//pkg/api/v1:go_default_library",
"//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
"//vendor:k8s.io/apiserver/pkg/admission",
],
@ -26,8 +25,8 @@ go_library(
srcs = ["admission.go"],
tags = ["automanaged"],
deps = [
"//pkg/api/v1:go_default_library",
"//vendor:github.com/golang/glog",
"//pkg/api:go_default_library",
"//vendor:k8s.io/apimachinery/pkg/api/errors",
"//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
"//vendor:k8s.io/apiserver/pkg/admission",
],

33
plugin/pkg/admission/defaulttolerationseconds/admission.go
View File

@ -21,11 +21,10 @@ import (
"fmt"
"io"
"github.com/golang/glog"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apiserver/pkg/admission"
"k8s.io/kubernetes/pkg/api/v1"
"k8s.io/kubernetes/pkg/api"
)
var (
@ -62,28 +61,32 @@ func NewDefaultTolerationSeconds() admission.Interface {
}
func (p *plugin) Admit(attributes admission.Attributes) (err error) {
if attributes.GetResource().GroupResource() != v1.Resource("pods") {
if attributes.GetResource().GroupResource() != api.Resource("pods") {
return nil
}
pod, ok := attributes.GetObject().(*v1.Pod)
if !ok {
glog.Errorf("expected pod but got %s", attributes.GetKind().Kind)
if len(attributes.GetSubresource()) > 0 {
// only run the checks below on pods proper and not subresources
return nil
}
pod, ok := attributes.GetObject().(*api.Pod)
if !ok {
return errors.NewBadRequest(fmt.Sprintf("expected *api.Pod but got %T", attributes.GetObject()))
}
tolerations := pod.Spec.Tolerations
toleratesNodeNotReady := false
toleratesNodeUnreachable := false
for _, toleration := range tolerations {
if (toleration.Key == metav1.TaintNodeNotReady || len(toleration.Key) == 0) &&
(toleration.Effect == v1.TaintEffectNoExecute || len(toleration.Effect) == 0) {
(toleration.Effect == api.TaintEffectNoExecute || len(toleration.Effect) == 0) {
toleratesNodeNotReady = true
}
if (toleration.Key == metav1.TaintNodeUnreachable || len(toleration.Key) == 0) &&
(toleration.Effect == v1.TaintEffectNoExecute || len(toleration.Effect) == 0) {
(toleration.Effect == api.TaintEffectNoExecute || len(toleration.Effect) == 0) {
toleratesNodeUnreachable = true
}
}
@ -94,10 +97,10 @@ func (p *plugin) Admit(attributes admission.Attributes) (err error) {
}
if !toleratesNodeNotReady {
_, err := v1.AddOrUpdateTolerationInPod(pod, &v1.Toleration{
_, err := api.AddOrUpdateTolerationInPod(pod, &api.Toleration{
Key: metav1.TaintNodeNotReady,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: defaultNotReadyTolerationSeconds,
})
if err != nil {
@ -107,10 +110,10 @@ func (p *plugin) Admit(attributes admission.Attributes) (err error) {
}
if !toleratesNodeUnreachable {
_, err := v1.AddOrUpdateTolerationInPod(pod, &v1.Toleration{
_, err := api.AddOrUpdateTolerationInPod(pod, &api.Toleration{
Key: metav1.TaintNodeUnreachable,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: defaultUnreachableTolerationSeconds,
})
if err != nil {

165
plugin/pkg/admission/defaulttolerationseconds/admission_test.go
View File

@ -22,7 +22,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apiserver/pkg/admission"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/v1"
)
func TestForgivenessAdmission(t *testing.T) {
@ -35,27 +34,27 @@ func TestForgivenessAdmission(t *testing.T) {
handler := NewDefaultTolerationSeconds()
tests := []struct {
description string
requestedPod v1.Pod
expectedPod v1.Pod
requestedPod api.Pod
expectedPod api.Pod
}{
{
description: "pod has no tolerations, expect add tolerations for `notread:NoExecute` and `unreachable:NoExecute`",
requestedPod: v1.Pod{
Spec: v1.PodSpec{},
requestedPod: api.Pod{
Spec: api.PodSpec{},
},
expectedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
expectedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Key: metav1.TaintNodeNotReady,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: &defaultTolerationSeconds,
},
{
Key: metav1.TaintNodeUnreachable,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: &defaultTolerationSeconds,
},
},
@ -64,39 +63,39 @@ func TestForgivenessAdmission(t *testing.T) {
},
{
description: "pod has tolerations, but none is for taint `notread:NoExecute` or `unreachable:NoExecute`, expect add tolerations for `notread:NoExecute` and `unreachable:NoExecute`",
requestedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
requestedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Key: "foo",
Operator: v1.TolerationOpEqual,
Operator: api.TolerationOpEqual,
Value: "bar",
Effect: v1.TaintEffectNoSchedule,
Effect: api.TaintEffectNoSchedule,
TolerationSeconds: genTolerationSeconds(700),
},
},
},
},
expectedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
expectedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Key: "foo",
Operator: v1.TolerationOpEqual,
Operator: api.TolerationOpEqual,
Value: "bar",
Effect: v1.TaintEffectNoSchedule,
Effect: api.TaintEffectNoSchedule,
TolerationSeconds: genTolerationSeconds(700),
},
{
Key: metav1.TaintNodeNotReady,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: &defaultTolerationSeconds,
},
{
Key: metav1.TaintNodeUnreachable,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: &defaultTolerationSeconds,
},
},
@ -105,31 +104,31 @@ func TestForgivenessAdmission(t *testing.T) {
},
{
description: "pod specified a toleration for taint `notReady:NoExecute`, expect add toleration for `unreachable:NoExecute`",
requestedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
requestedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Key: metav1.TaintNodeNotReady,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(700),
},
},
},
},
expectedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
expectedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Key: metav1.TaintNodeNotReady,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(700),
},
{
Key: metav1.TaintNodeUnreachable,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: &defaultTolerationSeconds,
},
},
@ -138,31 +137,31 @@ func TestForgivenessAdmission(t *testing.T) {
},
{
description: "pod specified a toleration for taint `unreachable:NoExecute`, expect add toleration for `notReady:NoExecute`",
requestedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
requestedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Key: metav1.TaintNodeUnreachable,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(700),
},
},
},
},
expectedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
expectedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Key: metav1.TaintNodeUnreachable,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(700),
},
{
Key: metav1.TaintNodeNotReady,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: &defaultTolerationSeconds,
},
},
@ -171,37 +170,37 @@ func TestForgivenessAdmission(t *testing.T) {
},
{
description: "pod specified tolerations for both `notread:NoExecute` and `unreachable:NoExecute`, expect no change",
requestedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
requestedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Key: metav1.TaintNodeNotReady,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(700),
},
{
Key: metav1.TaintNodeUnreachable,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(700),
},
},
},
},
expectedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
expectedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Key: metav1.TaintNodeNotReady,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(700),
},
{
Key: metav1.TaintNodeUnreachable,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(700),
},
},
@ -210,29 +209,29 @@ func TestForgivenessAdmission(t *testing.T) {
},
{
description: "pod specified toleration for taint `unreachable`, expect add toleration for `notReady:NoExecute`",
requestedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
requestedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Key: metav1.TaintNodeUnreachable,
Operator: v1.TolerationOpExists,
Operator: api.TolerationOpExists,
TolerationSeconds: genTolerationSeconds(700),
},
},
},
},
expectedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
expectedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Key: metav1.TaintNodeUnreachable,
Operator: v1.TolerationOpExists,
Operator: api.TolerationOpExists,
TolerationSeconds: genTolerationSeconds(700),
},
{
Key: metav1.TaintNodeNotReady,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoExecute,
TolerationSeconds: genTolerationSeconds(300),
},
},
@ -241,18 +240,18 @@ func TestForgivenessAdmission(t *testing.T) {
},
{
description: "pod has wildcard toleration for all kind of taints, expect no change",
requestedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
{Operator: v1.TolerationOpExists, TolerationSeconds: genTolerationSeconds(700)},
requestedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{Operator: api.TolerationOpExists, TolerationSeconds: genTolerationSeconds(700)},
},
},
},
expectedPod: v1.Pod{
Spec: v1.PodSpec{
Tolerations: []v1.Toleration{
expectedPod: api.Pod{
Spec: api.PodSpec{
Tolerations: []api.Toleration{
{
Operator: v1.TolerationOpExists,
Operator: api.TolerationOpExists,
TolerationSeconds: genTolerationSeconds(700),
},
},
@ -262,7 +261,7 @@ func TestForgivenessAdmission(t *testing.T) {
}
for _, test := range tests {
err := handler.Admit(admission.NewAttributesRecord(&test.requestedPod, nil, api.Kind("Pod").WithVersion("version"), "foo", "name", v1.Resource("pods").WithVersion("version"), "", "ignored", nil))
err := handler.Admit(admission.NewAttributesRecord(&test.requestedPod, nil, api.Kind("Pod").WithVersion("version"), "foo", "name", api.Resource("pods").WithVersion("version"), "", "ignored", nil))
if err != nil {
t.Errorf("[%s]: unexpected error %v for pod %+v", test.description, err, test.requestedPod)
}

38
staging/src/k8s.io/client-go/pkg/api/helpers.go
View File

@ -465,6 +465,44 @@ const (
ObjectTTLAnnotationKey string = "node.alpha.kubernetes.io/ttl"
)
// AddOrUpdateTolerationInPod tries to add a toleration to the pod's toleration list.
// Returns true if something was updated, false otherwise.
func AddOrUpdateTolerationInPod(pod *Pod, toleration *Toleration) (bool, error) {
podTolerations := pod.Spec.Tolerations
var newTolerations []Toleration
updated := false
for i := range podTolerations {
if toleration.MatchToleration(&podTolerations[i]) {
if Semantic.DeepEqual(toleration, podTolerations[i]) {
return false, nil
}
newTolerations = append(newTolerations, *toleration)
updated = true
continue
}
newTolerations = append(newTolerations, podTolerations[i])
}
if !updated {
newTolerations = append(newTolerations, *toleration)
}
pod.Spec.Tolerations = newTolerations
return true, nil
}
// MatchToleration checks if the toleration matches tolerationToMatch. Tolerations are unique by <key,effect,operator,value>,
// if the two tolerations have same <key,effect,operator,value> combination, regard as they match.
// TODO: uniqueness check for tolerations in api validations.
func (t *Toleration) MatchToleration(tolerationToMatch *Toleration) bool {
return t.Key == tolerationToMatch.Key &&
t.Effect == tolerationToMatch.Effect &&
t.Operator == tolerationToMatch.Operator &&
t.Value == tolerationToMatch.Value
}
// TolerationToleratesTaint checks if the toleration tolerates the taint.
func TolerationToleratesTaint(toleration *Toleration, taint *Taint) bool {
if len(toleration.Effect) != 0 && toleration.Effect != taint.Effect {

104
test/integration/defaulttolerationseconds/defaulttolerationseconds_test.go Normal file
View File

@ -0,0 +1,104 @@
// +build integration,!no-etcd
/*
Copyright 2017 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 defaulttolerationseconds
import (
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
restclient "k8s.io/client-go/rest"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/v1"
"k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
"k8s.io/kubernetes/plugin/pkg/admission/defaulttolerationseconds"
"k8s.io/kubernetes/test/integration/framework"
)
func TestAdmission(t *testing.T) {
masterConfig := framework.NewMasterConfig()
masterConfig.GenericConfig.EnableProfiling = true
masterConfig.GenericConfig.EnableMetrics = true
masterConfig.GenericConfig.AdmissionControl = defaulttolerationseconds.NewDefaultTolerationSeconds()
_, s := framework.RunAMaster(masterConfig)
defer s.Close()
client := clientset.NewForConfigOrDie(&restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &api.Registry.GroupOrDie(v1.GroupName).GroupVersion}})
ns := framework.CreateTestingNamespace("default-toleration-seconds", s, t)
defer framework.DeleteTestingNamespace(ns, s, t)
pod := v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns.Name,
Name: "foo",
},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "test",
Image: "an-image",
},
},
},
}
updatedPod, err := client.Core().Pods(pod.Namespace).Create(&pod)
if err != nil {
t.Fatalf("error creating pod: %v", err)
}
var defaultSeconds int64 = 300
nodeNotReady := v1.Toleration{
Key: metav1.TaintNodeNotReady,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
TolerationSeconds: &defaultSeconds,
}
nodeUnreachable := v1.Toleration{
Key: metav1.TaintNodeUnreachable,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoExecute,
TolerationSeconds: &defaultSeconds,
}
found := 0
tolerations := updatedPod.Spec.Tolerations
for i := range tolerations {
if found == 2 {
break
}
if tolerations[i].MatchToleration(&nodeNotReady) {
if api.Semantic.DeepEqual(tolerations[i], nodeNotReady) {
found++
continue
}
}
if tolerations[i].MatchToleration(&nodeUnreachable) {
if api.Semantic.DeepEqual(tolerations[i], nodeUnreachable) {
found++
continue
}
}
}
if found != 2 {
t.Fatalf("unexpected tolerations: %v\n", updatedPod.Spec.Tolerations)
}
}