From 3b64c1b01d983e8f8f156a6dd92f9b3722d08c1e Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Fri, 17 Sep 2021 14:08:18 -0700 Subject: [PATCH] sched: de-duplicate plugin registration logic by using FactoryAdapter --- .../default_preemption_test.go | 8 ++--- .../interpodaffinity/filtering_test.go | 29 +++++-------------- .../plugins/interpodaffinity/scoring_test.go | 17 ++++------- pkg/scheduler/generic_scheduler_test.go | 25 +++++----------- pkg/scheduler/scheduler_test.go | 4 +-- 5 files changed, 22 insertions(+), 61 deletions(-) diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go index 95b13ed21ca..751214672a2 100644 --- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go +++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go @@ -93,9 +93,7 @@ func getDefaultDefaultPreemptionArgs() *config.DefaultPreemptionArgs { return dpa } -var nodeResourcesFitFunc = func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return noderesources.NewFit(plArgs, fh, feature.Features{}) -} +var nodeResourcesFitFunc = frameworkruntime.FactoryAdapter(feature.Features{}, noderesources.NewFit) func TestPostFilter(t *testing.T) { onePodRes := map[v1.ResourceName]string{v1.ResourcePods: "1"} @@ -526,9 +524,7 @@ func TestDryRunPreemption(t *testing.T) { name: "pod with anti-affinity is preempted", registerPlugins: []st.RegisterPluginFunc{ st.RegisterPluginAsExtensions(noderesources.FitName, nodeResourcesFitFunc, "Filter", "PreFilter"), - st.RegisterPluginAsExtensions(interpodaffinity.Name, func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return interpodaffinity.New(plArgs, fh, feature.Features{}) - }, "Filter", "PreFilter"), + st.RegisterPluginAsExtensions(interpodaffinity.Name, frameworkruntime.FactoryAdapter(feature.Features{}, interpodaffinity.New), "Filter", "PreFilter"), }, nodeNames: []string{"node1", "node2"}, testPods: []*v1.Pod{ diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go b/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go index a03dca019fe..f243459fbdc 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go @@ -30,6 +30,7 @@ import ( "k8s.io/kubernetes/pkg/scheduler/framework" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature" plugintesting "k8s.io/kubernetes/pkg/scheduler/framework/plugins/testing" + frameworkruntime "k8s.io/kubernetes/pkg/scheduler/framework/runtime" "k8s.io/kubernetes/pkg/scheduler/internal/cache" ) @@ -996,12 +997,8 @@ func TestRequiredAffinitySingleNode(t *testing.T) { t.Run(test.name, func(t *testing.T) { ctx := context.Background() snapshot := cache.NewSnapshot(test.pods, []*v1.Node{test.node}) - n := func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return New(plArgs, fh, feature.Features{ - EnablePodAffinityNamespaceSelector: !test.disableNSSelector, - }) - } - p := plugintesting.SetupPluginWithInformers(ctx, t, n, &config.InterPodAffinityArgs{}, snapshot, namespaces) + fts := feature.Features{EnablePodAffinityNamespaceSelector: !test.disableNSSelector} + p := plugintesting.SetupPluginWithInformers(ctx, t, frameworkruntime.FactoryAdapter(fts, New), &config.InterPodAffinityArgs{}, snapshot, namespaces) state := framework.NewCycleState() preFilterStatus := p.(framework.PreFilterPlugin).PreFilter(ctx, state, test.pod) if !preFilterStatus.IsSuccess() { @@ -1865,10 +1862,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { t.Run(test.name, func(t *testing.T) { ctx := context.Background() snapshot := cache.NewSnapshot(test.pods, test.nodes) - n := func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return New(plArgs, fh, feature.Features{}) - } - p := plugintesting.SetupPluginWithInformers(ctx, t, n, &config.InterPodAffinityArgs{}, snapshot, + p := plugintesting.SetupPluginWithInformers(ctx, t, frameworkruntime.FactoryAdapter(feature.Features{}, New), &config.InterPodAffinityArgs{}, snapshot, []runtime.Object{ &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "NS1"}}, }) @@ -1893,10 +1887,7 @@ func TestPreFilterDisabled(t *testing.T) { nodeInfo := framework.NewNodeInfo() node := v1.Node{} nodeInfo.SetNode(&node) - n := func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return New(plArgs, fh, feature.Features{}) - } - p := plugintesting.SetupPlugin(t, n, &config.InterPodAffinityArgs{}, cache.NewEmptySnapshot()) + p := plugintesting.SetupPlugin(t, frameworkruntime.FactoryAdapter(feature.Features{}, New), &config.InterPodAffinityArgs{}, cache.NewEmptySnapshot()) cycleState := framework.NewCycleState() gotStatus := p.(framework.FilterPlugin).Filter(context.Background(), cycleState, pod, nodeInfo) wantStatus := framework.AsStatus(fmt.Errorf(`error reading "PreFilterInterPodAffinity" from cycleState: %w`, framework.ErrNotFound)) @@ -2164,10 +2155,7 @@ func TestPreFilterStateAddRemovePod(t *testing.T) { // getMeta creates predicate meta data given the list of pods. getState := func(pods []*v1.Pod) (*InterPodAffinity, *framework.CycleState, *preFilterState, *cache.Snapshot) { snapshot := cache.NewSnapshot(pods, test.nodes) - n := func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return New(plArgs, fh, feature.Features{}) - } - p := plugintesting.SetupPlugin(t, n, &config.InterPodAffinityArgs{}, snapshot) + p := plugintesting.SetupPlugin(t, frameworkruntime.FactoryAdapter(feature.Features{}, New), &config.InterPodAffinityArgs{}, snapshot) cycleState := framework.NewCycleState() preFilterStatus := p.(framework.PreFilterPlugin).PreFilter(ctx, cycleState, test.pendingPod) if !preFilterStatus.IsSuccess() { @@ -2450,10 +2438,7 @@ func TestGetTPMapMatchingIncomingAffinityAntiAffinity(t *testing.T) { t.Run(tt.name, func(t *testing.T) { snapshot := cache.NewSnapshot(tt.existingPods, tt.nodes) l, _ := snapshot.NodeInfos().List() - n := func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return New(plArgs, fh, feature.Features{}) - } - p := plugintesting.SetupPlugin(t, n, &config.InterPodAffinityArgs{}, snapshot) + p := plugintesting.SetupPlugin(t, frameworkruntime.FactoryAdapter(feature.Features{}, New), &config.InterPodAffinityArgs{}, snapshot) gotAffinityPodsMap, gotAntiAffinityPodsMap := p.(*InterPodAffinity).getIncomingAffinityAntiAffinityCounts(framework.NewPodInfo(tt.pod), l, true) if !reflect.DeepEqual(gotAffinityPodsMap, tt.wantAffinityPodsMap) { t.Errorf("getTPMapMatchingIncomingAffinityAntiAffinity() gotAffinityPodsMap = %#v, want %#v", gotAffinityPodsMap, tt.wantAffinityPodsMap) diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go b/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go index fb90f9b82ce..6eb0f541096 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go @@ -29,6 +29,7 @@ import ( "k8s.io/kubernetes/pkg/scheduler/framework" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature" plugintesting "k8s.io/kubernetes/pkg/scheduler/framework/plugins/testing" + frameworkruntime "k8s.io/kubernetes/pkg/scheduler/framework/runtime" "k8s.io/kubernetes/pkg/scheduler/internal/cache" ) @@ -743,12 +744,8 @@ func TestPreferredAffinity(t *testing.T) { t.Run(test.name, func(t *testing.T) { ctx := context.Background() state := framework.NewCycleState() - n := func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return New(plArgs, fh, feature.Features{ - EnablePodAffinityNamespaceSelector: !test.disableNSSelector, - }) - } - p := plugintesting.SetupPluginWithInformers(ctx, t, n, &config.InterPodAffinityArgs{HardPodAffinityWeight: 1}, cache.NewSnapshot(test.pods, test.nodes), namespaces) + fts := feature.Features{EnablePodAffinityNamespaceSelector: !test.disableNSSelector} + p := plugintesting.SetupPluginWithInformers(ctx, t, frameworkruntime.FactoryAdapter(fts, New), &config.InterPodAffinityArgs{HardPodAffinityWeight: 1}, cache.NewSnapshot(test.pods, test.nodes), namespaces) status := p.(framework.PreScorePlugin).PreScore(ctx, state, test.pod, test.nodes) if !status.IsSuccess() { if !strings.Contains(status.Message(), test.wantStatus.Message()) { @@ -909,12 +906,8 @@ func TestPreferredAffinityWithHardPodAffinitySymmetricWeight(t *testing.T) { t.Run(test.name, func(t *testing.T) { ctx := context.Background() state := framework.NewCycleState() - n := func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return New(plArgs, fh, feature.Features{ - EnablePodAffinityNamespaceSelector: !test.disableNSSelector, - }) - } - p := plugintesting.SetupPluginWithInformers(ctx, t, n, &config.InterPodAffinityArgs{HardPodAffinityWeight: test.hardPodAffinityWeight}, cache.NewSnapshot(test.pods, test.nodes), namespaces) + fts := feature.Features{EnablePodAffinityNamespaceSelector: !test.disableNSSelector} + p := plugintesting.SetupPluginWithInformers(ctx, t, frameworkruntime.FactoryAdapter(fts, New), &config.InterPodAffinityArgs{HardPodAffinityWeight: test.hardPodAffinityWeight}, cache.NewSnapshot(test.pods, test.nodes), namespaces) status := p.(framework.PreScorePlugin).PreScore(ctx, state, test.pod, test.nodes) if !status.IsSuccess() { t.Errorf("unexpected error: %v", status) diff --git a/pkg/scheduler/generic_scheduler_test.go b/pkg/scheduler/generic_scheduler_test.go index 07ed33bac11..cb78ea3e3b8 100644 --- a/pkg/scheduler/generic_scheduler_test.go +++ b/pkg/scheduler/generic_scheduler_test.go @@ -447,6 +447,7 @@ func TestFindNodesThatPassExtenders(t *testing.T) { } func TestGenericScheduler(t *testing.T) { + fts := feature.Features{} tests := []struct { name string registerPlugins []st.RegisterPluginFunc @@ -605,9 +606,7 @@ func TestGenericScheduler(t *testing.T) { // Pod with existing PVC registerPlugins: []st.RegisterPluginFunc{ st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New), - st.RegisterPreFilterPlugin(volumebinding.Name, func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return volumebinding.New(plArgs, fh, feature.Features{}) - }), + st.RegisterPreFilterPlugin(volumebinding.Name, frameworkruntime.FactoryAdapter(fts, volumebinding.New)), st.RegisterFilterPlugin("TrueFilter", st.NewTrueFilterPlugin), st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New), }, @@ -640,9 +639,7 @@ func TestGenericScheduler(t *testing.T) { // Pod with non existing PVC registerPlugins: []st.RegisterPluginFunc{ st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New), - st.RegisterPreFilterPlugin(volumebinding.Name, func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return volumebinding.New(plArgs, fh, feature.Features{}) - }), + st.RegisterPreFilterPlugin(volumebinding.Name, frameworkruntime.FactoryAdapter(fts, volumebinding.New)), st.RegisterFilterPlugin("TrueFilter", st.NewTrueFilterPlugin), st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New), }, @@ -691,9 +688,7 @@ func TestGenericScheduler(t *testing.T) { // Pod with deleting PVC registerPlugins: []st.RegisterPluginFunc{ st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New), - st.RegisterPreFilterPlugin(volumebinding.Name, func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return volumebinding.New(plArgs, fh, feature.Features{}) - }), + st.RegisterPreFilterPlugin(volumebinding.Name, frameworkruntime.FactoryAdapter(fts, volumebinding.New)), st.RegisterFilterPlugin("TrueFilter", st.NewTrueFilterPlugin), st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New), }, @@ -1314,17 +1309,11 @@ func TestZeroRequest(t *testing.T) { informerFactory := informers.NewSharedInformerFactory(client, 0) snapshot := internalcache.NewSnapshot(test.pods, test.nodes) - + fts := feature.Features{} pluginRegistrations := []st.RegisterPluginFunc{ st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New), - st.RegisterScorePlugin(noderesources.FitName, - func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return noderesources.NewFit(plArgs, fh, feature.Features{}) - }, - 1), - st.RegisterScorePlugin(noderesources.BalancedAllocationName, func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return noderesources.NewBalancedAllocation(plArgs, fh, feature.Features{}) - }, 1), + st.RegisterScorePlugin(noderesources.FitName, frameworkruntime.FactoryAdapter(fts, noderesources.NewFit), 1), + st.RegisterScorePlugin(noderesources.BalancedAllocationName, frameworkruntime.FactoryAdapter(fts, noderesources.NewBalancedAllocation), 1), st.RegisterScorePlugin(selectorspread.Name, selectorspread.New, 1), st.RegisterPreScorePlugin(selectorspread.Name, selectorspread.New), st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New), diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index c1c0f0b1ce6..e3deee6ae07 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -866,9 +866,7 @@ func TestSchedulerFailedSchedulingReasons(t *testing.T) { fns := []st.RegisterPluginFunc{ st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New), st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New), - st.RegisterPluginAsExtensions(noderesources.FitName, func(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return noderesources.NewFit(plArgs, fh, feature.Features{}) - }, "Filter", "PreFilter"), + st.RegisterPluginAsExtensions(noderesources.FitName, frameworkruntime.FactoryAdapter(feature.Features{}, noderesources.NewFit), "Filter", "PreFilter"), } scheduler, _, errChan := setupTestScheduler(queuedPodStore, scache, informerFactory, nil, fns...)