From ebda9584f9dc9ee4a7437c0653a22326cb0acadc Mon Sep 17 00:00:00 2001 From: "Bobby (Babak) Salamat" Date: Thu, 22 Feb 2018 17:38:17 -0800 Subject: [PATCH 1/3] Auto-create system critical prioity classes at API server startup --- pkg/apis/scheduling/types.go | 8 +++ pkg/apis/scheduling/validation/validation.go | 9 +-- .../scheduling/validation/validation_test.go | 4 -- pkg/kubelet/types/pod_update.go | 4 +- .../scheduling/rest/storage_scheduling.go | 72 +++++++++++++++++++ pkg/scheduler/api/types.go | 14 ---- plugin/pkg/admission/priority/admission.go | 40 +++++------ .../pkg/admission/priority/admission_test.go | 51 +++++++++---- test/e2e/scheduling/preemption.go | 41 ++++++++++- 9 files changed, 180 insertions(+), 63 deletions(-) diff --git a/pkg/apis/scheduling/types.go b/pkg/apis/scheduling/types.go index d5db894a8d0..29c950157c6 100644 --- a/pkg/apis/scheduling/types.go +++ b/pkg/apis/scheduling/types.go @@ -23,9 +23,17 @@ const ( // that do not specify any priority class and there is no priority class // marked as default. DefaultPriorityWhenNoDefaultClassExists = 0 + // HighestUserDefinablePriority is the highest priority for user defined priority classes. Priority values larger than 1 billion are reserved for Kubernetes system use. + HighestUserDefinablePriority = int32(1000000000) + // SystemCriticalPriority is the beginning of the range of priority values for critical system components. + SystemCriticalPriority = 2 * HighestUserDefinablePriority // SystemPriorityClassPrefix is the prefix reserved for system priority class names. Other priority // classes are not allowed to start with this prefix. SystemPriorityClassPrefix = "system-" + // NOTE: In order to avoid conflict of names with user-defined priority classes, all the names must + // start with SystemPriorityClassPrefix. + SystemClusterCritical = SystemPriorityClassPrefix + "cluster-critical" + SystemNodeCritical = SystemPriorityClassPrefix + "node-critical" ) // +genclient diff --git a/pkg/apis/scheduling/validation/validation.go b/pkg/apis/scheduling/validation/validation.go index c8b85ef85d4..bd446b25585 100644 --- a/pkg/apis/scheduling/validation/validation.go +++ b/pkg/apis/scheduling/validation/validation.go @@ -17,8 +17,6 @@ limitations under the License. package validation import ( - "strings" - "k8s.io/apimachinery/pkg/util/validation/field" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/apis/scheduling" @@ -26,12 +24,7 @@ import ( // ValidatePriorityClassName checks whether the given priority class name is valid. func ValidatePriorityClassName(name string, prefix bool) []string { - var allErrs []string - if strings.HasPrefix(name, scheduling.SystemPriorityClassPrefix) { - allErrs = append(allErrs, "priority class names with '"+scheduling.SystemPriorityClassPrefix+"' prefix are reserved for system use only") - } - allErrs = append(allErrs, apivalidation.NameIsDNSSubdomain(name, prefix)...) - return allErrs + return apivalidation.NameIsDNSSubdomain(name, prefix) } // ValidatePriorityClass tests whether required fields in the PriorityClass are diff --git a/pkg/apis/scheduling/validation/validation_test.go b/pkg/apis/scheduling/validation/validation_test.go index e35ee426936..8b0cec40b6c 100644 --- a/pkg/apis/scheduling/validation/validation_test.go +++ b/pkg/apis/scheduling/validation/validation_test.go @@ -53,10 +53,6 @@ func TestValidatePriorityClass(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "tier&1", Namespace: ""}, Value: 100, }, - "invalid system name": { - ObjectMeta: metav1.ObjectMeta{Name: scheduling.SystemPriorityClassPrefix + "test"}, - Value: 100, - }, } for k, v := range errorCases { diff --git a/pkg/kubelet/types/pod_update.go b/pkg/kubelet/types/pod_update.go index 62116985fd3..c176a08bf0b 100644 --- a/pkg/kubelet/types/pod_update.go +++ b/pkg/kubelet/types/pod_update.go @@ -22,7 +22,7 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kubeapi "k8s.io/kubernetes/pkg/apis/core" - schedulerapi "k8s.io/kubernetes/pkg/scheduler/api" + "k8s.io/kubernetes/pkg/apis/scheduling" ) const ( @@ -168,7 +168,7 @@ func IsCriticalPodBasedOnPriority(ns string, priority int32) bool { if ns != kubeapi.NamespaceSystem { return false } - if priority >= schedulerapi.SystemCriticalPriority { + if priority >= scheduling.SystemCriticalPriority { return true } return false diff --git a/pkg/registry/scheduling/rest/storage_scheduling.go b/pkg/registry/scheduling/rest/storage_scheduling.go index 3074fb7d4af..807a4ae8a6c 100644 --- a/pkg/registry/scheduling/rest/storage_scheduling.go +++ b/pkg/registry/scheduling/rest/storage_scheduling.go @@ -17,6 +17,15 @@ limitations under the License. package rest import ( + "fmt" + "time" + + "github.com/golang/glog" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/registry/rest" genericapiserver "k8s.io/apiserver/pkg/server" @@ -24,11 +33,16 @@ import ( "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/scheduling" schedulingapiv1alpha1 "k8s.io/kubernetes/pkg/apis/scheduling/v1alpha1" + schedulingclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/scheduling/internalversion" priorityclassstore "k8s.io/kubernetes/pkg/registry/scheduling/priorityclass/storage" ) +const PostStartHookName = "scheduling/bootstrap-system-priority-classes" + type RESTStorageProvider struct{} +var _ genericapiserver.PostStartHookProvider = RESTStorageProvider{} + func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(scheduling.GroupName, legacyscheme.Registry, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) @@ -49,6 +63,64 @@ func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstora return storage } +func (p RESTStorageProvider) PostStartHook() (string, genericapiserver.PostStartHookFunc, error) { + return PostStartHookName, AddSystemPriorityClasses(), nil +} + +func AddSystemPriorityClasses() genericapiserver.PostStartHookFunc { + priorityClasses := []*scheduling.PriorityClass{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: scheduling.SystemNodeCritical, + }, + Value: scheduling.SystemCriticalPriority + 1000, + Description: "Used for system critical pods that must not be moved from their current node.", + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: scheduling.SystemClusterCritical, + }, + Value: scheduling.SystemCriticalPriority, + Description: "Used for system critical pods that must run in the cluster, but can be moved to another node if necessary.", + }, + } + return func(hookContext genericapiserver.PostStartHookContext) error { + // Adding system priority classes is important. If they fail to add, many critical system + // components may fail and cluster may break. + err := wait.Poll(1*time.Second, 30*time.Second, func() (done bool, err error) { + schedClientSet, err := schedulingclient.NewForConfig(hookContext.LoopbackClientConfig) + if err != nil { + utilruntime.HandleError(fmt.Errorf("unable to initialize client: %v", err)) + return false, nil + } + + for _, pc := range priorityClasses { + _, err := schedClientSet.PriorityClasses().Get(pc.Name, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + _, err := schedClientSet.PriorityClasses().Create(pc) + if err != nil { + return false, err + } else { + glog.Infof("created PriorityClass %s with value %v", pc.Name, pc.Value) + } + } else { + // Unable to get the priority class for reasons other than "not found". + return false, err + } + } + } + glog.Infof("all system priority classes are created successfully.") + return true, nil + }) + // if we're never able to make it through initialization, kill the API server. + if err != nil { + return fmt.Errorf("unable to add default system priority classes: %v", err) + } + return nil + } +} + func (p RESTStorageProvider) GroupName() string { return scheduling.GroupName } diff --git a/pkg/scheduler/api/types.go b/pkg/scheduler/api/types.go index cfc8d219ec1..4d87b3853dc 100644 --- a/pkg/scheduler/api/types.go +++ b/pkg/scheduler/api/types.go @@ -36,14 +36,6 @@ const ( MaxPriority = 10 // MaxWeight defines the max weight value. MaxWeight = MaxInt / MaxPriority - // HighestUserDefinablePriority is the highest priority for user defined priority classes. Priority values larger than 1 billion are reserved for Kubernetes system use. - HighestUserDefinablePriority = int32(1000000000) - // SystemCriticalPriority is the beginning of the range of priority values for critical system components. - SystemCriticalPriority = 2 * HighestUserDefinablePriority - // NOTE: In order to avoid conflict of names with user-defined priority classes, all the names must - // start with scheduling.SystemPriorityClassPrefix which is by default "system-". - SystemClusterCritical = "system-cluster-critical" - SystemNodeCritical = "system-node-critical" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -259,12 +251,6 @@ type HostPriority struct { // HostPriorityList declares a []HostPriority type. type HostPriorityList []HostPriority -// SystemPriorityClasses defines special priority classes which are used by system critical pods that should not be preempted by workload pods. -var SystemPriorityClasses = map[string]int32{ - SystemClusterCritical: SystemCriticalPriority, - SystemNodeCritical: SystemCriticalPriority + 1000, -} - func (h HostPriorityList) Len() int { return len(h) } diff --git a/plugin/pkg/admission/priority/admission.go b/plugin/pkg/admission/priority/admission.go index 2c7e22242cb..97add55a8d2 100644 --- a/plugin/pkg/admission/priority/admission.go +++ b/plugin/pkg/admission/priority/admission.go @@ -19,10 +19,12 @@ package priority import ( "fmt" "io" + "strings" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/authentication/user" utilfeature "k8s.io/apiserver/pkg/util/feature" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/scheduling" @@ -32,7 +34,6 @@ import ( "k8s.io/kubernetes/pkg/features" kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" kubelettypes "k8s.io/kubernetes/pkg/kubelet/types" - schedulerapi "k8s.io/kubernetes/pkg/scheduler/api" ) const ( @@ -154,7 +155,7 @@ func (p *priorityPlugin) admitPod(a admission.Attributes) error { if len(pod.Spec.PriorityClassName) == 0 && utilfeature.DefaultFeatureGate.Enabled(features.ExperimentalCriticalPodAnnotation) && kubelettypes.IsCritical(a.GetNamespace(), pod.Annotations) { - pod.Spec.PriorityClassName = schedulerapi.SystemClusterCritical + pod.Spec.PriorityClassName = scheduling.SystemClusterCritical } if len(pod.Spec.PriorityClassName) == 0 { var err error @@ -163,22 +164,17 @@ func (p *priorityPlugin) admitPod(a admission.Attributes) error { return fmt.Errorf("failed to get default priority class: %v", err) } } else { - // First try to resolve by system priority classes. - priority, ok = schedulerapi.SystemPriorityClasses[pod.Spec.PriorityClassName] - if !ok { - // Now that we didn't find any system priority, try resolving by user defined priority classes. - pc, err := p.lister.Get(pod.Spec.PriorityClassName) - - if err != nil { - if errors.IsNotFound(err) { - return admission.NewForbidden(a, fmt.Errorf("no PriorityClass with name %v was found", pod.Spec.PriorityClassName)) - } - - return fmt.Errorf("failed to get PriorityClass with name %s: %v", pod.Spec.PriorityClassName, err) + // Try resolving the priority class name. + pc, err := p.lister.Get(pod.Spec.PriorityClassName) + if err != nil { + if errors.IsNotFound(err) { + return admission.NewForbidden(a, fmt.Errorf("no PriorityClass with name %v was found", pod.Spec.PriorityClassName)) } - priority = pc.Value + return fmt.Errorf("failed to get PriorityClass with name %s: %v", pod.Spec.PriorityClassName, err) } + + priority = pc.Value } pod.Spec.Priority = &priority } @@ -192,11 +188,15 @@ func (p *priorityPlugin) validatePriorityClass(a admission.Attributes) error { if !ok { return errors.NewBadRequest("resource was marked with kind PriorityClass but was unable to be converted") } - if pc.Value > schedulerapi.HighestUserDefinablePriority { - return admission.NewForbidden(a, fmt.Errorf("maximum allowed value of a user defined priority is %v", schedulerapi.HighestUserDefinablePriority)) - } - if _, ok := schedulerapi.SystemPriorityClasses[pc.Name]; ok { - return admission.NewForbidden(a, fmt.Errorf("the name of the priority class is a reserved name for system use only: %v", pc.Name)) + // API server adds system critical priority classes at bootstrapping. We should + // not enforce restrictions on adding system level priority classes for API server. + if userInfo := a.GetUserInfo(); userInfo == nil || userInfo.GetName() != user.APIServerUser { + if pc.Value > scheduling.HighestUserDefinablePriority { + return admission.NewForbidden(a, fmt.Errorf("maximum allowed value of a user defined priority is %v", scheduling.HighestUserDefinablePriority)) + } + if strings.HasPrefix(pc.Name, scheduling.SystemPriorityClassPrefix) { + return admission.NewForbidden(a, fmt.Errorf("priority class names with '"+scheduling.SystemPriorityClassPrefix+"' prefix are reserved for system use only")) + } } // If the new PriorityClass tries to be the default priority, make sure that no other priority class is marked as default. if pc.GlobalDefault { diff --git a/plugin/pkg/admission/priority/admission_test.go b/plugin/pkg/admission/priority/admission_test.go index 2fb2a113420..bda6a8a4eea 100644 --- a/plugin/pkg/admission/priority/admission_test.go +++ b/plugin/pkg/admission/priority/admission_test.go @@ -24,13 +24,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/authentication/user" utilfeature "k8s.io/apiserver/pkg/util/feature" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/scheduling" informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/features" - schedulerapi "k8s.io/kubernetes/pkg/scheduler/api" ) func addPriorityClasses(ctrl *priorityPlugin, priorityClasses []*scheduling.PriorityClass) { @@ -75,6 +75,17 @@ var nondefaultClass1 = &scheduling.PriorityClass{ Description: "Just a test priority class", } +var systemClusterCritical = &scheduling.PriorityClass{ + TypeMeta: metav1.TypeMeta{ + Kind: "PriorityClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: scheduling.SystemClusterCritical, + }, + Value: scheduling.SystemCriticalPriority, + GlobalDefault: true, +} + func TestPriorityClassAdmission(t *testing.T) { var tooHighPriorityClass = &scheduling.PriorityClass{ TypeMeta: metav1.TypeMeta{ @@ -83,7 +94,7 @@ func TestPriorityClassAdmission(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "toohighclass", }, - Value: schedulerapi.HighestUserDefinablePriority + 1, + Value: scheduling.HighestUserDefinablePriority + 1, Description: "Just a test priority class", } @@ -92,42 +103,56 @@ func TestPriorityClassAdmission(t *testing.T) { Kind: "PriorityClass", }, ObjectMeta: metav1.ObjectMeta{ - Name: schedulerapi.SystemClusterCritical, + Name: scheduling.SystemPriorityClassPrefix + "test", }, - Value: schedulerapi.HighestUserDefinablePriority + 1, - Description: "Name conflicts with system priority class names", + Value: scheduling.HighestUserDefinablePriority + 1, + Description: "Name has system critical prefix", } tests := []struct { name string existingClasses []*scheduling.PriorityClass newClass *scheduling.PriorityClass + userInfo user.Info expectError bool }{ { "one default class", []*scheduling.PriorityClass{}, defaultClass1, + nil, false, }, { "more than one default classes", []*scheduling.PriorityClass{defaultClass1}, defaultClass2, + nil, true, }, { "too high PriorityClass value", []*scheduling.PriorityClass{}, tooHighPriorityClass, + nil, true, }, { "system name conflict", []*scheduling.PriorityClass{}, systemClass, + nil, true, }, + { + "system name allowed for API server", + []*scheduling.PriorityClass{}, + systemClass, + &user.DefaultInfo{ + Name: user.APIServerUser, + }, + false, + }, } for _, test := range tests { @@ -146,7 +171,7 @@ func TestPriorityClassAdmission(t *testing.T) { scheduling.Resource("priorityclasses").WithVersion("version"), "", admission.Create, - nil, + test.userInfo, ) err := ctrl.Validate(attrs) glog.Infof("Got %v", err) @@ -322,7 +347,7 @@ func TestPodAdmission(t *testing.T) { Name: containerName, }, }, - PriorityClassName: schedulerapi.SystemClusterCritical, + PriorityClassName: scheduling.SystemClusterCritical, }, }, // pod[5]: mirror Pod with a system priority class name @@ -419,9 +444,9 @@ func TestPodAdmission(t *testing.T) { }, { "pod with a system priority class", - []*scheduling.PriorityClass{}, + []*scheduling.PriorityClass{systemClusterCritical}, *pods[4], - schedulerapi.SystemCriticalPriority, + scheduling.SystemCriticalPriority, false, }, { @@ -440,9 +465,9 @@ func TestPodAdmission(t *testing.T) { }, { "mirror pod with system priority class", - []*scheduling.PriorityClass{}, + []*scheduling.PriorityClass{systemClusterCritical}, *pods[5], - schedulerapi.SystemCriticalPriority, + scheduling.SystemCriticalPriority, false, }, { @@ -454,9 +479,9 @@ func TestPodAdmission(t *testing.T) { }, { "pod with critical pod annotation", - []*scheduling.PriorityClass{}, + []*scheduling.PriorityClass{systemClusterCritical}, *pods[7], - schedulerapi.SystemCriticalPriority, + scheduling.SystemCriticalPriority, false, }, } diff --git a/test/e2e/scheduling/preemption.go b/test/e2e/scheduling/preemption.go index 1867ec4cc46..64a29dc61f8 100644 --- a/test/e2e/scheduling/preemption.go +++ b/test/e2e/scheduling/preemption.go @@ -26,7 +26,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" - schedulerapi "k8s.io/kubernetes/pkg/scheduler/api" + "k8s.io/kubernetes/pkg/apis/scheduling" "k8s.io/kubernetes/test/e2e/framework" . "github.com/onsi/ginkgo" @@ -168,7 +168,7 @@ var _ = SIGDescribe("SchedulerPreemption [Serial] [Feature:PodPreemption]", func // Create a critical pod and make sure it is scheduled. runPausePod(f, pausePodConfig{ Name: "critical-pod", - PriorityClassName: schedulerapi.SystemClusterCritical, + PriorityClassName: scheduling.SystemClusterCritical, Resources: &v1.ResourceRequirements{ Requests: podRes, }, @@ -311,3 +311,40 @@ var _ = SIGDescribe("SchedulerPreemption [Serial] [Feature:PodPreemption]", func } }) }) + +var _ = SIGDescribe("PodPriorityResolution [Serial] [Feature:PodPreemption]", func() { + var cs clientset.Interface + var ns string + f := framework.NewDefaultFramework("sched-pod-priority") + + BeforeEach(func() { + cs = f.ClientSet + ns = f.Namespace.Name + + err := framework.CheckTestingNSDeletedExcept(cs, ns) + framework.ExpectNoError(err) + }) + + // This test verifies that when a higher priority pod is created and no node with + // enough resources is found, scheduler preempts a lower priority pod to schedule + // the high priority pod. + It("validates critical system priorities are created and resolved", func() { + var podRes v1.ResourceList + // Create pods that use system critical priorities and + By("Create pods that use critical system priorities.") + systemPriorityClasses := []string{ + scheduling.SystemNodeCritical, scheduling.SystemClusterCritical, + } + for i, spc := range systemPriorityClasses { + pod := createPausePod(f, pausePodConfig{ + Name: fmt.Sprintf("pod%d-%v", i, spc), + PriorityClassName: spc, + Resources: &v1.ResourceRequirements{ + Requests: podRes, + }, + }) + Expect(pod.Spec.Priority).NotTo(BeNil()) + framework.Logf("Created pod: %v", pod.Name) + } + }) +}) From 515ba9e8d430f252275ca2a5231a16c3e1fdbfa5 Mon Sep 17 00:00:00 2001 From: "Bobby (Babak) Salamat" Date: Tue, 27 Feb 2018 11:40:31 -0800 Subject: [PATCH 2/3] autogenerated files --- pkg/apis/scheduling/BUILD | 9 +++++++++ pkg/kubelet/types/BUILD | 2 +- pkg/registry/scheduling/priorityclass/storage/BUILD | 4 ++++ pkg/registry/scheduling/rest/BUILD | 6 ++++++ plugin/pkg/admission/priority/BUILD | 3 +-- test/e2e/scheduling/BUILD | 2 +- 6 files changed, 22 insertions(+), 4 deletions(-) diff --git a/pkg/apis/scheduling/BUILD b/pkg/apis/scheduling/BUILD index 58811a51216..b8a7a39696c 100644 --- a/pkg/apis/scheduling/BUILD +++ b/pkg/apis/scheduling/BUILD @@ -3,12 +3,14 @@ package(default_visibility = ["//visibility:public"]) load( "@io_bazel_rules_go//go:def.bzl", "go_library", + "go_test", ) go_library( name = "go_default_library", srcs = [ "doc.go", + "helpers.go", "register.go", "types.go", "zz_generated.deepcopy.go", @@ -39,3 +41,10 @@ filegroup( ], tags = ["automanaged"], ) + +go_test( + name = "go_default_test", + srcs = ["helpers_test.go"], + embed = [":go_default_library"], + deps = ["//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library"], +) diff --git a/pkg/kubelet/types/BUILD b/pkg/kubelet/types/BUILD index 762979d5baf..c362d096de2 100644 --- a/pkg/kubelet/types/BUILD +++ b/pkg/kubelet/types/BUILD @@ -18,7 +18,7 @@ go_library( importpath = "k8s.io/kubernetes/pkg/kubelet/types", deps = [ "//pkg/apis/core:go_default_library", - "//pkg/scheduler/api:go_default_library", + "//pkg/apis/scheduling:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", diff --git a/pkg/registry/scheduling/priorityclass/storage/BUILD b/pkg/registry/scheduling/priorityclass/storage/BUILD index 7c0f95e785e..472ea8dee94 100644 --- a/pkg/registry/scheduling/priorityclass/storage/BUILD +++ b/pkg/registry/scheduling/priorityclass/storage/BUILD @@ -17,6 +17,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/fields:go_default_library", "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/generic:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/generic/testing:go_default_library", "//vendor/k8s.io/apiserver/pkg/storage/etcd/testing:go_default_library", @@ -30,7 +31,10 @@ go_library( deps = [ "//pkg/apis/scheduling:go_default_library", "//pkg/registry/scheduling/priorityclass:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/generic:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/generic/registry:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library", diff --git a/pkg/registry/scheduling/rest/BUILD b/pkg/registry/scheduling/rest/BUILD index 54e6dd25ce1..f3e3297d728 100644 --- a/pkg/registry/scheduling/rest/BUILD +++ b/pkg/registry/scheduling/rest/BUILD @@ -13,7 +13,13 @@ go_library( "//pkg/api/legacyscheme:go_default_library", "//pkg/apis/scheduling:go_default_library", "//pkg/apis/scheduling/v1alpha1:go_default_library", + "//pkg/client/clientset_generated/internalclientset/typed/scheduling/internalversion:go_default_library", "//pkg/registry/scheduling/priorityclass/storage:go_default_library", + "//vendor/github.com/golang/glog:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/generic:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library", "//vendor/k8s.io/apiserver/pkg/server:go_default_library", diff --git a/plugin/pkg/admission/priority/BUILD b/plugin/pkg/admission/priority/BUILD index 3f9959806f4..51922c77e84 100644 --- a/plugin/pkg/admission/priority/BUILD +++ b/plugin/pkg/admission/priority/BUILD @@ -16,10 +16,10 @@ go_test( "//pkg/client/informers/informers_generated/internalversion:go_default_library", "//pkg/controller:go_default_library", "//pkg/features:go_default_library", - "//pkg/scheduler/api:go_default_library", "//vendor/github.com/golang/glog:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", + "//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library", "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", ], ) @@ -37,7 +37,6 @@ go_library( "//pkg/features:go_default_library", "//pkg/kubeapiserver/admission:go_default_library", "//pkg/kubelet/types:go_default_library", - "//pkg/scheduler/api:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", diff --git a/test/e2e/scheduling/BUILD b/test/e2e/scheduling/BUILD index 9ee18722387..1e1cda3a2c4 100644 --- a/test/e2e/scheduling/BUILD +++ b/test/e2e/scheduling/BUILD @@ -22,10 +22,10 @@ go_library( "//pkg/api/v1/pod:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/extensions:go_default_library", + "//pkg/apis/scheduling:go_default_library", "//pkg/kubelet/apis:go_default_library", "//pkg/quota/evaluator/core:go_default_library", "//pkg/scheduler/algorithm/priorities/util:go_default_library", - "//pkg/scheduler/api:go_default_library", "//pkg/util/version:go_default_library", "//test/e2e/common:go_default_library", "//test/e2e/framework:go_default_library", From 9592a9ecf45d4dddb0009b0c8b2bd90b6c063a65 Mon Sep 17 00:00:00 2001 From: "Bobby (Babak) Salamat" Date: Thu, 1 Mar 2018 00:47:48 -0800 Subject: [PATCH 3/3] Allow system critical priority classes in API validation --- hack/.golint_failures | 1 - pkg/apis/scheduling/helpers.go | 65 +++++++++++++++++++ pkg/apis/scheduling/helpers_test.go | 54 +++++++++++++++ pkg/apis/scheduling/validation/validation.go | 21 ++++-- .../scheduling/validation/validation_test.go | 17 +++++ .../priorityclass/storage/storage.go | 24 +++++-- .../priorityclass/storage/storage_test.go | 17 +++++ .../scheduling/priorityclass/strategy.go | 4 +- .../scheduling/rest/storage_scheduling.go | 21 +----- plugin/pkg/admission/priority/admission.go | 12 ---- .../pkg/admission/priority/admission_test.go | 27 +------- test/e2e/scheduling/preemption.go | 8 +-- 12 files changed, 193 insertions(+), 78 deletions(-) create mode 100644 pkg/apis/scheduling/helpers.go create mode 100644 pkg/apis/scheduling/helpers_test.go diff --git a/hack/.golint_failures b/hack/.golint_failures index 95a0b7fb99b..45fb6e1aad7 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -324,7 +324,6 @@ pkg/registry/storage/rest pkg/registry/storage/storageclass pkg/registry/storage/storageclass/storage pkg/routes -pkg/scheduler/api pkg/security/apparmor pkg/security/podsecuritypolicy pkg/security/podsecuritypolicy/group diff --git a/pkg/apis/scheduling/helpers.go b/pkg/apis/scheduling/helpers.go new file mode 100644 index 00000000000..58b3799751c --- /dev/null +++ b/pkg/apis/scheduling/helpers.go @@ -0,0 +1,65 @@ +/* +Copyright 2018 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 scheduling + +import ( + "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// SystemPriorityClasses define system priority classes that are auto-created at cluster bootstrapping. +// Our API validation logic ensures that any priority class that has a system prefix or its value +// is higher than HighestUserDefinablePriority is equal to one of these SystemPriorityClasses. +var systemPriorityClasses = []*PriorityClass{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: SystemNodeCritical, + }, + Value: SystemCriticalPriority + 1000, + Description: "Used for system critical pods that must not be moved from their current node.", + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: SystemClusterCritical, + }, + Value: SystemCriticalPriority, + Description: "Used for system critical pods that must run in the cluster, but can be moved to another node if necessary.", + }, +} + +// SystemPriorityClasses returns the list of system priority classes. +// NOTE: be careful not to modify any of elements of the returned array directly. +func SystemPriorityClasses() []*PriorityClass { + return systemPriorityClasses +} + +// IsKnownSystemPriorityClass checks that "pc" is equal to one of the system PriorityClasses. +// It ignores "description", labels, annotations, etc. of the PriorityClass. +func IsKnownSystemPriorityClass(pc *PriorityClass) (bool, error) { + for _, spc := range systemPriorityClasses { + if spc.Name == pc.Name { + if spc.Value != pc.Value { + return false, fmt.Errorf("value of %v PriorityClass must be %v", spc.Name, spc.Value) + } + if spc.GlobalDefault != pc.GlobalDefault { + return false, fmt.Errorf("globalDefault of %v PriorityClass must be %v", spc.Name, spc.GlobalDefault) + } + return true, nil + } + } + return false, fmt.Errorf("%v is not a known system priority class", pc.Name) +} diff --git a/pkg/apis/scheduling/helpers_test.go b/pkg/apis/scheduling/helpers_test.go new file mode 100644 index 00000000000..b0fe132b838 --- /dev/null +++ b/pkg/apis/scheduling/helpers_test.go @@ -0,0 +1,54 @@ +/* +Copyright 2018 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 scheduling + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestIsKnownSystemPriorityClass(t *testing.T) { + tests := []struct { + name string + pc *PriorityClass + expected bool + }{ + { + name: "system priority class", + pc: SystemPriorityClasses()[0], + expected: true, + }, + { + name: "non-system priority class", + pc: &PriorityClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: SystemNodeCritical, + }, + Value: SystemCriticalPriority, // This is the value of system cluster critical + Description: "Used for system critical pods that must not be moved from their current node.", + }, + expected: false, + }, + } + + for _, test := range tests { + if is, err := IsKnownSystemPriorityClass(test.pc); test.expected != is { + t.Errorf("Test [%v]: Expected %v, but got %v. Error: %v", test.name, test.expected, is, err) + } + } +} diff --git a/pkg/apis/scheduling/validation/validation.go b/pkg/apis/scheduling/validation/validation.go index bd446b25585..5f2998157ed 100644 --- a/pkg/apis/scheduling/validation/validation.go +++ b/pkg/apis/scheduling/validation/validation.go @@ -17,22 +17,29 @@ limitations under the License. package validation import ( + "fmt" + "strings" + "k8s.io/apimachinery/pkg/util/validation/field" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/apis/scheduling" ) -// ValidatePriorityClassName checks whether the given priority class name is valid. -func ValidatePriorityClassName(name string, prefix bool) []string { - return apivalidation.NameIsDNSSubdomain(name, prefix) -} - // ValidatePriorityClass tests whether required fields in the PriorityClass are // set correctly. func ValidatePriorityClass(pc *scheduling.PriorityClass) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, apivalidation.ValidateObjectMeta(&pc.ObjectMeta, false, ValidatePriorityClassName, field.NewPath("metadata"))...) - // The "Value" field can be any valid integer. So, no need to validate. + allErrs = append(allErrs, apivalidation.ValidateObjectMeta(&pc.ObjectMeta, false, apivalidation.NameIsDNSSubdomain, field.NewPath("metadata"))...) + // If the priorityClass starts with a system prefix, it must be one of the + // predefined system priority classes. + if strings.HasPrefix(pc.Name, scheduling.SystemPriorityClassPrefix) { + if is, err := scheduling.IsKnownSystemPriorityClass(pc); !is { + allErrs = append(allErrs, field.Forbidden(field.NewPath("metadata", "name"), "priority class names with '"+scheduling.SystemPriorityClassPrefix+"' prefix are reserved for system use only. error: "+err.Error())) + } + } else if pc.Value > scheduling.HighestUserDefinablePriority { + // Non-system critical priority classes are not allowed to have a value larger than HighestUserDefinablePriority. + allErrs = append(allErrs, field.Forbidden(field.NewPath("value"), fmt.Sprintf("maximum allowed value of a user defined priority is %v", scheduling.HighestUserDefinablePriority))) + } return allErrs } diff --git a/pkg/apis/scheduling/validation/validation_test.go b/pkg/apis/scheduling/validation/validation_test.go index 8b0cec40b6c..3d7373e207e 100644 --- a/pkg/apis/scheduling/validation/validation_test.go +++ b/pkg/apis/scheduling/validation/validation_test.go @@ -25,6 +25,7 @@ import ( ) func TestValidatePriorityClass(t *testing.T) { + spcs := scheduling.SystemPriorityClasses() successCases := map[string]scheduling.PriorityClass{ "no description": { ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: ""}, @@ -36,6 +37,12 @@ func TestValidatePriorityClass(t *testing.T) { GlobalDefault: false, Description: "Used for the highest priority pods.", }, + "system node critical": { + ObjectMeta: metav1.ObjectMeta{Name: spcs[0].Name, Namespace: ""}, + Value: spcs[0].Value, + GlobalDefault: spcs[0].GlobalDefault, + Description: "system priority class 0", + }, } for k, v := range successCases { @@ -53,6 +60,16 @@ func TestValidatePriorityClass(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "tier&1", Namespace: ""}, Value: 100, }, + "incorrect system class name": { + ObjectMeta: metav1.ObjectMeta{Name: spcs[0].Name, Namespace: ""}, + Value: 0, + GlobalDefault: spcs[0].GlobalDefault, + }, + "incorrect system class value": { + ObjectMeta: metav1.ObjectMeta{Name: "system-something", Namespace: ""}, + Value: spcs[0].Value, + GlobalDefault: spcs[0].GlobalDefault, + }, } for k, v := range errorCases { diff --git a/pkg/registry/scheduling/priorityclass/storage/storage.go b/pkg/registry/scheduling/priorityclass/storage/storage.go index b4203621199..da78b01e1eb 100644 --- a/pkg/registry/scheduling/priorityclass/storage/storage.go +++ b/pkg/registry/scheduling/priorityclass/storage/storage.go @@ -17,11 +17,16 @@ limitations under the License. package storage import ( + "errors" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" genericregistry "k8s.io/apiserver/pkg/registry/generic/registry" "k8s.io/apiserver/pkg/registry/rest" - schedulingapi "k8s.io/kubernetes/pkg/apis/scheduling" + "k8s.io/kubernetes/pkg/apis/scheduling" "k8s.io/kubernetes/pkg/registry/scheduling/priorityclass" ) @@ -33,9 +38,9 @@ type REST struct { // NewREST returns a RESTStorage object that will work against priority classes. func NewREST(optsGetter generic.RESTOptionsGetter) *REST { store := &genericregistry.Store{ - NewFunc: func() runtime.Object { return &schedulingapi.PriorityClass{} }, - NewListFunc: func() runtime.Object { return &schedulingapi.PriorityClassList{} }, - DefaultQualifiedResource: schedulingapi.Resource("priorityclasses"), + NewFunc: func() runtime.Object { return &scheduling.PriorityClass{} }, + NewListFunc: func() runtime.Object { return &scheduling.PriorityClassList{} }, + DefaultQualifiedResource: scheduling.Resource("priorityclasses"), CreateStrategy: priorityclass.Strategy, UpdateStrategy: priorityclass.Strategy, @@ -56,3 +61,14 @@ var _ rest.ShortNamesProvider = &REST{} func (r *REST) ShortNames() []string { return []string{"pc"} } + +// Delete ensures that system priority classes are not deleted. +func (r *REST) Delete(ctx genericapirequest.Context, name string, options *metav1.DeleteOptions) (runtime.Object, bool, error) { + for _, spc := range scheduling.SystemPriorityClasses() { + if name == spc.Name { + return nil, false, apierrors.NewForbidden(scheduling.Resource("priorityclasses"), spc.Name, errors.New("this is a system priority class and cannot be deleted")) + } + } + + return r.Store.Delete(ctx, name, options) +} diff --git a/pkg/registry/scheduling/priorityclass/storage/storage_test.go b/pkg/registry/scheduling/priorityclass/storage/storage_test.go index 8ef95fd59f5..01a081ea5d9 100644 --- a/pkg/registry/scheduling/priorityclass/storage/storage_test.go +++ b/pkg/registry/scheduling/priorityclass/storage/storage_test.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing" etcdtesting "k8s.io/apiserver/pkg/storage/etcd/testing" @@ -105,6 +106,22 @@ func TestDelete(t *testing.T) { test.TestDelete(validNewPriorityClass()) } +// TestDeleteSystemPriorityClass checks that system priority classes cannot be deleted. +func TestDeleteSystemPriorityClass(t *testing.T) { + storage, server := newStorage(t) + defer server.Terminate(t) + defer storage.Store.DestroyFunc() + key := "test/system-node-critical" + ctx := genericapirequest.NewContext() + pc := scheduling.SystemPriorityClasses()[0] + if err := storage.Store.Storage.Create(ctx, key, pc, nil, 0); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, _, err := storage.Delete(ctx, pc.Name, nil); err == nil { + t.Error("expected to receive an error") + } +} + func TestGet(t *testing.T) { storage, server := newStorage(t) defer server.Terminate(t) diff --git a/pkg/registry/scheduling/priorityclass/strategy.go b/pkg/registry/scheduling/priorityclass/strategy.go index f39dce1df1a..45fb3eacb0b 100644 --- a/pkg/registry/scheduling/priorityclass/strategy.go +++ b/pkg/registry/scheduling/priorityclass/strategy.go @@ -68,9 +68,7 @@ func (priorityClassStrategy) AllowCreateOnUpdate() bool { // ValidateUpdate is the default update validation for an end user. func (priorityClassStrategy) ValidateUpdate(ctx genericapirequest.Context, obj, old runtime.Object) field.ErrorList { - validationErrorList := validation.ValidatePriorityClass(obj.(*scheduling.PriorityClass)) - updateErrorList := validation.ValidatePriorityClassUpdate(obj.(*scheduling.PriorityClass), old.(*scheduling.PriorityClass)) - return append(validationErrorList, updateErrorList...) + return validation.ValidatePriorityClassUpdate(obj.(*scheduling.PriorityClass), old.(*scheduling.PriorityClass)) } // AllowUnconditionalUpdate is the default update policy for PriorityClass objects. diff --git a/pkg/registry/scheduling/rest/storage_scheduling.go b/pkg/registry/scheduling/rest/storage_scheduling.go index 807a4ae8a6c..f9f40249689 100644 --- a/pkg/registry/scheduling/rest/storage_scheduling.go +++ b/pkg/registry/scheduling/rest/storage_scheduling.go @@ -68,22 +68,6 @@ func (p RESTStorageProvider) PostStartHook() (string, genericapiserver.PostStart } func AddSystemPriorityClasses() genericapiserver.PostStartHookFunc { - priorityClasses := []*scheduling.PriorityClass{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: scheduling.SystemNodeCritical, - }, - Value: scheduling.SystemCriticalPriority + 1000, - Description: "Used for system critical pods that must not be moved from their current node.", - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: scheduling.SystemClusterCritical, - }, - Value: scheduling.SystemCriticalPriority, - Description: "Used for system critical pods that must run in the cluster, but can be moved to another node if necessary.", - }, - } return func(hookContext genericapiserver.PostStartHookContext) error { // Adding system priority classes is important. If they fail to add, many critical system // components may fail and cluster may break. @@ -94,7 +78,7 @@ func AddSystemPriorityClasses() genericapiserver.PostStartHookFunc { return false, nil } - for _, pc := range priorityClasses { + for _, pc := range scheduling.SystemPriorityClasses() { _, err := schedClientSet.PriorityClasses().Get(pc.Name, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { @@ -106,11 +90,12 @@ func AddSystemPriorityClasses() genericapiserver.PostStartHookFunc { } } else { // Unable to get the priority class for reasons other than "not found". + glog.Warningf("unable to get PriorityClass %v: %v. Retrying...", pc.Name, err) return false, err } } } - glog.Infof("all system priority classes are created successfully.") + glog.Infof("all system priority classes are created successfully or already exist.") return true, nil }) // if we're never able to make it through initialization, kill the API server. diff --git a/plugin/pkg/admission/priority/admission.go b/plugin/pkg/admission/priority/admission.go index 97add55a8d2..8f3d62469b1 100644 --- a/plugin/pkg/admission/priority/admission.go +++ b/plugin/pkg/admission/priority/admission.go @@ -19,12 +19,10 @@ package priority import ( "fmt" "io" - "strings" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" "k8s.io/apiserver/pkg/admission" - "k8s.io/apiserver/pkg/authentication/user" utilfeature "k8s.io/apiserver/pkg/util/feature" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/scheduling" @@ -188,16 +186,6 @@ func (p *priorityPlugin) validatePriorityClass(a admission.Attributes) error { if !ok { return errors.NewBadRequest("resource was marked with kind PriorityClass but was unable to be converted") } - // API server adds system critical priority classes at bootstrapping. We should - // not enforce restrictions on adding system level priority classes for API server. - if userInfo := a.GetUserInfo(); userInfo == nil || userInfo.GetName() != user.APIServerUser { - if pc.Value > scheduling.HighestUserDefinablePriority { - return admission.NewForbidden(a, fmt.Errorf("maximum allowed value of a user defined priority is %v", scheduling.HighestUserDefinablePriority)) - } - if strings.HasPrefix(pc.Name, scheduling.SystemPriorityClassPrefix) { - return admission.NewForbidden(a, fmt.Errorf("priority class names with '"+scheduling.SystemPriorityClassPrefix+"' prefix are reserved for system use only")) - } - } // If the new PriorityClass tries to be the default priority, make sure that no other priority class is marked as default. if pc.GlobalDefault { dpc, err := p.getDefaultPriorityClass() diff --git a/plugin/pkg/admission/priority/admission_test.go b/plugin/pkg/admission/priority/admission_test.go index bda6a8a4eea..2a9e3a1b33c 100644 --- a/plugin/pkg/admission/priority/admission_test.go +++ b/plugin/pkg/admission/priority/admission_test.go @@ -87,17 +87,6 @@ var systemClusterCritical = &scheduling.PriorityClass{ } func TestPriorityClassAdmission(t *testing.T) { - var tooHighPriorityClass = &scheduling.PriorityClass{ - TypeMeta: metav1.TypeMeta{ - Kind: "PriorityClass", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "toohighclass", - }, - Value: scheduling.HighestUserDefinablePriority + 1, - Description: "Just a test priority class", - } - var systemClass = &scheduling.PriorityClass{ TypeMeta: metav1.TypeMeta{ Kind: "PriorityClass", @@ -131,21 +120,7 @@ func TestPriorityClassAdmission(t *testing.T) { true, }, { - "too high PriorityClass value", - []*scheduling.PriorityClass{}, - tooHighPriorityClass, - nil, - true, - }, - { - "system name conflict", - []*scheduling.PriorityClass{}, - systemClass, - nil, - true, - }, - { - "system name allowed for API server", + "system name and value are allowed by admission controller", []*scheduling.PriorityClass{}, systemClass, &user.DefaultInfo{ diff --git a/test/e2e/scheduling/preemption.go b/test/e2e/scheduling/preemption.go index 64a29dc61f8..b67c6bf8130 100644 --- a/test/e2e/scheduling/preemption.go +++ b/test/e2e/scheduling/preemption.go @@ -325,11 +325,8 @@ var _ = SIGDescribe("PodPriorityResolution [Serial] [Feature:PodPreemption]", fu framework.ExpectNoError(err) }) - // This test verifies that when a higher priority pod is created and no node with - // enough resources is found, scheduler preempts a lower priority pod to schedule - // the high priority pod. + // This test verifies that system critical priorities are created automatically and resolved properly. It("validates critical system priorities are created and resolved", func() { - var podRes v1.ResourceList // Create pods that use system critical priorities and By("Create pods that use critical system priorities.") systemPriorityClasses := []string{ @@ -339,9 +336,6 @@ var _ = SIGDescribe("PodPriorityResolution [Serial] [Feature:PodPreemption]", fu pod := createPausePod(f, pausePodConfig{ Name: fmt.Sprintf("pod%d-%v", i, spc), PriorityClassName: spc, - Resources: &v1.ResourceRequirements{ - Requests: podRes, - }, }) Expect(pod.Spec.Priority).NotTo(BeNil()) framework.Logf("Created pod: %v", pod.Name)