From 6b292822f5c6513bcb162b1b573eceae4ff91428 Mon Sep 17 00:00:00 2001 From: "Bobby (Babak) Salamat" Date: Fri, 16 Feb 2018 14:35:50 -0800 Subject: [PATCH 1/4] Pick the PriorityClass with the lowest value of priority in case more than one global default exists --- plugin/pkg/admission/priority/admission.go | 9 +++++++-- plugin/pkg/admission/priority/admission_test.go | 8 ++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/plugin/pkg/admission/priority/admission.go b/plugin/pkg/admission/priority/admission.go index 5a0c98d278c..3750ec86aff 100644 --- a/plugin/pkg/admission/priority/admission.go +++ b/plugin/pkg/admission/priority/admission.go @@ -231,12 +231,17 @@ func (p *PriorityPlugin) getDefaultPriorityClass() (*scheduling.PriorityClass, e if err != nil { return nil, err } + // In case more than one global default priority class is added as a result of a race condition, + // we pick the one with the lowest priority value. + var defaultPC *scheduling.PriorityClass for _, pci := range list { if pci.GlobalDefault { - return pci, nil + if defaultPC == nil || defaultPC.Value > pci.Value { + defaultPC = pci + } } } - return nil, nil + return defaultPC, nil } func (p *PriorityPlugin) getDefaultPriority() (int32, error) { diff --git a/plugin/pkg/admission/priority/admission_test.go b/plugin/pkg/admission/priority/admission_test.go index c9122ac63f0..06963588c61 100644 --- a/plugin/pkg/admission/priority/admission_test.go +++ b/plugin/pkg/admission/priority/admission_test.go @@ -189,6 +189,14 @@ func TestDefaultPriority(t *testing.T) { expectedDefaultBefore: scheduling.DefaultPriorityWhenNoDefaultClassExists, expectedDefaultAfter: defaultClass1.Value, }, + { + name: "multiple default classes resolves to the minimum value among them", + classesBefore: []*scheduling.PriorityClass{defaultClass1, defaultClass2}, + classesAfter: []*scheduling.PriorityClass{defaultClass2}, + attributes: admission.NewAttributesRecord(nil, nil, pcKind, "", defaultClass1.Name, pcResource, "", admission.Delete, nil), + expectedDefaultBefore: defaultClass1.Value, + expectedDefaultAfter: defaultClass2.Value, + }, { name: "delete default priority class", classesBefore: []*scheduling.PriorityClass{defaultClass1}, From 5f7354679ea1201d56373c687554a750b44fc16d Mon Sep 17 00:00:00 2001 From: "Bobby (Babak) Salamat" Date: Fri, 16 Feb 2018 17:49:29 -0800 Subject: [PATCH 2/4] Add API docs for multiple PriorityClasses marked as globalDefault --- pkg/apis/scheduling/types.go | 5 ++++- staging/src/k8s.io/api/scheduling/v1alpha1/types.go | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/apis/scheduling/types.go b/pkg/apis/scheduling/types.go index 5130d624a03..1e81eb66190 100644 --- a/pkg/apis/scheduling/types.go +++ b/pkg/apis/scheduling/types.go @@ -44,8 +44,11 @@ type PriorityClass struct { // receive when they have the name of this class in their pod spec. Value int32 - // GlobalDefault specifies whether this PriorityClass should be considered as + // globalDefault specifies whether this PriorityClass should be considered as // the default priority for pods that do not have any priority class. + // Only one PriorityClass with can be marked as `globalDefault`. Due to race conditions, more than + // one PriorityClasses might be created with their `globalDefault` field set to true. In that case, + // the smallest value of such global default PriorityClasses will be used as the default priority. // +optional GlobalDefault bool diff --git a/staging/src/k8s.io/api/scheduling/v1alpha1/types.go b/staging/src/k8s.io/api/scheduling/v1alpha1/types.go index 07bf337fb14..bba67b4df5d 100644 --- a/staging/src/k8s.io/api/scheduling/v1alpha1/types.go +++ b/staging/src/k8s.io/api/scheduling/v1alpha1/types.go @@ -39,6 +39,9 @@ type PriorityClass struct { // globalDefault specifies whether this PriorityClass should be considered as // the default priority for pods that do not have any priority class. + // Only one PriorityClass with can be marked as `globalDefault`. Due to race conditions, more than + // one PriorityClasses might be created with their `globalDefault` field set to true. In that case, + // the smallest value of such global default PriorityClasses will be used as the default priority. // +optional GlobalDefault bool `json:"globalDefault,omitempty" protobuf:"bytes,3,opt,name=globalDefault"` From 0b0884f1462adced0843f6a367529d0b1264d475 Mon Sep 17 00:00:00 2001 From: "Bobby (Babak) Salamat" Date: Sun, 18 Feb 2018 17:47:06 -0800 Subject: [PATCH 3/4] Changed API doc --- pkg/apis/scheduling/types.go | 4 ++-- staging/src/k8s.io/api/scheduling/v1alpha1/types.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/apis/scheduling/types.go b/pkg/apis/scheduling/types.go index 1e81eb66190..d5db894a8d0 100644 --- a/pkg/apis/scheduling/types.go +++ b/pkg/apis/scheduling/types.go @@ -46,8 +46,8 @@ type PriorityClass struct { // globalDefault specifies whether this PriorityClass should be considered as // the default priority for pods that do not have any priority class. - // Only one PriorityClass with can be marked as `globalDefault`. Due to race conditions, more than - // one PriorityClasses might be created with their `globalDefault` field set to true. In that case, + // Only one PriorityClass can be marked as `globalDefault`. However, if more than + // one PriorityClasses exists with their `globalDefault` field set to true, // the smallest value of such global default PriorityClasses will be used as the default priority. // +optional GlobalDefault bool diff --git a/staging/src/k8s.io/api/scheduling/v1alpha1/types.go b/staging/src/k8s.io/api/scheduling/v1alpha1/types.go index bba67b4df5d..21e3df0af96 100644 --- a/staging/src/k8s.io/api/scheduling/v1alpha1/types.go +++ b/staging/src/k8s.io/api/scheduling/v1alpha1/types.go @@ -39,8 +39,8 @@ type PriorityClass struct { // globalDefault specifies whether this PriorityClass should be considered as // the default priority for pods that do not have any priority class. - // Only one PriorityClass with can be marked as `globalDefault`. Due to race conditions, more than - // one PriorityClasses might be created with their `globalDefault` field set to true. In that case, + // Only one PriorityClass can be marked as `globalDefault`. However, if more than + // one PriorityClasses exists with their `globalDefault` field set to true, // the smallest value of such global default PriorityClasses will be used as the default priority. // +optional GlobalDefault bool `json:"globalDefault,omitempty" protobuf:"bytes,3,opt,name=globalDefault"` From af0d7459a7fe65d6ea970754f209e16032dd0521 Mon Sep 17 00:00:00 2001 From: "Bobby (Babak) Salamat" Date: Sun, 18 Feb 2018 17:56:37 -0800 Subject: [PATCH 4/4] autogenerated files --- api/openapi-spec/swagger.json | 2 +- api/swagger-spec/scheduling.k8s.io_v1alpha1.json | 2 +- docs/api-reference/scheduling.k8s.io/v1alpha1/definitions.html | 2 +- staging/src/k8s.io/api/scheduling/v1alpha1/generated.proto | 3 +++ .../api/scheduling/v1alpha1/types_swagger_doc_generated.go | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 0754abf13f6..6465e23d031 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -83654,7 +83654,7 @@ "type": "string" }, "globalDefault": { - "description": "globalDefault specifies whether this PriorityClass should be considered as the default priority for pods that do not have any priority class.", + "description": "globalDefault specifies whether this PriorityClass should be considered as the default priority for pods that do not have any priority class. Only one PriorityClass can be marked as `globalDefault`. However, if more than one PriorityClasses exists with their `globalDefault` field set to true, the smallest value of such global default PriorityClasses will be used as the default priority.", "type": "boolean" }, "kind": { diff --git a/api/swagger-spec/scheduling.k8s.io_v1alpha1.json b/api/swagger-spec/scheduling.k8s.io_v1alpha1.json index d2a6aedc565..f6a1545826a 100644 --- a/api/swagger-spec/scheduling.k8s.io_v1alpha1.json +++ b/api/swagger-spec/scheduling.k8s.io_v1alpha1.json @@ -799,7 +799,7 @@ }, "globalDefault": { "type": "boolean", - "description": "globalDefault specifies whether this PriorityClass should be considered as the default priority for pods that do not have any priority class." + "description": "globalDefault specifies whether this PriorityClass should be considered as the default priority for pods that do not have any priority class. Only one PriorityClass can be marked as `globalDefault`. However, if more than one PriorityClasses exists with their `globalDefault` field set to true, the smallest value of such global default PriorityClasses will be used as the default priority." }, "description": { "type": "string", diff --git a/docs/api-reference/scheduling.k8s.io/v1alpha1/definitions.html b/docs/api-reference/scheduling.k8s.io/v1alpha1/definitions.html index 5c480a3ca0f..24e467ad970 100755 --- a/docs/api-reference/scheduling.k8s.io/v1alpha1/definitions.html +++ b/docs/api-reference/scheduling.k8s.io/v1alpha1/definitions.html @@ -1341,7 +1341,7 @@ Examples:

globalDefault

-

globalDefault specifies whether this PriorityClass should be considered as the default priority for pods that do not have any priority class.

+

globalDefault specifies whether this PriorityClass should be considered as the default priority for pods that do not have any priority class. Only one PriorityClass can be marked as globalDefault. However, if more than one PriorityClasses exists with their globalDefault field set to true, the smallest value of such global default PriorityClasses will be used as the default priority.

false

boolean

false

diff --git a/staging/src/k8s.io/api/scheduling/v1alpha1/generated.proto b/staging/src/k8s.io/api/scheduling/v1alpha1/generated.proto index 588ef9718b0..e964a76dd4d 100644 --- a/staging/src/k8s.io/api/scheduling/v1alpha1/generated.proto +++ b/staging/src/k8s.io/api/scheduling/v1alpha1/generated.proto @@ -43,6 +43,9 @@ message PriorityClass { // globalDefault specifies whether this PriorityClass should be considered as // the default priority for pods that do not have any priority class. + // Only one PriorityClass can be marked as `globalDefault`. However, if more than + // one PriorityClasses exists with their `globalDefault` field set to true, + // the smallest value of such global default PriorityClasses will be used as the default priority. // +optional optional bool globalDefault = 3; diff --git a/staging/src/k8s.io/api/scheduling/v1alpha1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/scheduling/v1alpha1/types_swagger_doc_generated.go index 4b68bf04a47..9080dd9d68c 100644 --- a/staging/src/k8s.io/api/scheduling/v1alpha1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/scheduling/v1alpha1/types_swagger_doc_generated.go @@ -31,7 +31,7 @@ var map_PriorityClass = map[string]string{ "": "PriorityClass defines mapping from a priority class name to the priority integer value. The value can be any valid integer.", "metadata": "Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata", "value": "The value of this priority class. This is the actual priority that pods receive when they have the name of this class in their pod spec.", - "globalDefault": "globalDefault specifies whether this PriorityClass should be considered as the default priority for pods that do not have any priority class.", + "globalDefault": "globalDefault specifies whether this PriorityClass should be considered as the default priority for pods that do not have any priority class. Only one PriorityClass can be marked as `globalDefault`. However, if more than one PriorityClasses exists with their `globalDefault` field set to true, the smallest value of such global default PriorityClasses will be used as the default priority.", "description": "description is an arbitrary string that usually provides guidelines on when this priority class should be used.", }