From d81023db30cf91886706cc320cf23ca44c6539ca Mon Sep 17 00:00:00 2001 From: kerthcet Date: Sun, 4 Feb 2024 14:59:26 +0800 Subject: [PATCH 1/2] When matching clusterEvent, we should consider the "*" additionally Signed-off-by: kerthcet --- pkg/scheduler/framework/types.go | 7 +++ pkg/scheduler/framework/types_test.go | 49 +++++++++++++++++++ .../internal/queue/scheduling_queue.go | 2 +- .../internal/queue/scheduling_queue_test.go | 44 +++++++++++++++++ 4 files changed, 101 insertions(+), 1 deletion(-) diff --git a/pkg/scheduler/framework/types.go b/pkg/scheduler/framework/types.go index 188f5aa2f24..72c35eef8b6 100644 --- a/pkg/scheduler/framework/types.go +++ b/pkg/scheduler/framework/types.go @@ -144,6 +144,13 @@ func (ce ClusterEvent) IsWildCard() bool { return ce.Resource == WildCard && ce.ActionType == All } +// Match returns true if ClusterEvent is matched with the coming event. +// If the ce.Resource is "*", there's no requirement for the coming event' Resource. +// Contrarily, if the coming event's Resource is "*", the ce.Resource should only be "*". +func (ce ClusterEvent) Match(event ClusterEvent) bool { + return ce.IsWildCard() || (ce.Resource == WildCard || ce.Resource == event.Resource) && ce.ActionType&event.ActionType != 0 +} + func UnrollWildCardResource() []ClusterEventWithHint { return []ClusterEventWithHint{ {Event: ClusterEvent{Resource: Pod, ActionType: All}}, diff --git a/pkg/scheduler/framework/types_test.go b/pkg/scheduler/framework/types_test.go index 2a5b3ec8957..cd3b5f7115b 100644 --- a/pkg/scheduler/framework/types_test.go +++ b/pkg/scheduler/framework/types_test.go @@ -1608,3 +1608,52 @@ func TestCalculatePodResourcesWithResize(t *testing.T) { }) } } + +func TestCloudEvent_Match(t *testing.T) { + testCases := []struct { + name string + event ClusterEvent + comingEvent ClusterEvent + wantResult bool + }{ + { + name: "wildcard event matches with all kinds of coming events", + event: ClusterEvent{Resource: WildCard, ActionType: All}, + comingEvent: ClusterEvent{Resource: Pod, ActionType: UpdateNodeLabel}, + wantResult: true, + }, + { + name: "event with resource = 'Pod' matching with coming events carries same actionType", + event: ClusterEvent{Resource: Pod, ActionType: UpdateNodeLabel | UpdateNodeTaint}, + comingEvent: ClusterEvent{Resource: Pod, ActionType: UpdateNodeLabel}, + wantResult: true, + }, + { + name: "event with resource = '*' matching with coming events carries same actionType", + event: ClusterEvent{Resource: WildCard, ActionType: UpdateNodeLabel}, + comingEvent: ClusterEvent{Resource: Pod, ActionType: UpdateNodeLabel}, + wantResult: true, + }, + { + name: "event with resource = '*' matching with coming events carries different actionType", + event: ClusterEvent{Resource: WildCard, ActionType: UpdateNodeLabel}, + comingEvent: ClusterEvent{Resource: Pod, ActionType: UpdateNodeAllocatable}, + wantResult: false, + }, + { + name: "event matching with coming events carries '*' resources", + event: ClusterEvent{Resource: Pod, ActionType: UpdateNodeLabel}, + comingEvent: ClusterEvent{Resource: WildCard, ActionType: UpdateNodeLabel}, + wantResult: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := tc.event.Match(tc.comingEvent) + if got != tc.wantResult { + t.Fatalf("unexpected result") + } + }) + } +} diff --git a/pkg/scheduler/internal/queue/scheduling_queue.go b/pkg/scheduler/internal/queue/scheduling_queue.go index ab52a19bff2..4a7222ece46 100644 --- a/pkg/scheduler/internal/queue/scheduling_queue.go +++ b/pkg/scheduler/internal/queue/scheduling_queue.go @@ -441,7 +441,7 @@ func (p *PriorityQueue) isPodWorthRequeuing(logger klog.Logger, pInfo *framework pod := pInfo.Pod queueStrategy := queueSkip for eventToMatch, hintfns := range hintMap { - if eventToMatch.Resource != event.Resource || eventToMatch.ActionType&event.ActionType == 0 { + if !eventToMatch.Match(event) { continue } diff --git a/pkg/scheduler/internal/queue/scheduling_queue_test.go b/pkg/scheduler/internal/queue/scheduling_queue_test.go index 25e998677aa..02250dbbd35 100644 --- a/pkg/scheduler/internal/queue/scheduling_queue_test.go +++ b/pkg/scheduler/internal/queue/scheduling_queue_test.go @@ -3550,6 +3550,50 @@ func Test_isPodWorthRequeuing(t *testing.T) { }, }, }, + { + name: "If event with '*' Resource, queueing hint function for specified Resource is also executed", + podInfo: &framework.QueuedPodInfo{ + UnschedulablePlugins: sets.New("fooPlugin1"), + PodInfo: mustNewPodInfo(st.MakePod().Name("pod1").Namespace("ns1").UID("1").Obj()), + }, + event: framework.ClusterEvent{Resource: framework.Node, ActionType: framework.Add}, + oldObj: nil, + newObj: st.MakeNode().Obj(), + expected: queueAfterBackoff, + expectedExecutionCount: 1, + queueingHintMap: QueueingHintMapPerProfile{ + "": { + framework.ClusterEvent{Resource: framework.WildCard, ActionType: framework.Add}: { + { + PluginName: "fooPlugin1", + QueueingHintFn: queueHintReturnQueue, + }, + }, + }, + }, + }, + { + name: "If event is a wildcard one, queueing hint function for all kinds of events is executed", + podInfo: &framework.QueuedPodInfo{ + UnschedulablePlugins: sets.New("fooPlugin1"), + PodInfo: mustNewPodInfo(st.MakePod().Name("pod1").Namespace("ns1").UID("1").Obj()), + }, + event: framework.ClusterEvent{Resource: framework.Node, ActionType: framework.UpdateNodeLabel | framework.UpdateNodeTaint}, + oldObj: nil, + newObj: st.MakeNode().Obj(), + expected: queueAfterBackoff, + expectedExecutionCount: 1, + queueingHintMap: QueueingHintMapPerProfile{ + "": { + framework.ClusterEvent{Resource: framework.WildCard, ActionType: framework.All}: { + { + PluginName: "fooPlugin1", + QueueingHintFn: queueHintReturnQueue, + }, + }, + }, + }, + }, } for _, test := range tests { From f97dec28407b12bf071aacfa52f8fa351f24c080 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Mon, 5 Feb 2024 11:46:59 +0800 Subject: [PATCH 2/2] Add comments about wildcard clusterEvent Signed-off-by: kerthcet --- pkg/scheduler/framework/types.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/scheduler/framework/types.go b/pkg/scheduler/framework/types.go index 72c35eef8b6..fc79e4afe13 100644 --- a/pkg/scheduler/framework/types.go +++ b/pkg/scheduler/framework/types.go @@ -83,7 +83,15 @@ const ( CSINode GVK = "storage.k8s.io/CSINode" CSIDriver GVK = "storage.k8s.io/CSIDriver" CSIStorageCapacity GVK = "storage.k8s.io/CSIStorageCapacity" - WildCard GVK = "*" + + // WildCard is a special GVK to match all resources. + // e.g., If you register `{Resource: "*", ActionType: All}` in EventsToRegister, + // all coming clusterEvents will be admitted. Be careful to register it, it will + // increase the computing pressure in requeueing unless you really need it. + // + // Meanwhile, if the coming clusterEvent is a wildcard one, all pods + // will be moved from unschedulablePod pool to activeQ/backoffQ forcibly. + WildCard GVK = "*" ) type ClusterEventWithHint struct { @@ -147,6 +155,10 @@ func (ce ClusterEvent) IsWildCard() bool { // Match returns true if ClusterEvent is matched with the coming event. // If the ce.Resource is "*", there's no requirement for the coming event' Resource. // Contrarily, if the coming event's Resource is "*", the ce.Resource should only be "*". +// +// Note: we have a special case here when the coming event is a wildcard event, +// it will force all Pods to move to activeQ/backoffQ, +// but we take it as an unmatched event unless the ce is also a wildcard one. func (ce ClusterEvent) Match(event ClusterEvent) bool { return ce.IsWildCard() || (ce.Resource == WildCard || ce.Resource == event.Resource) && ce.ActionType&event.ActionType != 0 }