diff --git a/pkg/scheduler/backend/queue/scheduling_queue_test.go b/pkg/scheduler/backend/queue/scheduling_queue_test.go index 77a1c80577d..0467d8efb34 100644 --- a/pkg/scheduler/backend/queue/scheduling_queue_test.go +++ b/pkg/scheduler/backend/queue/scheduling_queue_test.go @@ -1534,9 +1534,13 @@ func TestPriorityQueue_Activate(t *testing.T) { type preEnqueuePlugin struct { allowlists []string + name string } func (pl *preEnqueuePlugin) Name() string { + if pl.name != "" { + return pl.name + } return "preEnqueuePlugin" } @@ -3182,8 +3186,11 @@ var ( add = func(t *testing.T, logger klog.Logger, queue *PriorityQueue, pInfo *framework.QueuedPodInfo) { queue.Add(logger, pInfo.Pod) } - pop = func(_ *testing.T, logger klog.Logger, queue *PriorityQueue, _ *framework.QueuedPodInfo) { - queue.Pop(logger) + pop = func(t *testing.T, logger klog.Logger, queue *PriorityQueue, _ *framework.QueuedPodInfo) { + _, err := queue.Pop(logger) + if err != nil { + t.Fatalf("Unexpected error during Pop: %v", err) + } } popAndRequeueAsUnschedulable = func(t *testing.T, logger klog.Logger, queue *PriorityQueue, pInfo *framework.QueuedPodInfo) { // To simulate the pod is failed in scheduling in the real world, Pop() the pod from activeQ before AddUnschedulableIfNotPresent() below. @@ -4652,116 +4659,144 @@ func TestPriorityQueue_GetPod(t *testing.T) { } func TestUnschedulablePodsMetric(t *testing.T) { - preenqueuePluginName := "preEnqueuePlugin" - queueable := "queueable" - timestamp := time.Now() - pInfos := makeQueuedPodInfos(30, "x", queueable, timestamp) + type step func(t *testing.T, logger klog.Logger, q *PriorityQueue) - makeGated := func(pInfos []*framework.QueuedPodInfo) []*framework.QueuedPodInfo { - for _, pInfo := range pInfos { - setQueuedPodInfoGated(pInfo, preenqueuePluginName, []fwk.ClusterEvent{framework.EventUnschedulableTimeout}) + addPod := func(pInfo *framework.QueuedPodInfo) step { + return func(t *testing.T, logger klog.Logger, q *PriorityQueue) { + add(t, logger, q, pInfo) + } + } + deletePod := func(pInfo *framework.QueuedPodInfo) step { + return func(t *testing.T, logger klog.Logger, q *PriorityQueue) { + deletePod(t, logger, q, pInfo) + } + } + popPod := func() step { + return func(t *testing.T, logger klog.Logger, q *PriorityQueue) { + pop(t, logger, q, nil) + } + } + moveAllToActiveOrBackoffQ := func() step { + return func(t *testing.T, logger klog.Logger, q *PriorityQueue) { + moveAllToActiveOrBackoffQ(t, logger, q, nil) + } + } + updatePluginAllowList := func(pluginName string, list []string) step { + return func(t *testing.T, logger klog.Logger, q *PriorityQueue) { + q.preEnqueuePluginMap[""][pluginName].(*preEnqueuePlugin).allowlists = list } - return pInfos } - tests := []struct { - name string - operations []operation - operands [][]*framework.QueuedPodInfo - expectedMetric int - }{ - { - name: "Unschedulable pods metric must be 0 after a pod is gated, ungated, re-queued, and eventually popped from the scheduling queue", - operations: []operation{ - updatePluginToGateAllPods, - add, - moveAllToActiveOrBackoffQ, - moveAllToActiveOrBackoffQ, - updatePluginToUngateAllPods, - moveAllToActiveOrBackoffQ, - pop, - }, - operands: [][]*framework.QueuedPodInfo{ - {nil}, - pInfos[:1], - {nil}, - {nil}, - {nil}, - {nil}, - {nil}, - }, - expectedMetric: 0, - }, - { - name: "Unschedulable pods metric must be 0 after pod is gated and then deleted", - operations: []operation{ - updatePluginToGateAllPods, - add, - moveAllToActiveOrBackoffQ, - moveAllToActiveOrBackoffQ, - deletePod, - }, - operands: [][]*framework.QueuedPodInfo{ - {nil}, - pInfos[:1], - {nil}, - {nil}, - pInfos[:1], - }, - expectedMetric: 0, - }, - { - name: "Unschedulable pods metric must be 1 after pod is gated multiple time by simillar plugin", - operations: []operation{ - updatePluginToGateAllPods, - add, - moveAllToActiveOrBackoffQ, - moveAllToActiveOrBackoffQ, - }, - operands: [][]*framework.QueuedPodInfo{ - {nil}, - pInfos[:1], - {nil}, - {nil}, - }, - expectedMetric: 1, - }, - { - name: "Unshedulable pods metric must be 0 after non gated pods is added and then deleted", - operations: []operation{ - add, - deletePod, - }, - operands: [][]*framework.QueuedPodInfo{ - pInfos[:1], - pInfos[:1], - }, - expectedMetric: 0, - }, - { - name: "Unschedulable pods metric should not be duplicate if gated pods added and then gated with the same plugin again", - operations: []operation{ - updatePluginToGateAllPods, - add, - moveAllToActiveOrBackoffQ, - }, - operands: [][]*framework.QueuedPodInfo{ - {nil}, - makeGated(pInfos[:10]), - {nil}, - }, - expectedMetric: 10, - }, + pluginName1 := "plugin1" + pluginName2 := "plugin2" + queueable := "queueable" + timestamp := time.Now() + pod := &framework.QueuedPodInfo{ + PodInfo: mustNewPodInfo( + st.MakePod().Name("podA").Namespace("namespaceA").Label(queueable, "").UID("someUid").Obj()), + Timestamp: timestamp, + UnschedulablePlugins: sets.New[string](), } resetMetrics := func() { - metrics.UnschedulableReason(preenqueuePluginName, "").Set(0) + metrics.UnschedulableReason(pluginName1, "").Set(0) + metrics.UnschedulableReason(pluginName2, "").Set(0) } - resetPodInfos := func() { - for i := range pInfos { - pInfos[i].Attempts = 0 - pInfos[i].UnschedulableCount = 0 - } + + makeGated := func(pInfo *framework.QueuedPodInfo) *framework.QueuedPodInfo { + return setQueuedPodInfoGated(pInfo.DeepCopy(), pluginName1, []fwk.ClusterEvent{framework.EventUnschedulableTimeout}) + } + + tests := []struct { + name string + steps []step + expectedMetrics []int + }{ + { + name: "Unschedulable pods metric must be 0 after a pod is gated, ungated, re-queued, and eventually popped from the scheduling queue", + steps: []step{ + updatePluginAllowList(pluginName1, []string{}), + addPod(pod), + moveAllToActiveOrBackoffQ(), + updatePluginAllowList(pluginName1, []string{queueable}), + moveAllToActiveOrBackoffQ(), + popPod(), + }, + expectedMetrics: []int{0, 0}, + }, + { + name: "Unschedulable pods metric must be 0 after pod is gated and then deleted", + steps: []step{ + updatePluginAllowList(pluginName1, []string{}), + addPod(pod), + moveAllToActiveOrBackoffQ(), + deletePod(pod), + }, + expectedMetrics: []int{0, 0}, + }, + { + name: "Unschedulable pods metric must be 1 after pod is gated multiple time by the same plugin", + steps: []step{ + updatePluginAllowList(pluginName1, []string{}), + addPod(pod), + moveAllToActiveOrBackoffQ(), + }, + expectedMetrics: []int{1, 0}, + }, + { + name: "Unschedulable pods metric must be 0 after non gated pods is added and then deleted", + steps: []step{ + addPod(pod), + deletePod(pod), + }, + expectedMetrics: []int{0, 0}, + }, + { + name: "Unschedulable pods metric should not be duplicate if gated pods added and then gated with the same plugin again", + steps: []step{ + updatePluginAllowList(pluginName1, []string{}), + addPod(makeGated(pod)), + moveAllToActiveOrBackoffQ(), + }, + expectedMetrics: []int{1, 0}, + }, + { + name: "Unschedulable pods metric should be 0 if pod was gated by two plugins sequentially and then ungated and popped", + steps: []step{ + updatePluginAllowList(pluginName1, []string{}), + addPod(pod), + updatePluginAllowList(pluginName1, []string{queueable}), + updatePluginAllowList(pluginName2, []string{}), + moveAllToActiveOrBackoffQ(), + updatePluginAllowList(pluginName2, []string{queueable}), + moveAllToActiveOrBackoffQ(), + popPod(), + }, + expectedMetrics: []int{0, 0}, + }, + { + name: "Unschedulable pods metric should be 0 if pod was gated by two plugins sequentially and then deleted", + steps: []step{ + updatePluginAllowList(pluginName1, []string{}), + addPod(pod), + updatePluginAllowList(pluginName1, []string{queueable}), + updatePluginAllowList(pluginName2, []string{}), + moveAllToActiveOrBackoffQ(), + deletePod(pod), + }, + expectedMetrics: []int{0, 0}, + }, + { + name: "Unschedulable pods metric should be 1 for both plugins if pod was gated by two plugins sequentially", + steps: []step{ + updatePluginAllowList(pluginName1, []string{}), + addPod(pod), + updatePluginAllowList(pluginName1, []string{queueable}), + updatePluginAllowList(pluginName2, []string{}), + moveAllToActiveOrBackoffQ(), + }, + expectedMetrics: []int{1, 1}, + }, } for _, tt := range tests { @@ -4770,34 +4805,41 @@ func TestUnschedulablePodsMetric(t *testing.T) { ctx, cancel := context.WithCancel(ctx) defer cancel() resetMetrics() - resetPodInfos() m := makeEmptyQueueingHintMapPerProfile() m[""][framework.EventUnschedulableTimeout] = []*QueueingHintFunction{ { - PluginName: preenqueuePluginName, + PluginName: pluginName1, + QueueingHintFn: queueHintReturnQueue, + }, + { + PluginName: pluginName2, QueueingHintFn: queueHintReturnQueue, }, } - preenq := map[string]map[string]fwk.PreEnqueuePlugin{"": {(&preEnqueuePlugin{}).Name(): &preEnqueuePlugin{allowlists: []string{queueable}}}} - recorder := metrics.NewMetricsAsyncRecorder(3, 20*time.Microsecond, ctx.Done()) - queue := NewTestQueue(ctx, newDefaultQueueSort(), WithClock(testingclock.NewFakeClock(timestamp)), WithPreEnqueuePluginMap(preenq), WithMetricsRecorder(recorder), WithQueueingHintMapPerProfile(m)) + plugin1 := preEnqueuePlugin{name: pluginName1, allowlists: []string{queueable}} + plugin2 := preEnqueuePlugin{name: pluginName2, allowlists: []string{queueable}} - for i, op := range tt.operations { - for _, pInfo := range tt.operands[i] { - op(t, logger, queue, pInfo) + preenq := map[string]map[string]fwk.PreEnqueuePlugin{"": {pluginName1: &plugin1, pluginName2: &plugin2}} + recorder := metrics.NewMetricsAsyncRecorder(3, 20*time.Microsecond, ctx.Done()) + q := NewTestQueue(ctx, newDefaultQueueSort(), WithClock(testingclock.NewFakeClock(timestamp)), WithPreEnqueuePluginMap(preenq), WithMetricsRecorder(recorder), WithQueueingHintMapPerProfile(m)) + + for _, step := range tt.steps { + step(t, logger, q) + } + + for i, pluginName := range []string{pluginName1, pluginName2} { + val, err := testutil.GetGaugeMetricValue(metrics.UnschedulableReason(pluginName, "")) + + if err != nil { + t.Errorf("Error while collection metric value:\n%s", err) + } + if int(val) != tt.expectedMetrics[i] { + t.Errorf("Unexpected metric for plugin %s result expected %d, actual %d", pluginName, tt.expectedMetrics[i], int(val)) } } - val, err := testutil.GetGaugeMetricValue(metrics.UnschedulableReason(preenqueuePluginName, "")) - - if err != nil { - t.Errorf("Error while collection metric value:\n%s", err) - } - if int(val) != tt.expectedMetric { - t.Errorf("Unexpected metric result expected %d, actual %d", tt.expectedMetric, int(val)) - } }) } }