From b26e4dfa7f5c7e524e23f5a7686d29b85ff5688b Mon Sep 17 00:00:00 2001 From: Michael Taufen Date: Mon, 25 Jun 2018 18:26:02 -0700 Subject: [PATCH] port setNodeDiskPressureCondition to Setter abstraction, add test --- pkg/kubelet/kubelet_node_status.go | 58 +--------- pkg/kubelet/nodestatus/setters.go | 61 +++++++++++ pkg/kubelet/nodestatus/setters_test.go | 142 +++++++++++++++++++++++++ 3 files changed, 204 insertions(+), 57 deletions(-) diff --git a/pkg/kubelet/kubelet_node_status.go b/pkg/kubelet/kubelet_node_status.go index 907b39e9ef8..9b8f7044c1c 100644 --- a/pkg/kubelet/kubelet_node_status.go +++ b/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), diff --git a/pkg/kubelet/nodestatus/setters.go b/pkg/kubelet/nodestatus/setters.go index f3c32746bc1..647b7acdc22 100644 --- a/pkg/kubelet/nodestatus/setters.go +++ b/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 diff --git a/pkg/kubelet/nodestatus/setters_test.go b/pkg/kubelet/nodestatus/setters_test.go index e5bfb890757..e9a15ca9d8a 100644 --- a/pkg/kubelet/nodestatus/setters_test.go +++ b/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), + } +}