From f3ece885b2eae2657e9c09fabd361aa8daec5a37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Dzikowski?= Date: Tue, 27 Oct 2026 09:32:32 +0000 Subject: [PATCH] scheduler: log error when PostFilter plugin does not implement PodGroupPostFilterPlugin --- pkg/scheduler/framework/runtime/framework.go | 15 +- .../framework/runtime/framework_test.go | 223 ++++++++++++++++++ 2 files changed, 237 insertions(+), 1 deletion(-) diff --git a/pkg/scheduler/framework/runtime/framework.go b/pkg/scheduler/framework/runtime/framework.go index bcfffae38ab..ac4f3bb96e3 100644 --- a/pkg/scheduler/framework/runtime/framework.go +++ b/pkg/scheduler/framework/runtime/framework.go @@ -524,7 +524,8 @@ func NewFramework(ctx context.Context, r Registry, profile *config.KubeScheduler f.computeBatchablePlugins() } - // Put default preemption as the only RankNodesPlugin + // Put default preemption as the only RankNodesPlugin and check that all PostFilter + // plugins are also enabled at PodGroupPostFilter, as PodGroups only run the latter. if utilfeature.DefaultFeatureGate.Enabled(features.GenericWorkload) { hasPodGroupPostFilter := slices.ContainsFunc(f.podGroupPostFilterPlugins, func(pl fwk.PodGroupPostFilterPlugin) bool { return pl.Name() == names.DefaultPreemption @@ -541,6 +542,18 @@ func NewFramework(ctx context.Context, r Registry, profile *config.KubeScheduler } else { logger.V(2).Info("GenericWorkload is enabled, but default preemption plugin does not fulfill RankNodesPlugin interface. Node ranking will not be used.") } + + for _, pl := range f.postFilterPlugins { + if !slices.ContainsFunc(f.podGroupPostFilterPlugins, func(pgpl fwk.PodGroupPostFilterPlugin) bool { + return pgpl.Name() == pl.Name() + }) { + msg := "PostFilter plugin does not implement PodGroupPostFilterPlugin; it will not be run for PodGroups or CompositePodGroups" + if _, ok := pl.(fwk.PodGroupPostFilterPlugin); ok { + msg = "PostFilter plugin implements PodGroupPostFilterPlugin but is not enabled; it will not be run for PodGroups or CompositePodGroups" + } + logger.Error(nil, msg, "plugin", pl.Name(), "profile", f.profileName) + } + } } if options.captureProfile != nil { diff --git a/pkg/scheduler/framework/runtime/framework_test.go b/pkg/scheduler/framework/runtime/framework_test.go index 01e394b06bf..c0e76f0ab01 100644 --- a/pkg/scheduler/framework/runtime/framework_test.go +++ b/pkg/scheduler/framework/runtime/framework_test.go @@ -40,6 +40,7 @@ import ( clientsetfake "k8s.io/client-go/kubernetes/fake" featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/component-base/metrics/testutil" + "k8s.io/klog/v2" "k8s.io/klog/v2/ktesting" fwk "k8s.io/kube-scheduler/framework" "k8s.io/kubernetes/pkg/features" @@ -72,6 +73,10 @@ const ( placementGeneratePlugin = "placement-generate-plugin" placementGeneratePlugin2 = "placement-generate-plugin-2" defaultPreemptionPlugin = names.DefaultPreemption + postFilterOnlyPlugin1 = "post-filter-only-plugin-1" + postFilterOnlyPlugin2 = "post-filter-only-plugin-2" + filterAndPostFilterPlugin = "filter-and-post-filter-plugin" + bothPostFilterPlugin = "post-filter-and-pod-group-post-filter-plugin" testProfileName = "test-profile" testPercentageOfNodesToScore = 35 @@ -496,6 +501,10 @@ var registry = func() Registry { r.Register(placementGeneratePlugin2, newTestPlacementGeneratePlugin2) r.Register(placementScorePlugin1, newPlacementScorePluginFactory(placementScorePlugin1)) r.Register(defaultPreemptionPlugin, newMockDefaultPreemptionPlugin) + r.Register(postFilterOnlyPlugin1, newPostFilterOnlyPluginFactory(postFilterOnlyPlugin1)) + r.Register(postFilterOnlyPlugin2, newPostFilterOnlyPluginFactory(postFilterOnlyPlugin2)) + r.Register(filterAndPostFilterPlugin, newFilterAndPostFilterPlugin) + r.Register(bothPostFilterPlugin, newBothPostFilterPlugin) return r }() @@ -802,6 +811,220 @@ func TestPodGroupPostFilterPlugins(t *testing.T) { } } +func TestPostFilterWithoutPodGroupPostFilterWarning(t *testing.T) { + const ( + wantNotImplementedMsg = "PostFilter plugin does not implement PodGroupPostFilterPlugin; it will not be run for PodGroups or CompositePodGroups" + wantNotEnabledMsg = "PostFilter plugin implements PodGroupPostFilterPlugin but is not enabled; it will not be run for PodGroups or CompositePodGroups" + ) + tests := []struct { + name string + genericWorkloadEnabled bool + multiPointPlugins []config.Plugin + filterPlugins []config.Plugin + postFilterPlugins []config.Plugin + podGroupPostFilterPlugins []config.Plugin + podGroupPostFilterDisabled []config.Plugin + wantErrorLogs []ktesting.LogEntry + }{ + { + name: "GenericWorkload enabled, PostFilter plugin is also enabled at PodGroupPostFilter", + genericWorkloadEnabled: true, + postFilterPlugins: []config.Plugin{ + {Name: defaultPreemptionPlugin}, + }, + podGroupPostFilterPlugins: []config.Plugin{ + {Name: defaultPreemptionPlugin}, + }, + }, + { + name: "GenericWorkload enabled, PostFilter plugin implementing PodGroupPostFilterPlugin is enabled only at PostFilter", + genericWorkloadEnabled: true, + postFilterPlugins: []config.Plugin{ + {Name: defaultPreemptionPlugin}, + }, + wantErrorLogs: []ktesting.LogEntry{ + {Type: ktesting.LogError, Message: wantNotEnabledMsg, ParameterKVList: []any{"plugin", defaultPreemptionPlugin, "profile", testProfileName}}, + }, + }, + { + name: "GenericWorkload enabled, PostFilter plugin does not implement PodGroupPostFilterPlugin", + genericWorkloadEnabled: true, + postFilterPlugins: []config.Plugin{ + {Name: postFilterOnlyPlugin1}, + }, + wantErrorLogs: []ktesting.LogEntry{ + {Type: ktesting.LogError, Message: wantNotImplementedMsg, ParameterKVList: []any{"plugin", postFilterOnlyPlugin1, "profile", testProfileName}}, + }, + }, + { + name: "GenericWorkload enabled, multiple PostFilter plugins with and without PodGroupPostFilterPlugin", + genericWorkloadEnabled: true, + postFilterPlugins: []config.Plugin{ + {Name: defaultPreemptionPlugin}, + {Name: postFilterOnlyPlugin1}, + {Name: postFilterOnlyPlugin2}, + }, + podGroupPostFilterPlugins: []config.Plugin{ + {Name: defaultPreemptionPlugin}, + }, + wantErrorLogs: []ktesting.LogEntry{ + {Type: ktesting.LogError, Message: wantNotImplementedMsg, ParameterKVList: []any{"plugin", postFilterOnlyPlugin1, "profile", testProfileName}}, + {Type: ktesting.LogError, Message: wantNotImplementedMsg, ParameterKVList: []any{"plugin", postFilterOnlyPlugin2, "profile", testProfileName}}, + }, + }, + { + name: "GenericWorkload disabled, PostFilter plugin is also enabled at PodGroupPostFilter", + genericWorkloadEnabled: false, + postFilterPlugins: []config.Plugin{ + {Name: defaultPreemptionPlugin}, + }, + podGroupPostFilterPlugins: []config.Plugin{ + {Name: defaultPreemptionPlugin}, + }, + }, + { + name: "GenericWorkload disabled, PostFilter plugin implementing PodGroupPostFilterPlugin is enabled only at PostFilter", + genericWorkloadEnabled: false, + postFilterPlugins: []config.Plugin{ + {Name: defaultPreemptionPlugin}, + }, + }, + { + name: "GenericWorkload disabled, PostFilter plugin does not implement PodGroupPostFilterPlugin", + genericWorkloadEnabled: false, + postFilterPlugins: []config.Plugin{ + {Name: postFilterOnlyPlugin1}, + }, + }, + { + name: "GenericWorkload enabled, MultiPoint plugin does not implement PodGroupPostFilterPlugin", + genericWorkloadEnabled: true, + multiPointPlugins: []config.Plugin{ + {Name: postFilterOnlyPlugin1}, + {Name: bothPostFilterPlugin}, + }, + wantErrorLogs: []ktesting.LogEntry{ + {Type: ktesting.LogError, Message: wantNotImplementedMsg, ParameterKVList: []any{"plugin", postFilterOnlyPlugin1, "profile", testProfileName}}, + }, + }, + { + name: "GenericWorkload enabled, MultiPoint plugin disabled at PodGroupPostFilter", + genericWorkloadEnabled: true, + multiPointPlugins: []config.Plugin{ + {Name: bothPostFilterPlugin}, + }, + podGroupPostFilterDisabled: []config.Plugin{ + {Name: bothPostFilterPlugin}, + }, + wantErrorLogs: []ktesting.LogEntry{ + {Type: ktesting.LogError, Message: wantNotEnabledMsg, ParameterKVList: []any{"plugin", bothPostFilterPlugin, "profile", testProfileName}}, + }, + }, + { + name: "GenericWorkload enabled, plugin implementing PostFilter is enabled only at Filter", + genericWorkloadEnabled: true, + filterPlugins: []config.Plugin{ + {Name: filterAndPostFilterPlugin}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GenericWorkload, tc.genericWorkloadEnabled) + + logger := ktesting.NewLogger(t, ktesting.NewConfig(ktesting.BufferLogs(true))) + ctx, cancel := context.WithCancel(klog.NewContext(context.Background(), logger)) + defer cancel() + + profile := config.KubeSchedulerProfile{ + SchedulerName: testProfileName, + Plugins: &config.Plugins{ + MultiPoint: config.PluginSet{ + Enabled: tc.multiPointPlugins, + }, + Filter: config.PluginSet{ + Enabled: tc.filterPlugins, + }, + PostFilter: config.PluginSet{ + Enabled: tc.postFilterPlugins, + }, + PodGroupPostFilter: config.PluginSet{ + Enabled: tc.podGroupPostFilterPlugins, + Disabled: tc.podGroupPostFilterDisabled, + }, + }, + } + if _, err := newFrameworkWithQueueSortAndBind(ctx, registry, profile); err != nil { + t.Fatalf("Unexpected framework initialization error: %v", err) + } + var gotErrorLogs []ktesting.LogEntry + for _, entry := range logger.GetSink().(ktesting.Underlier).GetBuffer().Data() { + if entry.Type == ktesting.LogError { + gotErrorLogs = append(gotErrorLogs, entry) + } + } + if diff := cmp.Diff(tc.wantErrorLogs, gotErrorLogs, cmpopts.IgnoreFields(ktesting.LogEntry{}, "Timestamp")); diff != "" { + t.Errorf("Unexpected error logs (-want,+got):\n%s", diff) + } + }) + } +} + +type testPostFilterOnlyPlugin struct { + name string +} + +func (pl *testPostFilterOnlyPlugin) Name() string { + return pl.name +} + +func (pl *testPostFilterOnlyPlugin) PostFilter(_ context.Context, _ fwk.CycleState, _ *v1.Pod, _ fwk.NodeToStatusReader) (*fwk.PostFilterResult, *fwk.Status) { + return nil, nil +} + +var _ fwk.PostFilterPlugin = &testPostFilterOnlyPlugin{} + +func newPostFilterOnlyPluginFactory(name string) func(context.Context, runtime.Object, fwk.Handle) (fwk.Plugin, error) { + return func(context.Context, runtime.Object, fwk.Handle) (fwk.Plugin, error) { + return &testPostFilterOnlyPlugin{name: name}, nil + } +} + +// testFilterAndPostFilterPlugin implements FilterPlugin and PostFilterPlugin, but not PodGroupPostFilterPlugin. +type testFilterAndPostFilterPlugin struct { + testPostFilterOnlyPlugin +} + +func (pl *testFilterAndPostFilterPlugin) Filter(_ context.Context, _ fwk.CycleState, _ *v1.Pod, _ fwk.NodeInfo) *fwk.Status { + return nil +} + +var ( + _ fwk.FilterPlugin = &testFilterAndPostFilterPlugin{} + _ fwk.PostFilterPlugin = &testFilterAndPostFilterPlugin{} +) + +func newFilterAndPostFilterPlugin(_ context.Context, _ runtime.Object, _ fwk.Handle) (fwk.Plugin, error) { + return &testFilterAndPostFilterPlugin{testPostFilterOnlyPlugin: testPostFilterOnlyPlugin{name: filterAndPostFilterPlugin}}, nil +} + +// testBothPostFilterPlugin implements PostFilterPlugin and PodGroupPostFilterPlugin only, so +// MultiPoint expansion does not add it to QueueSort or Bind. +type testBothPostFilterPlugin struct { + testPostFilterOnlyPlugin +} + +func (pl *testBothPostFilterPlugin) PodGroupPostFilter(_ context.Context, _ fwk.PodGroupCycleState, _ fwk.PodGroupInfo, _ fwk.PodGroupSchedulingFunc) (*fwk.PodGroupPostFilterResult, *fwk.Status) { + return nil, nil +} + +var _ fwk.PodGroupPostFilterPlugin = &testBothPostFilterPlugin{} + +func newBothPostFilterPlugin(_ context.Context, _ runtime.Object, _ fwk.Handle) (fwk.Plugin, error) { + return &testBothPostFilterPlugin{testPostFilterOnlyPlugin: testPostFilterOnlyPlugin{name: bothPostFilterPlugin}}, nil +} + // mockDefaultPreemptionPlugin is used to simulate a default preemption plugin that implements PodGroupPostFilterPlugin type mockDefaultPreemptionPlugin struct { TestPlugin