From b21b298074d2fdcec504db738a0c29e24de34fee Mon Sep 17 00:00:00 2001 From: Wen Gao Date: Thu, 5 Mar 2020 14:41:18 +0800 Subject: [PATCH] add arg for noderesourcesfit plugin to support ignore a group of extended resources --- pkg/kubelet/lifecycle/predicate.go | 4 +- pkg/scheduler/apis/config/types_pluginargs.go | 5 + .../config/v1beta1/zz_generated.conversion.go | 2 + .../apis/config/zz_generated.deepcopy.go | 5 + .../framework/plugins/noderesources/BUILD | 2 + .../framework/plugins/noderesources/fit.go | 66 +++++++++--- .../plugins/noderesources/fit_test.go | 102 +++++++++++++++++- .../config/v1beta1/types_pluginargs.go | 6 ++ .../config/v1beta1/zz_generated.deepcopy.go | 5 + 9 files changed, 176 insertions(+), 21 deletions(-) diff --git a/pkg/kubelet/lifecycle/predicate.go b/pkg/kubelet/lifecycle/predicate.go index 866cce0f4d1..fb05bb050c2 100644 --- a/pkg/kubelet/lifecycle/predicate.go +++ b/pkg/kubelet/lifecycle/predicate.go @@ -19,7 +19,7 @@ package lifecycle import ( "fmt" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/klog/v2" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/kubelet/util/format" @@ -225,7 +225,7 @@ func GeneralPredicates(pod *v1.Pod, nodeInfo *schedulerframework.NodeInfo) ([]Pr } var reasons []PredicateFailureReason - for _, r := range noderesources.Fits(pod, nodeInfo, nil) { + for _, r := range noderesources.Fits(pod, nodeInfo) { reasons = append(reasons, &InsufficientResourceError{ ResourceName: r.ResourceName, Requested: r.Requested, diff --git a/pkg/scheduler/apis/config/types_pluginargs.go b/pkg/scheduler/apis/config/types_pluginargs.go index d8964a73ee4..24dd2f6ffda 100644 --- a/pkg/scheduler/apis/config/types_pluginargs.go +++ b/pkg/scheduler/apis/config/types_pluginargs.go @@ -57,6 +57,11 @@ type NodeResourcesFitArgs struct { // IgnoredResources is the list of resources that NodeResources fit filter // should ignore. IgnoredResources []string + // IgnoredResourceGroups defines the list of resource groups that NodeResources fit filter should ignore. + // e.g. if group is ["example.com"], it will ignore all resource names that begin + // with "example.com", such as "example.com/aaa" and "example.com/bbb". + // A resource group name can't contain '/'. + IgnoredResourceGroups []string } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/scheduler/apis/config/v1beta1/zz_generated.conversion.go b/pkg/scheduler/apis/config/v1beta1/zz_generated.conversion.go index 2ba5c02ed22..2ef0c10557c 100644 --- a/pkg/scheduler/apis/config/v1beta1/zz_generated.conversion.go +++ b/pkg/scheduler/apis/config/v1beta1/zz_generated.conversion.go @@ -416,6 +416,7 @@ func Convert_config_NodeLabelArgs_To_v1beta1_NodeLabelArgs(in *config.NodeLabelA func autoConvert_v1beta1_NodeResourcesFitArgs_To_config_NodeResourcesFitArgs(in *v1beta1.NodeResourcesFitArgs, out *config.NodeResourcesFitArgs, s conversion.Scope) error { out.IgnoredResources = *(*[]string)(unsafe.Pointer(&in.IgnoredResources)) + out.IgnoredResourceGroups = *(*[]string)(unsafe.Pointer(&in.IgnoredResourceGroups)) return nil } @@ -426,6 +427,7 @@ func Convert_v1beta1_NodeResourcesFitArgs_To_config_NodeResourcesFitArgs(in *v1b func autoConvert_config_NodeResourcesFitArgs_To_v1beta1_NodeResourcesFitArgs(in *config.NodeResourcesFitArgs, out *v1beta1.NodeResourcesFitArgs, s conversion.Scope) error { out.IgnoredResources = *(*[]string)(unsafe.Pointer(&in.IgnoredResources)) + out.IgnoredResourceGroups = *(*[]string)(unsafe.Pointer(&in.IgnoredResourceGroups)) return nil } diff --git a/pkg/scheduler/apis/config/zz_generated.deepcopy.go b/pkg/scheduler/apis/config/zz_generated.deepcopy.go index 093f2db86a1..2e44ffee96c 100644 --- a/pkg/scheduler/apis/config/zz_generated.deepcopy.go +++ b/pkg/scheduler/apis/config/zz_generated.deepcopy.go @@ -285,6 +285,11 @@ func (in *NodeResourcesFitArgs) DeepCopyInto(out *NodeResourcesFitArgs) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.IgnoredResourceGroups != nil { + in, out := &in.IgnoredResourceGroups, &out.IgnoredResourceGroups + *out = make([]string, len(*in)) + copy(*out, *in) + } return } diff --git a/pkg/scheduler/framework/plugins/noderesources/BUILD b/pkg/scheduler/framework/plugins/noderesources/BUILD index ca0b1ad4848..3930266e15a 100644 --- a/pkg/scheduler/framework/plugins/noderesources/BUILD +++ b/pkg/scheduler/framework/plugins/noderesources/BUILD @@ -23,8 +23,10 @@ go_library( "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource: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/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/apiserver/pkg/util/feature:go_default_library", "//vendor/k8s.io/klog/v2:go_default_library", ], diff --git a/pkg/scheduler/framework/plugins/noderesources/fit.go b/pkg/scheduler/framework/plugins/noderesources/fit.go index 94a44e1ddd9..3d3dc25d876 100644 --- a/pkg/scheduler/framework/plugins/noderesources/fit.go +++ b/pkg/scheduler/framework/plugins/noderesources/fit.go @@ -19,10 +19,13 @@ package noderesources import ( "context" "fmt" + "strings" 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" utilfeature "k8s.io/apiserver/pkg/util/feature" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/features" @@ -44,7 +47,8 @@ const ( // Fit is a plugin that checks if a node has sufficient resources. type Fit struct { - ignoredResources sets.String + ignoredResources sets.String + ignoredResourceGroups sets.String } // preFilterState computed at PreFilter and used at Filter. @@ -62,16 +66,48 @@ func (f *Fit) Name() string { return FitName } +func validateFitArgs(args config.NodeResourcesFitArgs) error { + var allErrs field.ErrorList + resPath := field.NewPath("ignoredResources") + for i, res := range args.IgnoredResources { + path := resPath.Index(i) + if errs := metav1validation.ValidateLabelName(res, path); len(errs) != 0 { + allErrs = append(allErrs, errs...) + } + } + + groupPath := field.NewPath("ignoredResourceGroups") + for i, group := range args.IgnoredResourceGroups { + path := groupPath.Index(i) + if strings.Contains(group, "/") { + allErrs = append(allErrs, field.Invalid(path, group, "resource group name can't contain '/'")) + } + if errs := metav1validation.ValidateLabelName(group, path); len(errs) != 0 { + allErrs = append(allErrs, errs...) + } + } + + if len(allErrs) == 0 { + return nil + } + return allErrs.ToAggregate() +} + // NewFit initializes a new plugin and returns it. func NewFit(plArgs runtime.Object, _ framework.FrameworkHandle) (framework.Plugin, error) { args, err := getFitArgs(plArgs) if err != nil { return nil, err } - fit := &Fit{ - ignoredResources: sets.NewString(args.IgnoredResources...), + + if err := validateFitArgs(args); err != nil { + return nil, err } - return fit, nil + + return &Fit{ + ignoredResources: sets.NewString(args.IgnoredResources...), + ignoredResourceGroups: sets.NewString(args.IgnoredResourceGroups...), + }, nil } func getFitArgs(obj runtime.Object) (config.NodeResourcesFitArgs, error) { @@ -162,7 +198,7 @@ func (f *Fit) Filter(ctx context.Context, cycleState *framework.CycleState, pod return framework.NewStatus(framework.Error, err.Error()) } - insufficientResources := fitsRequest(s, nodeInfo, f.ignoredResources) + insufficientResources := fitsRequest(s, nodeInfo, f.ignoredResources, f.ignoredResourceGroups) if len(insufficientResources) != 0 { // We will keep all failure reasons. @@ -187,11 +223,11 @@ type InsufficientResource struct { } // Fits checks if node have enough resources to host the pod. -func Fits(pod *v1.Pod, nodeInfo *framework.NodeInfo, ignoredExtendedResources sets.String) []InsufficientResource { - return fitsRequest(computePodResourceRequest(pod), nodeInfo, ignoredExtendedResources) +func Fits(pod *v1.Pod, nodeInfo *framework.NodeInfo) []InsufficientResource { + return fitsRequest(computePodResourceRequest(pod), nodeInfo, nil, nil) } -func fitsRequest(podRequest *preFilterState, nodeInfo *framework.NodeInfo, ignoredExtendedResources sets.String) []InsufficientResource { +func fitsRequest(podRequest *preFilterState, nodeInfo *framework.NodeInfo, ignoredExtendedResources, ignoredResourceGroups sets.String) []InsufficientResource { insufficientResources := make([]InsufficientResource, 0, 4) allowedPodNumber := nodeInfo.Allocatable.AllowedPodNumber @@ -205,10 +241,6 @@ func fitsRequest(podRequest *preFilterState, nodeInfo *framework.NodeInfo, ignor }) } - if ignoredExtendedResources == nil { - ignoredExtendedResources = sets.NewString() - } - if podRequest.MilliCPU == 0 && podRequest.Memory == 0 && podRequest.EphemeralStorage == 0 && @@ -246,9 +278,13 @@ func fitsRequest(podRequest *preFilterState, nodeInfo *framework.NodeInfo, ignor for rName, rQuant := range podRequest.ScalarResources { if v1helper.IsExtendedResourceName(rName) { - // If this resource is one of the extended resources that should be - // ignored, we will skip checking it. - if ignoredExtendedResources.Has(string(rName)) { + // If this resource is one of the extended resources that should be ignored, we will skip checking it. + // rName is guaranteed to have a slash due to API validation. + var rNamePrefix string + if ignoredResourceGroups.Len() > 0 { + rNamePrefix = strings.Split(string(rName), "/")[0] + } + if ignoredExtendedResources.Has(string(rName)) || ignoredResourceGroups.Has(rNamePrefix) { continue } } diff --git a/pkg/scheduler/framework/plugins/noderesources/fit_test.go b/pkg/scheduler/framework/plugins/noderesources/fit_test.go index d3470e181e5..4e44447a45b 100644 --- a/pkg/scheduler/framework/plugins/noderesources/fit_test.go +++ b/pkg/scheduler/framework/plugins/noderesources/fit_test.go @@ -20,13 +20,13 @@ import ( "context" "fmt" "reflect" + "strings" "testing" - "k8s.io/kubernetes/pkg/scheduler/apis/config" - v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" + "k8s.io/kubernetes/pkg/scheduler/apis/config" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" ) @@ -367,6 +367,31 @@ func TestEnoughRequests(t *testing.T) { wantStatus: framework.NewStatus(framework.Unschedulable, getErrReason(v1.ResourceMemory)), wantInsufficientResources: []InsufficientResource{{v1.ResourceMemory, getErrReason(v1.ResourceMemory), 16, 5, 20}}, }, + { + pod: newResourcePod( + framework.Resource{ + MilliCPU: 1, + Memory: 1, + ScalarResources: map[v1.ResourceName]int64{ + extendedResourceB: 1, + kubernetesIOResourceA: 1, + }}), + nodeInfo: framework.NewNodeInfo(newResourcePod(framework.Resource{MilliCPU: 0, Memory: 0})), + args: config.NodeResourcesFitArgs{ + IgnoredResourceGroups: []string{"example.com"}, + }, + name: "skip checking ignored extended resource via resource groups", + wantStatus: framework.NewStatus(framework.Unschedulable, fmt.Sprintf("Insufficient %v", kubernetesIOResourceA)), + wantInsufficientResources: []InsufficientResource{ + { + ResourceName: kubernetesIOResourceA, + Reason: fmt.Sprintf("Insufficient %v", kubernetesIOResourceA), + Requested: 1, + Used: 0, + Capacity: 0, + }, + }, + }, } for _, test := range enoughPodsTests { @@ -389,9 +414,9 @@ func TestEnoughRequests(t *testing.T) { t.Errorf("status does not match: %v, want: %v", gotStatus, test.wantStatus) } - gotInsufficientResources := Fits(test.pod, test.nodeInfo, p.(*Fit).ignoredResources) + gotInsufficientResources := fitsRequest(computePodResourceRequest(test.pod), test.nodeInfo, p.(*Fit).ignoredResources, p.(*Fit).ignoredResourceGroups) if !reflect.DeepEqual(gotInsufficientResources, test.wantInsufficientResources) { - t.Errorf("insufficient resources do not match: %v, want: %v", gotInsufficientResources, test.wantInsufficientResources) + t.Errorf("insufficient resources do not match: %+v, want: %v", gotInsufficientResources, test.wantInsufficientResources) } }) } @@ -529,3 +554,72 @@ func TestStorageRequests(t *testing.T) { } } + +func TestValidateFitArgs(t *testing.T) { + argsTest := []struct { + name string + args config.NodeResourcesFitArgs + expect string + }{ + { + name: "IgnoredResources: too long value", + args: config.NodeResourcesFitArgs{ + IgnoredResources: []string{fmt.Sprintf("longvalue%s", strings.Repeat("a", 64))}, + }, + expect: "name part must be no more than 63 characters", + }, + { + name: "IgnoredResources: name is empty", + args: config.NodeResourcesFitArgs{ + IgnoredResources: []string{"example.com/"}, + }, + expect: "name part must be non-empty", + }, + { + name: "IgnoredResources: name has too many slash", + args: config.NodeResourcesFitArgs{ + IgnoredResources: []string{"example.com/aaa/bbb"}, + }, + expect: "a qualified name must consist of alphanumeric characters", + }, + { + name: "IgnoredResources: valid args", + args: config.NodeResourcesFitArgs{ + IgnoredResources: []string{"example.com"}, + }, + }, + { + name: "IgnoredResourceGroups: valid args ", + args: config.NodeResourcesFitArgs{ + IgnoredResourceGroups: []string{"example.com"}, + }, + }, + { + name: "IgnoredResourceGroups: illegal args", + args: config.NodeResourcesFitArgs{ + IgnoredResourceGroups: []string{"example.com/"}, + }, + expect: "name part must be non-empty", + }, + { + name: "IgnoredResourceGroups: name is too long", + args: config.NodeResourcesFitArgs{ + IgnoredResourceGroups: []string{strings.Repeat("a", 64)}, + }, + expect: "name part must be no more than 63 characters", + }, + { + name: "IgnoredResourceGroups: name cannot be contain slash", + args: config.NodeResourcesFitArgs{ + IgnoredResourceGroups: []string{"example.com/aa"}, + }, + expect: "resource group name can't contain '/'", + }, + } + + for _, test := range argsTest { + if err := validateFitArgs(test.args); err != nil && !strings.Contains(err.Error(), test.expect) { + t.Errorf("case[%v]: error details do not include %v", test.name, err) + } + } +} diff --git a/staging/src/k8s.io/kube-scheduler/config/v1beta1/types_pluginargs.go b/staging/src/k8s.io/kube-scheduler/config/v1beta1/types_pluginargs.go index 3a1b3ecb581..de94e6f887f 100644 --- a/staging/src/k8s.io/kube-scheduler/config/v1beta1/types_pluginargs.go +++ b/staging/src/k8s.io/kube-scheduler/config/v1beta1/types_pluginargs.go @@ -64,6 +64,12 @@ type NodeResourcesFitArgs struct { // should ignore. // +listType=atomic IgnoredResources []string `json:"ignoredResources,omitempty"` + // IgnoredResourceGroups defines the list of resource groups that NodeResources fit filter should ignore. + // e.g. if group is ["example.com"], it will ignore all resource names that begin + // with "example.com", such as "example.com/aaa" and "example.com/bbb". + // A resource group name can't contain '/'. + // +listType=atomic + IgnoredResourceGroups []string `json:"ignoredResourceGroups,omitempty"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/staging/src/k8s.io/kube-scheduler/config/v1beta1/zz_generated.deepcopy.go b/staging/src/k8s.io/kube-scheduler/config/v1beta1/zz_generated.deepcopy.go index 070a8ecd39f..d3df960cad5 100644 --- a/staging/src/k8s.io/kube-scheduler/config/v1beta1/zz_generated.deepcopy.go +++ b/staging/src/k8s.io/kube-scheduler/config/v1beta1/zz_generated.deepcopy.go @@ -215,6 +215,11 @@ func (in *NodeResourcesFitArgs) DeepCopyInto(out *NodeResourcesFitArgs) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.IgnoredResourceGroups != nil { + in, out := &in.IgnoredResourceGroups, &out.IgnoredResourceGroups + *out = make([]string, len(*in)) + copy(*out, *in) + } return }