diff --git a/pkg/apis/flowcontrol/validation/BUILD b/pkg/apis/flowcontrol/validation/BUILD index 62575f0e9eb..8e2aa88803a 100644 --- a/pkg/apis/flowcontrol/validation/BUILD +++ b/pkg/apis/flowcontrol/validation/BUILD @@ -11,6 +11,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/api/validation: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/shufflesharding:go_default_library", ], ) diff --git a/pkg/apis/flowcontrol/validation/validation.go b/pkg/apis/flowcontrol/validation/validation.go index e40bb610e2d..78430e592a0 100644 --- a/pkg/apis/flowcontrol/validation/validation.go +++ b/pkg/apis/flowcontrol/validation/validation.go @@ -18,12 +18,13 @@ package validation import ( "fmt" - "math/big" + "strings" "k8s.io/apimachinery/pkg/api/validation" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/util/shufflesharding" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/apis/flowcontrol" ) @@ -55,10 +56,6 @@ var supportedSubjectKinds = sets.NewString( flowcontrol.GroupKind, ) -const ( - maxHashBits = 60 -) - func ValidateFlowSchema(fs *flowcontrol.FlowSchema) field.ErrorList { allErrs := apivalidation.ValidateObjectMeta(&fs.ObjectMeta, false, ValidateFlowSchemaName, field.NewPath("metadata")) allErrs = append(allErrs, ValidateFlowSchemaSpec(&fs.Spec, field.NewPath("spec"))...) @@ -217,6 +214,13 @@ func ValidatePriorityLevelConfigurationSpec(spec *flowcontrol.PriorityLevelConfi if spec.HandSize < 0 { allErrs = append(allErrs, field.Invalid(fldPath.Child("handSize"), spec.HandSize, "must be positive")) } + if spec.HandSize > spec.Queues { + allErrs = append(allErrs, field.Invalid(fldPath.Child("handSize"), spec.HandSize, + fmt.Sprintf("should not be greater than queues (%d)", spec.Queues))) + } + if errs := shufflesharding.ValidateParameters(int(spec.Queues), int(spec.HandSize)); len(errs) > 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("handSize"), spec.HandSize, strings.Join(errs, "; "))) + } } else { if spec.AssuredConcurrencyShares != 0 { allErrs = append(allErrs, field.Forbidden(fldPath.Child("assuredConcurrencyShares"), "must be zero for exempt priority")) @@ -231,14 +235,6 @@ func ValidatePriorityLevelConfigurationSpec(spec *flowcontrol.PriorityLevelConfi allErrs = append(allErrs, field.Forbidden(fldPath.Child("handSize"), "must be zero for exempt priority")) } } - if spec.HandSize > spec.Queues { - allErrs = append(allErrs, field.Invalid(fldPath.Child("handSize"), spec.HandSize, - fmt.Sprintf("should not be greater than queues (%d)", spec.Queues))) - } - if !validateShuffleShardingParameters(spec.HandSize, spec.Queues) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("handSize"), spec.HandSize, - fmt.Sprintf("falling factorial of handSize (%d) and queues (%d) exceeds %d bits", spec.HandSize, spec.Queues, maxHashBits))) - } return allErrs } @@ -248,11 +244,6 @@ func ValidatePriorityLevelConfigurationStatus(status *flowcontrol.PriorityLevelC return allErrs } -func validateShuffleShardingParameters(handSize, queues int32) bool { - // TODO: performance impact from bit-int multiplication? - return int32(big.NewInt(int64(queues)).BitLen())*handSize <= maxHashBits -} - func hasWildcard(operations []string) bool { for _, o := range operations { if o == "*" { diff --git a/pkg/apis/flowcontrol/validation/validation_test.go b/pkg/apis/flowcontrol/validation/validation_test.go index 19111b00a93..d51e6d751a5 100644 --- a/pkg/apis/flowcontrol/validation/validation_test.go +++ b/pkg/apis/flowcontrol/validation/validation_test.go @@ -416,6 +416,7 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) { field.Invalid(field.NewPath("spec").Child("assuredConcurrencyShares"), int32(0), "must be positive"), field.Invalid(field.NewPath("spec").Child("queueLengthLimit"), int32(0), "must be positive"), field.Invalid(field.NewPath("spec").Child("queues"), int32(0), "must be positive"), + field.Invalid(field.NewPath("spec").Child("handSize"), int32(0), "handSize is not positive; deckSize is not positive"), }, }, { @@ -432,7 +433,7 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) { }, }, expectedErrors: field.ErrorList{ - field.Invalid(field.NewPath("spec").Child("handSize"), int32(8), "falling factorial of handSize (8) and queues (512) exceeds 60 bits"), + field.Invalid(field.NewPath("spec").Child("handSize"), int32(8), "more than 60 bits of entropy required"), }, }, { @@ -449,7 +450,7 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) { }, }, expectedErrors: field.ErrorList{ - field.Invalid(field.NewPath("spec").Child("handSize"), int32(10), "falling factorial of handSize (10) and queues (128) exceeds 60 bits"), + field.Invalid(field.NewPath("spec").Child("handSize"), int32(10), "more than 60 bits of entropy required"), }, }, { @@ -466,7 +467,7 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) { }, }, expectedErrors: field.ErrorList{ - field.Invalid(field.NewPath("spec").Child("handSize"), int32(3), "falling factorial of handSize (3) and queues (2147483647) exceeds 60 bits"), + field.Invalid(field.NewPath("spec").Child("handSize"), int32(3), "more than 60 bits of entropy required"), }, }, { @@ -499,6 +500,7 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) { }, expectedErrors: field.ErrorList{ field.Invalid(field.NewPath("spec").Child("handSize"), int32(8), "should not be greater than queues (7)"), + field.Invalid(field.NewPath("spec").Child("handSize"), int32(8), "handSize is greater than deckSize"), }, }, }