diff --git a/pkg/controller/nodelifecycle/node_lifecycle_controller.go b/pkg/controller/nodelifecycle/node_lifecycle_controller.go index 24d68a8dfe6..3aff066e832 100644 --- a/pkg/controller/nodelifecycle/node_lifecycle_controller.go +++ b/pkg/controller/nodelifecycle/node_lifecycle_controller.go @@ -734,7 +734,7 @@ func (nc *Controller) monitorNodeHealth(ctx context.Context) error { } return } - nc.processTaintBaseEviction(ctx, node, &observedReadyCondition) + nc.processTaintBaseEviction(ctx, node, currentReadyCondition) _, needsRetry := nc.nodesToRetry.Load(node.Name) switch { @@ -765,11 +765,11 @@ func (nc *Controller) monitorNodeHealth(ctx context.Context) error { return nil } -func (nc *Controller) processTaintBaseEviction(ctx context.Context, node *v1.Node, observedReadyCondition *v1.NodeCondition) { +func (nc *Controller) processTaintBaseEviction(ctx context.Context, node *v1.Node, currentReadyCondition *v1.NodeCondition) { decisionTimestamp := nc.now() // Check eviction timeout against decisionTimestamp logger := klog.FromContext(ctx) - switch observedReadyCondition.Status { + switch currentReadyCondition.Status { case v1.ConditionFalse: // We want to update the taint straight away if Node is already tainted with the UnreachableTaint if taintutils.TaintExists(node.Spec.Taints, UnreachableTaintTemplate) { @@ -781,7 +781,7 @@ func (nc *Controller) processTaintBaseEviction(ctx context.Context, node *v1.Nod logger.V(2).Info("Node is NotReady. Adding it to the Taint queue", "node", klog.KObj(node), "timeStamp", decisionTimestamp) } case v1.ConditionUnknown: - // We want to update the taint straight away if Node is already tainted with the UnreachableTaint + // We want to update the taint straight away if Node is already tainted with the NotReadyTaintTemplate if taintutils.TaintExists(node.Spec.Taints, NotReadyTaintTemplate) { taintToAdd := *UnreachableTaintTemplate if !controllerutil.SwapNodeControllerTaint(ctx, nc.kubeClient, []*v1.Taint{&taintToAdd}, []*v1.Taint{NotReadyTaintTemplate}, node) { diff --git a/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go b/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go index 6b6e337f981..01c8d603bf4 100644 --- a/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go +++ b/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go @@ -2523,6 +2523,191 @@ func TestApplyNoExecuteTaints(t *testing.T) { } } +// TestApplyNoExecuteTaintsToUnreachableNode ensures a NoExecute taint is applied to node that hasn't posted the +// node status for a period greater than nodeMonitorGracePeriod. +func TestApplyNoExecuteTaintsToUnreachableNode(t *testing.T) { + // TODO: Remove skip once https://github.com/kubernetes/kubernetes/pull/114607 merges. + if goruntime.GOOS == "windows" { + t.Skip("Skipping test on Windows.") + } + time1 := metav1.Date(2017, 1, 1, 12, 0, 0, 0, time.UTC) + // time2 is set to NodeMonitorGracePeriod plus 1 second after time1. + time2 := metav1.Time{Time: time1.Add(testNodeMonitorGracePeriod + time.Second)} + + fakeNodeHandler := &testutil.FakeNodeHandler{ + Existing: []*v1.Node{ + // Unreachable Taint with effect 'NoExecute' should be applied to this node after 2015-01-01 12:00:40. + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + Labels: map[string]string{ + v1.LabelTopologyRegion: "region1", + v1.LabelTopologyZone: "zone1", + v1.LabelFailureDomainBetaRegion: "region1", + v1.LabelFailureDomainBetaZone: "zone1", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + LastHeartbeatTime: time1, + LastTransitionTime: time1, + }, + }, + }, + }, + // Because of the logic that prevents NC from evicting anything when all Nodes are NotReady + // we need second healthy node in tests. + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + Labels: map[string]string{ + v1.LabelTopologyRegion: "region1", + v1.LabelTopologyZone: "zone1", + v1.LabelFailureDomainBetaRegion: "region1", + v1.LabelFailureDomainBetaZone: "zone1", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + LastHeartbeatTime: time1, + LastTransitionTime: time1, + }, + }, + }, + }, + }, + Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}), + } + _, ctx := ktesting.NewTestContext(t) + nodeController, _ := newNodeLifecycleControllerFromClient( + ctx, + fakeNodeHandler, + testRateLimiterQPS, + testRateLimiterQPS, + testLargeClusterThreshold, + testUnhealthyThreshold, + testNodeMonitorGracePeriod, + testNodeStartupGracePeriod, + testNodeMonitorPeriod, + ) + // No taint should be added when the NodeReady conditions were not set longer ago than NodeMonitorGracePeriod. + nodeController.now = func() metav1.Time { return time1 } + nodeController.recorder = testutil.NewFakeRecorder() + nodeController.getPodsAssignedToNode = fakeGetPodsAssignedToNode(fakeNodeHandler.Clientset) + if err := nodeController.syncNodeStore(fakeNodeHandler); err != nil { + t.Errorf("unexpected error: %v", err) + } + if err := nodeController.monitorNodeHealth(ctx); err != nil { + t.Errorf("unexpected error: %v", err) + } + nodeController.doNoExecuteTaintingPass(ctx) + node0, err := fakeNodeHandler.Get(ctx, "node0", metav1.GetOptions{}) + if err != nil { + t.Errorf("Can't get current node0...") + return + } + if len(node0.Spec.Taints) > 0 { + t.Errorf("Got unexpected taints %v", node0.Spec.Taints) + } + node1, err := fakeNodeHandler.Get(ctx, "node1", metav1.GetOptions{}) + if err != nil { + t.Errorf("Can't get current node1...") + return + } + if len(node1.Spec.Taints) > 0 { + t.Errorf("Got unexpected taints %v", node1.Spec.Taints) + } + + // Advance the clock to time2 to expire the previous NodeReady condition. + nodeController.now = func() metav1.Time { return time2 } + // Only node1 sends a new heartbeat at time2. + node1.Status = v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + LastHeartbeatTime: time2, + LastTransitionTime: time1, + }, + }, + } + _, err = fakeNodeHandler.UpdateStatus(ctx, node1, metav1.UpdateOptions{}) + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + // Unreachable taint should be added to node0 and not node1 because the former's NodeReadyCondition was set longer ago than NodeMonitorGracePeriod. + if err := nodeController.syncNodeStore(fakeNodeHandler); err != nil { + t.Errorf("unexpected error: %v", err) + } + if err := nodeController.monitorNodeHealth(ctx); err != nil { + t.Errorf("unexpected error: %v", err) + } + if err := nodeController.syncNodeStore(fakeNodeHandler); err != nil { + t.Errorf("unexpected error: %v", err) + } + nodeController.doNoExecuteTaintingPass(ctx) + + node0, err = fakeNodeHandler.Get(ctx, "node0", metav1.GetOptions{}) + if err != nil { + t.Errorf("Can't get current node0...") + return + } + if !taintutils.TaintExists(node0.Spec.Taints, UnreachableTaintTemplate) { + t.Errorf("Can't find taint %v in %v", UnreachableTaintTemplate, node0.Spec.Taints) + } + node1, err = fakeNodeHandler.Get(ctx, "node1", metav1.GetOptions{}) + if err != nil { + t.Errorf("Can't get current node1...") + return + } + if len(node1.Spec.Taints) > 0 { + t.Errorf("Got unexpected taints %v", node1.Spec.Taints) + } + + // Unreachable taint should be deleted from node0 after it sends a new heartbeat. + node0.Status = v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + LastHeartbeatTime: time2, + LastTransitionTime: time2, + }, + }, + } + _, err = fakeNodeHandler.UpdateStatus(ctx, node0, metav1.UpdateOptions{}) + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + if err := nodeController.syncNodeStore(fakeNodeHandler); err != nil { + t.Errorf("unexpected error: %v", err) + } + if err := nodeController.monitorNodeHealth(ctx); err != nil { + t.Errorf("unexpected error: %v", err) + } + nodeController.doNoExecuteTaintingPass(ctx) + + node0, err = fakeNodeHandler.Get(ctx, "node0", metav1.GetOptions{}) + if err != nil { + t.Errorf("Can't get current node0...") + return + } + // We should not see any taint on the node. + if len(node0.Spec.Taints) > 0 { + t.Errorf("Got unexpected taints %v", node0.Spec.Taints) + } +} + // TestApplyNoExecuteTaintsToNodesEnqueueTwice ensures we taint every node with NoExecute even if enqueued twice func TestApplyNoExecuteTaintsToNodesEnqueueTwice(t *testing.T) { // TODO: Remove skip once https://github.com/kubernetes/kubernetes/pull/114607 merges.