diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go index 772c25de25b..7324d336770 100644 --- a/pkg/controller/deployment/util/deployment_util.go +++ b/pkg/controller/deployment/util/deployment_util.go @@ -378,24 +378,34 @@ func FindActiveOrLatest(newRS *apps.ReplicaSet, oldRSs []*apps.ReplicaSet) *apps // GetDesiredReplicasAnnotation returns the number of desired replicas func GetDesiredReplicasAnnotation(logger klog.Logger, rs *apps.ReplicaSet) (int32, bool) { - return getIntFromAnnotation(logger, rs, DesiredReplicasAnnotation) + return getNonNegativeInt32FromAnnotationVerbose(logger, rs, DesiredReplicasAnnotation) } func getMaxReplicasAnnotation(logger klog.Logger, rs *apps.ReplicaSet) (int32, bool) { - return getIntFromAnnotation(logger, rs, MaxReplicasAnnotation) + return getNonNegativeInt32FromAnnotationVerbose(logger, rs, MaxReplicasAnnotation) } -func getIntFromAnnotation(logger klog.Logger, rs *apps.ReplicaSet, annotationKey string) (int32, bool) { +func getNonNegativeInt32FromAnnotationVerbose(logger klog.Logger, rs *apps.ReplicaSet, annotationKey string) (int32, bool) { + value, ok, err := getNonNegativeInt32FromAnnotation(rs, annotationKey) + if err != nil { + logger.V(2).Info("Could not convert the value with annotation key for the replica set", "annotationValue", rs.Annotations[annotationKey], "annotationKey", annotationKey, "replicaSet", klog.KObj(rs)) + } + return value, ok +} + +func getNonNegativeInt32FromAnnotation(rs *apps.ReplicaSet, annotationKey string) (int32, bool, error) { annotationValue, ok := rs.Annotations[annotationKey] if !ok { - return int32(0), false + return int32(0), false, nil } - intValue, err := strconv.Atoi(annotationValue) + intValue, err := strconv.ParseUint(annotationValue, 10, 32) if err != nil { - logger.V(2).Info("Could not convert the value with annotation key for the replica set", "annotationValue", annotationValue, "annotationKey", annotationKey, "replicaSet", klog.KObj(rs)) - return int32(0), false + return int32(0), false, err } - return int32(intValue), true + if intValue > math.MaxInt32 { + return int32(0), false, fmt.Errorf("value %d is out of range (higher than %d)", intValue, math.MaxInt32) + } + return int32(intValue), true, nil } // SetReplicasAnnotations sets the desiredReplicas and maxReplicas into the annotations diff --git a/pkg/controller/deployment/util/deployment_util_test.go b/pkg/controller/deployment/util/deployment_util_test.go index b02311bd410..486c738f99d 100644 --- a/pkg/controller/deployment/util/deployment_util_test.go +++ b/pkg/controller/deployment/util/deployment_util_test.go @@ -23,6 +23,7 @@ import ( "reflect" "sort" "strconv" + "strings" "testing" "time" @@ -1020,6 +1021,87 @@ func TestMaxUnavailable(t *testing.T) { } } +func TestGetNonNegativeInt32FromAnnotation(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + expectedValue int32 + expectedValid bool + expectedErr string + }{ + { + name: "invalid empty", + }, + { + name: "invalid", + annotations: map[string]string{"test": "invalid", "foo": "2"}, + expectedErr: "invalid syntax", + }, + { + name: "invalid negative ", + annotations: map[string]string{"test": "-1", "foo": "2"}, + expectedErr: "invalid syntax", + }, + { + name: "valid zero", + annotations: map[string]string{"test": "0", "foo": "2"}, + expectedValue: 0, + expectedValid: true, + }, + { + name: "valid", + annotations: map[string]string{"test": "13", "foo": "2"}, + expectedValue: 13, + expectedValid: true, + }, + { + name: "valid max", + annotations: map[string]string{"test": fmt.Sprintf("%d", math.MaxInt32), "foo": "2"}, + expectedValue: math.MaxInt32, + expectedValid: true, + }, + { + name: "invalid max out of range", + annotations: map[string]string{"test": fmt.Sprintf("%d", uint32(math.MaxInt32)+1), "foo": "2"}, + expectedErr: "out of range", + }, + { + name: "invalid max out of range 2", + annotations: map[string]string{"test": fmt.Sprintf("%d", uint64(math.MaxUint32)+1), "foo": "2"}, + expectedErr: "out of range", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + tDeployment := generateDeployment("nginx") + tRS := generateRS(tDeployment) + tRS.Annotations = test.annotations + value, valid, err := getNonNegativeInt32FromAnnotation(&tRS, "test") + if test.expectedValue != value { + t.Fatalf("expected value:%v, got:%v", test.expectedValue, value) + } + if test.expectedValid != valid { + t.Fatalf("expected valid:%v, got:%v", test.expectedValid, valid) + } + if err != nil && !strings.Contains(err.Error(), test.expectedErr) { + t.Fatalf("unexpected error, expected: %s, got %v", test.expectedErr, err) + } + if err == nil && len(test.expectedErr) != 0 { + t.Fatalf("didn't return error expected %s", test.expectedErr) + } + logger, _ := ktesting.NewTestContext(t) + value, valid = getNonNegativeInt32FromAnnotationVerbose(logger, &tRS, "test") + if test.expectedValue != value { + t.Fatalf("expected value:%v, got:%v", test.expectedValue, value) + } + if test.expectedValid != valid { + t.Fatalf("expected valid:%v, got:%v", test.expectedValid, valid) + } + }) + } +} + // Set of simple tests for annotation related util functions func TestAnnotationUtils(t *testing.T) {