From 8a01aed83fd26b40d6957a3915569c1c19876ad7 Mon Sep 17 00:00:00 2001 From: Abhishek Gupta Date: Mon, 30 Mar 2015 18:37:19 -0700 Subject: [PATCH] Adding validations for scheduler Policy --- plugin/pkg/scheduler/api/types.go | 1 + plugin/pkg/scheduler/api/v1/types.go | 1 + .../scheduler/api/validation/validation.go | 38 +++++++++++++ .../api/validation/validation_test.go | 53 +++++++++++++++++++ plugin/pkg/scheduler/factory/factory.go | 6 +++ 5 files changed, 99 insertions(+) create mode 100644 plugin/pkg/scheduler/api/validation/validation.go create mode 100644 plugin/pkg/scheduler/api/validation/validation_test.go diff --git a/plugin/pkg/scheduler/api/types.go b/plugin/pkg/scheduler/api/types.go index a1cac22bad4..1dabeb9f9c3 100644 --- a/plugin/pkg/scheduler/api/types.go +++ b/plugin/pkg/scheduler/api/types.go @@ -43,6 +43,7 @@ type PriorityPolicy struct { // For the Kubernetes provided priority functions, the name is the identifier of the pre-defined priority function Name string `json:"name"` // The numeric multiplier for the minion scores that the priority function generates + // The weight should be non-zero and can be a positive or a negative integer Weight int `json:"weight"` // Holds the parameters to configure the given priority function Argument *PriorityArgument `json:"argument"` diff --git a/plugin/pkg/scheduler/api/v1/types.go b/plugin/pkg/scheduler/api/v1/types.go index fefb278ad85..1defe4ed4e9 100644 --- a/plugin/pkg/scheduler/api/v1/types.go +++ b/plugin/pkg/scheduler/api/v1/types.go @@ -43,6 +43,7 @@ type PriorityPolicy struct { // For the Kubernetes provided priority functions, the name is the identifier of the pre-defined priority function Name string `json:"name"` // The numeric multiplier for the minion scores that the priority function generates + // The weight should be non-zero and can be a positive or a negative integer Weight int `json:"weight"` // Holds the parameters to configure the given priority function Argument *PriorityArgument `json:"argument"` diff --git a/plugin/pkg/scheduler/api/validation/validation.go b/plugin/pkg/scheduler/api/validation/validation.go new file mode 100644 index 00000000000..ba439042703 --- /dev/null +++ b/plugin/pkg/scheduler/api/validation/validation.go @@ -0,0 +1,38 @@ +/* +Copyright 2015 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "fmt" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" + schedulerapi "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/scheduler/api" +) + +// Validate checks for errors in the Config +// It does not return early so that it can find as many errors as possible +func ValidatePolicy(policy schedulerapi.Policy) error { + validationErrors := make([]error, 0) + + for _, priority := range policy.Priorities { + if priority.Weight == 0 { + validationErrors = append(validationErrors, fmt.Errorf("Priority %s should have a non-zero weight applied to it", priority.Name)) + } + } + + return errors.NewAggregate(validationErrors) +} diff --git a/plugin/pkg/scheduler/api/validation/validation_test.go b/plugin/pkg/scheduler/api/validation/validation_test.go new file mode 100644 index 00000000000..bbc0cddc49e --- /dev/null +++ b/plugin/pkg/scheduler/api/validation/validation_test.go @@ -0,0 +1,53 @@ +/* +Copyright 2015 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/scheduler/api" +) + +func TestValidatePriorityWithNoWeight(t *testing.T) { + policy := api.Policy{Priorities: []api.PriorityPolicy{{Name: "NoWeightPriority"}}} + if ValidatePolicy(policy) == nil { + t.Errorf("Expected error about priority weight being zero") + } +} + +func TestValidatePriorityWithZeroWeight(t *testing.T) { + policy := api.Policy{Priorities: []api.PriorityPolicy{{Name: "NoWeightPriority", Weight: 0}}} + if ValidatePolicy(policy) == nil { + t.Errorf("Expected error about priority weight being zero") + } +} + +func TestValidatePriorityWithNonZeroWeight(t *testing.T) { + policy := api.Policy{Priorities: []api.PriorityPolicy{{Name: "WeightPriority", Weight: 2}}} + errs := ValidatePolicy(policy) + if ValidatePolicy(policy) != nil { + t.Errorf("Unexpected errors %v", errs) + } +} + +func TestValidatePriorityWithNegativeWeight(t *testing.T) { + policy := api.Policy{Priorities: []api.PriorityPolicy{{Name: "WeightPriority", Weight: -2}}} + errs := ValidatePolicy(policy) + if ValidatePolicy(policy) != nil { + t.Errorf("Unexpected errors %v", errs) + } +} diff --git a/plugin/pkg/scheduler/factory/factory.go b/plugin/pkg/scheduler/factory/factory.go index ff869e3126d..5380daff811 100644 --- a/plugin/pkg/scheduler/factory/factory.go +++ b/plugin/pkg/scheduler/factory/factory.go @@ -31,6 +31,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/scheduler" schedulerapi "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/scheduler/api" + "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/scheduler/api/validation" "github.com/golang/glog" ) @@ -87,6 +88,11 @@ func (f *ConfigFactory) CreateFromProvider(providerName string) (*scheduler.Conf func (f *ConfigFactory) CreateFromConfig(policy schedulerapi.Policy) (*scheduler.Config, error) { glog.V(2).Infof("creating scheduler from configuration: %v", policy) + // validate the policy configuration + if err := validation.ValidatePolicy(policy); err != nil { + return nil, err + } + predicateKeys := util.NewStringSet() for _, predicate := range policy.Predicates { glog.V(2).Infof("Registering predicate: %s", predicate.Name)