add GetValueFromIntOrPercent helper funcs

pkg/controller/deployment/deployment_controller.go

@@ -38,6 +38,7 @@ import (
 	deploymentutil "k8s.io/kubernetes/pkg/util/deployment"
 	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 	"k8s.io/kubernetes/pkg/util/integer"
+	intstrutil "k8s.io/kubernetes/pkg/util/intstr"
 	labelsutil "k8s.io/kubernetes/pkg/util/labels"
 	podutil "k8s.io/kubernetes/pkg/util/pod"
 	utilruntime "k8s.io/kubernetes/pkg/util/runtime"
@@ -767,13 +768,11 @@ func (dc *DeploymentController) reconcileNewReplicaSet(allRSs []*extensions.Repl
 		return true, err
 	}
 	// Check if we can scale up.
-	maxSurge, isPercent, err := intstrutil.GetIntOrPercentValue(&deployment.Spec.Strategy.RollingUpdate.MaxSurge)
+	maxSurge, err := intstrutil.GetValueFromIntOrPercent(&deployment.Spec.Strategy.RollingUpdate.MaxSurge, deployment.Spec.Replicas)
 	if err != nil {
-		return false, fmt.Errorf("invalid value for MaxSurge: %v", err)
+		return false, err
-	}
-	if isPercent {
-		maxSurge = intstrutil.GetValueFromPercent(maxSurge, deployment.Spec.Replicas)
 	}
+
 	// Find the total number of pods
 	currentPodCount := deploymentutil.GetReplicaCountForReplicaSets(allRSs)
 	maxTotalPods := deployment.Spec.Replicas + maxSurge
@@ -815,12 +814,9 @@ func (dc *DeploymentController) reconcileOldReplicaSets(allRSs []*extensions.Rep
 		return false, fmt.Errorf("could not find available pods: %v", err)
 	}
 
-	maxUnavailable, isPercent, err := intstrutil.GetIntOrPercentValue(&deployment.Spec.Strategy.RollingUpdate.MaxUnavailable)
+	maxUnavailable, err := intstrutil.GetValueFromIntOrPercent(&deployment.Spec.Strategy.RollingUpdate.MaxUnavailable, deployment.Spec.Replicas)
 	if err != nil {
-		return false, fmt.Errorf("invalid value for MaxUnavailable: %v", err)
+		return false, err
-	}
-	if isPercent {
-		maxUnavailable = intstrutil.GetValueFromPercent(maxUnavailable, deployment.Spec.Replicas)
 	}
 
 	// Check if we can scale down. We can scale down in the following 2 cases:
@@ -919,13 +915,11 @@ func (dc *DeploymentController) cleanupUnhealthyReplicas(oldRSs []*extensions.Re
 // scaleDownOldReplicaSetsForRollingUpdate scales down old replica sets when deployment strategy is "RollingUpdate".
 // Need check maxUnavailable to ensure availability
 func (dc *DeploymentController) scaleDownOldReplicaSetsForRollingUpdate(allRSs []*extensions.ReplicaSet, oldRSs []*extensions.ReplicaSet, deployment extensions.Deployment) (int, error) {
-	maxUnavailable, isPercent, err := intstrutil.GetIntOrPercentValue(&deployment.Spec.Strategy.RollingUpdate.MaxUnavailable)
+	maxUnavailable, err := intstrutil.GetValueFromIntOrPercent(&deployment.Spec.Strategy.RollingUpdate.MaxUnavailable, deployment.Spec.Replicas)
 	if err != nil {
-		return 0, fmt.Errorf("invalid value for MaxUnavailable: %v", err)
+		return 0, err
-	}
-	if isPercent {
-		maxUnavailable = intstrutil.GetValueFromPercent(maxUnavailable, deployment.Spec.Replicas)
 	}
+
 	// Check if we can scale down.
 	minAvailable := deployment.Spec.Replicas - maxUnavailable
 	minReadySeconds := deployment.Spec.MinReadySeconds

pkg/util/intstr/intstr.go

@@ -116,7 +116,18 @@ func (intstr *IntOrString) Fuzz(c fuzz.Continue) {
 	}
 }
 
-func GetIntOrPercentValue(intOrStr *IntOrString) (int, bool, error) {
+func GetValueFromIntOrPercent(intOrPercent *IntOrString, total int) (int, error) {
+	value, isPercent, err := getIntOrPercentValue(intOrPercent)
+	if err != nil {
+		return 0, fmt.Errorf("invalid value for IntOrString: %v", err)
+	}
+	if isPercent {
+		value = int(math.Ceil(float64(value) * (float64(total)) / 100))
+	}
+	return value, nil
+}
+
+func getIntOrPercentValue(intOrStr *IntOrString) (int, bool, error) {
 	switch intOrStr.Type {
 	case Int:
 		return intOrStr.IntValue(), false, nil
@@ -130,7 +141,3 @@ func GetIntOrPercentValue(intOrStr *IntOrString) (int, bool, error) {
 	}
 	return 0, false, fmt.Errorf("invalid value: neither int nor percentage")
 }
-
-func GetValueFromPercent(percent int, value int) int {
-	return int(math.Ceil(float64(percent) * (float64(value)) / 100))
-}

pkg/util/intstr/intstr_test.go

@@ -109,3 +109,52 @@ func TestIntOrStringMarshalJSONUnmarshalYAML(t *testing.T) {
 		}
 	}
 }
+
+func TestGetValueFromIntOrPercent(t *testing.T) {
+	tests := []struct {
+		input     IntOrString
+		total     int
+		expectErr bool
+		expectVal int
+	}{
+		{
+			input:     FromInt(123),
+			expectErr: false,
+			expectVal: 123,
+		},
+		{
+			input:     FromString("90%"),
+			total:     100,
+			expectErr: false,
+			expectVal: 90,
+		},
+		{
+			input:     FromString("%"),
+			expectErr: true,
+		},
+		{
+			input:     FromString("90#"),
+			expectErr: true,
+		},
+		{
+			input:     FromString("#%"),
+			expectErr: true,
+		},
+	}
+
+	for i, test := range tests {
+		t.Logf("test case %d", i)
+		value, err := GetValueFromIntOrPercent(&test.input, test.total)
+		if test.expectErr && err == nil {
+			t.Errorf("expected error, but got none")
+			continue
+		}
+		if !test.expectErr && err != nil {
+			t.Errorf("unexpected err: %v", err)
+			continue
+		}
+		if test.expectVal != value {
+			t.Errorf("expected %v, but got %v", test.expectErr, value)
+		}
+	}
+}