Skip to content

Commit fa8092f

Browse files
committed
support UpdatePodScaleDown instead of UpdatePodRequest
1 parent 0dee497 commit fa8092f

File tree

7 files changed

+42
-22
lines changed

7 files changed

+42
-22
lines changed

pkg/scheduler/framework/events.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ var (
5757
assignedPodOtherUpdate = ClusterEvent{Resource: Pod, ActionType: updatePodOther, Label: "AssignedPodUpdate"}
5858
// AssignedPodDelete is the event when an assigned pod is deleted.
5959
AssignedPodDelete = ClusterEvent{Resource: Pod, ActionType: Delete, Label: "AssignedPodDelete"}
60-
// PodRequestChange is the event when a pod's resource request is changed.
61-
PodRequestChange = ClusterEvent{Resource: Pod, ActionType: UpdatePodRequest, Label: "PodRequestChange"}
60+
// PodRequestScaledDown is the event when a pod's resource request is scaled down.
61+
PodRequestScaledDown = ClusterEvent{Resource: Pod, ActionType: UpdatePodScaleDown, Label: "PodRequestScaledDown"}
6262
// PodLabelChange is the event when a pod's label is changed.
6363
PodLabelChange = ClusterEvent{Resource: Pod, ActionType: UpdatePodLabel, Label: "PodLabelChange"}
6464
// NodeSpecUnschedulableChange is the event when unschedulable node spec is changed.
@@ -108,7 +108,7 @@ var (
108108
func PodSchedulingPropertiesChange(newPod *v1.Pod, oldPod *v1.Pod) (events []ClusterEvent) {
109109
podChangeExtracters := []podChangeExtractor{
110110
extractPodLabelsChange,
111-
extractPodResourceRequestChange,
111+
extractPodScaleDown,
112112
}
113113

114114
for _, fn := range podChangeExtracters {
@@ -128,13 +128,27 @@ func PodSchedulingPropertiesChange(newPod *v1.Pod, oldPod *v1.Pod) (events []Clu
128128

129129
type podChangeExtractor func(newNode *v1.Pod, oldNode *v1.Pod) *ClusterEvent
130130

131-
func extractPodResourceRequestChange(newPod, oldPod *v1.Pod) *ClusterEvent {
131+
// extractPodScaleDown interprets the update of a pod and returns PodRequestScaledDown event if any pod's resource request(s) is scaled down.
132+
func extractPodScaleDown(newPod, oldPod *v1.Pod) *ClusterEvent {
132133
opt := resource.PodResourcesOptions{
133134
InPlacePodVerticalScalingEnabled: utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling),
134135
}
135-
if !equality.Semantic.DeepEqual(resource.PodRequests(newPod, opt), resource.PodRequests(oldPod, opt)) {
136-
return &PodRequestChange
136+
newPodRequests := resource.PodRequests(newPod, opt)
137+
oldPodRequests := resource.PodRequests(oldPod, opt)
138+
139+
for rName, oldReq := range oldPodRequests {
140+
newReq, ok := newPodRequests[rName]
141+
if !ok {
142+
// The resource request of rName is removed.
143+
return &PodRequestScaledDown
144+
}
145+
146+
if oldReq.MilliValue() > newReq.MilliValue() {
147+
// The resource request of rName is scaled down.
148+
return &PodRequestScaledDown
149+
}
137150
}
151+
138152
return nil
139153
}
140154

pkg/scheduler/framework/events_test.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ func Test_podSchedulingPropertiesChange(t *testing.T) {
291291
},
292292
},
293293
}
294-
podWithBigRequestAndLabel := &v1.Pod{
294+
podWithSmallRequestAndLabel := &v1.Pod{
295295
ObjectMeta: metav1.ObjectMeta{
296296
Labels: map[string]string{"foo": "bar"},
297297
},
@@ -300,7 +300,7 @@ func Test_podSchedulingPropertiesChange(t *testing.T) {
300300
{
301301
Name: "app",
302302
Resources: v1.ResourceRequirements{
303-
Requests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("101m")},
303+
Requests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("100m")},
304304
},
305305
},
306306
},
@@ -309,7 +309,7 @@ func Test_podSchedulingPropertiesChange(t *testing.T) {
309309
ContainerStatuses: []v1.ContainerStatus{
310310
{
311311
Name: "app",
312-
AllocatedResources: v1.ResourceList{v1.ResourceCPU: resource.MustParse("101m")},
312+
AllocatedResources: v1.ResourceList{v1.ResourceCPU: resource.MustParse("100m")},
313313
},
314314
},
315315
},
@@ -347,16 +347,22 @@ func Test_podSchedulingPropertiesChange(t *testing.T) {
347347
want: []ClusterEvent{PodLabelChange},
348348
},
349349
{
350-
name: "only pod's resource request is updated",
350+
name: "pod's resource request is scaled down",
351+
oldPod: podWithBigRequest,
352+
newPod: podWithSmallRequest,
353+
want: []ClusterEvent{PodRequestScaledDown},
354+
},
355+
{
356+
name: "pod's resource request is scaled up",
351357
oldPod: podWithSmallRequest,
352358
newPod: podWithBigRequest,
353-
want: []ClusterEvent{PodRequestChange},
359+
want: []ClusterEvent{assignedPodOtherUpdate},
354360
},
355361
{
356362
name: "both pod's resource request and label are updated",
357-
oldPod: podWithSmallRequest,
358-
newPod: podWithBigRequestAndLabel,
359-
want: []ClusterEvent{PodLabelChange, PodRequestChange},
363+
oldPod: podWithBigRequest,
364+
newPod: podWithSmallRequestAndLabel,
365+
want: []ClusterEvent{PodLabelChange, PodRequestScaledDown},
360366
},
361367
{
362368
name: "untracked properties of pod is updated",

pkg/scheduler/framework/plugins/noderesources/fit.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ func (f *Fit) EventsToRegister(_ context.Context) ([]framework.ClusterEventWithH
252252
if f.enableInPlacePodVerticalScaling {
253253
// If InPlacePodVerticalScaling (KEP 1287) is enabled, then PodRequestUpdate event should be registered
254254
// for this plugin since a Pod update may free up resources that make other Pods schedulable.
255-
podActionType |= framework.UpdatePodRequest
255+
podActionType |= framework.UpdatePodScaleDown
256256
}
257257
return []framework.ClusterEventWithHint{
258258
{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
296296
return framework.QueueSkip, nil
297297
}
298298

299-
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))
299+
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))
300300
return framework.Queue, nil
301301
}
302302

pkg/scheduler/framework/plugins/noderesources/fit_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1095,7 +1095,7 @@ func TestEventsToRegister(t *testing.T) {
10951095
"Register events with InPlacePodVerticalScaling feature enabled",
10961096
true,
10971097
[]framework.ClusterEventWithHint{
1098-
{Event: framework.ClusterEvent{Resource: "Pod", ActionType: framework.UpdatePodRequest | framework.Delete}},
1098+
{Event: framework.ClusterEvent{Resource: "Pod", ActionType: framework.UpdatePodScaleDown | framework.Delete}},
10991099
{Event: framework.ClusterEvent{Resource: "Node", ActionType: framework.Add | framework.Update}},
11001100
},
11011101
},

pkg/scheduler/framework/types.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ const (
6666
// It's better to narrow down the scope of the event by using them instead of Update event
6767
// for better performance in requeueing.
6868
UpdatePodLabel
69-
// UpdatePodRequest is a update for pod's resource request calculated by resource.PodRequests() function.
70-
UpdatePodRequest
69+
// UpdatePodScaleDown is an update for pod's scale down (i.e., any resource request is reduced).
70+
UpdatePodScaleDown
7171

7272
// updatePodOther is a update for pod's other fields.
7373
// It's used only for the internal event handling, and thus unexported.
@@ -76,7 +76,7 @@ const (
7676
All ActionType = 1<<iota - 1
7777

7878
// Use the general Update type if you don't either know or care the specific sub-Update type to use.
79-
Update = UpdateNodeAllocatable | UpdateNodeLabel | UpdateNodeTaint | UpdateNodeCondition | UpdateNodeAnnotation | UpdatePodLabel | UpdatePodRequest | updatePodOther
79+
Update = UpdateNodeAllocatable | UpdateNodeLabel | UpdateNodeTaint | UpdateNodeCondition | UpdateNodeAnnotation | UpdatePodLabel | UpdatePodScaleDown | updatePodOther
8080
)
8181

8282
// GVK is short for group/version/kind, which can uniquely represent a particular API resource.

pkg/scheduler/internal/queue/scheduling_queue.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,7 @@ func (p *PriorityQueue) AssignedPodAdded(logger klog.Logger, pod *v1.Pod) {
11691169
// may make pending pods with matching affinity terms schedulable.
11701170
func (p *PriorityQueue) AssignedPodUpdated(logger klog.Logger, oldPod, newPod *v1.Pod, event framework.ClusterEvent) {
11711171
p.lock.Lock()
1172-
if event.Resource == framework.Pod && event.ActionType&framework.UpdatePodRequest != 0 {
1172+
if event.Resource == framework.Pod && event.ActionType&framework.UpdatePodScaleDown != 0 {
11731173
// In this case, we don't want to pre-filter Pods by getUnschedulablePodsWithCrossTopologyTerm
11741174
// because Pod related events may make Pods that were rejected by NodeResourceFit schedulable.
11751175
p.moveAllToActiveOrBackoffQueue(logger, framework.AssignedPodUpdate, oldPod, newPod, nil)

pkg/scheduler/scheduler_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -893,7 +893,7 @@ func Test_UnionedGVKs(t *testing.T) {
893893
name: "plugins with default profile (InPlacePodVerticalScaling: enabled)",
894894
plugins: schedulerapi.PluginSet{Enabled: defaults.PluginsV1.MultiPoint.Enabled},
895895
want: map[framework.GVK]framework.ActionType{
896-
framework.Pod: framework.Add | framework.UpdatePodLabel | framework.UpdatePodRequest | framework.Delete,
896+
framework.Pod: framework.Add | framework.UpdatePodLabel | framework.UpdatePodScaleDown | framework.Delete,
897897
framework.Node: framework.All,
898898
framework.CSINode: framework.All - framework.Delete,
899899
framework.CSIDriver: framework.All - framework.Delete,

0 commit comments

Comments
 (0)