Merge pull request #120816 from tnqn/fix-unreachable-taint-delay

NoExecute taint should be added when a Node's ready condition becomes Unknown
This commit is contained in:
Kubernetes Prow Robot
2025-06-03 00:54:44 -07:00
committed by GitHub
2 changed files with 189 additions and 4 deletions

View File

@@ -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) {

View File

@@ -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.