From 50d511d4b8a3ee5783440fa4d735ce097b910a3e Mon Sep 17 00:00:00 2001 From: Haosdent Huang Date: Tue, 21 Jan 2020 11:47:52 +0800 Subject: [PATCH] Deprecate scheduler's framework.plugins.RegistryArgs --- pkg/scheduler/factory_test.go | 2 +- pkg/scheduler/framework/plugins/BUILD | 1 - pkg/scheduler/framework/plugins/registry.go | 35 +++++++------------ .../framework/plugins/volumebinding/BUILD | 1 + .../plugins/volumebinding/volume_binding.go | 9 ++--- .../volumebinding/volume_binding_test.go | 6 ++-- .../internal/queue/scheduling_queue_test.go | 2 +- pkg/scheduler/scheduler.go | 4 +-- pkg/scheduler/scheduler_test.go | 23 ++++++------ 9 files changed, 38 insertions(+), 45 deletions(-) diff --git a/pkg/scheduler/factory_test.go b/pkg/scheduler/factory_test.go index 759e36dbbe4..e34ce027ae9 100644 --- a/pkg/scheduler/factory_test.go +++ b/pkg/scheduler/factory_test.go @@ -435,7 +435,7 @@ func newConfigFactoryWithFrameworkRegistry( func newConfigFactory( client clientset.Interface, hardPodAffinitySymmetricWeight int32, stopCh <-chan struct{}) *Configurator { return newConfigFactoryWithFrameworkRegistry(client, hardPodAffinitySymmetricWeight, stopCh, - frameworkplugins.NewInTreeRegistry(&frameworkplugins.RegistryArgs{})) + frameworkplugins.NewInTreeRegistry()) } type fakeExtender struct { diff --git a/pkg/scheduler/framework/plugins/BUILD b/pkg/scheduler/framework/plugins/BUILD index 1bd66d44321..6081076ae07 100644 --- a/pkg/scheduler/framework/plugins/BUILD +++ b/pkg/scheduler/framework/plugins/BUILD @@ -30,7 +30,6 @@ go_library( "//pkg/scheduler/framework/plugins/volumerestrictions:go_default_library", "//pkg/scheduler/framework/plugins/volumezone:go_default_library", "//pkg/scheduler/framework/v1alpha1:go_default_library", - "//pkg/scheduler/volumebinder:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", diff --git a/pkg/scheduler/framework/plugins/registry.go b/pkg/scheduler/framework/plugins/registry.go index 38906da993b..cd4ad049711 100644 --- a/pkg/scheduler/framework/plugins/registry.go +++ b/pkg/scheduler/framework/plugins/registry.go @@ -17,7 +17,6 @@ limitations under the License. package plugins import ( - "k8s.io/apimachinery/pkg/runtime" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/defaultpodtopologyspread" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/imagelocality" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/interpodaffinity" @@ -37,18 +36,12 @@ import ( "k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumerestrictions" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumezone" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" - "k8s.io/kubernetes/pkg/scheduler/volumebinder" ) -// RegistryArgs arguments needed to create default plugin factories. -type RegistryArgs struct { - VolumeBinder *volumebinder.VolumeBinder -} - // NewInTreeRegistry builds the registry with all the in-tree plugins. // A scheduler that runs out of tree plugins can register additional plugins // through the WithFrameworkOutOfTreeRegistry option. -func NewInTreeRegistry(args *RegistryArgs) framework.Registry { +func NewInTreeRegistry() framework.Registry { return framework.Registry{ defaultpodtopologyspread.Name: defaultpodtopologyspread.New, imagelocality.Name: imagelocality.New, @@ -65,19 +58,17 @@ func NewInTreeRegistry(args *RegistryArgs) framework.Registry { noderesources.LeastAllocatedName: noderesources.NewLeastAllocated, noderesources.RequestedToCapacityRatioName: noderesources.NewRequestedToCapacityRatio, noderesources.ResourceLimitsName: noderesources.NewResourceLimits, - volumebinding.Name: func(_ *runtime.Unknown, _ framework.FrameworkHandle) (framework.Plugin, error) { - return volumebinding.NewFromVolumeBinder(args.VolumeBinder), nil - }, - volumerestrictions.Name: volumerestrictions.New, - volumezone.Name: volumezone.New, - nodevolumelimits.CSIName: nodevolumelimits.NewCSI, - nodevolumelimits.EBSName: nodevolumelimits.NewEBS, - nodevolumelimits.GCEPDName: nodevolumelimits.NewGCEPD, - nodevolumelimits.AzureDiskName: nodevolumelimits.NewAzureDisk, - nodevolumelimits.CinderName: nodevolumelimits.NewCinder, - interpodaffinity.Name: interpodaffinity.New, - nodelabel.Name: nodelabel.New, - serviceaffinity.Name: serviceaffinity.New, - queuesort.Name: queuesort.New, + volumebinding.Name: volumebinding.New, + volumerestrictions.Name: volumerestrictions.New, + volumezone.Name: volumezone.New, + nodevolumelimits.CSIName: nodevolumelimits.NewCSI, + nodevolumelimits.EBSName: nodevolumelimits.NewEBS, + nodevolumelimits.GCEPDName: nodevolumelimits.NewGCEPD, + nodevolumelimits.AzureDiskName: nodevolumelimits.NewAzureDisk, + nodevolumelimits.CinderName: nodevolumelimits.NewCinder, + interpodaffinity.Name: interpodaffinity.New, + nodelabel.Name: nodelabel.New, + serviceaffinity.Name: serviceaffinity.New, + queuesort.Name: queuesort.New, } } diff --git a/pkg/scheduler/framework/plugins/volumebinding/BUILD b/pkg/scheduler/framework/plugins/volumebinding/BUILD index dcdd72415b6..70dfbd50880 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/BUILD +++ b/pkg/scheduler/framework/plugins/volumebinding/BUILD @@ -10,6 +10,7 @@ go_library( "//pkg/scheduler/nodeinfo:go_default_library", "//pkg/scheduler/volumebinder:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", ], ) diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go index 305573e9aba..772dad6f220 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go @@ -20,6 +20,7 @@ import ( "context" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo" "k8s.io/kubernetes/pkg/scheduler/volumebinder" @@ -97,9 +98,9 @@ func (pl *VolumeBinding) Filter(ctx context.Context, cs *framework.CycleState, p return nil } -// NewFromVolumeBinder initializes a new plugin with volume binder and returns it. -func NewFromVolumeBinder(volumeBinder *volumebinder.VolumeBinder) framework.Plugin { +// New initializes a new plugin with volume binder and returns it. +func New(_ *runtime.Unknown, fh framework.FrameworkHandle) (framework.Plugin, error) { return &VolumeBinding{ - binder: volumeBinder, - } + binder: fh.VolumeBinder(), + }, nil } diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go index 557112e9780..390e0c58392 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go @@ -111,8 +111,10 @@ func TestVolumeBinding(t *testing.T) { nodeInfo := schedulernodeinfo.NewNodeInfo() nodeInfo.SetNode(item.node) fakeVolumeBinder := volumebinder.NewFakeVolumeBinder(item.volumeBinderConfig) - p := NewFromVolumeBinder(fakeVolumeBinder) - gotStatus := p.(framework.FilterPlugin).Filter(context.Background(), nil, item.pod, nodeInfo) + p := &VolumeBinding{ + binder: fakeVolumeBinder, + } + gotStatus := p.Filter(context.Background(), nil, item.pod, nodeInfo) if !reflect.DeepEqual(gotStatus, item.wantStatus) { t.Errorf("status does not match: %v, want: %v", gotStatus, item.wantStatus) } diff --git a/pkg/scheduler/internal/queue/scheduling_queue_test.go b/pkg/scheduler/internal/queue/scheduling_queue_test.go index f820def41a7..dc06a74dba2 100644 --- a/pkg/scheduler/internal/queue/scheduling_queue_test.go +++ b/pkg/scheduler/internal/queue/scheduling_queue_test.go @@ -169,7 +169,7 @@ func newDefaultFramework() framework.Framework { pl, pls := defaultCfg.FrameworkPlugins, defaultCfg.FrameworkPluginConfig fakeClient := fake.NewSimpleClientset() fwk, err := framework.NewFramework( - frameworkplugins.NewInTreeRegistry(&frameworkplugins.RegistryArgs{}), + frameworkplugins.NewInTreeRegistry(), pl, pls, framework.WithClientSet(fakeClient), diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index 71f158d5e7c..da83daa833b 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -267,9 +267,7 @@ func New(client clientset.Interface, time.Duration(options.bindTimeoutSeconds)*time.Second, ) - registry := frameworkplugins.NewInTreeRegistry(&frameworkplugins.RegistryArgs{ - VolumeBinder: volumeBinder, - }) + registry := frameworkplugins.NewInTreeRegistry() if err := registry.Merge(options.frameworkOutOfTreeRegistry); err != nil { return nil, err } diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index 1417bd97725..4c9bb4829c2 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -182,7 +182,7 @@ func TestSchedulerCreation(t *testing.T) { // Test case for when a plugin name in frameworkOutOfTreeRegistry already exist in defaultRegistry. fakeFrameworkPluginName := "" - for name := range frameworkplugins.NewInTreeRegistry(&frameworkplugins.RegistryArgs{}) { + for name := range frameworkplugins.NewInTreeRegistry() { fakeFrameworkPluginName = name break } @@ -485,7 +485,7 @@ func TestSchedulerNoPhantomPodAfterDelete(t *testing.T) { func setupTestSchedulerWithOnePodOnNode(t *testing.T, queuedPodStore *clientcache.FIFO, scache internalcache.Cache, informerFactory informers.SharedInformerFactory, stop chan struct{}, pod *v1.Pod, node *v1.Node, fns ...st.RegisterPluginFunc) (*Scheduler, chan *v1.Binding, chan error) { - scheduler, bindingChan, errChan := setupTestScheduler(queuedPodStore, scache, informerFactory, nil, fns...) + scheduler, bindingChan, errChan := setupTestScheduler(queuedPodStore, scache, informerFactory, nil, nil, fns...) informerFactory.Start(stop) informerFactory.WaitForCacheSync(stop) @@ -569,7 +569,7 @@ func TestSchedulerFailedSchedulingReasons(t *testing.T) { st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New), st.RegisterFilterPlugin("PodFitsResources", noderesources.NewFit), } - scheduler, _, errChan := setupTestScheduler(queuedPodStore, scache, informerFactory, nil, fns...) + scheduler, _, errChan := setupTestScheduler(queuedPodStore, scache, informerFactory, nil, nil, fns...) informerFactory.Start(stop) informerFactory.WaitForCacheSync(stop) @@ -596,8 +596,12 @@ func TestSchedulerFailedSchedulingReasons(t *testing.T) { // queuedPodStore: pods queued before processing. // scache: scheduler cache that might contain assumed pods. -func setupTestScheduler(queuedPodStore *clientcache.FIFO, scache internalcache.Cache, informerFactory informers.SharedInformerFactory, recorder events.EventRecorder, fns ...st.RegisterPluginFunc) (*Scheduler, chan *v1.Binding, chan error) { +func setupTestScheduler(queuedPodStore *clientcache.FIFO, scache internalcache.Cache, informerFactory informers.SharedInformerFactory, recorder events.EventRecorder, fakeVolumeBinder *volumebinder.VolumeBinder, fns ...st.RegisterPluginFunc) (*Scheduler, chan *v1.Binding, chan error) { registry := framework.Registry{} + if fakeVolumeBinder == nil { + // Create default volume binder if it didn't set. + fakeVolumeBinder = volumebinder.NewFakeVolumeBinder(&volumescheduling.FakeVolumeBinderConfig{AllBound: true}) + } // TODO: instantiate the plugins dynamically. plugins := &schedulerapi.Plugins{ QueueSort: &schedulerapi.PluginSet{}, @@ -608,7 +612,7 @@ func setupTestScheduler(queuedPodStore *clientcache.FIFO, scache internalcache.C for _, f := range fns { f(®istry, plugins, pluginConfigs) } - fwk, _ := framework.NewFramework(registry, plugins, pluginConfigs) + fwk, _ := framework.NewFramework(registry, plugins, pluginConfigs, framework.WithVolumeBinder(fakeVolumeBinder)) algo := core.NewGenericScheduler( scache, internalqueue.NewSchedulingQueue(nil), @@ -648,7 +652,7 @@ func setupTestScheduler(queuedPodStore *clientcache.FIFO, scache internalcache.C podConditionUpdater: fakePodConditionUpdater{}, podPreemptor: fakePodPreemptor{}, Framework: fwk, - VolumeBinder: volumebinder.NewFakeVolumeBinder(&volumescheduling.FakeVolumeBinderConfig{AllBound: true}), + VolumeBinder: fakeVolumeBinder, client: client, } @@ -674,14 +678,11 @@ func setupTestSchedulerWithVolumeBinding(fakeVolumeBinder *volumebinder.VolumeBi informerFactory := informers.NewSharedInformerFactory(client, 0) recorder := broadcaster.NewRecorder(scheme.Scheme, "scheduler") - volumeBindingNewFunc := func(_ *runtime.Unknown, _ framework.FrameworkHandle) (framework.Plugin, error) { - return volumebinding.NewFromVolumeBinder(fakeVolumeBinder), nil - } fns := []st.RegisterPluginFunc{ st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New), - st.RegisterFilterPlugin(volumebinding.Name, volumeBindingNewFunc), + st.RegisterFilterPlugin(volumebinding.Name, volumebinding.New), } - s, bindingChan, errChan := setupTestScheduler(queuedPodStore, scache, informerFactory, recorder, fns...) + s, bindingChan, errChan := setupTestScheduler(queuedPodStore, scache, informerFactory, recorder, fakeVolumeBinder, fns...) informerFactory.Start(stop) informerFactory.WaitForCacheSync(stop) s.VolumeBinder = fakeVolumeBinder