Skip to content

Commit d654b49

Browse files
authored
Merge pull request kubernetes#73097 from bsalamat/fix_taint_nodes
Add NotReady taint to new nodes during admission
2 parents 402263f + 37be9f5 commit d654b49

File tree

8 files changed

+270
-2
lines changed

8 files changed

+270
-2
lines changed

pkg/kubeapiserver/options/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ go_library(
4040
"//plugin/pkg/admission/namespace/autoprovision:go_default_library",
4141
"//plugin/pkg/admission/namespace/exists:go_default_library",
4242
"//plugin/pkg/admission/noderestriction:go_default_library",
43+
"//plugin/pkg/admission/nodetaint:go_default_library",
4344
"//plugin/pkg/admission/podnodeselector:go_default_library",
4445
"//plugin/pkg/admission/podpreset:go_default_library",
4546
"//plugin/pkg/admission/podtolerationrestriction:go_default_library",

pkg/kubeapiserver/options/plugins.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
"k8s.io/kubernetes/plugin/pkg/admission/namespace/autoprovision"
3939
"k8s.io/kubernetes/plugin/pkg/admission/namespace/exists"
4040
"k8s.io/kubernetes/plugin/pkg/admission/noderestriction"
41+
"k8s.io/kubernetes/plugin/pkg/admission/nodetaint"
4142
"k8s.io/kubernetes/plugin/pkg/admission/podnodeselector"
4243
"k8s.io/kubernetes/plugin/pkg/admission/podpreset"
4344
"k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction"
@@ -72,6 +73,7 @@ var AllOrderedPlugins = []string{
7273
limitranger.PluginName, // LimitRanger
7374
serviceaccount.PluginName, // ServiceAccount
7475
noderestriction.PluginName, // NodeRestriction
76+
nodetaint.PluginName, // TaintNodesByCondition
7577
alwayspullimages.PluginName, // AlwaysPullImages
7678
imagepolicy.PluginName, // ImagePolicyWebhook
7779
podsecuritypolicy.PluginName, // PodSecurityPolicy
@@ -111,6 +113,7 @@ func RegisterAllAdmissionPlugins(plugins *admission.Plugins) {
111113
autoprovision.Register(plugins)
112114
exists.Register(plugins)
113115
noderestriction.Register(plugins)
116+
nodetaint.Register(plugins)
114117
label.Register(plugins) // DEPRECATED in favor of NewPersistentVolumeLabelController in CCM
115118
podnodeselector.Register(plugins)
116119
podpreset.Register(plugins)
@@ -143,5 +146,9 @@ func DefaultOffAdmissionPlugins() sets.String {
143146
defaultOnPlugins.Insert(podpriority.PluginName) //PodPriority
144147
}
145148

149+
if utilfeature.DefaultFeatureGate.Enabled(features.TaintNodesByCondition) {
150+
defaultOnPlugins.Insert(nodetaint.PluginName) //TaintNodesByCondition
151+
}
152+
146153
return sets.NewString(AllOrderedPlugins...).Difference(defaultOnPlugins)
147154
}

plugin/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ filegroup(
2525
"//plugin/pkg/admission/namespace/autoprovision:all-srcs",
2626
"//plugin/pkg/admission/namespace/exists:all-srcs",
2727
"//plugin/pkg/admission/noderestriction:all-srcs",
28+
"//plugin/pkg/admission/nodetaint:all-srcs",
2829
"//plugin/pkg/admission/podnodeselector:all-srcs",
2930
"//plugin/pkg/admission/podpreset:all-srcs",
3031
"//plugin/pkg/admission/podtolerationrestriction:all-srcs",
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "go_default_library",
5+
srcs = ["admission.go"],
6+
importpath = "k8s.io/kubernetes/plugin/pkg/admission/nodetaint",
7+
visibility = ["//visibility:public"],
8+
deps = [
9+
"//pkg/apis/core:go_default_library",
10+
"//pkg/features:go_default_library",
11+
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
12+
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
13+
],
14+
)
15+
16+
go_test(
17+
name = "go_default_test",
18+
srcs = ["admission_test.go"],
19+
embed = [":go_default_library"],
20+
deps = [
21+
"//pkg/apis/core:go_default_library",
22+
"//pkg/features:go_default_library",
23+
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
24+
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
25+
"//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
26+
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
27+
],
28+
)
29+
30+
filegroup(
31+
name = "package-srcs",
32+
srcs = glob(["**"]),
33+
tags = ["automanaged"],
34+
visibility = ["//visibility:private"],
35+
)
36+
37+
filegroup(
38+
name = "all-srcs",
39+
srcs = [":package-srcs"],
40+
tags = ["automanaged"],
41+
visibility = ["//visibility:public"],
42+
)
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/*
2+
Copyright 2019 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package nodetaint
18+
19+
import (
20+
"fmt"
21+
"io"
22+
"k8s.io/apiserver/pkg/admission"
23+
utilfeature "k8s.io/apiserver/pkg/util/feature"
24+
api "k8s.io/kubernetes/pkg/apis/core"
25+
"k8s.io/kubernetes/pkg/features"
26+
)
27+
28+
const (
29+
// PluginName is the name of the plugin.
30+
PluginName = "TaintNodesByCondition"
31+
// TaintNodeNotReady is the not-ready label as specified in the API.
32+
TaintNodeNotReady = "node.kubernetes.io/not-ready"
33+
)
34+
35+
// Register registers a plugin
36+
func Register(plugins *admission.Plugins) {
37+
plugins.Register(PluginName, func(config io.Reader) (admission.Interface, error) {
38+
return NewPlugin(), nil
39+
})
40+
}
41+
42+
// NewPlugin creates a new NodeTaint admission plugin.
43+
// This plugin identifies requests from nodes
44+
func NewPlugin() *Plugin {
45+
return &Plugin{
46+
Handler: admission.NewHandler(admission.Create),
47+
features: utilfeature.DefaultFeatureGate,
48+
}
49+
}
50+
51+
// Plugin holds state for and implements the admission plugin.
52+
type Plugin struct {
53+
*admission.Handler
54+
// allows overriding for testing
55+
features utilfeature.FeatureGate
56+
}
57+
58+
var (
59+
_ = admission.Interface(&Plugin{})
60+
)
61+
62+
var (
63+
nodeResource = api.Resource("nodes")
64+
)
65+
66+
// Admit is the main function that checks node identity and adds taints as needed.
67+
func (p *Plugin) Admit(a admission.Attributes) error {
68+
// If TaintNodesByCondition is not enabled, we don't need to do anything.
69+
if !p.features.Enabled(features.TaintNodesByCondition) {
70+
return nil
71+
}
72+
73+
// Our job is just to taint nodes.
74+
if a.GetResource().GroupResource() != nodeResource || a.GetSubresource() != "" {
75+
return nil
76+
}
77+
78+
node, ok := a.GetObject().(*api.Node)
79+
if !ok {
80+
return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject()))
81+
}
82+
83+
// Taint node with NotReady taint at creation if TaintNodesByCondition is
84+
// enabled. This is needed to make sure that nodes are added to the cluster
85+
// with the NotReady taint. Otherwise, a new node may receive the taint with
86+
// some delay causing pods to be scheduled on a not-ready node.
87+
// Node controller will remove the taint when the node becomes ready.
88+
addNotReadyTaint(node)
89+
return nil
90+
}
91+
92+
func addNotReadyTaint(node *api.Node) {
93+
notReadyTaint := api.Taint{
94+
Key: TaintNodeNotReady,
95+
Effect: api.TaintEffectNoSchedule,
96+
}
97+
for _, taint := range node.Spec.Taints {
98+
if taint.MatchTaint(notReadyTaint) {
99+
// the taint already exists.
100+
return
101+
}
102+
}
103+
node.Spec.Taints = append(node.Spec.Taints, notReadyTaint)
104+
}
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/*
2+
Copyright 2019 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package nodetaint
18+
19+
import (
20+
"reflect"
21+
"testing"
22+
23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
"k8s.io/apiserver/pkg/admission"
25+
"k8s.io/apiserver/pkg/authentication/user"
26+
utilfeature "k8s.io/apiserver/pkg/util/feature"
27+
api "k8s.io/kubernetes/pkg/apis/core"
28+
"k8s.io/kubernetes/pkg/features"
29+
)
30+
31+
var (
32+
enableTaintNodesByCondition = utilfeature.NewFeatureGate()
33+
disableTaintNodesByCondition = utilfeature.NewFeatureGate()
34+
)
35+
36+
func init() {
37+
if err := enableTaintNodesByCondition.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.TaintNodesByCondition: {Default: true}}); err != nil {
38+
panic(err)
39+
}
40+
if err := disableTaintNodesByCondition.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.TaintNodesByCondition: {Default: false}}); err != nil {
41+
panic(err)
42+
}
43+
}
44+
45+
func Test_nodeTaints(t *testing.T) {
46+
var (
47+
mynode = &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}}
48+
resource = api.Resource("nodes").WithVersion("v1")
49+
notReadyTaint = api.Taint{Key: TaintNodeNotReady, Effect: api.TaintEffectNoSchedule}
50+
notReadyCondition = api.NodeCondition{Type: api.NodeReady, Status: api.ConditionFalse}
51+
myNodeObjMeta = metav1.ObjectMeta{Name: "mynode"}
52+
myNodeObj = api.Node{ObjectMeta: myNodeObjMeta}
53+
myTaintedNodeObj = api.Node{ObjectMeta: myNodeObjMeta,
54+
Spec: api.NodeSpec{Taints: []api.Taint{notReadyTaint}}}
55+
myUnreadyNodeObj = api.Node{ObjectMeta: myNodeObjMeta,
56+
Status: api.NodeStatus{Conditions: []api.NodeCondition{notReadyCondition}}}
57+
nodeKind = api.Kind("Node").WithVersion("v1")
58+
)
59+
tests := []struct {
60+
name string
61+
node api.Node
62+
oldNode api.Node
63+
features utilfeature.FeatureGate
64+
operation admission.Operation
65+
expectedTaints []api.Taint
66+
}{
67+
{
68+
name: "notReady taint is added on creation",
69+
node: myNodeObj,
70+
features: enableTaintNodesByCondition,
71+
operation: admission.Create,
72+
expectedTaints: []api.Taint{notReadyTaint},
73+
},
74+
{
75+
name: "NotReady taint is not added when TaintNodesByCondition is disabled",
76+
node: myNodeObj,
77+
features: disableTaintNodesByCondition,
78+
operation: admission.Create,
79+
expectedTaints: nil,
80+
},
81+
{
82+
name: "already tainted node is not tainted again",
83+
node: myTaintedNodeObj,
84+
features: enableTaintNodesByCondition,
85+
operation: admission.Create,
86+
expectedTaints: []api.Taint{notReadyTaint},
87+
},
88+
{
89+
name: "NotReady taint is added to an unready node as well",
90+
node: myUnreadyNodeObj,
91+
features: enableTaintNodesByCondition,
92+
operation: admission.Create,
93+
expectedTaints: []api.Taint{notReadyTaint},
94+
},
95+
}
96+
for _, tt := range tests {
97+
t.Run(tt.name, func(t *testing.T) {
98+
attributes := admission.NewAttributesRecord(&tt.node, &tt.oldNode, nodeKind, myNodeObj.Namespace, myNodeObj.Name, resource, "", tt.operation, false, mynode)
99+
c := NewPlugin()
100+
if tt.features != nil {
101+
c.features = tt.features
102+
}
103+
err := c.Admit(attributes)
104+
if err != nil {
105+
t.Errorf("nodePlugin.Admit() error = %v", err)
106+
}
107+
node, _ := attributes.GetObject().(*api.Node)
108+
if !reflect.DeepEqual(node.Spec.Taints, tt.expectedTaints) {
109+
t.Errorf("Unexpected Node taints. Got %v\nExpected: %v", node.Spec.Taints, tt.expectedTaints)
110+
}
111+
})
112+
}
113+
}

test/e2e_node/services/apiserver.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func (a *APIServer) Start() error {
5555
}
5656
o.ServiceClusterIPRange = *ipnet
5757
o.AllowPrivileged = true
58-
o.Admission.GenericAdmission.DisablePlugins = []string{"ServiceAccount"}
58+
o.Admission.GenericAdmission.DisablePlugins = []string{"ServiceAccount", "TaintNodesByCondition"}
5959
errCh := make(chan error)
6060
go func() {
6161
defer close(errCh)

test/integration/auth/node_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func TestNodeAuthorizer(t *testing.T) {
8888
"--enable-admission-plugins", "NodeRestriction",
8989
// The "default" SA is not installed, causing the ServiceAccount plugin to retry for ~1s per
9090
// API request.
91-
"--disable-admission-plugins", "ServiceAccount",
91+
"--disable-admission-plugins", "ServiceAccount,TaintNodesByCondition",
9292
}, framework.SharedEtcd())
9393
defer server.TearDownFn()
9494

0 commit comments

Comments
 (0)