diff --git a/plugin/pkg/scheduler/api/types.go b/plugin/pkg/scheduler/api/types.go index 90d671e8eec..2f89d7544cc 100644 --- a/plugin/pkg/scheduler/api/types.go +++ b/plugin/pkg/scheduler/api/types.go @@ -24,6 +24,14 @@ import ( "k8s.io/kubernetes/pkg/api/v1" ) +const ( + MaxUint = ^uint(0) + MaxInt = int(MaxUint >> 1) + MaxTotalPriority = MaxInt + MaxPriority = 10 + MaxWeight = MaxInt / MaxPriority +) + type Policy struct { metav1.TypeMeta // Holds the information to configure the fit predicate functions diff --git a/plugin/pkg/scheduler/api/validation/validation.go b/plugin/pkg/scheduler/api/validation/validation.go index 7f8f36c2102..2e557c09d2f 100644 --- a/plugin/pkg/scheduler/api/validation/validation.go +++ b/plugin/pkg/scheduler/api/validation/validation.go @@ -29,8 +29,8 @@ func ValidatePolicy(policy schedulerapi.Policy) error { var validationErrors []error for _, priority := range policy.Priorities { - if priority.Weight <= 0 { - validationErrors = append(validationErrors, fmt.Errorf("Priority %s should have a positive weight applied to it", priority.Name)) + if priority.Weight <= 0 || priority.Weight >= schedulerapi.MaxWeight { + validationErrors = append(validationErrors, fmt.Errorf("Priority %s should have a positive weight applied to it or it has overflown", priority.Name)) } } diff --git a/plugin/pkg/scheduler/api/validation/validation_test.go b/plugin/pkg/scheduler/api/validation/validation_test.go index 4ddd44aa25b..afc684dca64 100644 --- a/plugin/pkg/scheduler/api/validation/validation_test.go +++ b/plugin/pkg/scheduler/api/validation/validation_test.go @@ -51,6 +51,13 @@ func TestValidatePriorityWithNegativeWeight(t *testing.T) { } } +func TestValidatePriorityWithOverFlowWeight(t *testing.T) { + policy := api.Policy{Priorities: []api.PriorityPolicy{{Name: "WeightPriority", Weight: api.MaxWeight}}} + if ValidatePolicy(policy) == nil { + t.Errorf("Expected error about priority weight not being overflown.") + } +} + func TestValidateExtenderWithNonNegativeWeight(t *testing.T) { extenderPolicy := api.Policy{ExtenderConfigs: []api.ExtenderConfig{{URLPrefix: "http://127.0.0.1:8081/extender", FilterVerb: "filter", Weight: 2}}} errs := ValidatePolicy(extenderPolicy) diff --git a/plugin/pkg/scheduler/factory/plugins.go b/plugin/pkg/scheduler/factory/plugins.go index fe6799aef1e..df1c6febfc5 100644 --- a/plugin/pkg/scheduler/factory/plugins.go +++ b/plugin/pkg/scheduler/factory/plugins.go @@ -23,13 +23,12 @@ import ( "strings" "sync" + "github.com/golang/glog" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm" "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm/predicates" "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm/priorities" schedulerapi "k8s.io/kubernetes/plugin/pkg/scheduler/api" - - "github.com/golang/glog" ) // PluginFactoryArgs are passed to all plugin factory functions. @@ -356,9 +355,25 @@ func getPriorityFunctionConfigs(names sets.String, args PluginFactoryArgs) ([]al }) } } + if err := validateSelectedConfigs(configs); err != nil { + return nil, err + } return configs, nil } +// validateSelectedConfigs validates the config weights to avoid the overflow. +func validateSelectedConfigs(configs []algorithm.PriorityConfig) error { + var totalPriority int + for _, config := range configs { + // Checks totalPriority against MaxTotalPriority to avoid overflow + if config.Weight*schedulerapi.MaxPriority > schedulerapi.MaxTotalPriority-totalPriority { + return fmt.Errorf("Total priority of priority functions has overflown") + } + totalPriority += config.Weight * schedulerapi.MaxPriority + } + return nil +} + var validName = regexp.MustCompile("^[a-zA-Z0-9]([-a-zA-Z0-9]*[a-zA-Z0-9])$") func validateAlgorithmNameOrDie(name string) { diff --git a/plugin/pkg/scheduler/factory/plugins_test.go b/plugin/pkg/scheduler/factory/plugins_test.go index dbfe1663031..d14324bcfd4 100644 --- a/plugin/pkg/scheduler/factory/plugins_test.go +++ b/plugin/pkg/scheduler/factory/plugins_test.go @@ -16,7 +16,11 @@ limitations under the License. package factory -import "testing" +import ( + "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm" + "k8s.io/kubernetes/plugin/pkg/scheduler/api" + "testing" +) func TestAlgorithmNameValidation(t *testing.T) { algorithmNamesShouldValidate := []string{ @@ -39,3 +43,39 @@ func TestAlgorithmNameValidation(t *testing.T) { } } } + +func TestValidatePriorityConfigOverFlow(t *testing.T) { + tests := []struct { + description string + configs []algorithm.PriorityConfig + expected bool + }{ + { + description: "one of the weights is MaxInt", + configs: []algorithm.PriorityConfig{{Weight: api.MaxInt}, {Weight: 5}}, + expected: true, + }, + { + description: "after multiplication with MaxPriority the weight is larger than MaxWeight", + configs: []algorithm.PriorityConfig{{Weight: api.MaxInt/api.MaxPriority + api.MaxPriority}, {Weight: 5}}, + expected: true, + }, + { + description: "normal weights", + configs: []algorithm.PriorityConfig{{Weight: 10000}, {Weight: 5}}, + expected: false, + }, + } + for _, test := range tests { + err := validateSelectedConfigs(test.configs) + if test.expected { + if err == nil { + t.Errorf("Expected Overflow for %s", test.description) + } + } else { + if err != nil { + t.Errorf("Did not expect an overflow for %s", test.description) + } + } + } +}