NodeController sets NodeTaints instead of deleting Pods

cmd/kube-controller-manager/app/controllermanager.go

@@ -444,6 +444,7 @@ func StartControllers(controllers map[string]InitFunc, s *options.CMServer, root
 		int(s.NodeCIDRMaskSize),
 		s.AllocateNodeCIDRs,
 		s.EnableTaintManager,
+		s.UseTaintBasedEvictions,
 	)
 	if err != nil {
 		return fmt.Errorf("failed to initialize nodecontroller: %v", err)

cmd/kube-controller-manager/app/options/options.go

@@ -105,6 +105,7 @@ func NewCMServer() *CMServer {
 			ClusterSigningKeyFile:    "/etc/kubernetes/ca/ca.key",
 			ReconcilerSyncLoopPeriod: metav1.Duration{Duration: 60 * time.Second},
 			EnableTaintManager:       true,
+			UseTaintBasedEvictions:   false,
 		},
 	}
 	s.LeaderElection.LeaderElect = true
@@ -198,6 +199,7 @@ func (s *CMServer) AddFlags(fs *pflag.FlagSet, allControllers []string, disabled
 	fs.BoolVar(&s.DisableAttachDetachReconcilerSync, "disable-attach-detach-reconcile-sync", false, "Disable volume attach detach reconciler sync. Disabling this may cause volumes to be mismatched with pods. Use wisely.")
 	fs.DurationVar(&s.ReconcilerSyncLoopPeriod.Duration, "attach-detach-reconcile-sync-period", s.ReconcilerSyncLoopPeriod.Duration, "The reconciler sync wait time between volume attach detach. This duration must be larger than one second, and increasing this value from the default may allow for volumes to be mismatched with pods.")
 	fs.BoolVar(&s.EnableTaintManager, "enable-taint-manager", s.EnableTaintManager, "WARNING: Beta feature. If set to true enables NoExecute Taints and will evict all not-tolerating Pod running on Nodes tainted with this kind of Taints.")
+	fs.BoolVar(&s.UseTaintBasedEvictions, "use-taint-based-evictions", s.UseTaintBasedEvictions, "WARNING: Alpha feature. If set to true NodeController will use taints to evict Pods from notReady and unreachable Nodes.")
 
 	leaderelection.BindFlags(&s.LeaderElection, fs)
 

hack/verify-flags/known-flags.txt

@@ -639,6 +639,8 @@ upgrade-image
 upgrade-target
 use-kubernetes-cluster-service
 use-service-account-credentials
+use-kubernetes-version
+use-taint-based-evictions
 user-whitelist
 verb
 verify-only

pkg/apis/componentconfig/types.go

@@ -794,6 +794,8 @@ type KubeControllerManagerConfiguration struct {
 	// If set to true enables NoExecute Taints and will evict all not-tolerating
 	// Pod running on Nodes tainted with this kind of Taints.
 	EnableTaintManager bool
+	// If set to true NodeController will use taints to evict Pods from notReady and unreachable Nodes.
+	UseTaintBasedEvictions bool
 }
 
 // VolumeConfiguration contains *all* enumerated flags meant to configure all volume

pkg/controller/controller_utils.go

@@ -873,7 +873,22 @@ func AddOrUpdateTaintOnNode(c clientset.Interface, nodeName string, taint *v1.Ta
 
 // RemoveTaintOffNode is for cleaning up taints temporarily added to node,
 // won't fail if target taint doesn't exist or has been removed.
-func RemoveTaintOffNode(c clientset.Interface, nodeName string, taint *v1.Taint) error {
+// If passed a node it'll check if there's anything to be done, if taint is not present it won't issue
+// any API calls.
+func RemoveTaintOffNode(c clientset.Interface, nodeName string, taint *v1.Taint, node *v1.Node) error {
+	// Short circuit for limiting amout of API calls.
+	if node != nil {
+		match := false
+		for i := range node.Spec.Taints {
+			if node.Spec.Taints[i].MatchTaint(taint) {
+				match = true
+				break
+			}
+		}
+		if !match {
+			return nil
+		}
+	}
 	firstTry := true
 	return clientretry.RetryOnConflict(UpdateTaintBackoff, func() error {
 		var err error

pkg/controller/node/nodecontroller_test.go

@@ -100,6 +100,7 @@ func NewNodeControllerFromClient(
 		nodeCIDRMaskSize,
 		allocateNodeCIDRs,
 		false,
+		false,
 	)
 	if err != nil {
 		return nil, err
@@ -573,11 +574,15 @@ func TestMonitorNodeStatusEvictPods(t *testing.T) {
 		}
 		zones := testutil.GetZones(item.fakeNodeHandler)
 		for _, zone := range zones {
-			nodeController.zonePodEvictor[zone].Try(func(value TimedValue) (bool, time.Duration) {
-				nodeUid, _ := value.UID.(string)
-				deletePods(item.fakeNodeHandler, nodeController.recorder, value.Value, nodeUid, nodeController.daemonSetInformer.Lister())
-				return true, 0
-			})
+			if _, ok := nodeController.zonePodEvictor[zone]; ok {
+				nodeController.zonePodEvictor[zone].Try(func(value TimedValue) (bool, time.Duration) {
+					nodeUid, _ := value.UID.(string)
+					deletePods(item.fakeNodeHandler, nodeController.recorder, value.Value, nodeUid, nodeController.daemonSetInformer.Lister())
+					return true, 0
+				})
+			} else {
+				t.Fatalf("Zone %v was unitialized!", zone)
+			}
 		}
 
 		podEvicted := false

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go

@@ -167,7 +167,7 @@ func init() {
 	addControllerRole(rbac.ClusterRole{
 		ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "node-controller"},
 		Rules: []rbac.PolicyRule{
-			rbac.NewRule("get", "list", "update", "delete").Groups(legacyGroup).Resources("nodes").RuleOrDie(),
+			rbac.NewRule("get", "list", "update", "delete", "patch").Groups(legacyGroup).Resources("nodes").RuleOrDie(),
 			rbac.NewRule("update").Groups(legacyGroup).Resources("nodes/status").RuleOrDie(),
 			// used for pod eviction
 			rbac.NewRule("update").Groups(legacyGroup).Resources("pods/status").RuleOrDie(),

test/e2e/framework/util.go

@@ -82,6 +82,7 @@ import (
 	gcecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/gce"
 	"k8s.io/kubernetes/pkg/controller"
 	deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util"
+	nodectlr "k8s.io/kubernetes/pkg/controller/node"
 	"k8s.io/kubernetes/pkg/kubectl"
 	"k8s.io/kubernetes/pkg/kubelet/util/format"
 	"k8s.io/kubernetes/pkg/master/ports"
@@ -774,7 +775,7 @@ func WaitForMatchPodsCondition(c clientset.Interface, opts metav1.ListOptions, d
 		if len(conditionNotMatch) <= 0 {
 			return err
 		}
-		Logf("%d pods are not %s", len(conditionNotMatch), desc)
+		Logf("%d pods are not %s: %v", len(conditionNotMatch), desc, conditionNotMatch)
 	}
 	return fmt.Errorf("gave up waiting for matching pods to be '%s' after %v", desc, timeout)
 }
@@ -2496,7 +2497,7 @@ func ExpectNodeHasLabel(c clientset.Interface, nodeName string, labelKey string,
 }
 
 func RemoveTaintOffNode(c clientset.Interface, nodeName string, taint v1.Taint) {
-	ExpectNoError(controller.RemoveTaintOffNode(c, nodeName, &taint))
+	ExpectNoError(controller.RemoveTaintOffNode(c, nodeName, &taint, nil))
 	VerifyThatTaintIsGone(c, nodeName, &taint)
 }
 
@@ -2525,14 +2526,24 @@ func VerifyThatTaintIsGone(c clientset.Interface, nodeName string, taint *v1.Tai
 
 func ExpectNodeHasTaint(c clientset.Interface, nodeName string, taint *v1.Taint) {
 	By("verifying the node has the taint " + taint.ToString())
+	if has, err := NodeHasTaint(c, nodeName, taint); !has {
+		ExpectNoError(err)
+		Failf("Failed to find taint %s on node %s", taint.ToString(), nodeName)
+	}
+}
+
+func NodeHasTaint(c clientset.Interface, nodeName string, taint *v1.Taint) (bool, error) {
 	node, err := c.Core().Nodes().Get(nodeName, metav1.GetOptions{})
-	ExpectNoError(err)
+	if err != nil {
+		return false, err
+	}
 
 	nodeTaints := node.Spec.Taints
 
 	if len(nodeTaints) == 0 || !v1.TaintExists(nodeTaints, taint) {
-		Failf("Failed to find taint %s on node %s", taint.ToString(), nodeName)
+		return false, nil
 	}
+	return true, nil
 }
 
 func getScalerForKind(internalClientset internalclientset.Interface, kind schema.GroupKind) (kubectl.Scaler, error) {
@@ -3961,7 +3972,47 @@ func isNodeConditionSetAsExpected(node *v1.Node, conditionType v1.NodeConditionT
 	for _, cond := range node.Status.Conditions {
 		// Ensure that the condition type and the status matches as desired.
 		if cond.Type == conditionType {
-			if (cond.Status == v1.ConditionTrue) == wantTrue {
+			// For NodeReady condition we need to check Taints as well
+			if cond.Type == v1.NodeReady {
+				hasNodeControllerTaints := false
+				// For NodeReady we need to check if Taints are gone as well
+				taints := node.Spec.Taints
+				for _, taint := range taints {
+					if taint.MatchTaint(nodectlr.UnreachableTaintTemplate) || taint.MatchTaint(nodectlr.NotReadyTaintTemplate) {
+						hasNodeControllerTaints = true
+						break
+					}
+				}
+				if wantTrue {
+					if (cond.Status == v1.ConditionTrue) && !hasNodeControllerTaints {
+						return true
+					} else {
+						msg := ""
+						if !hasNodeControllerTaints {
+							msg = fmt.Sprintf("Condition %s of node %s is %v instead of %t. Reason: %v, message: %v",
+								conditionType, node.Name, cond.Status == v1.ConditionTrue, wantTrue, cond.Reason, cond.Message)
+						} else {
+							msg = fmt.Sprintf("Condition %s of node %s is %v, but Node is tainted by NodeController with %v. Failure",
+								conditionType, node.Name, cond.Status == v1.ConditionTrue, taints)
+						}
+						if !silent {
+							Logf(msg)
+						}
+						return false
+					}
+				} else {
+					// TODO: check if the Node is tainted once we enable NC notReady/unreachable taints by default
+					if cond.Status != v1.ConditionTrue {
+						return true
+					}
+					if !silent {
+						Logf("Condition %s of node %s is %v instead of %t. Reason: %v, message: %v",
+							conditionType, node.Name, cond.Status == v1.ConditionTrue, wantTrue, cond.Reason, cond.Message)
+					}
+					return false
+				}
+			}
+			if (wantTrue && (cond.Status == v1.ConditionTrue)) || (!wantTrue && (cond.Status != v1.ConditionTrue)) {
 				return true
 			} else {
 				if !silent {
@@ -3971,6 +4022,7 @@ func isNodeConditionSetAsExpected(node *v1.Node, conditionType v1.NodeConditionT
 				return false
 			}
 		}
+
 	}
 	if !silent {
 		Logf("Couldn't find condition %v on node %v", conditionType, node.Name)

test/e2e/network_partition.go

@@ -27,10 +27,13 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apimachinery/pkg/watch"
+
 	"k8s.io/client-go/tools/cache"
+
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
+	nodepkg "k8s.io/kubernetes/pkg/controller/node"
 	"k8s.io/kubernetes/test/e2e/framework"
 	testutils "k8s.io/kubernetes/test/utils"
 
@@ -476,4 +479,181 @@ var _ = framework.KubeDescribe("Network Partition [Disruptive] [Slow]", func() {
 			}
 		})
 	})
+
+	framework.KubeDescribe("Pods", func() {
+		Context("should be evicted from unready Node", func() {
+			BeforeEach(func() {
+				framework.SkipUnlessProviderIs("gce", "gke", "aws")
+				framework.SkipUnlessNodeCountIsAtLeast(2)
+			})
+
+			// What happens in this test:
+			//	Network traffic from a node to master is cut off to simulate network partition
+			// Expect to observe:
+			// 1. Node is marked NotReady after timeout by nodecontroller (40seconds)
+			// 2. All pods on node are marked NotReady shortly after #1
+			// 3. After enough time passess all Pods are evicted from the given Node
+			It("[Feature:TaintEviction] All pods on the unreachable node should be marked as NotReady upon the node turn NotReady "+
+				"AND all pods should be evicted after eviction timeout passes", func() {
+				By("choose a node - we will block all network traffic on this node")
+				var podOpts metav1.ListOptions
+				nodes := framework.GetReadySchedulableNodesOrDie(c)
+				framework.FilterNodes(nodes, func(node v1.Node) bool {
+					if !framework.IsNodeConditionSetAsExpected(&node, v1.NodeReady, true) {
+						return false
+					}
+					podOpts = metav1.ListOptions{FieldSelector: fields.OneTermEqualSelector(api.PodHostField, node.Name).String()}
+					pods, err := c.Core().Pods(metav1.NamespaceAll).List(podOpts)
+					if err != nil || len(pods.Items) <= 0 {
+						return false
+					}
+					return true
+				})
+				if len(nodes.Items) <= 0 {
+					framework.Failf("No eligible node were found: %d", len(nodes.Items))
+				}
+				node := nodes.Items[0]
+				podOpts = metav1.ListOptions{FieldSelector: fields.OneTermEqualSelector(api.PodHostField, node.Name).String()}
+				if err := framework.WaitForMatchPodsCondition(c, podOpts, "Running and Ready", podReadyTimeout, testutils.PodRunningReadyOrSucceeded); err != nil {
+					framework.Failf("Pods on node %s are not ready and running within %v: %v", node.Name, podReadyTimeout, err)
+				}
+				pods, err := c.Core().Pods(metav1.NamespaceAll).List(podOpts)
+				framework.ExpectNoError(err)
+				podTolerationTimes := map[string]time.Duration{}
+				// This test doesn't add tolerations by itself, but because they may be present in the cluster
+				// it needs to account for that.
+				for _, pod := range pods.Items {
+					namespacedName := fmt.Sprintf("%v/%v", pod.Namespace, pod.Name)
+					tolerations := pod.Spec.Tolerations
+					framework.ExpectNoError(err)
+					for _, toleration := range tolerations {
+						if toleration.ToleratesTaint(nodepkg.UnreachableTaintTemplate) {
+							if toleration.TolerationSeconds != nil {
+								podTolerationTimes[namespacedName] = time.Duration(*toleration.TolerationSeconds) * time.Second
+								break
+							} else {
+								podTolerationTimes[namespacedName] = -1
+							}
+						}
+					}
+					if _, ok := podTolerationTimes[namespacedName]; !ok {
+						podTolerationTimes[namespacedName] = 0
+					}
+				}
+				neverEvictedPods := []string{}
+				maxTolerationTime := time.Duration(0)
+				for podName, tolerationTime := range podTolerationTimes {
+					if tolerationTime < 0 {
+						neverEvictedPods = append(neverEvictedPods, podName)
+					} else {
+						if tolerationTime > maxTolerationTime {
+							maxTolerationTime = tolerationTime
+						}
+					}
+				}
+				framework.Logf(
+					"Only %v should be running after partition. Maximum TolerationSeconds among other Pods is %v",
+					neverEvictedPods,
+					maxTolerationTime,
+				)
+
+				By("Set up watch on node status")
+				nodeSelector := fields.OneTermEqualSelector("metadata.name", node.Name)
+				stopCh := make(chan struct{})
+				newNode := make(chan *v1.Node)
+				var controller cache.Controller
+				_, controller = cache.NewInformer(
+					&cache.ListWatch{
+						ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
+							options.FieldSelector = nodeSelector.String()
+							obj, err := f.ClientSet.Core().Nodes().List(options)
+							return runtime.Object(obj), err
+						},
+						WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) {
+							options.FieldSelector = nodeSelector.String()
+							return f.ClientSet.Core().Nodes().Watch(options)
+						},
+					},
+					&v1.Node{},
+					0,
+					cache.ResourceEventHandlerFuncs{
+						UpdateFunc: func(oldObj, newObj interface{}) {
+							n, ok := newObj.(*v1.Node)
+							Expect(ok).To(Equal(true))
+							newNode <- n
+
+						},
+					},
+				)
+
+				defer func() {
+					// Will not explicitly close newNode channel here due to
+					// race condition where stopCh and newNode are closed but informer onUpdate still executes.
+					close(stopCh)
+				}()
+				go controller.Run(stopCh)
+
+				By(fmt.Sprintf("Block traffic from node %s to the master", node.Name))
+				host := framework.GetNodeExternalIP(&node)
+				master := framework.GetMasterAddress(c)
+				defer func() {
+					By(fmt.Sprintf("Unblock traffic from node %s to the master", node.Name))
+					framework.UnblockNetwork(host, master)
+
+					if CurrentGinkgoTestDescription().Failed {
+						return
+					}
+
+					By("Expect to observe node status change from NotReady to Ready after network connectivity recovers")
+					expectNodeReadiness(true, newNode)
+				}()
+
+				framework.BlockNetwork(host, master)
+
+				By("Expect to observe node and pod status change from Ready to NotReady after network partition")
+				expectNodeReadiness(false, newNode)
+				framework.ExpectNoError(wait.Poll(1*time.Second, timeout, func() (bool, error) {
+					return framework.NodeHasTaint(c, node.Name, nodepkg.UnreachableTaintTemplate)
+				}))
+				if err = framework.WaitForMatchPodsCondition(c, podOpts, "NotReady", podNotReadyTimeout, testutils.PodNotReady); err != nil {
+					framework.Failf("Pods on node %s did not become NotReady within %v: %v", node.Name, podNotReadyTimeout, err)
+				}
+
+				sleepTime := maxTolerationTime + 20*time.Second
+				By(fmt.Sprintf("Sleeping for %v and checking if all Pods were evicted", sleepTime))
+				time.Sleep(sleepTime)
+				pods, err = c.Core().Pods(v1.NamespaceAll).List(podOpts)
+				framework.ExpectNoError(err)
+				seenRunning := []string{}
+				for _, pod := range pods.Items {
+					namespacedName := fmt.Sprintf("%v/%v", pod.Namespace, pod.Name)
+					shouldBeTerminating := true
+					for _, neverEvictedPod := range neverEvictedPods {
+						if neverEvictedPod == namespacedName {
+							shouldBeTerminating = false
+						}
+					}
+					if pod.DeletionTimestamp == nil {
+						seenRunning = append(seenRunning, namespacedName)
+						if shouldBeTerminating {
+							framework.Failf("Pod %v should have been deleted but was seen running", namespacedName)
+						}
+					}
+				}
+
+				for _, neverEvictedPod := range neverEvictedPods {
+					running := false
+					for _, runningPod := range seenRunning {
+						if runningPod == neverEvictedPod {
+							running = true
+							break
+						}
+					}
+					if !running {
+						framework.Failf("Pod %v was evicted even though it shouldn't", neverEvictedPod)
+					}
+				}
+			})
+		})
+	})
 })