port setNodeDiskPressureCondition to Setter abstraction, add test

pkg/kubelet/kubelet_node_status.go

@@ -780,62 +780,6 @@ func (kl *Kubelet) setNodePIDPressureCondition(node *v1.Node) {
 	}
 }
 
-// setNodeDiskPressureCondition for the node.
-// TODO: this needs to move somewhere centralized...
-func (kl *Kubelet) setNodeDiskPressureCondition(node *v1.Node) {
-	currentTime := metav1.NewTime(kl.clock.Now())
-	var condition *v1.NodeCondition
-
-	// Check if NodeDiskPressure condition already exists and if it does, just pick it up for update.
-	for i := range node.Status.Conditions {
-		if node.Status.Conditions[i].Type == v1.NodeDiskPressure {
-			condition = &node.Status.Conditions[i]
-		}
-	}
-
-	newCondition := false
-	// If the NodeDiskPressure condition doesn't exist, create one
-	if condition == nil {
-		condition = &v1.NodeCondition{
-			Type:   v1.NodeDiskPressure,
-			Status: v1.ConditionUnknown,
-		}
-		// cannot be appended to node.Status.Conditions here because it gets
-		// copied to the slice. So if we append to the slice here none of the
-		// updates we make below are reflected in the slice.
-		newCondition = true
-	}
-
-	// Update the heartbeat time
-	condition.LastHeartbeatTime = currentTime
-
-	// Note: The conditions below take care of the case when a new NodeDiskPressure condition is
-	// created and as well as the case when the condition already exists. When a new condition
-	// is created its status is set to v1.ConditionUnknown which matches either
-	// condition.Status != v1.ConditionTrue or
-	// condition.Status != v1.ConditionFalse in the conditions below depending on whether
-	// the kubelet is under disk pressure or not.
-	if kl.evictionManager.IsUnderDiskPressure() {
-		if condition.Status != v1.ConditionTrue {
-			condition.Status = v1.ConditionTrue
-			condition.Reason = "KubeletHasDiskPressure"
-			condition.Message = "kubelet has disk pressure"
-			condition.LastTransitionTime = currentTime
-			kl.recordNodeStatusEvent(v1.EventTypeNormal, "NodeHasDiskPressure")
-		}
-	} else if condition.Status != v1.ConditionFalse {
-		condition.Status = v1.ConditionFalse
-		condition.Reason = "KubeletHasNoDiskPressure"
-		condition.Message = "kubelet has no disk pressure"
-		condition.LastTransitionTime = currentTime
-		kl.recordNodeStatusEvent(v1.EventTypeNormal, "NodeHasNoDiskPressure")
-	}
-
-	if newCondition {
-		node.Status.Conditions = append(node.Status.Conditions, *condition)
-	}
-}
-
 // record if node schedulable change.
 func (kl *Kubelet) recordNodeSchedulableEvent(node *v1.Node) {
 	kl.lastNodeUnschedulableLock.Lock()
@@ -903,7 +847,7 @@ func (kl *Kubelet) defaultNodeStatusFuncs() []func(*v1.Node) error {
 		withoutError(kl.setNodeStatusInfo),
 		nodestatus.OutOfDiskCondition(kl.clock.Now, kl.recordNodeStatusEvent),
 		nodestatus.MemoryPressureCondition(kl.clock.Now, kl.evictionManager.IsUnderMemoryPressure, kl.recordNodeStatusEvent),
-		withoutError(kl.setNodeDiskPressureCondition),
+		nodestatus.DiskPressureCondition(kl.clock.Now, kl.evictionManager.IsUnderDiskPressure, kl.recordNodeStatusEvent),
 		withoutError(kl.setNodePIDPressureCondition),
 		withoutError(kl.setNodeReadyCondition),
 		withoutError(kl.setNodeVolumesInUseStatus),

pkg/kubelet/nodestatus/setters.go

@@ -203,6 +203,67 @@ func MemoryPressureCondition(nowFunc func() time.Time, // typically Kubelet.cloc
 	}
 }
 
+// DiskPressureCondition returns a Setter that updates the v1.NodeDiskPressure condition on the node.
+func DiskPressureCondition(nowFunc func() time.Time, // typically Kubelet.clock.Now
+	pressureFunc func() bool, // typically Kubelet.evictionManager.IsUnderDiskPressure
+	recordEventFunc func(eventType, event string), // typically Kubelet.recordNodeStatusEvent
+) Setter {
+	return func(node *v1.Node) error {
+		currentTime := metav1.NewTime(nowFunc())
+		var condition *v1.NodeCondition
+
+		// Check if NodeDiskPressure condition already exists and if it does, just pick it up for update.
+		for i := range node.Status.Conditions {
+			if node.Status.Conditions[i].Type == v1.NodeDiskPressure {
+				condition = &node.Status.Conditions[i]
+			}
+		}
+
+		newCondition := false
+		// If the NodeDiskPressure condition doesn't exist, create one
+		if condition == nil {
+			condition = &v1.NodeCondition{
+				Type:   v1.NodeDiskPressure,
+				Status: v1.ConditionUnknown,
+			}
+			// cannot be appended to node.Status.Conditions here because it gets
+			// copied to the slice. So if we append to the slice here none of the
+			// updates we make below are reflected in the slice.
+			newCondition = true
+		}
+
+		// Update the heartbeat time
+		condition.LastHeartbeatTime = currentTime
+
+		// Note: The conditions below take care of the case when a new NodeDiskPressure condition is
+		// created and as well as the case when the condition already exists. When a new condition
+		// is created its status is set to v1.ConditionUnknown which matches either
+		// condition.Status != v1.ConditionTrue or
+		// condition.Status != v1.ConditionFalse in the conditions below depending on whether
+		// the kubelet is under disk pressure or not.
+		if pressureFunc() {
+			if condition.Status != v1.ConditionTrue {
+				condition.Status = v1.ConditionTrue
+				condition.Reason = "KubeletHasDiskPressure"
+				condition.Message = "kubelet has disk pressure"
+				condition.LastTransitionTime = currentTime
+				recordEventFunc(v1.EventTypeNormal, "NodeHasDiskPressure")
+			}
+		} else if condition.Status != v1.ConditionFalse {
+			condition.Status = v1.ConditionFalse
+			condition.Reason = "KubeletHasNoDiskPressure"
+			condition.Message = "kubelet has no disk pressure"
+			condition.LastTransitionTime = currentTime
+			recordEventFunc(v1.EventTypeNormal, "NodeHasNoDiskPressure")
+		}
+
+		if newCondition {
+			node.Status.Conditions = append(node.Status.Conditions, *condition)
+		}
+		return nil
+	}
+}
+
 // OutOfDiskCondition returns a Setter that updates the v1.NodeOutOfDisk condition on the node.
 // TODO(#65658): remove this condition
 func OutOfDiskCondition(nowFunc func() time.Time, // typically Kubelet.clock.Now

pkg/kubelet/nodestatus/setters_test.go

@@ -306,6 +306,127 @@ func TestMemoryPressureCondition(t *testing.T) {
 	}
 }
 
+func TestDiskPressureCondition(t *testing.T) {
+	now := time.Now()
+	before := now.Add(-time.Second)
+	nowFunc := func() time.Time { return now }
+
+	cases := []struct {
+		desc             string
+		node             *v1.Node
+		pressure         bool
+		expectConditions []v1.NodeCondition
+		expectEvents     []testEvent
+	}{
+		{
+			desc:             "new, no pressure",
+			node:             &v1.Node{},
+			pressure:         false,
+			expectConditions: []v1.NodeCondition{*makeDiskPressureCondition(false, now, now)},
+			expectEvents: []testEvent{
+				{
+					eventType: v1.EventTypeNormal,
+					event:     "NodeHasNoDiskPressure",
+				},
+			},
+		},
+		{
+			desc:             "new, pressure",
+			node:             &v1.Node{},
+			pressure:         true,
+			expectConditions: []v1.NodeCondition{*makeDiskPressureCondition(true, now, now)},
+			expectEvents: []testEvent{
+				{
+					eventType: v1.EventTypeNormal,
+					event:     "NodeHasDiskPressure",
+				},
+			},
+		},
+		{
+			desc: "transition to pressure",
+			node: &v1.Node{
+				Status: v1.NodeStatus{
+					Conditions: []v1.NodeCondition{*makeDiskPressureCondition(false, before, before)},
+				},
+			},
+			pressure:         true,
+			expectConditions: []v1.NodeCondition{*makeDiskPressureCondition(true, now, now)},
+			expectEvents: []testEvent{
+				{
+					eventType: v1.EventTypeNormal,
+					event:     "NodeHasDiskPressure",
+				},
+			},
+		},
+		{
+			desc: "transition to no pressure",
+			node: &v1.Node{
+				Status: v1.NodeStatus{
+					Conditions: []v1.NodeCondition{*makeDiskPressureCondition(true, before, before)},
+				},
+			},
+			pressure:         false,
+			expectConditions: []v1.NodeCondition{*makeDiskPressureCondition(false, now, now)},
+			expectEvents: []testEvent{
+				{
+					eventType: v1.EventTypeNormal,
+					event:     "NodeHasNoDiskPressure",
+				},
+			},
+		},
+		{
+			desc: "pressure, no transition",
+			node: &v1.Node{
+				Status: v1.NodeStatus{
+					Conditions: []v1.NodeCondition{*makeDiskPressureCondition(true, before, before)},
+				},
+			},
+			pressure:         true,
+			expectConditions: []v1.NodeCondition{*makeDiskPressureCondition(true, before, now)},
+			expectEvents:     []testEvent{},
+		},
+		{
+			desc: "no pressure, no transition",
+			node: &v1.Node{
+				Status: v1.NodeStatus{
+					Conditions: []v1.NodeCondition{*makeDiskPressureCondition(false, before, before)},
+				},
+			},
+			pressure:         false,
+			expectConditions: []v1.NodeCondition{*makeDiskPressureCondition(false, before, now)},
+			expectEvents:     []testEvent{},
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.desc, func(t *testing.T) {
+			events := []testEvent{}
+			recordEventFunc := func(eventType, event string) {
+				events = append(events, testEvent{
+					eventType: eventType,
+					event:     event,
+				})
+			}
+			pressureFunc := func() bool {
+				return tc.pressure
+			}
+			// construct setter
+			setter := DiskPressureCondition(nowFunc, pressureFunc, recordEventFunc)
+			// call setter on node
+			if err := setter(tc.node); err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			// check expected condition
+			assert.True(t, apiequality.Semantic.DeepEqual(tc.expectConditions, tc.node.Status.Conditions),
+				"Diff: %s", diff.ObjectDiff(tc.expectConditions, tc.node.Status.Conditions))
+			// check expected events
+			require.Equal(t, len(tc.expectEvents), len(events))
+			for i := range tc.expectEvents {
+				assert.Equal(t, tc.expectEvents[i], events[i])
+			}
+		})
+	}
+}
+
 // Test Helpers:
 
 // sortableNodeAddress is a type for sorting []v1.NodeAddress
@@ -347,3 +468,24 @@ func makeMemoryPressureCondition(pressure bool, transition, heartbeat time.Time)
 		LastHeartbeatTime:  metav1.NewTime(heartbeat),
 	}
 }
+
+func makeDiskPressureCondition(pressure bool, transition, heartbeat time.Time) *v1.NodeCondition {
+	if pressure {
+		return &v1.NodeCondition{
+			Type:               v1.NodeDiskPressure,
+			Status:             v1.ConditionTrue,
+			Reason:             "KubeletHasDiskPressure",
+			Message:            "kubelet has disk pressure",
+			LastTransitionTime: metav1.NewTime(transition),
+			LastHeartbeatTime:  metav1.NewTime(heartbeat),
+		}
+	}
+	return &v1.NodeCondition{
+		Type:               v1.NodeDiskPressure,
+		Status:             v1.ConditionFalse,
+		Reason:             "KubeletHasNoDiskPressure",
+		Message:            "kubelet has no disk pressure",
+		LastTransitionTime: metav1.NewTime(transition),
+		LastHeartbeatTime:  metav1.NewTime(heartbeat),
+	}
+}