From fa8092f83896101e950da0e0f77db70247992f66 Mon Sep 17 00:00:00 2001 From: Kensei Nakada Date: Sat, 20 Jul 2024 19:15:16 +0900 Subject: [PATCH] support UpdatePodScaleDown instead of UpdatePodRequest --- pkg/scheduler/framework/events.go | 26 ++++++++++++++----- pkg/scheduler/framework/events_test.go | 22 ++++++++++------ .../framework/plugins/noderesources/fit.go | 4 +-- .../plugins/noderesources/fit_test.go | 2 +- pkg/scheduler/framework/types.go | 6 ++--- .../internal/queue/scheduling_queue.go | 2 +- pkg/scheduler/scheduler_test.go | 2 +- 7 files changed, 42 insertions(+), 22 deletions(-) diff --git a/pkg/scheduler/framework/events.go b/pkg/scheduler/framework/events.go index 82043fc0d85..89422a36dbd 100644 --- a/pkg/scheduler/framework/events.go +++ b/pkg/scheduler/framework/events.go @@ -57,8 +57,8 @@ var ( assignedPodOtherUpdate = ClusterEvent{Resource: Pod, ActionType: updatePodOther, Label: "AssignedPodUpdate"} // AssignedPodDelete is the event when an assigned pod is deleted. AssignedPodDelete = ClusterEvent{Resource: Pod, ActionType: Delete, Label: "AssignedPodDelete"} - // PodRequestChange is the event when a pod's resource request is changed. - PodRequestChange = ClusterEvent{Resource: Pod, ActionType: UpdatePodRequest, Label: "PodRequestChange"} + // PodRequestScaledDown is the event when a pod's resource request is scaled down. + PodRequestScaledDown = ClusterEvent{Resource: Pod, ActionType: UpdatePodScaleDown, Label: "PodRequestScaledDown"} // PodLabelChange is the event when a pod's label is changed. PodLabelChange = ClusterEvent{Resource: Pod, ActionType: UpdatePodLabel, Label: "PodLabelChange"} // NodeSpecUnschedulableChange is the event when unschedulable node spec is changed. @@ -108,7 +108,7 @@ var ( func PodSchedulingPropertiesChange(newPod *v1.Pod, oldPod *v1.Pod) (events []ClusterEvent) { podChangeExtracters := []podChangeExtractor{ extractPodLabelsChange, - extractPodResourceRequestChange, + extractPodScaleDown, } for _, fn := range podChangeExtracters { @@ -128,13 +128,27 @@ func PodSchedulingPropertiesChange(newPod *v1.Pod, oldPod *v1.Pod) (events []Clu type podChangeExtractor func(newNode *v1.Pod, oldNode *v1.Pod) *ClusterEvent -func extractPodResourceRequestChange(newPod, oldPod *v1.Pod) *ClusterEvent { +// extractPodScaleDown interprets the update of a pod and returns PodRequestScaledDown event if any pod's resource request(s) is scaled down. +func extractPodScaleDown(newPod, oldPod *v1.Pod) *ClusterEvent { opt := resource.PodResourcesOptions{ InPlacePodVerticalScalingEnabled: utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling), } - if !equality.Semantic.DeepEqual(resource.PodRequests(newPod, opt), resource.PodRequests(oldPod, opt)) { - return &PodRequestChange + newPodRequests := resource.PodRequests(newPod, opt) + oldPodRequests := resource.PodRequests(oldPod, opt) + + for rName, oldReq := range oldPodRequests { + newReq, ok := newPodRequests[rName] + if !ok { + // The resource request of rName is removed. + return &PodRequestScaledDown + } + + if oldReq.MilliValue() > newReq.MilliValue() { + // The resource request of rName is scaled down. + return &PodRequestScaledDown + } } + return nil } diff --git a/pkg/scheduler/framework/events_test.go b/pkg/scheduler/framework/events_test.go index e3905887e87..2e0bb110663 100644 --- a/pkg/scheduler/framework/events_test.go +++ b/pkg/scheduler/framework/events_test.go @@ -291,7 +291,7 @@ func Test_podSchedulingPropertiesChange(t *testing.T) { }, }, } - podWithBigRequestAndLabel := &v1.Pod{ + podWithSmallRequestAndLabel := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"foo": "bar"}, }, @@ -300,7 +300,7 @@ func Test_podSchedulingPropertiesChange(t *testing.T) { { Name: "app", Resources: v1.ResourceRequirements{ - Requests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("101m")}, + Requests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("100m")}, }, }, }, @@ -309,7 +309,7 @@ func Test_podSchedulingPropertiesChange(t *testing.T) { ContainerStatuses: []v1.ContainerStatus{ { Name: "app", - AllocatedResources: v1.ResourceList{v1.ResourceCPU: resource.MustParse("101m")}, + AllocatedResources: v1.ResourceList{v1.ResourceCPU: resource.MustParse("100m")}, }, }, }, @@ -347,16 +347,22 @@ func Test_podSchedulingPropertiesChange(t *testing.T) { want: []ClusterEvent{PodLabelChange}, }, { - name: "only pod's resource request is updated", + name: "pod's resource request is scaled down", + oldPod: podWithBigRequest, + newPod: podWithSmallRequest, + want: []ClusterEvent{PodRequestScaledDown}, + }, + { + name: "pod's resource request is scaled up", oldPod: podWithSmallRequest, newPod: podWithBigRequest, - want: []ClusterEvent{PodRequestChange}, + want: []ClusterEvent{assignedPodOtherUpdate}, }, { name: "both pod's resource request and label are updated", - oldPod: podWithSmallRequest, - newPod: podWithBigRequestAndLabel, - want: []ClusterEvent{PodLabelChange, PodRequestChange}, + oldPod: podWithBigRequest, + newPod: podWithSmallRequestAndLabel, + want: []ClusterEvent{PodLabelChange, PodRequestScaledDown}, }, { name: "untracked properties of pod is updated", diff --git a/pkg/scheduler/framework/plugins/noderesources/fit.go b/pkg/scheduler/framework/plugins/noderesources/fit.go index 98708b4a8ab..c680bf3e5fe 100644 --- a/pkg/scheduler/framework/plugins/noderesources/fit.go +++ b/pkg/scheduler/framework/plugins/noderesources/fit.go @@ -252,7 +252,7 @@ func (f *Fit) EventsToRegister(_ context.Context) ([]framework.ClusterEventWithH if f.enableInPlacePodVerticalScaling { // If InPlacePodVerticalScaling (KEP 1287) is enabled, then PodRequestUpdate event should be registered // for this plugin since a Pod update may free up resources that make other Pods schedulable. - podActionType |= framework.UpdatePodRequest + podActionType |= framework.UpdatePodScaleDown } return []framework.ClusterEventWithHint{ {Event: framework.ClusterEvent{Resource: framework.Pod, ActionType: podActionType}, QueueingHintFn: f.isSchedulableAfterPodChange}, @@ -296,7 +296,7 @@ func (f *Fit) isSchedulableAfterPodChange(logger klog.Logger, pod *v1.Pod, oldOb return framework.QueueSkip, nil } - logger.V(5).Info("the max request resources of another scheduled pod got reduced and it may make the unscheduled pod schedulable", "pod", klog.KObj(pod), "modifiedPod", klog.KObj(modifiedPod)) + logger.V(5).Info("another scheduled pod or the target pod itself got scaled down, and it may make the unscheduled pod schedulable", "pod", klog.KObj(pod), "modifiedPod", klog.KObj(modifiedPod)) return framework.Queue, nil } diff --git a/pkg/scheduler/framework/plugins/noderesources/fit_test.go b/pkg/scheduler/framework/plugins/noderesources/fit_test.go index 19a80935225..263e661abde 100644 --- a/pkg/scheduler/framework/plugins/noderesources/fit_test.go +++ b/pkg/scheduler/framework/plugins/noderesources/fit_test.go @@ -1095,7 +1095,7 @@ func TestEventsToRegister(t *testing.T) { "Register events with InPlacePodVerticalScaling feature enabled", true, []framework.ClusterEventWithHint{ - {Event: framework.ClusterEvent{Resource: "Pod", ActionType: framework.UpdatePodRequest | framework.Delete}}, + {Event: framework.ClusterEvent{Resource: "Pod", ActionType: framework.UpdatePodScaleDown | framework.Delete}}, {Event: framework.ClusterEvent{Resource: "Node", ActionType: framework.Add | framework.Update}}, }, }, diff --git a/pkg/scheduler/framework/types.go b/pkg/scheduler/framework/types.go index a90f26dce1b..50a38b951e1 100644 --- a/pkg/scheduler/framework/types.go +++ b/pkg/scheduler/framework/types.go @@ -66,8 +66,8 @@ const ( // It's better to narrow down the scope of the event by using them instead of Update event // for better performance in requeueing. UpdatePodLabel - // UpdatePodRequest is a update for pod's resource request calculated by resource.PodRequests() function. - UpdatePodRequest + // UpdatePodScaleDown is an update for pod's scale down (i.e., any resource request is reduced). + UpdatePodScaleDown // updatePodOther is a update for pod's other fields. // It's used only for the internal event handling, and thus unexported. @@ -76,7 +76,7 @@ const ( All ActionType = 1<