Skip to content

Commit

Permalink
Merge pull request #76362 from yue9944882/chore/follow-up-prune-inter…
Browse files Browse the repository at this point in the history
…nal-client-scheduling-v1-api

Follow-up of #71136: Switch system priority class usage to versioned (v1) api
  • Loading branch information
k8s-ci-robot committed Oct 24, 2019
2 parents 31b7e3d + 09cf42d commit 5238e1c
Show file tree
Hide file tree
Showing 14 changed files with 60 additions and 54 deletions.
14 changes: 1 addition & 13 deletions pkg/apis/scheduling/BUILD
@@ -1,16 +1,11 @@
package(default_visibility = ["//visibility:public"])

load(
"@io_bazel_rules_go//go:def.bzl",
"go_library",
"go_test",
)
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "go_default_library",
srcs = [
"doc.go",
"helpers.go",
"register.go",
"types.go",
"zz_generated.deepcopy.go",
Expand Down Expand Up @@ -45,10 +40,3 @@ filegroup(
],
tags = ["automanaged"],
)

go_test(
name = "go_default_test",
srcs = ["helpers_test.go"],
embed = [":go_default_library"],
deps = ["//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library"],
)
9 changes: 8 additions & 1 deletion pkg/apis/scheduling/v1/BUILD
Expand Up @@ -5,6 +5,7 @@ go_library(
srcs = [
"defaults.go",
"doc.go",
"helpers.go",
"register.go",
"zz_generated.conversion.go",
"zz_generated.defaults.go",
Expand All @@ -17,6 +18,7 @@ go_library(
"//pkg/features:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/api/scheduling/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/conversion:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
Expand All @@ -40,14 +42,19 @@ filegroup(

go_test(
name = "go_default_test",
srcs = ["defaults_test.go"],
srcs = [
"defaults_test.go",
"helpers_test.go",
],
embed = [":go_default_library"],
deps = [
"//pkg/api/legacyscheme:go_default_library",
"//pkg/api/testapi:go_default_library",
"//pkg/apis/scheduling:go_default_library",
"//pkg/features:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/api/scheduling/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
Expand Down
@@ -1,5 +1,5 @@
/*
Copyright 2018 The Kubernetes Authors.
Copyright 2019 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.
Expand All @@ -14,52 +14,54 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package scheduling
package v1

import (
"fmt"
"k8s.io/api/scheduling/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/kubernetes/pkg/apis/scheduling"
)

// 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{
var systemPriorityClasses = []*v1.PriorityClass{
{
ObjectMeta: metav1.ObjectMeta{
Name: SystemNodeCritical,
Name: scheduling.SystemNodeCritical,
},
Value: SystemCriticalPriority + 1000,
Value: scheduling.SystemCriticalPriority + 1000,
Description: "Used for system critical pods that must not be moved from their current node.",
},
{
ObjectMeta: metav1.ObjectMeta{
Name: SystemClusterCritical,
Name: scheduling.SystemClusterCritical,
},
Value: SystemCriticalPriority,
Value: scheduling.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 {
func SystemPriorityClasses() []*v1.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 {
// IsKnownSystemPriorityClass returns true if there's any of the system priority classes exactly
// matches "name", "value", "globalDefault". otherwise it will return an error.
func IsKnownSystemPriorityClass(name string, value int32, globalDefault bool) (bool, error) {
for _, spc := range SystemPriorityClasses() {
if spc.Name == name {
if spc.Value != value {
return false, fmt.Errorf("value of %v PriorityClass must be %v", spc.Name, spc.Value)
}
if spc.GlobalDefault != pc.GlobalDefault {
if spc.GlobalDefault != 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)
return false, fmt.Errorf("%v is not a known system priority class", name)
}
@@ -1,5 +1,5 @@
/*
Copyright 2018 The Kubernetes Authors.
Copyright 2019 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.
Expand All @@ -14,18 +14,20 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package scheduling
package v1

import (
"testing"

v1 "k8s.io/api/scheduling/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/kubernetes/pkg/apis/scheduling"
)

func TestIsKnownSystemPriorityClass(t *testing.T) {
tests := []struct {
name string
pc *PriorityClass
pc *v1.PriorityClass
expected bool
}{
{
Expand All @@ -35,19 +37,19 @@ func TestIsKnownSystemPriorityClass(t *testing.T) {
},
{
name: "non-system priority class",
pc: &PriorityClass{
pc: &v1.PriorityClass{
ObjectMeta: metav1.ObjectMeta{
Name: SystemNodeCritical,
Name: scheduling.SystemNodeCritical,
},
Value: SystemCriticalPriority, // This is the value of system cluster critical
Value: scheduling.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 {
if is, err := IsKnownSystemPriorityClass(test.pc.Name, test.pc.Value, test.pc.GlobalDefault); test.expected != is {
t.Errorf("Test [%v]: Expected %v, but got %v. Error: %v", test.name, test.expected, is, err)
}
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/scheduling/validation/BUILD
Expand Up @@ -12,6 +12,7 @@ go_test(
embed = [":go_default_library"],
deps = [
"//pkg/apis/scheduling:go_default_library",
"//pkg/apis/scheduling/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
],
Expand All @@ -24,6 +25,7 @@ go_library(
deps = [
"//pkg/apis/core/validation:go_default_library",
"//pkg/apis/scheduling:go_default_library",
"//pkg/apis/scheduling/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/validation:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
],
Expand Down
3 changes: 2 additions & 1 deletion pkg/apis/scheduling/validation/validation.go
Expand Up @@ -24,6 +24,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
"k8s.io/kubernetes/pkg/apis/scheduling"
schedulingapiv1 "k8s.io/kubernetes/pkg/apis/scheduling/v1"
)

// ValidatePriorityClass tests whether required fields in the PriorityClass are
Expand All @@ -34,7 +35,7 @@ func ValidatePriorityClass(pc *scheduling.PriorityClass) field.ErrorList {
// 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 {
if is, err := schedulingapiv1.IsKnownSystemPriorityClass(pc.Name, pc.Value, pc.GlobalDefault); !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 {
Expand Down
3 changes: 2 additions & 1 deletion pkg/apis/scheduling/validation/validation_test.go
Expand Up @@ -22,10 +22,11 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/kubernetes/pkg/apis/scheduling"
schedulingapiv1 "k8s.io/kubernetes/pkg/apis/scheduling/v1"
)

func TestValidatePriorityClass(t *testing.T) {
spcs := scheduling.SystemPriorityClasses()
spcs := schedulingapiv1.SystemPriorityClasses()
successCases := map[string]scheduling.PriorityClass{
"no description": {
ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: ""},
Expand Down
2 changes: 2 additions & 0 deletions pkg/registry/scheduling/priorityclass/storage/BUILD
Expand Up @@ -12,6 +12,7 @@ go_test(
embed = [":go_default_library"],
deps = [
"//pkg/apis/scheduling:go_default_library",
"//pkg/apis/scheduling/v1:go_default_library",
"//pkg/registry/registrytest:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
Expand All @@ -31,6 +32,7 @@ go_library(
importpath = "k8s.io/kubernetes/pkg/registry/scheduling/priorityclass/storage",
deps = [
"//pkg/apis/scheduling:go_default_library",
"//pkg/apis/scheduling/v1:go_default_library",
"//pkg/printers:go_default_library",
"//pkg/printers/internalversion:go_default_library",
"//pkg/printers/storage:go_default_library",
Expand Down
3 changes: 2 additions & 1 deletion pkg/registry/scheduling/priorityclass/storage/storage.go
Expand Up @@ -26,6 +26,7 @@ import (
genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/kubernetes/pkg/apis/scheduling"
schedulingapiv1 "k8s.io/kubernetes/pkg/apis/scheduling/v1"
"k8s.io/kubernetes/pkg/printers"
printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
printerstorage "k8s.io/kubernetes/pkg/printers/storage"
Expand Down Expand Up @@ -68,7 +69,7 @@ func (r *REST) ShortNames() []string {

// Delete ensures that system priority classes are not deleted.
func (r *REST) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) {
for _, spc := range scheduling.SystemPriorityClasses() {
for _, spc := range schedulingapiv1.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"))
}
Expand Down
9 changes: 7 additions & 2 deletions pkg/registry/scheduling/priorityclass/storage/storage_test.go
Expand Up @@ -29,6 +29,7 @@ import (
"k8s.io/apiserver/pkg/registry/rest"
etcd3testing "k8s.io/apiserver/pkg/storage/etcd3/testing"
"k8s.io/kubernetes/pkg/apis/scheduling"
schedulingapiv1 "k8s.io/kubernetes/pkg/apis/scheduling/v1"
"k8s.io/kubernetes/pkg/registry/registrytest"
)

Expand Down Expand Up @@ -118,8 +119,12 @@ func TestDeleteSystemPriorityClass(t *testing.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, false); err != nil {
pc := schedulingapiv1.SystemPriorityClasses()[0]
internalPc := &scheduling.PriorityClass{}
if err := schedulingapiv1.Convert_v1_PriorityClass_To_scheduling_PriorityClass(pc, internalPc, nil); err != nil {
t.Fatal(err)
}
if err := storage.Store.Storage.Create(ctx, key, internalPc, nil, 0, false); err != nil {
t.Fatalf("unexpected error: %v", err)
}
if _, _, err := storage.Delete(ctx, pc.Name, rest.ValidateAllObjectFunc, nil); err == nil {
Expand Down
3 changes: 1 addition & 2 deletions pkg/registry/scheduling/rest/BUILD
Expand Up @@ -16,7 +16,6 @@ go_library(
"//pkg/apis/scheduling/v1alpha1:go_default_library",
"//pkg/apis/scheduling/v1beta1:go_default_library",
"//pkg/registry/scheduling/priorityclass/storage:go_default_library",
"//staging/src/k8s.io/api/scheduling/v1beta1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
Expand All @@ -25,7 +24,7 @@ go_library(
"//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server/storage:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/typed/scheduling/v1beta1:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/typed/scheduling/v1:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
],
)
Expand Down
12 changes: 3 additions & 9 deletions pkg/registry/scheduling/rest/storage_scheduling.go
Expand Up @@ -22,7 +22,6 @@ import (

"k8s.io/klog"

schedulingv1beta1 "k8s.io/api/scheduling/v1beta1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand All @@ -31,7 +30,7 @@ import (
"k8s.io/apiserver/pkg/registry/rest"
genericapiserver "k8s.io/apiserver/pkg/server"
serverstorage "k8s.io/apiserver/pkg/server/storage"
schedulingclient "k8s.io/client-go/kubernetes/typed/scheduling/v1beta1"
schedulingclient "k8s.io/client-go/kubernetes/typed/scheduling/v1"
"k8s.io/kubernetes/pkg/api/legacyscheme"
"k8s.io/kubernetes/pkg/apis/scheduling"
schedulingapiv1 "k8s.io/kubernetes/pkg/apis/scheduling/v1"
Expand Down Expand Up @@ -123,16 +122,11 @@ func AddSystemPriorityClasses() genericapiserver.PostStartHookFunc {
return false, nil
}

for _, pc := range scheduling.SystemPriorityClasses() {
for _, pc := range schedulingapiv1.SystemPriorityClasses() {
_, err := schedClientSet.PriorityClasses().Get(pc.Name, metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
// TODO: Remove this explicit conversion after scheduling api move to v1
v1beta1PriorityClass := &schedulingv1beta1.PriorityClass{}
if err := schedulingapiv1beta1.Convert_scheduling_PriorityClass_To_v1beta1_PriorityClass(pc, v1beta1PriorityClass, nil); err != nil {
return false, err
}
_, err := schedClientSet.PriorityClasses().Create(v1beta1PriorityClass)
_, err := schedClientSet.PriorityClasses().Create(pc)
if err != nil && !apierrors.IsAlreadyExists(err) {
return false, err
} else {
Expand Down
1 change: 1 addition & 0 deletions plugin/pkg/admission/priority/BUILD
Expand Up @@ -35,6 +35,7 @@ go_library(
deps = [
"//pkg/apis/core:go_default_library",
"//pkg/apis/scheduling:go_default_library",
"//pkg/apis/scheduling/v1:go_default_library",
"//pkg/features:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/api/scheduling/v1:go_default_library",
Expand Down
3 changes: 2 additions & 1 deletion plugin/pkg/admission/priority/admission.go
Expand Up @@ -35,6 +35,7 @@ import (
"k8s.io/kubernetes/pkg/apis/core"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/scheduling"
schedulingapiv1 "k8s.io/kubernetes/pkg/apis/scheduling/v1"
"k8s.io/kubernetes/pkg/features"
)

Expand Down Expand Up @@ -144,7 +145,7 @@ func priorityClassPermittedInNamespace(priorityClassName string, namespace strin
// Only allow system priorities in the system namespace. This is to prevent abuse or incorrect
// usage of these priorities. Pods created at these priorities could preempt system critical
// components.
for _, spc := range scheduling.SystemPriorityClasses() {
for _, spc := range schedulingapiv1.SystemPriorityClasses() {
if spc.Name == priorityClassName && namespace != metav1.NamespaceSystem {
return false
}
Expand Down

0 comments on commit 5238e1c

Please sign in to comment.