rename the roundtrip annotation, forbid it in v1

This commit is contained in:
Abu Kashem 2023-10-30 18:22:51 -04:00
parent 233bc2449d
commit b8cd792b36
No known key found for this signature in database
GPG Key ID: E5ECC1124B5F9C68
15 changed files with 47 additions and 36 deletions

View File

@ -11608,7 +11608,7 @@
"description": "`limitResponse` indicates what to do with requests that can not be executed right now"
},
"nominalConcurrencyShares": {
"description": "`nominalConcurrencyShares` (NCS) contributes to the computation of the NominalConcurrencyLimit (NominalCL) of this level. This is the number of execution seats available at this priority level. This is used both for requests dispatched from this priority level as well as requests dispatched from other priority levels borrowing seats from this level. The server's concurrency limit (ServerCL) is divided among the Limited priority levels in proportion to their NCS values:\n\nNominalCL(i) = ceil( ServerCL * NCS(i) / sum_ncs ) sum_ncs = sum[priority level k] NCS(k)\n\nBigger numbers mean a larger nominal concurrency limit, at the expense of every other priority level.\n\nIf not specified, this field defaults to a value of 30.\n\nSetting this field to zero allows for the construction of a \"jail\" for this priority level that is used to hold some request(s)",
"description": "`nominalConcurrencyShares` (NCS) contributes to the computation of the NominalConcurrencyLimit (NominalCL) of this level. This is the number of execution seats available at this priority level. This is used both for requests dispatched from this priority level as well as requests dispatched from other priority levels borrowing seats from this level. The server's concurrency limit (ServerCL) is divided among the Limited priority levels in proportion to their NCS values:\n\nNominalCL(i) = ceil( ServerCL * NCS(i) / sum_ncs ) sum_ncs = sum[priority level k] NCS(k)\n\nBigger numbers mean a larger nominal concurrency limit, at the expense of every other priority level.\n\nIf not specified, this field defaults to a value of 30.\n\nSetting this field to zero supports the construction of a \"jail\" for this priority level that is used to hold some request(s)",
"format": "int32",
"type": "integer"
}

View File

@ -290,7 +290,7 @@
"description": "`limitResponse` indicates what to do with requests that can not be executed right now"
},
"nominalConcurrencyShares": {
"description": "`nominalConcurrencyShares` (NCS) contributes to the computation of the NominalConcurrencyLimit (NominalCL) of this level. This is the number of execution seats available at this priority level. This is used both for requests dispatched from this priority level as well as requests dispatched from other priority levels borrowing seats from this level. The server's concurrency limit (ServerCL) is divided among the Limited priority levels in proportion to their NCS values:\n\nNominalCL(i) = ceil( ServerCL * NCS(i) / sum_ncs ) sum_ncs = sum[priority level k] NCS(k)\n\nBigger numbers mean a larger nominal concurrency limit, at the expense of every other priority level.\n\nIf not specified, this field defaults to a value of 30.\n\nSetting this field to zero allows for the construction of a \"jail\" for this priority level that is used to hold some request(s)",
"description": "`nominalConcurrencyShares` (NCS) contributes to the computation of the NominalConcurrencyLimit (NominalCL) of this level. This is the number of execution seats available at this priority level. This is used both for requests dispatched from this priority level as well as requests dispatched from other priority levels borrowing seats from this level. The server's concurrency limit (ServerCL) is divided among the Limited priority levels in proportion to their NCS values:\n\nNominalCL(i) = ceil( ServerCL * NCS(i) / sum_ncs ) sum_ncs = sum[priority level k] NCS(k)\n\nBigger numbers mean a larger nominal concurrency limit, at the expense of every other priority level.\n\nIf not specified, this field defaults to a value of 30.\n\nSetting this field to zero supports the construction of a \"jail\" for this priority level that is used to hold some request(s)",
"format": "int32",
"type": "integer"
}

View File

@ -420,7 +420,7 @@ type LimitedPriorityLevelConfiguration struct {
// at the expense of every other priority level.
// This field has a default value of 30.
//
// Setting this field to zero allows for the construction of a
// Setting this field to zero supports the construction of a
// "jail" for this priority level that is used to hold some request(s)
//
// +optional

View File

@ -57,22 +57,22 @@ func Convert_flowcontrol_PriorityLevelConfiguration_To_v1beta3_PriorityLevelConf
}
func dropPriorityLevelConcurrencyShareDefaultAnnotation(in map[string]string) (map[string]string, bool) {
if _, ok := in[v1beta3.PriorityLevelConcurrencyShareDefaultKey]; !ok {
if _, ok := in[v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey]; !ok {
return in, false
}
out := copyStringMap(in)
delete(out, v1beta3.PriorityLevelConcurrencyShareDefaultKey)
delete(out, v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey)
return out, true
}
func addPriorityLevelConcurrencyShareDefaultAnnotation(in map[string]string) (map[string]string, bool) {
if _, ok := in[v1beta3.PriorityLevelConcurrencyShareDefaultKey]; ok {
if _, ok := in[v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey]; ok {
return in, false
}
out := copyStringMap(in)
out[v1beta3.PriorityLevelConcurrencyShareDefaultKey] = ""
out[v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey] = ""
return out, true
}

View File

@ -69,7 +69,7 @@ func TestConvert_v1beta3_PriorityLevelConfiguration_To_flowcontrol_PriorityLevel
name: "v1beta3 object, the roundtrip annotation is set, NominalConcurrencyShares is zero; the internal object should not have the roundtrip annotation set",
in: inObjFn(0, map[string]string{
"foo": "bar",
v1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
}),
expected: outObjFn(0, map[string]string{
"foo": "bar",
@ -81,7 +81,7 @@ func TestConvert_v1beta3_PriorityLevelConfiguration_To_flowcontrol_PriorityLevel
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"foo": "bar",
v1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
},
},
},
@ -169,7 +169,7 @@ func TestConvert_flowcontrol_PriorityLevelConfiguration_To_v1beta3_PriorityLevel
}),
expected: outObjFn(0, map[string]string{
"foo": "bar",
v1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
}),
},
{
@ -185,33 +185,33 @@ func TestConvert_flowcontrol_PriorityLevelConfiguration_To_v1beta3_PriorityLevel
name: "internal object, the roundtrip annotation is set, NominalConcurrencyShares is 0, no error expected",
in: inObjFn(0, map[string]string{
"foo": "bar",
v1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
}),
expected: outObjFn(0, map[string]string{
"foo": "bar",
v1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
}),
},
{
name: "internal object, the roundtrip annotation is set with a non-empty value, NominalConcurrencyShares is 0, the annotation value should be preserved",
in: inObjFn(0, map[string]string{
"foo": "bar",
v1beta3.PriorityLevelConcurrencyShareDefaultKey: "non-empty",
v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "non-empty",
}),
expected: outObjFn(0, map[string]string{
"foo": "bar",
v1beta3.PriorityLevelConcurrencyShareDefaultKey: "non-empty",
v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "non-empty",
}),
},
{
name: "internal object, the roundtrip annotation is set with a non-empty value, NominalConcurrencyShares is not 0, the annotation value should be preserved",
in: inObjFn(1, map[string]string{
"foo": "bar",
v1beta3.PriorityLevelConcurrencyShareDefaultKey: "non-empty",
v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "non-empty",
}),
expected: outObjFn(1, map[string]string{
"foo": "bar",
v1beta3.PriorityLevelConcurrencyShareDefaultKey: "non-empty",
v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "non-empty",
}),
},
}

View File

@ -52,7 +52,7 @@ func SetDefaults_PriorityLevelConfiguration(in *v1beta3.PriorityLevelConfigurati
// field only when:
// a) NominalConcurrencyShares == 0, and
// b) the roundtrip annotation is not set
if _, ok := in.Annotations[v1beta3.PriorityLevelConcurrencyShareDefaultKey]; !ok && limited.NominalConcurrencyShares == 0 {
if _, ok := in.Annotations[v1beta3.PriorityLevelPreserveZeroConcurrencySharesKey]; !ok && limited.NominalConcurrencyShares == 0 {
limited.NominalConcurrencyShares = PriorityLevelConfigurationDefaultNominalConcurrencyShares
}
}

View File

@ -141,7 +141,7 @@ func TestDefaultWithPriorityLevelConfiguration(t *testing.T) {
original: &flowcontrolv1beta3.PriorityLevelConfiguration{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
flowcontrolv1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
flowcontrolv1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
},
},
Spec: flowcontrolv1beta3.PriorityLevelConfigurationSpec{
@ -157,7 +157,7 @@ func TestDefaultWithPriorityLevelConfiguration(t *testing.T) {
expected: &flowcontrolv1beta3.PriorityLevelConfiguration{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
flowcontrolv1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
flowcontrolv1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
},
},
Spec: flowcontrolv1beta3.PriorityLevelConfigurationSpec{
@ -177,7 +177,7 @@ func TestDefaultWithPriorityLevelConfiguration(t *testing.T) {
original: &flowcontrolv1beta3.PriorityLevelConfiguration{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
flowcontrolv1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
flowcontrolv1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
},
},
Spec: flowcontrolv1beta3.PriorityLevelConfigurationSpec{
@ -193,7 +193,7 @@ func TestDefaultWithPriorityLevelConfiguration(t *testing.T) {
expected: &flowcontrolv1beta3.PriorityLevelConfiguration{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
flowcontrolv1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
flowcontrolv1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
},
},
Spec: flowcontrolv1beta3.PriorityLevelConfigurationSpec{
@ -213,7 +213,7 @@ func TestDefaultWithPriorityLevelConfiguration(t *testing.T) {
original: &flowcontrolv1beta3.PriorityLevelConfiguration{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
flowcontrolv1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
flowcontrolv1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
},
},
Spec: flowcontrolv1beta3.PriorityLevelConfigurationSpec{
@ -229,7 +229,7 @@ func TestDefaultWithPriorityLevelConfiguration(t *testing.T) {
expected: &flowcontrolv1beta3.PriorityLevelConfiguration{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
flowcontrolv1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
flowcontrolv1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
},
},
Spec: flowcontrolv1beta3.PriorityLevelConfigurationSpec{

View File

@ -22,6 +22,7 @@ import (
flowcontrolv1beta1 "k8s.io/api/flowcontrol/v1beta1"
flowcontrolv1beta2 "k8s.io/api/flowcontrol/v1beta2"
flowcontrolv1beta3 "k8s.io/api/flowcontrol/v1beta3"
apiequality "k8s.io/apimachinery/pkg/api/equality"
apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
"k8s.io/apimachinery/pkg/runtime/schema"
@ -351,6 +352,13 @@ func ValidateFlowSchemaCondition(condition *flowcontrol.FlowSchemaCondition, fld
func ValidatePriorityLevelConfiguration(pl *flowcontrol.PriorityLevelConfiguration, requestGV schema.GroupVersion, opts PriorityLevelValidationOptions) field.ErrorList {
allErrs := apivalidation.ValidateObjectMeta(&pl.ObjectMeta, false, ValidatePriorityLevelConfigurationName, field.NewPath("metadata"))
// the roundtrip annotation is only for use in v1beta3, and after
// conversion, the internal object should not have the roundtrip
// annotation, so we should forbid it, if it's set.
if _, ok := pl.ObjectMeta.Annotations[flowcontrolv1beta3.PriorityLevelPreserveZeroConcurrencySharesKey]; ok {
allErrs = append(allErrs, field.Forbidden(field.NewPath("metadata").Child("annotations"), fmt.Sprintf("annotation '%s' is forbidden", flowcontrolv1beta3.PriorityLevelPreserveZeroConcurrencySharesKey)))
}
specPath := field.NewPath("spec")
allErrs = append(allErrs, ValidatePriorityLevelConfigurationSpec(&pl.Spec, requestGV, pl.Name, specPath, opts)...)
allErrs = append(allErrs, ValidateIfMandatoryPriorityLevelConfigurationObject(pl, specPath)...)

View File

@ -1107,12 +1107,12 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
field.Invalid(field.NewPath("spec").Child("limited").Child("limitResponse").Child("queuing").Child("handSize"), int32(8), "should not be greater than queues (7)"),
},
}, {
name: "the roundtrip annotation is not forbidden in v1beta3",
name: "the roundtrip annotation is forbidden",
priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: "with-forbidden-annotation",
Annotations: map[string]string{
flowcontrolv1beta3.PriorityLevelConcurrencyShareDefaultKey: "",
flowcontrolv1beta3.PriorityLevelPreserveZeroConcurrencySharesKey: "",
},
},
Spec: flowcontrol.PriorityLevelConfigurationSpec{
@ -1124,8 +1124,11 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
},
},
},
requestGV: &flowcontrolv1beta3.SchemeGroupVersion,
expectedErrors: field.ErrorList{},
// the internal object should never have the round trip annotation
requestGV: &schema.GroupVersion{},
expectedErrors: field.ErrorList{
field.Forbidden(field.NewPath("metadata").Child("annotations"), fmt.Sprintf("annotation '%s' is forbidden", flowcontrolv1beta3.PriorityLevelPreserveZeroConcurrencySharesKey)),
},
}}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {

View File

@ -33143,7 +33143,7 @@ func schema_k8sio_api_flowcontrol_v1_LimitedPriorityLevelConfiguration(ref commo
Properties: map[string]spec.Schema{
"nominalConcurrencyShares": {
SchemaProps: spec.SchemaProps{
Description: "`nominalConcurrencyShares` (NCS) contributes to the computation of the NominalConcurrencyLimit (NominalCL) of this level. This is the number of execution seats available at this priority level. This is used both for requests dispatched from this priority level as well as requests dispatched from other priority levels borrowing seats from this level. The server's concurrency limit (ServerCL) is divided among the Limited priority levels in proportion to their NCS values:\n\nNominalCL(i) = ceil( ServerCL * NCS(i) / sum_ncs ) sum_ncs = sum[priority level k] NCS(k)\n\nBigger numbers mean a larger nominal concurrency limit, at the expense of every other priority level.\n\nIf not specified, this field defaults to a value of 30.\n\nSetting this field to zero allows for the construction of a \"jail\" for this priority level that is used to hold some request(s)",
Description: "`nominalConcurrencyShares` (NCS) contributes to the computation of the NominalConcurrencyLimit (NominalCL) of this level. This is the number of execution seats available at this priority level. This is used both for requests dispatched from this priority level as well as requests dispatched from other priority levels borrowing seats from this level. The server's concurrency limit (ServerCL) is divided among the Limited priority levels in proportion to their NCS values:\n\nNominalCL(i) = ceil( ServerCL * NCS(i) / sum_ncs ) sum_ncs = sum[priority level k] NCS(k)\n\nBigger numbers mean a larger nominal concurrency limit, at the expense of every other priority level.\n\nIf not specified, this field defaults to a value of 30.\n\nSetting this field to zero supports the construction of a \"jail\" for this priority level that is used to hold some request(s)",
Type: []string{"integer"},
Format: "int32",
},

View File

@ -66,7 +66,7 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
}
if isZero && v == 0 {
obj.ObjectMeta.Annotations = map[string]string{}
obj.ObjectMeta.Annotations[flowcontrolv1beta3.PriorityLevelConcurrencyShareDefaultKey] = ""
obj.ObjectMeta.Annotations[flowcontrolv1beta3.PriorityLevelPreserveZeroConcurrencySharesKey] = ""
}
return obj
}
@ -272,7 +272,7 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
errExpected: nil,
},
{
name: "v1beta3, feature enabled, update, zero value, existing has non-zero, error expected",
name: "v1beta3, feature enabled, update, zero value, existing has non-zero, no error expected",
obj: v1beta3ObjFn(0, true),
old: internalObjFn(1),
zeroFeatureEnabled: true,

View File

@ -209,7 +209,7 @@ message LimitedPriorityLevelConfiguration {
//
// If not specified, this field defaults to a value of 30.
//
// Setting this field to zero allows for the construction of a
// Setting this field to zero supports the construction of a
// "jail" for this priority level that is used to hold some request(s)
//
// +optional

View File

@ -475,7 +475,7 @@ type LimitedPriorityLevelConfiguration struct {
//
// If not specified, this field defaults to a value of 30.
//
// Setting this field to zero allows for the construction of a
// Setting this field to zero supports the construction of a
// "jail" for this priority level that is used to hold some request(s)
//
// +optional

View File

@ -122,7 +122,7 @@ func (LimitResponse) SwaggerDoc() map[string]string {
var map_LimitedPriorityLevelConfiguration = map[string]string{
"": "LimitedPriorityLevelConfiguration specifies how to handle requests that are subject to limits. It addresses two issues:\n - How are requests for this priority level limited?\n - What should be done with requests that exceed the limit?",
"nominalConcurrencyShares": "`nominalConcurrencyShares` (NCS) contributes to the computation of the NominalConcurrencyLimit (NominalCL) of this level. This is the number of execution seats available at this priority level. This is used both for requests dispatched from this priority level as well as requests dispatched from other priority levels borrowing seats from this level. The server's concurrency limit (ServerCL) is divided among the Limited priority levels in proportion to their NCS values:\n\nNominalCL(i) = ceil( ServerCL * NCS(i) / sum_ncs ) sum_ncs = sum[priority level k] NCS(k)\n\nBigger numbers mean a larger nominal concurrency limit, at the expense of every other priority level.\n\nIf not specified, this field defaults to a value of 30.\n\nSetting this field to zero allows for the construction of a \"jail\" for this priority level that is used to hold some request(s)",
"nominalConcurrencyShares": "`nominalConcurrencyShares` (NCS) contributes to the computation of the NominalConcurrencyLimit (NominalCL) of this level. This is the number of execution seats available at this priority level. This is used both for requests dispatched from this priority level as well as requests dispatched from other priority levels borrowing seats from this level. The server's concurrency limit (ServerCL) is divided among the Limited priority levels in proportion to their NCS values:\n\nNominalCL(i) = ceil( ServerCL * NCS(i) / sum_ncs ) sum_ncs = sum[priority level k] NCS(k)\n\nBigger numbers mean a larger nominal concurrency limit, at the expense of every other priority level.\n\nIf not specified, this field defaults to a value of 30.\n\nSetting this field to zero supports the construction of a \"jail\" for this priority level that is used to hold some request(s)",
"limitResponse": "`limitResponse` indicates what to do with requests that can not be executed right now",
"lendablePercent": "`lendablePercent` prescribes the fraction of the level's NominalCL that can be borrowed by other priority levels. The value of this field must be between 0 and 100, inclusive, and it defaults to 0. The number of seats that other levels can borrow from this level, known as this level's LendableConcurrencyLimit (LendableCL), is defined as follows.\n\nLendableCL(i) = round( NominalCL(i) * lendablePercent(i)/100.0 )",
"borrowingLimitPercent": "`borrowingLimitPercent`, if present, configures a limit on how many seats this priority level can borrow from other priority levels. The limit is known as this level's BorrowingConcurrencyLimit (BorrowingCL) and is a limit on the total number of seats that this level may borrow at any one time. This field holds the ratio of that limit to the level's nominal concurrency limit. When this field is non-nil, it must hold a non-negative integer and the limit is calculated as follows.\n\nBorrowingCL(i) = round( NominalCL(i) * borrowingLimitPercent(i)/100.0 )\n\nThe value of this field can be more than 100, implying that this priority level can borrow a number of seats that is greater than its own nominal concurrency limit (NominalCL). When this field is left `nil`, the limit is effectively infinite.",

View File

@ -104,7 +104,7 @@ const (
)
const (
// This annotation applies to v1beta3 only.
// This annotation is only for use in v1beta3.
//
// The presence of this annotation in a v1beta3 object means that
// a zero value in the 'NominalConcurrencyShares' field means zero
@ -112,9 +112,9 @@ const (
//
// To set a zero value for the 'NominalConcurrencyShares' field in v1beta3,
// set the annotation to an empty string:
// "flowcontrol.k8s.io/concurrency-shares-defaults-to-zero": ""
// "flowcontrol.k8s.io/v1beta3-preserve-zero-concurrency-shares": ""
//
PriorityLevelConcurrencyShareDefaultKey = "flowcontrol.k8s.io/concurrency-shares-defaults-to-zero"
PriorityLevelPreserveZeroConcurrencySharesKey = "flowcontrol.k8s.io/v1beta3-preserve-zero-concurrency-shares"
)
// +genclient