github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Move Scheduler plugin args validation to apis/config/validation

Browse Source
This commit is contained in:
Rafal Wicha
2020-05-18 21:06:38 +01:00
parent 4e8b56e667
commit 85be9c1673
12 changed files with 380 additions and 326 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/scheduler/framework/plugins/podtopologyspread/BUILD
View File

@@ -12,16 +12,15 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/scheduler/apis/config:go_default_library",
"//pkg/scheduler/apis/config/validation:go_default_library",
"//pkg/scheduler/framework/plugins/helper:go_default_library",
"//pkg/scheduler/framework/v1alpha1:go_default_library",
"//pkg/scheduler/internal/parallelize:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/labels: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/apimachinery/pkg/util/validation/field:go_default_library",
"//staging/src/k8s.io/client-go/informers:go_default_library",
"//staging/src/k8s.io/client-go/listers/apps/v1:go_default_library",
"//staging/src/k8s.io/client-go/listers/core/v1:go_default_library",
@@ -33,7 +32,6 @@ go_test(
name = "go_default_test",
srcs = [
"filtering_test.go",
"plugin_test.go",
"scoring_test.go",
],
embed = [":go_default_library"],

72
pkg/scheduler/framework/plugins/podtopologyspread/plugin.go
View File

@@ -19,15 +19,12 @@ package podtopologyspread
import (
"fmt"
v1 "k8s.io/api/core/v1"
metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/client-go/informers"
appslisters "k8s.io/client-go/listers/apps/v1"
corelisters "k8s.io/client-go/listers/core/v1"
"k8s.io/kubernetes/pkg/scheduler/apis/config"
"k8s.io/kubernetes/pkg/scheduler/apis/config/validation"
framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
)
@@ -36,10 +33,6 @@ const (
ErrReasonConstraintsNotMatch = "node(s) didn't match pod topology spread constraints"
)
var (
supportedScheduleActions = sets.NewString(string(v1.DoNotSchedule), string(v1.ScheduleAnyway))
)
// PodTopologySpread is a plugin that ensures pod's topologySpreadConstraints is satisfied.
type PodTopologySpread struct {
args config.PodTopologySpreadArgs
@@ -79,7 +72,7 @@ func New(plArgs runtime.Object, h framework.FrameworkHandle) (framework.Plugin,
if err != nil {
return nil, err
}
if err := validateArgs(&args); err != nil {
if err := validation.ValidatePodTopologySpreadArgs(&args); err != nil {
return nil, err
}
pl := &PodTopologySpread{
@@ -109,64 +102,3 @@ func (pl *PodTopologySpread) setListers(factory informers.SharedInformerFactory)
pl.replicaSets = factory.Apps().V1().ReplicaSets().Lister()
pl.statefulSets = factory.Apps().V1().StatefulSets().Lister()
}
// validateArgs replicates the validation from
// pkg/apis/core/validation.validateTopologySpreadConstraints.
// This has the additional check for .labelSelector to be nil.
func validateArgs(args *config.PodTopologySpreadArgs) error {
var allErrs field.ErrorList
path := field.NewPath("defaultConstraints")
for i, c := range args.DefaultConstraints {
p := path.Index(i)
if c.MaxSkew <= 0 {
f := p.Child("maxSkew")
allErrs = append(allErrs, field.Invalid(f, c.MaxSkew, "must be greater than zero"))
}
allErrs = append(allErrs, validateTopologyKey(p.Child("topologyKey"), c.TopologyKey)...)
if err := validateWhenUnsatisfiable(p.Child("whenUnsatisfiable"), c.WhenUnsatisfiable); err != nil {
allErrs = append(allErrs, err)
}
if c.LabelSelector != nil {
f := field.Forbidden(p.Child("labelSelector"), "constraint must not define a selector, as they deduced for each pod")
allErrs = append(allErrs, f)
}
if err := validateConstraintNotRepeat(path, args.DefaultConstraints, i); err != nil {
allErrs = append(allErrs, err)
}
}
if len(allErrs) == 0 {
return nil
}
return allErrs.ToAggregate()
}
func validateTopologyKey(p *field.Path, v string) field.ErrorList {
var allErrs field.ErrorList
if len(v) == 0 {
allErrs = append(allErrs, field.Required(p, "can not be empty"))
} else {
allErrs = append(allErrs, metav1validation.ValidateLabelName(v, p)...)
}
return allErrs
}
func validateWhenUnsatisfiable(p *field.Path, v v1.UnsatisfiableConstraintAction) *field.Error {
if len(v) == 0 {
return field.Required(p, "can not be empty")
}
if !supportedScheduleActions.Has(string(v)) {
return field.NotSupported(p, v, supportedScheduleActions.List())
}
return nil
}
func validateConstraintNotRepeat(path *field.Path, constraints []v1.TopologySpreadConstraint, idx int) *field.Error {
c := &constraints[idx]
for i := range constraints[:idx] {
other := &constraints[i]
if c.TopologyKey == other.TopologyKey && c.WhenUnsatisfiable == other.WhenUnsatisfiable {
return field.Duplicate(path.Index(idx), fmt.Sprintf("{%v, %v}", c.TopologyKey, c.WhenUnsatisfiable))
}
}
return nil
}

153
pkg/scheduler/framework/plugins/podtopologyspread/plugin_test.go
View File

@@ -1,153 +0,0 @@
/*
Copyright 2020 The Kubernetes Authors.
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 podtopologyspread
import (
"strings"
"testing"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/kubernetes/pkg/scheduler/apis/config"
framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
"k8s.io/kubernetes/pkg/scheduler/internal/cache"
)
func TestNew(t *testing.T) {
cases := []struct {
name string
args config.PodTopologySpreadArgs
wantErr string
}{
{name: "empty args"},
{
name: "valid constraints",
args: config.PodTopologySpreadArgs{
DefaultConstraints: []v1.TopologySpreadConstraint{
{
MaxSkew: 1,
TopologyKey: "node",
WhenUnsatisfiable: v1.ScheduleAnyway,
},
{
MaxSkew: 5,
TopologyKey: "zone",
WhenUnsatisfiable: v1.DoNotSchedule,
},
},
},
},
{
name: "repeated constraints",
args: config.PodTopologySpreadArgs{
DefaultConstraints: []v1.TopologySpreadConstraint{
{
MaxSkew: 1,
TopologyKey: "node",
WhenUnsatisfiable: v1.ScheduleAnyway,
},
{
MaxSkew: 5,
TopologyKey: "node",
WhenUnsatisfiable: v1.ScheduleAnyway,
},
},
},
wantErr: "Duplicate value",
},
{
name: "unknown whenUnsatisfiable",
args: config.PodTopologySpreadArgs{
DefaultConstraints: []v1.TopologySpreadConstraint{
{
MaxSkew: 1,
TopologyKey: "node",
WhenUnsatisfiable: "Unknown",
},
},
},
wantErr: "Unsupported value",
},
{
name: "negative maxSkew",
args: config.PodTopologySpreadArgs{
DefaultConstraints: []v1.TopologySpreadConstraint{
{
MaxSkew: -1,
TopologyKey: "node",
WhenUnsatisfiable: v1.ScheduleAnyway,
},
},
},
wantErr: "must be greater than zero",
},
{
name: "empty topologyKey",
args: config.PodTopologySpreadArgs{
DefaultConstraints: []v1.TopologySpreadConstraint{
{
MaxSkew: 1,
WhenUnsatisfiable: v1.ScheduleAnyway,
},
},
},
wantErr: "can not be empty",
},
{
name: "with label selector",
args: config.PodTopologySpreadArgs{
DefaultConstraints: []v1.TopologySpreadConstraint{
{
MaxSkew: 1,
TopologyKey: "rack",
WhenUnsatisfiable: v1.ScheduleAnyway,
LabelSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"foo": "bar",
},
},
},
},
},
wantErr: "constraint must not define a selector",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
informerFactory := informers.NewSharedInformerFactory(fake.NewSimpleClientset(), 0)
f, err := framework.NewFramework(nil, nil, nil,
framework.WithSnapshotSharedLister(cache.NewSnapshot(nil, nil)),
framework.WithInformerFactory(informerFactory),
)
if err != nil {
t.Fatal(err)
}
_, err = New(&tc.args, f)
if len(tc.wantErr) != 0 {
if err == nil || !strings.Contains(err.Error(), tc.wantErr) {
t.Errorf("must fail, got error %q, want %q", err, tc.wantErr)
}
return
}
if err != nil {
t.Fatal(err)
}
})
}
}