Merge pull request #21044 from mqliang/deployment-util

Auto commit by PR queue bot

pkg/controller/deployment/deployment_controller.go

@@ -35,10 +35,10 @@ import (
 	"k8s.io/kubernetes/pkg/controller/framework"
 	"k8s.io/kubernetes/pkg/kubectl"
 	"k8s.io/kubernetes/pkg/runtime"
-	"k8s.io/kubernetes/pkg/util"
 	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"
@@ -768,13 +768,11 @@ func (dc *DeploymentController) reconcileNewReplicaSet(allRSs []*extensions.Repl
 		return true, err
 	}
 	// Check if we can scale up.
-	maxSurge, isPercent, err := util.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 = util.GetValueFromPercent(maxSurge, deployment.Spec.Replicas)
 	}
+
 	// Find the total number of pods
 	currentPodCount := deploymentutil.GetReplicaCountForReplicaSets(allRSs)
 	maxTotalPods := deployment.Spec.Replicas + maxSurge
@@ -816,12 +814,9 @@ func (dc *DeploymentController) reconcileOldReplicaSets(allRSs []*extensions.Rep
 		return false, fmt.Errorf("could not find available pods: %v", err)
 	}
 
-	maxUnavailable, isPercent, err := util.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 = util.GetValueFromPercent(maxUnavailable, deployment.Spec.Replicas)
 	}
 
 	// Check if we can scale down. We can scale down in the following 2 cases:
@@ -920,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 := util.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 = util.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

@@ -19,7 +19,9 @@ package intstr
 import (
 	"encoding/json"
 	"fmt"
+	"math"
 	"strconv"
+	"strings"
 
 	"github.com/google/gofuzz"
 )
@@ -113,3 +115,29 @@ func (intstr *IntOrString) Fuzz(c fuzz.Continue) {
 		c.Fuzz(&intstr.StrVal)
 	}
 }
+
+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
+	case String:
+		s := strings.Replace(intOrStr.StrVal, "%", "", -1)
+		v, err := strconv.Atoi(s)
+		if err != nil {
+			return 0, false, fmt.Errorf("invalid value %q: %v", intOrStr.StrVal, err)
+		}
+		return int(v), true, nil
+	}
+	return 0, false, fmt.Errorf("invalid value: neither int nor percentage")
+}

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)
+		}
+	}
+}

pkg/util/util.go

@@ -18,35 +18,11 @@ package util
 
 import (
 	"fmt"
-	"math"
 	"os"
 	"reflect"
 	"regexp"
-	"strconv"
-	"strings"
-
-	"k8s.io/kubernetes/pkg/util/intstr"
 )
 
-func GetIntOrPercentValue(intOrStr *intstr.IntOrString) (int, bool, error) {
-	switch intOrStr.Type {
-	case intstr.Int:
-		return intOrStr.IntValue(), false, nil
-	case intstr.String:
-		s := strings.Replace(intOrStr.StrVal, "%", "", -1)
-		v, err := strconv.Atoi(s)
-		if err != nil {
-			return 0, false, fmt.Errorf("invalid value %q: %v", intOrStr.StrVal, err)
-		}
-		return int(v), true, nil
-	}
-	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))
-}
-
 // Takes a list of strings and compiles them into a list of regular expressions
 func CompileRegexps(regexpStrings []string) ([]*regexp.Regexp, error) {
 	regexps := []*regexp.Regexp{}