From ffe4ae23f31d4697e0efdcb2bfc968a0460ca802 Mon Sep 17 00:00:00 2001 From: Xuzheng Chang Date: Mon, 25 Jul 2022 20:31:10 +0800 Subject: [PATCH] fix ambiguous comments of priorityClass update validation --- pkg/apis/scheduling/validation/validation.go | 9 +-- .../scheduling/validation/validation_test.go | 61 +++++++++++++------ 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/pkg/apis/scheduling/validation/validation.go b/pkg/apis/scheduling/validation/validation.go index 2cfbb744302..2812f9acbfd 100644 --- a/pkg/apis/scheduling/validation/validation.go +++ b/pkg/apis/scheduling/validation/validation.go @@ -49,14 +49,15 @@ func ValidatePriorityClass(pc *scheduling.PriorityClass) field.ErrorList { } // ValidatePriorityClassUpdate tests if required fields in the PriorityClass are -// set and are valid. PriorityClass does not allow updating Name, and Value. +// set and are valid. PriorityClass does not allow updating name, value, and preemptionPolicy. func ValidatePriorityClassUpdate(pc, oldPc *scheduling.PriorityClass) field.ErrorList { + // name is immutable and is checked by the ObjectMeta validator. allErrs := apivalidation.ValidateObjectMetaUpdate(&pc.ObjectMeta, &oldPc.ObjectMeta, field.NewPath("metadata")) - // Name is immutable and is checked by the ObjectMeta validator. + // value is immutable. if pc.Value != oldPc.Value { - allErrs = append(allErrs, field.Forbidden(field.NewPath("Value"), "may not be changed in an update.")) + allErrs = append(allErrs, field.Forbidden(field.NewPath("value"), "may not be changed in an update.")) } - // PreemptionPolicy is immutable and is checked by the ObjectMeta validator. + // preemptionPolicy is immutable. allErrs = append(allErrs, apivalidation.ValidateImmutableField(pc.PreemptionPolicy, oldPc.PreemptionPolicy, field.NewPath("preemptionPolicy"))...) return allErrs } diff --git a/pkg/apis/scheduling/validation/validation_test.go b/pkg/apis/scheduling/validation/validation_test.go index 785274b309a..1316b519c2e 100644 --- a/pkg/apis/scheduling/validation/validation_test.go +++ b/pkg/apis/scheduling/validation/validation_test.go @@ -21,6 +21,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/scheduling" schedulingapiv1 "k8s.io/kubernetes/pkg/apis/scheduling/v1" ) @@ -81,29 +82,37 @@ func TestValidatePriorityClass(t *testing.T) { } func TestValidatePriorityClassUpdate(t *testing.T) { + preemptLowerPriority := core.PreemptLowerPriority + preemptNever := core.PreemptNever + old := scheduling.PriorityClass{ - ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "1"}, - Value: 100, + ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "1"}, + Value: 100, + PreemptionPolicy: &preemptLowerPriority, } successCases := map[string]scheduling.PriorityClass{ "no change": { - ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, - Value: 100, - Description: "Used for the highest priority pods.", + ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, + Value: 100, + PreemptionPolicy: &preemptLowerPriority, + Description: "Used for the highest priority pods.", }, "change description": { - ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, - Value: 100, - Description: "A different description.", + ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, + Value: 100, + PreemptionPolicy: &preemptLowerPriority, + Description: "A different description.", }, "remove description": { - ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, - Value: 100, + ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, + Value: 100, + PreemptionPolicy: &preemptLowerPriority, }, "change globalDefault": { - ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, - Value: 100, - GlobalDefault: true, + ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, + Value: 100, + PreemptionPolicy: &preemptLowerPriority, + GlobalDefault: true, }, } @@ -119,31 +128,43 @@ func TestValidatePriorityClassUpdate(t *testing.T) { }{ "add namespace": { P: scheduling.PriorityClass{ - ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "foo", ResourceVersion: "2"}, - Value: 100, + ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "foo", ResourceVersion: "2"}, + Value: 100, + PreemptionPolicy: &preemptLowerPriority, }, T: field.ErrorTypeInvalid, }, "change name": { P: scheduling.PriorityClass{ - ObjectMeta: metav1.ObjectMeta{Name: "tier2", Namespace: "", ResourceVersion: "2"}, - Value: 100, + ObjectMeta: metav1.ObjectMeta{Name: "tier2", Namespace: "", ResourceVersion: "2"}, + Value: 100, + PreemptionPolicy: &preemptLowerPriority, }, T: field.ErrorTypeInvalid, }, "remove value": { P: scheduling.PriorityClass{ - ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, + ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, + PreemptionPolicy: &preemptLowerPriority, }, T: field.ErrorTypeForbidden, }, "change value": { P: scheduling.PriorityClass{ - ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, - Value: 101, + ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, + Value: 101, + PreemptionPolicy: &preemptLowerPriority, }, T: field.ErrorTypeForbidden, }, + "change preemptionPolicy": { + P: scheduling.PriorityClass{ + ObjectMeta: metav1.ObjectMeta{Name: "tier1", Namespace: "", ResourceVersion: "2"}, + Value: 100, + PreemptionPolicy: &preemptNever, + }, + T: field.ErrorTypeInvalid, + }, } for k, v := range errorCases {