From f9d1333144f145c215d10b67fd6fd1d59e2254c7 Mon Sep 17 00:00:00 2001 From: Bobby Salamat Date: Wed, 5 Apr 2017 16:59:24 -0700 Subject: [PATCH] Addressed reviewers comments --- hack/verify-flags/known-flags.txt | 1 + pkg/apis/componentconfig/types.go | 7 +- pkg/apis/componentconfig/v1alpha1/defaults.go | 3 + pkg/apis/componentconfig/v1alpha1/types.go | 7 +- plugin/cmd/kube-scheduler/app/configurator.go | 19 +++-- .../cmd/kube-scheduler/app/options/options.go | 5 +- .../algorithm/scheduler_interface.go | 6 ++ .../pkg/scheduler/core/generic_scheduler.go | 12 +++ plugin/pkg/scheduler/scheduler.go | 5 ++ plugin/pkg/scheduler/scheduler_test.go | 7 ++ test/integration/scheduler/scheduler_test.go | 76 ++++++++++++------- 11 files changed, 107 insertions(+), 41 deletions(-) diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index 730fb84075e..2a3c6e36090 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -522,6 +522,7 @@ pod-running-timeout pods-per-core policy-config-file policy-configmap +policy-configmap-namespace poll-interval portal-net prepull-images diff --git a/pkg/apis/componentconfig/types.go b/pkg/apis/componentconfig/types.go index 6e935c1fd9c..7b0f935cd79 100644 --- a/pkg/apis/componentconfig/types.go +++ b/pkg/apis/componentconfig/types.go @@ -603,9 +603,12 @@ type KubeSchedulerConfiguration struct { // the scheduler's policy config. If UseLegacyPolicyConfig is true, scheduler // uses PolicyConfigFile. If UseLegacyPolicyConfig is false and // PolicyConfigMapName is not empty, the ConfigMap object with this name must - // exist in the default system namespace ("kube-system") before scheduler - // initialization. + // exist in PolicyConfigMapNamespace before scheduler initialization. PolicyConfigMapName string + // PolicyConfigMapNamespace is the namespace where the above policy config map + // is located. If none is provided default system namespace ("kube-system") + // will be used. + PolicyConfigMapNamespace string // UseLegacyPolicyConfig tells the scheduler to ignore Policy ConfigMap and // to use PolicyConfigFile if available. UseLegacyPolicyConfig bool diff --git a/pkg/apis/componentconfig/v1alpha1/defaults.go b/pkg/apis/componentconfig/v1alpha1/defaults.go index bb99f8f3cd9..b5c7c3b3433 100644 --- a/pkg/apis/componentconfig/v1alpha1/defaults.go +++ b/pkg/apis/componentconfig/v1alpha1/defaults.go @@ -164,6 +164,9 @@ func SetDefaults_KubeSchedulerConfiguration(obj *KubeSchedulerConfiguration) { if obj.LockObjectName == "" { obj.LockObjectName = SchedulerDefaultLockObjectName } + if obj.PolicyConfigMapNamespace == "" { + obj.PolicyConfigMapNamespace = api.NamespaceSystem + } } func SetDefaults_LeaderElectionConfiguration(obj *LeaderElectionConfiguration) { diff --git a/pkg/apis/componentconfig/v1alpha1/types.go b/pkg/apis/componentconfig/v1alpha1/types.go index 82a5939da3f..48b487c4bad 100644 --- a/pkg/apis/componentconfig/v1alpha1/types.go +++ b/pkg/apis/componentconfig/v1alpha1/types.go @@ -138,9 +138,12 @@ type KubeSchedulerConfiguration struct { // the scheduler's policy config. If UseLegacyPolicyConfig is true, scheduler // uses PolicyConfigFile. If UseLegacyPolicyConfig is false and // PolicyConfigMapName is not empty, the ConfigMap object with this name must - // exist in the default system namespace ("kube-system") before scheduler - // initialization. + // exist in PolicyConfigMapNamespace before scheduler initialization. PolicyConfigMapName string `json:"policyConfigMapName"` + // PolicyConfigMapNamespace is the namespace where the above policy config map + // is located. If none is provided default system namespace ("kube-system") + // will be used. + PolicyConfigMapNamespace string `json:"policyConfigMapNamespace"` // UseLegacyPolicyConfig tells the scheduler to ignore Policy ConfigMap and // to use PolicyConfigFile if available. UseLegacyPolicyConfig bool `json:"useLegacyPolicyConfig"` diff --git a/plugin/cmd/kube-scheduler/app/configurator.go b/plugin/cmd/kube-scheduler/app/configurator.go index 6512928a539..1d0d9e08eb7 100644 --- a/plugin/cmd/kube-scheduler/app/configurator.go +++ b/plugin/cmd/kube-scheduler/app/configurator.go @@ -104,6 +104,7 @@ func CreateScheduler( s.PolicyConfigFile, s.AlgorithmProvider, s.PolicyConfigMapName, + s.PolicyConfigMapNamespace, s.UseLegacyPolicyConfig, } @@ -116,10 +117,11 @@ func CreateScheduler( // a scheduler from a user provided config file or ConfigMap object. type schedulerConfigurator struct { scheduler.Configurator - policyFile string - algorithmProvider string - policyConfigMap string - useLegacyPolicyConfig bool + policyFile string + algorithmProvider string + policyConfigMap string + policyConfigMapNamespace string + useLegacyPolicyConfig bool } // getSchedulerPolicyConfig finds and decodes scheduler's policy config. If no @@ -131,7 +133,8 @@ func (sc schedulerConfigurator) getSchedulerPolicyConfig() (*schedulerapi.Policy // If not in legacy mode, try to find policy ConfigMap. if !sc.useLegacyPolicyConfig && len(sc.policyConfigMap) != 0 { - policyConfigMap, err := sc.GetClient().CoreV1().ConfigMaps(metav1.NamespaceSystem).Get(sc.policyConfigMap, metav1.GetOptions{}) + namespace := sc.policyConfigMapNamespace + policyConfigMap, err := sc.GetClient().CoreV1().ConfigMaps(namespace).Get(sc.policyConfigMap, metav1.GetOptions{}) if err != nil { return nil, fmt.Errorf("Error getting scheduler policy ConfigMap: %v.", err) } @@ -150,8 +153,8 @@ func (sc schedulerConfigurator) getSchedulerPolicyConfig() (*schedulerapi.Policy } } - // If there we are in legacy mode or ConfigMap name is empty, try to use - // policy config file. + // If we are in legacy mode or ConfigMap name is empty, try to use policy + // config file. if !policyConfigMapFound { if _, err := os.Stat(sc.policyFile); err != nil { // No config file is found. @@ -171,7 +174,7 @@ func (sc schedulerConfigurator) getSchedulerPolicyConfig() (*schedulerapi.Policy } // Create implements the interface for the Configurator, hence it is exported -// even through the struct is not. +// even though the struct is not. func (sc schedulerConfigurator) Create() (*scheduler.Config, error) { policy, err := sc.getSchedulerPolicyConfig() if err != nil { diff --git a/plugin/cmd/kube-scheduler/app/options/options.go b/plugin/cmd/kube-scheduler/app/options/options.go index f2b9a635689..535c2696df0 100644 --- a/plugin/cmd/kube-scheduler/app/options/options.go +++ b/plugin/cmd/kube-scheduler/app/options/options.go @@ -63,8 +63,9 @@ func (s *SchedulerServer) AddFlags(fs *pflag.FlagSet) { fs.Int32Var(&s.Port, "port", s.Port, "The port that the scheduler's http service runs on") fs.StringVar(&s.Address, "address", s.Address, "The IP address to serve on (set to 0.0.0.0 for all interfaces)") fs.StringVar(&s.AlgorithmProvider, "algorithm-provider", s.AlgorithmProvider, "The scheduling algorithm provider to use, one of: "+factory.ListAlgorithmProviders()) - fs.StringVar(&s.PolicyConfigFile, "policy-config-file", s.PolicyConfigFile, "File with scheduler policy configuration. This file is used if policy ConfigMap is not provided or scheduler is using legacy policy config.") - fs.StringVar(&s.PolicyConfigMapName, "policy-configmap", s.PolicyConfigMapName, "Name of the ConfigMap object that contains scheduler's policy configuration. It must exist in the system namespace before scheduler initialization if scheduler is not using legacy policy config.") + fs.StringVar(&s.PolicyConfigFile, "policy-config-file", s.PolicyConfigFile, "File with scheduler policy configuration. This file is used if policy ConfigMap is not provided or --use-legacy-policy-config==true") + fs.StringVar(&s.PolicyConfigMapName, "policy-configmap", s.PolicyConfigMapName, "Name of the ConfigMap object that contains scheduler's policy configuration. It must exist in the system namespace before scheduler initialization if --use-legacy-policy-config==false") + fs.StringVar(&s.PolicyConfigMapNamespace, "policy-configmap-namespace", s.PolicyConfigMapNamespace, "The namespace where policy ConfigMap is located. The system namespace will be used if this is not provided or is empty.") fs.BoolVar(&s.UseLegacyPolicyConfig, "use-legacy-policy-config", false, "When set to true, scheduler will ignore policy ConfigMap and uses policy config file") fs.BoolVar(&s.EnableProfiling, "profiling", true, "Enable profiling via web interface host:port/debug/pprof/") fs.BoolVar(&s.EnableContentionProfiling, "contention-profiling", false, "Enable lock contention profiling, if profiling is enabled") diff --git a/plugin/pkg/scheduler/algorithm/scheduler_interface.go b/plugin/pkg/scheduler/algorithm/scheduler_interface.go index 09ec312caf8..ecdf935f85c 100644 --- a/plugin/pkg/scheduler/algorithm/scheduler_interface.go +++ b/plugin/pkg/scheduler/algorithm/scheduler_interface.go @@ -41,4 +41,10 @@ type SchedulerExtender interface { // onto machines. type ScheduleAlgorithm interface { Schedule(*v1.Pod, NodeLister) (selectedMachine string, err error) + // Predicates() returns a pointer to a map of predicate functions. This is + // exposed for testing. + Predicates() map[string]FitPredicate + // Prioritizers returns a slice of priority config. This is exposed for + // testing. + Prioritizers() []PriorityConfig } diff --git a/plugin/pkg/scheduler/core/generic_scheduler.go b/plugin/pkg/scheduler/core/generic_scheduler.go index 928b7ea7c75..a63a376fdb7 100644 --- a/plugin/pkg/scheduler/core/generic_scheduler.go +++ b/plugin/pkg/scheduler/core/generic_scheduler.go @@ -127,6 +127,18 @@ func (g *genericScheduler) Schedule(pod *v1.Pod, nodeLister algorithm.NodeLister return g.selectHost(priorityList) } +// Prioritizers returns a slice containing all the scheduler's priority +// functions and their config. It is exposed for testing only. +func (g *genericScheduler) Prioritizers() []algorithm.PriorityConfig { + return g.prioritizers +} + +// Predicates returns a map containing all the scheduler's predicate +// functions. It is exposed for testing only. +func (g *genericScheduler) Predicates() map[string]algorithm.FitPredicate { + return g.predicates +} + // selectHost takes a prioritized list of nodes and then picks one // in a round-robin manner from the nodes that had the highest score. func (g *genericScheduler) selectHost(priorityList schedulerapi.HostPriorityList) (string, error) { diff --git a/plugin/pkg/scheduler/scheduler.go b/plugin/pkg/scheduler/scheduler.go index d53f35baf29..c3cc7bf40fb 100644 --- a/plugin/pkg/scheduler/scheduler.go +++ b/plugin/pkg/scheduler/scheduler.go @@ -155,6 +155,11 @@ func (sched *Scheduler) Run() { go wait.Until(sched.scheduleOne, 0, sched.config.StopEverything) } +// Config return scheduler's config pointer. It is exposed for testing purposes. +func (sched *Scheduler) Config() *Config { + return sched.config +} + func (sched *Scheduler) scheduleOne() { pod := sched.config.NextPod() if pod.DeletionTimestamp != nil { diff --git a/plugin/pkg/scheduler/scheduler_test.go b/plugin/pkg/scheduler/scheduler_test.go index f438bd11eef..812fcb7c060 100644 --- a/plugin/pkg/scheduler/scheduler_test.go +++ b/plugin/pkg/scheduler/scheduler_test.go @@ -97,6 +97,13 @@ func (es mockScheduler) Schedule(pod *v1.Pod, ml algorithm.NodeLister) (string, return es.machine, es.err } +func (es mockScheduler) Predicates() map[string]algorithm.FitPredicate { + return nil +} +func (es mockScheduler) Prioritizers() []algorithm.PriorityConfig { + return nil +} + func TestScheduler(t *testing.T) { eventBroadcaster := record.NewBroadcaster() eventBroadcaster.StartLogging(t.Logf).Stop() diff --git a/test/integration/scheduler/scheduler_test.go b/test/integration/scheduler/scheduler_test.go index a8ec0a738bc..1fb14935f1b 100644 --- a/test/integration/scheduler/scheduler_test.go +++ b/test/integration/scheduler/scheduler_test.go @@ -42,8 +42,11 @@ import ( "k8s.io/kubernetes/plugin/cmd/kube-scheduler/app" "k8s.io/kubernetes/plugin/cmd/kube-scheduler/app/options" "k8s.io/kubernetes/plugin/pkg/scheduler" + "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm" _ "k8s.io/kubernetes/plugin/pkg/scheduler/algorithmprovider" + schedulerapi "k8s.io/kubernetes/plugin/pkg/scheduler/api" "k8s.io/kubernetes/plugin/pkg/scheduler/factory" + "k8s.io/kubernetes/plugin/pkg/scheduler/schedulercache" e2e "k8s.io/kubernetes/test/e2e/framework" "k8s.io/kubernetes/test/integration/framework" ) @@ -55,8 +58,25 @@ type nodeStateManager struct { makeUnSchedulable nodeMutationFunc } +func PredicateOne(pod *v1.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) { + return true, nil, nil +} + +func PredicateTwo(pod *v1.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) { + return true, nil, nil +} + +func PriorityOne(pod *v1.Pod, nodeNameToInfo map[string]*schedulercache.NodeInfo, nodes []*v1.Node) (schedulerapi.HostPriorityList, error) { + return []schedulerapi.HostPriority{}, nil +} + +func PriorityTwo(pod *v1.Pod, nodeNameToInfo map[string]*schedulercache.NodeInfo, nodes []*v1.Node) (schedulerapi.HostPriorityList, error) { + return []schedulerapi.HostPriority{}, nil +} + // TestSchedulerCreationFromConfigMap verifies that scheduler can be created -// from configurations provided by a ConfigMap object. +// from configurations provided by a ConfigMap object and then verifies that the +// configuration is applied correctly. func TestSchedulerCreationFromConfigMap(t *testing.T) { _, s := framework.RunAMaster(nil) defer s.Close() @@ -65,9 +85,14 @@ func TestSchedulerCreationFromConfigMap(t *testing.T) { defer framework.DeleteTestingNamespace(ns, s, t) clientSet := clientset.NewForConfigOrDie(&restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &api.Registry.GroupOrDie(v1.GroupName).GroupVersion}}) + defer clientSet.Core().Nodes().DeleteCollection(nil, metav1.ListOptions{}) informerFactory := informers.NewSharedInformerFactory(clientSet, 0) - defer clientSet.Core().Nodes().DeleteCollection(nil, metav1.ListOptions{}) + // Pre-register some predicate and priority functions + factory.RegisterFitPredicate("PredicateOne", PredicateOne) + factory.RegisterFitPredicate("PredicateTwo", PredicateTwo) + factory.RegisterPriorityFunction("PriorityOne", PriorityOne, 1) + factory.RegisterPriorityFunction("PriorityTwo", PriorityTwo, 1) // Add a ConfigMap object. configPolicyName := "scheduler-custom-policy-config" @@ -78,22 +103,17 @@ func TestSchedulerCreationFromConfigMap(t *testing.T) { "kind" : "Policy", "apiVersion" : "v1", "predicates" : [ - {"name" : "PodFitsHostPorts"}, - {"name" : "PodFitsResources"}, - {"name" : "NoDiskConflict"}, - {"name" : "NoVolumeZoneConflict"}, - {"name" : "MatchNodeSelector"}, - {"name" : "HostName"} + {"name" : "PredicateOne"}, + {"name" : "PredicateTwo"} ], "priorities" : [ - {"name" : "LeastRequestedPriority", "weight" : 1}, - {"name" : "BalancedResourceAllocation", "weight" : 1}, - {"name" : "ServiceSpreadingPriority", "weight" : 1}, - {"name" : "EqualPriority", "weight" : 1} + {"name" : "PriorityOne", "weight" : 1}, + {"name" : "PriorityTwo", "weight" : 5} ] }`, }, } + policyConfigMap.APIVersion = api.Registry.GroupOrDie(v1.GroupName).GroupVersion.String() clientSet.Core().ConfigMaps(metav1.NamespaceSystem).Create(&policyConfigMap) @@ -117,13 +137,21 @@ func TestSchedulerCreationFromConfigMap(t *testing.T) { t.Fatalf("Error creating scheduler: %v", err) } - stop := make(chan struct{}) - informerFactory.Start(stop) + // Verify that the config is applied correctly. + schedPredicates := sched.Config().Algorithm.Predicates() + schedPrioritizers := sched.Config().Algorithm.Prioritizers() + if len(schedPredicates) != 2 || len(schedPrioritizers) != 2 { + t.Errorf("Unexpected number of predicates or priority functions. Number of predicates: %v, number of prioritizers: %v", len(schedPredicates), len(schedPrioritizers)) + } + // Check a predicate and a priority function. + if schedPredicates["PredicateTwo"] == nil { + t.Errorf("Expected to have a PodFitsHostPorts predicate.") + } + if schedPrioritizers[1].Function == nil || schedPrioritizers[1].Weight != 5 { + t.Errorf("Unexpected prioritizer: func: %v, weight: %v", schedPrioritizers[1].Function, schedPrioritizers[1].Weight) + } - sched.Run() - defer close(stop) - - DoTestUnschedulableNodes(t, clientSet, ns, informerFactory.Core().V1().Nodes().Lister()) + defer close(sched.Config().StopEverything) } // TestSchedulerCreationFromNonExistentConfigMap ensures that creation of the @@ -160,11 +188,6 @@ func TestSchedulerCreationFromNonExistentConfigMap(t *testing.T) { if err == nil { t.Fatalf("Creation of scheduler didn't fail while the policy ConfigMap didn't exist.") } - - stop := make(chan struct{}) - informerFactory.Start(stop) - - defer close(stop) } // TestSchedulerCreationInLegacyMode ensures that creation of the scheduler @@ -178,7 +201,6 @@ func TestSchedulerCreationInLegacyMode(t *testing.T) { clientSet := clientset.NewForConfigOrDie(&restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &api.Registry.GroupOrDie(v1.GroupName).GroupVersion}}) defer clientSet.Core().Nodes().DeleteCollection(nil, metav1.ListOptions{}) - informerFactory := informers.NewSharedInformerFactory(clientSet, 0) eventBroadcaster := record.NewBroadcaster() @@ -204,11 +226,11 @@ func TestSchedulerCreationInLegacyMode(t *testing.T) { t.Fatalf("Creation of scheduler in legacy mode failed: %v", err) } - stop := make(chan struct{}) - informerFactory.Start(stop) + informerFactory.Start(sched.Config().StopEverything) + defer close(sched.Config().StopEverything) sched.Run() - defer close(stop) + DoTestUnschedulableNodes(t, clientSet, ns, informerFactory.Core().V1().Nodes().Lister()) } func TestUnschedulableNodes(t *testing.T) {