From d96cdb93c40a979d810ed2b9efe98234080a927d Mon Sep 17 00:00:00 2001 From: mqliang Date: Thu, 11 Feb 2016 14:19:31 +0800 Subject: [PATCH 1/2] move helper funcs to util/deployment.go from util.go --- .../deployment/deployment_controller.go | 13 +++++----- pkg/util/intstr/intstr.go | 21 ++++++++++++++++ pkg/util/util.go | 24 ------------------- 3 files changed, 27 insertions(+), 31 deletions(-) diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index 7b3d2eab03b..0a2e1063652 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -35,7 +35,6 @@ 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" @@ -768,12 +767,12 @@ 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, isPercent, err := intstrutil.GetIntOrPercentValue(&deployment.Spec.Strategy.RollingUpdate.MaxSurge) if err != nil { return false, fmt.Errorf("invalid value for MaxSurge: %v", err) } if isPercent { - maxSurge = util.GetValueFromPercent(maxSurge, deployment.Spec.Replicas) + maxSurge = intstrutil.GetValueFromPercent(maxSurge, deployment.Spec.Replicas) } // Find the total number of pods currentPodCount := deploymentutil.GetReplicaCountForReplicaSets(allRSs) @@ -816,12 +815,12 @@ 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, isPercent, err := intstrutil.GetIntOrPercentValue(&deployment.Spec.Strategy.RollingUpdate.MaxUnavailable) if err != nil { return false, fmt.Errorf("invalid value for MaxUnavailable: %v", err) } if isPercent { - maxUnavailable = util.GetValueFromPercent(maxUnavailable, deployment.Spec.Replicas) + maxUnavailable = intstrutil.GetValueFromPercent(maxUnavailable, deployment.Spec.Replicas) } // Check if we can scale down. We can scale down in the following 2 cases: @@ -920,12 +919,12 @@ 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, isPercent, err := intstrutil.GetIntOrPercentValue(&deployment.Spec.Strategy.RollingUpdate.MaxUnavailable) if err != nil { return 0, fmt.Errorf("invalid value for MaxUnavailable: %v", err) } if isPercent { - maxUnavailable = util.GetValueFromPercent(maxUnavailable, deployment.Spec.Replicas) + maxUnavailable = intstrutil.GetValueFromPercent(maxUnavailable, deployment.Spec.Replicas) } // Check if we can scale down. minAvailable := deployment.Spec.Replicas - maxUnavailable diff --git a/pkg/util/intstr/intstr.go b/pkg/util/intstr/intstr.go index 033d1d1724e..5d5ac58d116 100644 --- a/pkg/util/intstr/intstr.go +++ b/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,22 @@ func (intstr *IntOrString) Fuzz(c fuzz.Continue) { c.Fuzz(&intstr.StrVal) } } + +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") +} + +func GetValueFromPercent(percent int, value int) int { + return int(math.Ceil(float64(percent) * (float64(value)) / 100)) +} diff --git a/pkg/util/util.go b/pkg/util/util.go index 63da8ef5e76..97303b76ff0 100644 --- a/pkg/util/util.go +++ b/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{} From fcf0d6d720fec74d3a4749785b2b0708365efd1d Mon Sep 17 00:00:00 2001 From: mqliang Date: Thu, 11 Feb 2016 14:35:46 +0800 Subject: [PATCH 2/2] add GetValueFromIntOrPercent helper funcs --- .../deployment/deployment_controller.go | 24 ++++----- pkg/util/intstr/intstr.go | 17 +++++-- pkg/util/intstr/intstr_test.go | 49 +++++++++++++++++++ 3 files changed, 70 insertions(+), 20 deletions(-) diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index 0a2e1063652..8a40a79a67f 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/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) - } - if isPercent { - maxSurge = intstrutil.GetValueFromPercent(maxSurge, deployment.Spec.Replicas) + return false, err } + // 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) - } - if isPercent { - maxUnavailable = intstrutil.GetValueFromPercent(maxUnavailable, deployment.Spec.Replicas) + return false, err } // 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) - } - if isPercent { - maxUnavailable = intstrutil.GetValueFromPercent(maxUnavailable, deployment.Spec.Replicas) + return 0, err } + // Check if we can scale down. minAvailable := deployment.Spec.Replicas - maxUnavailable minReadySeconds := deployment.Spec.MinReadySeconds diff --git a/pkg/util/intstr/intstr.go b/pkg/util/intstr/intstr.go index 5d5ac58d116..05b572cf5a1 100644 --- a/pkg/util/intstr/intstr.go +++ b/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)) -} diff --git a/pkg/util/intstr/intstr_test.go b/pkg/util/intstr/intstr_test.go index 27939e9f157..fb36f9d6634 100644 --- a/pkg/util/intstr/intstr_test.go +++ b/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) + } + } +}