Merge pull request #73097 from bsalamat/fix_taint_nodes

Add NotReady taint to new nodes during admission

pkg/kubeapiserver/options/BUILD

@@ -40,6 +40,7 @@ go_library(
         "//plugin/pkg/admission/namespace/autoprovision:go_default_library",
         "//plugin/pkg/admission/namespace/exists:go_default_library",
         "//plugin/pkg/admission/noderestriction:go_default_library",
+        "//plugin/pkg/admission/nodetaint:go_default_library",
         "//plugin/pkg/admission/podnodeselector:go_default_library",
         "//plugin/pkg/admission/podpreset:go_default_library",
         "//plugin/pkg/admission/podtolerationrestriction:go_default_library",

pkg/kubeapiserver/options/plugins.go

@@ -38,6 +38,7 @@ import (
 	"k8s.io/kubernetes/plugin/pkg/admission/namespace/autoprovision"
 	"k8s.io/kubernetes/plugin/pkg/admission/namespace/exists"
 	"k8s.io/kubernetes/plugin/pkg/admission/noderestriction"
+	"k8s.io/kubernetes/plugin/pkg/admission/nodetaint"
 	"k8s.io/kubernetes/plugin/pkg/admission/podnodeselector"
 	"k8s.io/kubernetes/plugin/pkg/admission/podpreset"
 	"k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction"
@@ -72,6 +73,7 @@ var AllOrderedPlugins = []string{
 	limitranger.PluginName,                  // LimitRanger
 	serviceaccount.PluginName,               // ServiceAccount
 	noderestriction.PluginName,              // NodeRestriction
+	nodetaint.PluginName,                    // TaintNodesByCondition
 	alwayspullimages.PluginName,             // AlwaysPullImages
 	imagepolicy.PluginName,                  // ImagePolicyWebhook
 	podsecuritypolicy.PluginName,            // PodSecurityPolicy
@@ -111,6 +113,7 @@ func RegisterAllAdmissionPlugins(plugins *admission.Plugins) {
 	autoprovision.Register(plugins)
 	exists.Register(plugins)
 	noderestriction.Register(plugins)
+	nodetaint.Register(plugins)
 	label.Register(plugins) // DEPRECATED in favor of NewPersistentVolumeLabelController in CCM
 	podnodeselector.Register(plugins)
 	podpreset.Register(plugins)
@@ -143,5 +146,9 @@ func DefaultOffAdmissionPlugins() sets.String {
 		defaultOnPlugins.Insert(podpriority.PluginName) //PodPriority
 	}
 
+	if utilfeature.DefaultFeatureGate.Enabled(features.TaintNodesByCondition) {
+		defaultOnPlugins.Insert(nodetaint.PluginName) //TaintNodesByCondition
+	}
+
 	return sets.NewString(AllOrderedPlugins...).Difference(defaultOnPlugins)
 }

plugin/BUILD

@@ -25,6 +25,7 @@ filegroup(
         "//plugin/pkg/admission/namespace/autoprovision:all-srcs",
         "//plugin/pkg/admission/namespace/exists:all-srcs",
         "//plugin/pkg/admission/noderestriction:all-srcs",
+        "//plugin/pkg/admission/nodetaint:all-srcs",
         "//plugin/pkg/admission/podnodeselector:all-srcs",
         "//plugin/pkg/admission/podpreset:all-srcs",
         "//plugin/pkg/admission/podtolerationrestriction:all-srcs",

plugin/pkg/admission/nodetaint/BUILD

@@ -0,0 +1,42 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["admission.go"],
+    importpath = "k8s.io/kubernetes/plugin/pkg/admission/nodetaint",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/apis/core:go_default_library",
+        "//pkg/features:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
+    ],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["admission_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//pkg/apis/core:go_default_library",
+        "//pkg/features:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

plugin/pkg/admission/nodetaint/admission.go

@@ -0,0 +1,104 @@
+/*
+Copyright 2019 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 nodetaint
+
+import (
+	"fmt"
+	"io"
+	"k8s.io/apiserver/pkg/admission"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/features"
+)
+
+const (
+	// PluginName is the name of the plugin.
+	PluginName = "TaintNodesByCondition"
+	// TaintNodeNotReady is the not-ready label as specified in the API.
+	TaintNodeNotReady = "node.kubernetes.io/not-ready"
+)
+
+// Register registers a plugin
+func Register(plugins *admission.Plugins) {
+	plugins.Register(PluginName, func(config io.Reader) (admission.Interface, error) {
+		return NewPlugin(), nil
+	})
+}
+
+// NewPlugin creates a new NodeTaint admission plugin.
+// This plugin identifies requests from nodes
+func NewPlugin() *Plugin {
+	return &Plugin{
+		Handler:  admission.NewHandler(admission.Create),
+		features: utilfeature.DefaultFeatureGate,
+	}
+}
+
+// Plugin holds state for and implements the admission plugin.
+type Plugin struct {
+	*admission.Handler
+	// allows overriding for testing
+	features utilfeature.FeatureGate
+}
+
+var (
+	_ = admission.Interface(&Plugin{})
+)
+
+var (
+	nodeResource = api.Resource("nodes")
+)
+
+// Admit is the main function that checks node identity and adds taints as needed.
+func (p *Plugin) Admit(a admission.Attributes) error {
+	// If TaintNodesByCondition is not enabled, we don't need to do anything.
+	if !p.features.Enabled(features.TaintNodesByCondition) {
+		return nil
+	}
+
+	// Our job is just to taint nodes.
+	if a.GetResource().GroupResource() != nodeResource || a.GetSubresource() != "" {
+		return nil
+	}
+
+	node, ok := a.GetObject().(*api.Node)
+	if !ok {
+		return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject()))
+	}
+
+	// Taint node with NotReady taint at creation if TaintNodesByCondition is
+	// enabled. This is needed to make sure that nodes are added to the cluster
+	// with the NotReady taint. Otherwise, a new node may receive the taint with
+	// some delay causing pods to be scheduled on a not-ready node.
+	// Node controller will remove the taint when the node becomes ready.
+	addNotReadyTaint(node)
+	return nil
+}
+
+func addNotReadyTaint(node *api.Node) {
+	notReadyTaint := api.Taint{
+		Key:    TaintNodeNotReady,
+		Effect: api.TaintEffectNoSchedule,
+	}
+	for _, taint := range node.Spec.Taints {
+		if taint.MatchTaint(notReadyTaint) {
+			// the taint already exists.
+			return
+		}
+	}
+	node.Spec.Taints = append(node.Spec.Taints, notReadyTaint)
+}

plugin/pkg/admission/nodetaint/admission_test.go

@@ -0,0 +1,113 @@
+/*
+Copyright 2019 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 nodetaint
+
+import (
+	"reflect"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apiserver/pkg/admission"
+	"k8s.io/apiserver/pkg/authentication/user"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/features"
+)
+
+var (
+	enableTaintNodesByCondition  = utilfeature.NewFeatureGate()
+	disableTaintNodesByCondition = utilfeature.NewFeatureGate()
+)
+
+func init() {
+	if err := enableTaintNodesByCondition.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.TaintNodesByCondition: {Default: true}}); err != nil {
+		panic(err)
+	}
+	if err := disableTaintNodesByCondition.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.TaintNodesByCondition: {Default: false}}); err != nil {
+		panic(err)
+	}
+}
+
+func Test_nodeTaints(t *testing.T) {
+	var (
+		mynode            = &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}}
+		resource          = api.Resource("nodes").WithVersion("v1")
+		notReadyTaint     = api.Taint{Key: TaintNodeNotReady, Effect: api.TaintEffectNoSchedule}
+		notReadyCondition = api.NodeCondition{Type: api.NodeReady, Status: api.ConditionFalse}
+		myNodeObjMeta     = metav1.ObjectMeta{Name: "mynode"}
+		myNodeObj         = api.Node{ObjectMeta: myNodeObjMeta}
+		myTaintedNodeObj  = api.Node{ObjectMeta: myNodeObjMeta,
+			Spec: api.NodeSpec{Taints: []api.Taint{notReadyTaint}}}
+		myUnreadyNodeObj = api.Node{ObjectMeta: myNodeObjMeta,
+			Status: api.NodeStatus{Conditions: []api.NodeCondition{notReadyCondition}}}
+		nodeKind = api.Kind("Node").WithVersion("v1")
+	)
+	tests := []struct {
+		name           string
+		node           api.Node
+		oldNode        api.Node
+		features       utilfeature.FeatureGate
+		operation      admission.Operation
+		expectedTaints []api.Taint
+	}{
+		{
+			name:           "notReady taint is added on creation",
+			node:           myNodeObj,
+			features:       enableTaintNodesByCondition,
+			operation:      admission.Create,
+			expectedTaints: []api.Taint{notReadyTaint},
+		},
+		{
+			name:           "NotReady taint is not added when TaintNodesByCondition is disabled",
+			node:           myNodeObj,
+			features:       disableTaintNodesByCondition,
+			operation:      admission.Create,
+			expectedTaints: nil,
+		},
+		{
+			name:           "already tainted node is not tainted again",
+			node:           myTaintedNodeObj,
+			features:       enableTaintNodesByCondition,
+			operation:      admission.Create,
+			expectedTaints: []api.Taint{notReadyTaint},
+		},
+		{
+			name:           "NotReady taint is added to an unready node as well",
+			node:           myUnreadyNodeObj,
+			features:       enableTaintNodesByCondition,
+			operation:      admission.Create,
+			expectedTaints: []api.Taint{notReadyTaint},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			attributes := admission.NewAttributesRecord(&tt.node, &tt.oldNode, nodeKind, myNodeObj.Namespace, myNodeObj.Name, resource, "", tt.operation, false, mynode)
+			c := NewPlugin()
+			if tt.features != nil {
+				c.features = tt.features
+			}
+			err := c.Admit(attributes)
+			if err != nil {
+				t.Errorf("nodePlugin.Admit() error = %v", err)
+			}
+			node, _ := attributes.GetObject().(*api.Node)
+			if !reflect.DeepEqual(node.Spec.Taints, tt.expectedTaints) {
+				t.Errorf("Unexpected Node taints. Got %v\nExpected: %v", node.Spec.Taints, tt.expectedTaints)
+			}
+		})
+	}
+}

test/e2e_node/services/apiserver.go

@@ -55,7 +55,7 @@ func (a *APIServer) Start() error {
 	}
 	o.ServiceClusterIPRange = *ipnet
 	o.AllowPrivileged = true
-	o.Admission.GenericAdmission.DisablePlugins = []string{"ServiceAccount"}
+	o.Admission.GenericAdmission.DisablePlugins = []string{"ServiceAccount", "TaintNodesByCondition"}
 	errCh := make(chan error)
 	go func() {
 		defer close(errCh)

test/integration/auth/node_test.go

@@ -88,7 +88,7 @@ func TestNodeAuthorizer(t *testing.T) {
 		"--enable-admission-plugins", "NodeRestriction",
 		// The "default" SA is not installed, causing the ServiceAccount plugin to retry for ~1s per
 		// API request.
-		"--disable-admission-plugins", "ServiceAccount",
+		"--disable-admission-plugins", "ServiceAccount,TaintNodesByCondition",
 	}, framework.SharedEtcd())
 	defer server.TearDownFn()