Adds detail to initialScale validation error msgs (knative#12704)

* adds detail to initialScale validation error msgs Signed-off-by: Paul S. Schweigert <[email protected]> Originally, validateInitialScale returned an ErrInvalidValue for any invalid value. This PR adds more specific error messages for the cases when either a negative value is used or if initialScale=0 when not allowed by the cluster. * updating errors in other places Signed-off-by: Paul S. Schweigert <[email protected]>
rtnpro · Mar 8, 2022 · 25ce427 · 25ce427
1 parent afe46a9
commit 25ce427
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 5 deletions.
diff --git a/pkg/apis/autoscaling/annotation_validation.go b/pkg/apis/autoscaling/annotation_validation.go
@@ -244,8 +244,12 @@ func validateMetric(m map[string]string) *apis.FieldError {
 func validateInitialScale(config *autoscalerconfig.Config, m map[string]string) *apis.FieldError {
 	if k, v, ok := InitialScaleAnnotation.Get(m); ok {
 		initScaleInt, err := strconv.Atoi(v)
-		if err != nil || initScaleInt < 0 || (!config.AllowZeroInitialScale && initScaleInt == 0) {
+		if err != nil {
 			return apis.ErrInvalidValue(v, k)
+		} else if initScaleInt < 0 {
+			return apis.ErrInvalidValue(v, fmt.Sprintf("%s must be greater than 0", k))
+		} else if !config.AllowZeroInitialScale && initScaleInt == 0 {
+			return apis.ErrInvalidValue(v, fmt.Sprintf("%s=0 not allowed by cluster", k))
 		}
 	}
 	return nil

diff --git a/pkg/apis/autoscaling/annotation_validation_test.go b/pkg/apis/autoscaling/annotation_validation_test.go
@@ -338,7 +338,7 @@ func TestValidateAnnotations(t *testing.T) {
 	}, {
 		name:        "initial scale is zero but cluster doesn't allow",
 		annotations: map[string]string{InitialScaleAnnotationKey: "0"},
-		expectErr:   "invalid value: 0: autoscaling.knative.dev/initial-scale",
+		expectErr:   "invalid value: 0: " + InitialScaleAnnotationKey + "=0 not allowed by cluster",
 	}, {
 		name: "initial scale is zero and cluster allows",
 		configMutator: func(config *autoscalerconfig.Config) {
@@ -348,6 +348,10 @@ func TestValidateAnnotations(t *testing.T) {
 	}, {
 		name:        "initial scale is greater than 0",
 		annotations: map[string]string{InitialScaleAnnotationKey: "2"},
+	}, {
+		name:        "initial scale is less than 0",
+		annotations: map[string]string{InitialScaleAnnotationKey: "-1"},
+		expectErr:   "invalid value: -1: " + InitialScaleAnnotationKey + " must be greater than 0",
 	}, {
 		name:        "initial scale non-parseable",
 		annotations: map[string]string{InitialScaleAnnotationKey: "invalid"},

diff --git a/pkg/apis/serving/metadata_validation_test.go b/pkg/apis/serving/metadata_validation_test.go
@@ -170,7 +170,7 @@ func TestValidateObjectMetadata(t *testing.T) {
 				autoscaling.InitialScaleAnnotationKey: "-2",
 			},
 		},
-		expectErr: apis.ErrInvalidValue("-2", "annotations."+autoscaling.InitialScaleAnnotationKey),
+		expectErr: apis.ErrInvalidValue("-2", "annotations."+autoscaling.InitialScaleAnnotationKey+" must be greater than 0"),
 	}, {
 		name:             "cluster allows zero revision initial scale",
 		ctx:              config.ToContext(context.Background(), &config.Config{Autoscaler: &autoscalerconfig.Config{AllowZeroInitialScale: true}}),
@@ -190,7 +190,7 @@ func TestValidateObjectMetadata(t *testing.T) {
 				autoscaling.InitialScaleAnnotationKey: "0",
 			},
 		},
-		expectErr: apis.ErrInvalidValue("0", "annotations."+autoscaling.InitialScaleAnnotationKey),
+		expectErr: apis.ErrInvalidValue("0", "annotations."+autoscaling.InitialScaleAnnotationKey+"=0 not allowed by cluster"),
 	}, {
 		name:             "autoscaling annotations on a resource that doesn't allow them",
 		allowAutoscaling: false,

diff --git a/pkg/apis/serving/v1/revision_validation_test.go b/pkg/apis/serving/v1/revision_validation_test.go
@@ -946,7 +946,7 @@ func TestRevisionTemplateSpecValidation(t *testing.T) {
 		},
 		want: (&apis.FieldError{
 			Message: "invalid value: 0",
-			Paths:   []string{autoscaling.InitialScaleAnnotationKey},
+			Paths:   []string{fmt.Sprintf("%s=0 not allowed by cluster", autoscaling.InitialScaleAnnotationKey)},
 		}).ViaField("metadata.annotations"),
 	}, {
 		name: "Valid initial scale when cluster allows zero",