cleanup: refactor the way extracting Node smaller events

This commit is contained in:
Kensei Nakada 2024-07-12 19:06:07 +09:00
parent 9772ff2848
commit fe96bfa348
2 changed files with 146 additions and 128 deletions

View File

@ -37,18 +37,15 @@ const (
) )
var ( var (
// AssignedPodAdd is the event when a pod is added that causes pods with matching affinity terms // AssignedPodAdd is the event when an assigned pod is added.
// to be more schedulable.
AssignedPodAdd = framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.Add, Label: "AssignedPodAdd"} AssignedPodAdd = framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.Add, Label: "AssignedPodAdd"}
// NodeAdd is the event when a new node is added to the cluster. // NodeAdd is the event when a new node is added to the cluster.
NodeAdd = framework.ClusterEvent{Resource: framework.Node, ActionType: framework.Add, Label: "NodeAdd"} NodeAdd = framework.ClusterEvent{Resource: framework.Node, ActionType: framework.Add, Label: "NodeAdd"}
// AssignedPodUpdate is the event when a pod is updated that causes pods with matching affinity // AssignedPodUpdate is the event when an assigned pod is updated.
// terms to be more schedulable.
AssignedPodUpdate = framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.Update, Label: "AssignedPodUpdate"} AssignedPodUpdate = framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.Update, Label: "AssignedPodUpdate"}
// UnscheduledPodUpdate is the event when an unscheduled pod is updated. // UnscheduledPodUpdate is the event when an unscheduled pod is updated.
UnscheduledPodUpdate = framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.Update, Label: "UnschedulablePodUpdate"} UnscheduledPodUpdate = framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.Update, Label: "UnschedulablePodUpdate"}
// AssignedPodDelete is the event when a pod is deleted that causes pods with matching affinity // AssignedPodDelete is the event when an assigned pod is deleted.
// terms to be more schedulable.
AssignedPodDelete = framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.Delete, Label: "AssignedPodDelete"} AssignedPodDelete = framework.ClusterEvent{Resource: framework.Pod, ActionType: framework.Delete, Label: "AssignedPodDelete"}
// NodeSpecUnschedulableChange is the event when unschedulable node spec is changed. // NodeSpecUnschedulableChange is the event when unschedulable node spec is changed.
NodeSpecUnschedulableChange = framework.ClusterEvent{Resource: framework.Node, ActionType: framework.UpdateNodeTaint, Label: "NodeSpecUnschedulableChange"} NodeSpecUnschedulableChange = framework.ClusterEvent{Resource: framework.Node, ActionType: framework.UpdateNodeTaint, Label: "NodeSpecUnschedulableChange"}
@ -92,44 +89,49 @@ var (
UnschedulableTimeout = framework.ClusterEvent{Resource: framework.WildCard, ActionType: framework.All, Label: "UnschedulableTimeout"} UnschedulableTimeout = framework.ClusterEvent{Resource: framework.WildCard, ActionType: framework.All, Label: "UnschedulableTimeout"}
) )
func NodeSchedulingPropertiesChange(newNode *v1.Node, oldNode *v1.Node) []framework.ClusterEvent { // NodeSchedulingPropertiesChange interprets the update of a node and returns corresponding UpdateNodeXYZ event(s).
var events []framework.ClusterEvent func NodeSchedulingPropertiesChange(newNode *v1.Node, oldNode *v1.Node) (events []framework.ClusterEvent) {
nodeChangeExtracters := []nodeChangeExtractor{
if nodeSpecUnschedulableChanged(newNode, oldNode) { extractNodeSpecUnschedulableChange,
events = append(events, NodeSpecUnschedulableChange) extractNodeAllocatableChange,
} extractNodeLabelsChange,
if nodeAllocatableChanged(newNode, oldNode) { extractNodeTaintsChange,
events = append(events, NodeAllocatableChange) extractNodeConditionsChange,
} extractNodeAnnotationsChange,
if nodeLabelsChanged(newNode, oldNode) {
events = append(events, NodeLabelChange)
}
if nodeTaintsChanged(newNode, oldNode) {
events = append(events, NodeTaintChange)
}
if nodeConditionsChanged(newNode, oldNode) {
events = append(events, NodeConditionChange)
}
if nodeAnnotationsChanged(newNode, oldNode) {
events = append(events, NodeAnnotationChange)
} }
return events for _, fn := range nodeChangeExtracters {
if event := fn(newNode, oldNode); event != nil {
events = append(events, *event)
}
}
return
} }
func nodeAllocatableChanged(newNode *v1.Node, oldNode *v1.Node) bool { type nodeChangeExtractor func(newNode *v1.Node, oldNode *v1.Node) *framework.ClusterEvent
return !equality.Semantic.DeepEqual(oldNode.Status.Allocatable, newNode.Status.Allocatable)
func extractNodeAllocatableChange(newNode *v1.Node, oldNode *v1.Node) *framework.ClusterEvent {
if !equality.Semantic.DeepEqual(oldNode.Status.Allocatable, newNode.Status.Allocatable) {
return &NodeAllocatableChange
}
return nil
} }
func nodeLabelsChanged(newNode *v1.Node, oldNode *v1.Node) bool { func extractNodeLabelsChange(newNode *v1.Node, oldNode *v1.Node) *framework.ClusterEvent {
return !equality.Semantic.DeepEqual(oldNode.GetLabels(), newNode.GetLabels()) if !equality.Semantic.DeepEqual(oldNode.GetLabels(), newNode.GetLabels()) {
return &NodeLabelChange
}
return nil
} }
func nodeTaintsChanged(newNode *v1.Node, oldNode *v1.Node) bool { func extractNodeTaintsChange(newNode *v1.Node, oldNode *v1.Node) *framework.ClusterEvent {
return !equality.Semantic.DeepEqual(newNode.Spec.Taints, oldNode.Spec.Taints) if !equality.Semantic.DeepEqual(newNode.Spec.Taints, oldNode.Spec.Taints) {
return &NodeTaintChange
}
return nil
} }
func nodeConditionsChanged(newNode *v1.Node, oldNode *v1.Node) bool { func extractNodeConditionsChange(newNode *v1.Node, oldNode *v1.Node) *framework.ClusterEvent {
strip := func(conditions []v1.NodeCondition) map[v1.NodeConditionType]v1.ConditionStatus { strip := func(conditions []v1.NodeCondition) map[v1.NodeConditionType]v1.ConditionStatus {
conditionStatuses := make(map[v1.NodeConditionType]v1.ConditionStatus, len(conditions)) conditionStatuses := make(map[v1.NodeConditionType]v1.ConditionStatus, len(conditions))
for i := range conditions { for i := range conditions {
@ -137,13 +139,22 @@ func nodeConditionsChanged(newNode *v1.Node, oldNode *v1.Node) bool {
} }
return conditionStatuses return conditionStatuses
} }
return !equality.Semantic.DeepEqual(strip(oldNode.Status.Conditions), strip(newNode.Status.Conditions)) if !equality.Semantic.DeepEqual(strip(oldNode.Status.Conditions), strip(newNode.Status.Conditions)) {
return &NodeConditionChange
}
return nil
} }
func nodeSpecUnschedulableChanged(newNode *v1.Node, oldNode *v1.Node) bool { func extractNodeSpecUnschedulableChange(newNode *v1.Node, oldNode *v1.Node) *framework.ClusterEvent {
return newNode.Spec.Unschedulable != oldNode.Spec.Unschedulable && !newNode.Spec.Unschedulable if newNode.Spec.Unschedulable != oldNode.Spec.Unschedulable && !newNode.Spec.Unschedulable {
return &NodeSpecUnschedulableChange
}
return nil
} }
func nodeAnnotationsChanged(newNode *v1.Node, oldNode *v1.Node) bool { func extractNodeAnnotationsChange(newNode *v1.Node, oldNode *v1.Node) *framework.ClusterEvent {
return !equality.Semantic.DeepEqual(oldNode.GetAnnotations(), newNode.GetAnnotations()) if !equality.Semantic.DeepEqual(oldNode.GetAnnotations(), newNode.GetAnnotations()) {
return &NodeAnnotationChange
}
return nil
} }

View File

@ -1,9 +1,12 @@
/* /*
Copyright 2024 The Kubernetes Authors. Copyright 2024 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License"); Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. you may not use this file except in compliance with the License.
You may obtain a copy of the License at You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0 http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS, distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@ -25,152 +28,156 @@ import (
st "k8s.io/kubernetes/pkg/scheduler/testing" st "k8s.io/kubernetes/pkg/scheduler/testing"
) )
func TestNodeAllocatableChanged(t *testing.T) { func TestNodeAllocatableChange(t *testing.T) {
newQuantity := func(value int64) resource.Quantity { newQuantity := func(value int64) resource.Quantity {
return *resource.NewQuantity(value, resource.BinarySI) return *resource.NewQuantity(value, resource.BinarySI)
} }
for _, test := range []struct { for _, test := range []struct {
Name string name string
Changed bool // changed is true if it's expected that the function detects the change and returns event.
OldAllocatable v1.ResourceList changed bool
NewAllocatable v1.ResourceList oldAllocatable v1.ResourceList
newAllocatable v1.ResourceList
}{ }{
{ {
Name: "no allocatable resources changed", name: "no allocatable resources changed",
Changed: false, changed: false,
OldAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024)}, oldAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024)},
NewAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024)}, newAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024)},
}, },
{ {
Name: "new node has more allocatable resources", name: "new node has more allocatable resources",
Changed: true, changed: true,
OldAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024)}, oldAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024)},
NewAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024), v1.ResourceStorage: newQuantity(1024)}, newAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024), v1.ResourceStorage: newQuantity(1024)},
}, },
} { } {
t.Run(test.Name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
oldNode := &v1.Node{Status: v1.NodeStatus{Allocatable: test.OldAllocatable}} oldNode := &v1.Node{Status: v1.NodeStatus{Allocatable: test.oldAllocatable}}
newNode := &v1.Node{Status: v1.NodeStatus{Allocatable: test.NewAllocatable}} newNode := &v1.Node{Status: v1.NodeStatus{Allocatable: test.newAllocatable}}
changed := nodeAllocatableChanged(newNode, oldNode) changed := extractNodeAllocatableChange(newNode, oldNode) != nil
if changed != test.Changed { if changed != test.changed {
t.Errorf("nodeAllocatableChanged should be %t, got %t", test.Changed, changed) t.Errorf("nodeAllocatableChanged should be %t, got %t", test.changed, changed)
} }
}) })
} }
} }
func TestNodeLabelsChanged(t *testing.T) { func TestNodeLabelsChange(t *testing.T) {
for _, test := range []struct { for _, test := range []struct {
Name string name string
Changed bool // changed is true if it's expected that the function detects the change and returns event.
OldLabels map[string]string changed bool
NewLabels map[string]string oldLabels map[string]string
newLabels map[string]string
}{ }{
{ {
Name: "no labels changed", name: "no labels changed",
Changed: false, changed: false,
OldLabels: map[string]string{"foo": "bar"}, oldLabels: map[string]string{"foo": "bar"},
NewLabels: map[string]string{"foo": "bar"}, newLabels: map[string]string{"foo": "bar"},
}, },
// Labels changed. // Labels changed.
{ {
Name: "new node has more labels", name: "new object has more labels",
Changed: true, changed: true,
OldLabels: map[string]string{"foo": "bar"}, oldLabels: map[string]string{"foo": "bar"},
NewLabels: map[string]string{"foo": "bar", "test": "value"}, newLabels: map[string]string{"foo": "bar", "test": "value"},
}, },
} { } {
t.Run(test.Name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
oldNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Labels: test.OldLabels}} oldNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Labels: test.oldLabels}}
newNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Labels: test.NewLabels}} newNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Labels: test.newLabels}}
changed := nodeLabelsChanged(newNode, oldNode) changed := extractNodeLabelsChange(newNode, oldNode) != nil
if changed != test.Changed { if changed != test.changed {
t.Errorf("Test case %q failed: should be %t, got %t", test.Name, test.Changed, changed) t.Errorf("Test case %q failed: should be %t, got %t", test.name, test.changed, changed)
} }
}) })
} }
} }
func TestNodeTaintsChanged(t *testing.T) { func TestNodeTaintsChange(t *testing.T) {
for _, test := range []struct { for _, test := range []struct {
Name string name string
Changed bool // changed is true if it's expected that the function detects the change and returns event.
OldTaints []v1.Taint changed bool
NewTaints []v1.Taint oldTaints []v1.Taint
newTaints []v1.Taint
}{ }{
{ {
Name: "no taint changed", name: "no taint changed",
Changed: false, changed: false,
OldTaints: []v1.Taint{{Key: "key", Value: "value"}}, oldTaints: []v1.Taint{{Key: "key", Value: "value"}},
NewTaints: []v1.Taint{{Key: "key", Value: "value"}}, newTaints: []v1.Taint{{Key: "key", Value: "value"}},
}, },
{ {
Name: "taint value changed", name: "taint value changed",
Changed: true, changed: true,
OldTaints: []v1.Taint{{Key: "key", Value: "value1"}}, oldTaints: []v1.Taint{{Key: "key", Value: "value1"}},
NewTaints: []v1.Taint{{Key: "key", Value: "value2"}}, newTaints: []v1.Taint{{Key: "key", Value: "value2"}},
}, },
} { } {
t.Run(test.Name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
oldNode := &v1.Node{Spec: v1.NodeSpec{Taints: test.OldTaints}} oldNode := &v1.Node{Spec: v1.NodeSpec{Taints: test.oldTaints}}
newNode := &v1.Node{Spec: v1.NodeSpec{Taints: test.NewTaints}} newNode := &v1.Node{Spec: v1.NodeSpec{Taints: test.newTaints}}
changed := nodeTaintsChanged(newNode, oldNode) changed := extractNodeTaintsChange(newNode, oldNode) != nil
if changed != test.Changed { if changed != test.changed {
t.Errorf("Test case %q failed: should be %t, not %t", test.Name, test.Changed, changed) t.Errorf("Test case %q failed: should be %t, not %t", test.name, test.changed, changed)
} }
}) })
} }
} }
func TestNodeConditionsChanged(t *testing.T) { func TestNodeConditionsChange(t *testing.T) {
nodeConditionType := reflect.TypeOf(v1.NodeCondition{}) nodeConditionType := reflect.TypeOf(v1.NodeCondition{})
if nodeConditionType.NumField() != 6 { if nodeConditionType.NumField() != 6 {
t.Errorf("NodeCondition type has changed. The nodeConditionsChanged() function must be reevaluated.") t.Errorf("NodeCondition type has changed. The nodeConditionsChange() function must be reevaluated.")
} }
for _, test := range []struct { for _, test := range []struct {
Name string name string
Changed bool // changed is true if it's expected that the function detects the change and returns event.
OldConditions []v1.NodeCondition changed bool
NewConditions []v1.NodeCondition oldConditions []v1.NodeCondition
newConditions []v1.NodeCondition
}{ }{
{ {
Name: "no condition changed", name: "no condition changed",
Changed: false, changed: false,
OldConditions: []v1.NodeCondition{{Type: v1.NodeDiskPressure, Status: v1.ConditionTrue}}, oldConditions: []v1.NodeCondition{{Type: v1.NodeDiskPressure, Status: v1.ConditionTrue}},
NewConditions: []v1.NodeCondition{{Type: v1.NodeDiskPressure, Status: v1.ConditionTrue}}, newConditions: []v1.NodeCondition{{Type: v1.NodeDiskPressure, Status: v1.ConditionTrue}},
}, },
{ {
Name: "only LastHeartbeatTime changed", name: "only LastHeartbeatTime changed",
Changed: false, changed: false,
OldConditions: []v1.NodeCondition{{Type: v1.NodeDiskPressure, Status: v1.ConditionTrue, LastHeartbeatTime: metav1.Unix(1, 0)}}, oldConditions: []v1.NodeCondition{{Type: v1.NodeDiskPressure, Status: v1.ConditionTrue, LastHeartbeatTime: metav1.Unix(1, 0)}},
NewConditions: []v1.NodeCondition{{Type: v1.NodeDiskPressure, Status: v1.ConditionTrue, LastHeartbeatTime: metav1.Unix(2, 0)}}, newConditions: []v1.NodeCondition{{Type: v1.NodeDiskPressure, Status: v1.ConditionTrue, LastHeartbeatTime: metav1.Unix(2, 0)}},
}, },
{ {
Name: "new node has more healthy conditions", name: "new node has more healthy conditions",
Changed: true, changed: true,
OldConditions: []v1.NodeCondition{}, oldConditions: []v1.NodeCondition{},
NewConditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}, newConditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}},
}, },
{ {
Name: "new node has less unhealthy conditions", name: "new node has less unhealthy conditions",
Changed: true, changed: true,
OldConditions: []v1.NodeCondition{{Type: v1.NodeDiskPressure, Status: v1.ConditionTrue}}, oldConditions: []v1.NodeCondition{{Type: v1.NodeDiskPressure, Status: v1.ConditionTrue}},
NewConditions: []v1.NodeCondition{}, newConditions: []v1.NodeCondition{},
}, },
{ {
Name: "condition status changed", name: "condition status changed",
Changed: true, changed: true,
OldConditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}}, oldConditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}},
NewConditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}, newConditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}},
}, },
} { } {
t.Run(test.Name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
oldNode := &v1.Node{Status: v1.NodeStatus{Conditions: test.OldConditions}} oldNode := &v1.Node{Status: v1.NodeStatus{Conditions: test.oldConditions}}
newNode := &v1.Node{Status: v1.NodeStatus{Conditions: test.NewConditions}} newNode := &v1.Node{Status: v1.NodeStatus{Conditions: test.newConditions}}
changed := nodeConditionsChanged(newNode, oldNode) changed := extractNodeConditionsChange(newNode, oldNode) != nil
if changed != test.Changed { if changed != test.changed {
t.Errorf("Test case %q failed: should be %t, got %t", test.Name, test.Changed, changed) t.Errorf("Test case %q failed: should be %t, got %t", test.name, test.changed, changed)
} }
}) })
} }