From fcf0d6d720fec74d3a4749785b2b0708365efd1d Mon Sep 17 00:00:00 2001 From: mqliang Date: Thu, 11 Feb 2016 14:35:46 +0800 Subject: [PATCH] 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) + } + } +}