github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-22 03:11:40 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #89482 from renatoviana12/master

Browse Source 
fixed percentage behaviour in instr
This commit is contained in:
Kubernetes Prow Robot 2020-10-05 20:00:19 -07:00 committed by GitHub
commit 1a66eb7b8a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 143 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/apis/apps/v1/defaults_test.go
View File

@ -465,7 +465,7 @@ func TestSetDefaultDeployment(t *testing.T) {
func TestDefaultDeploymentAvailability(t *testing.T) {
d := roundTrip(t, runtime.Object(&appsv1.Deployment{})).(*appsv1.Deployment)
maxUnavailable, err := intstr.GetValueFromIntOrPercent(d.Spec.Strategy.RollingUpdate.MaxUnavailable, int(*(d.Spec.Replicas)), false)
maxUnavailable, err := intstr.GetScaledValueFromIntOrPercent(d.Spec.Strategy.RollingUpdate.MaxUnavailable, int(*(d.Spec.Replicas)), false)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

2
pkg/apis/apps/v1beta1/defaults_test.go
View File

@ -187,7 +187,7 @@ func TestSetDefaultDeployment(t *testing.T) {
func TestDefaultDeploymentAvailability(t *testing.T) {
d := roundTrip(t, runtime.Object(&appsv1beta1.Deployment{})).(*appsv1beta1.Deployment)
maxUnavailable, err := intstr.GetValueFromIntOrPercent(d.Spec.Strategy.RollingUpdate.MaxUnavailable, int(*(d.Spec.Replicas)), false)
maxUnavailable, err := intstr.GetScaledValueFromIntOrPercent(d.Spec.Strategy.RollingUpdate.MaxUnavailable, int(*(d.Spec.Replicas)), false)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

2
pkg/apis/apps/v1beta2/defaults_test.go
View File

@ -435,7 +435,7 @@ func TestSetDefaultDeployment(t *testing.T) {
func TestDefaultDeploymentAvailability(t *testing.T) {
d := roundTrip(t, runtime.Object(&appsv1beta2.Deployment{})).(*appsv1beta2.Deployment)
maxUnavailable, err := intstr.GetValueFromIntOrPercent(d.Spec.Strategy.RollingUpdate.MaxUnavailable, int(*(d.Spec.Replicas)), false)
maxUnavailable, err := intstr.GetScaledValueFromIntOrPercent(d.Spec.Strategy.RollingUpdate.MaxUnavailable, int(*(d.Spec.Replicas)), false)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

2
pkg/controller/daemon/update.go
View File

@ -415,7 +415,7 @@ func (dsc *DaemonSetsController) getUnavailableNumbers(ds *apps.DaemonSet, nodeL
numUnavailable++
}
}
maxUnavailable, err := intstrutil.GetValueFromIntOrPercent(ds.Spec.UpdateStrategy.RollingUpdate.MaxUnavailable, desiredNumberScheduled, true)
maxUnavailable, err := intstrutil.GetScaledValueFromIntOrPercent(ds.Spec.UpdateStrategy.RollingUpdate.MaxUnavailable, desiredNumberScheduled, true)
if err != nil {
return -1, -1, fmt.Errorf("invalid value for MaxUnavailable: %v", err)
}

6
pkg/controller/deployment/util/deployment_util.go
View File

@ -817,7 +817,7 @@ func NewRSNewReplicas(deployment *apps.Deployment, allRSs []*apps.ReplicaSet, ne
switch deployment.Spec.Strategy.Type {
case apps.RollingUpdateDeploymentStrategyType:
// Check if we can scale up.
maxSurge, err := intstrutil.GetValueFromIntOrPercent(deployment.Spec.Strategy.RollingUpdate.MaxSurge, int(*(deployment.Spec.Replicas)), true)
maxSurge, err := intstrutil.GetScaledValueFromIntOrPercent(deployment.Spec.Strategy.RollingUpdate.MaxSurge, int(*(deployment.Spec.Replicas)), true)
if err != nil {
return 0, err
}
@ -881,11 +881,11 @@ func WaitForObservedDeployment(getDeploymentFunc func() (*apps.Deployment, error
// 2 desired, max unavailable 0%, surge 1% - should scale new(+1), then old(-1), then new(+1), then old(-1)
// 1 desired, max unavailable 0%, surge 1% - should scale new(+1), then old(-1)
func ResolveFenceposts(maxSurge, maxUnavailable *intstrutil.IntOrString, desired int32) (int32, int32, error) {
surge, err := intstrutil.GetValueFromIntOrPercent(intstrutil.ValueOrDefault(maxSurge, intstrutil.FromInt(0)), int(desired), true)
surge, err := intstrutil.GetScaledValueFromIntOrPercent(intstrutil.ValueOrDefault(maxSurge, intstrutil.FromInt(0)), int(desired), true)
if err != nil {
return 0, 0, err
}
unavailable, err := intstrutil.GetValueFromIntOrPercent(intstrutil.ValueOrDefault(maxUnavailable, intstrutil.FromInt(0)), int(desired), false)
unavailable, err := intstrutil.GetScaledValueFromIntOrPercent(intstrutil.ValueOrDefault(maxUnavailable, intstrutil.FromInt(0)), int(desired), false)
if err != nil {
return 0, 0, err
}

4
pkg/controller/disruption/disruption.go
View File

@ -599,7 +599,7 @@ func (dc *DisruptionController) getExpectedPodCount(pdb *policy.PodDisruptionBud
return
}
var maxUnavailable int
maxUnavailable, err = intstr.GetValueFromIntOrPercent(pdb.Spec.MaxUnavailable, int(expectedCount), true)
maxUnavailable, err = intstr.GetScaledValueFromIntOrPercent(pdb.Spec.MaxUnavailable, int(expectedCount), true)
if err != nil {
return
}
@ -618,7 +618,7 @@ func (dc *DisruptionController) getExpectedPodCount(pdb *policy.PodDisruptionBud
}
var minAvailable int
minAvailable, err = intstr.GetValueFromIntOrPercent(pdb.Spec.MinAvailable, int(expectedCount), true)
minAvailable, err = intstr.GetScaledValueFromIntOrPercent(pdb.Spec.MinAvailable, int(expectedCount), true)
if err != nil {
return
}

51
staging/src/k8s.io/apimachinery/pkg/util/intstr/intstr.go
View File

@ -135,6 +135,33 @@ func ValueOrDefault(intOrPercent *IntOrString, defaultValue IntOrString) *IntOrS
return intOrPercent
}
// GetScaledValueFromIntOrPercent is meant to replace GetValueFromIntOrPercent.
// This method returns a scaled value from an IntOrString type. If the IntOrString
// is a percentage string value it's treated as a percentage and scaled appropriately
// in accordance to the total, if it's an int value it's treated as a a simple value and
// if it is a string value which is either non-numeric or numeric but lacking a trailing '%' it returns an error.
func GetScaledValueFromIntOrPercent(intOrPercent *IntOrString, total int, roundUp bool) (int, error) {
if intOrPercent == nil {
return 0, errors.New("nil value for IntOrString")
}
value, isPercent, err := getIntOrPercentValueSafely(intOrPercent)
if err != nil {
return 0, fmt.Errorf("invalid value for IntOrString: %v", err)
}
if isPercent {
if roundUp {
value = int(math.Ceil(float64(value) * (float64(total)) / 100))
} else {
value = int(math.Floor(float64(value) * (float64(total)) / 100))
}
}
return value, nil
}
// GetValueFromIntOrPercent was deprecated in favor of
// GetScaledValueFromIntOrPercent. This method was treating all int as a numeric value and all
// strings with or without a percent symbol as a percentage value.
// Deprecated
func GetValueFromIntOrPercent(intOrPercent *IntOrString, total int, roundUp bool) (int, error) {
if intOrPercent == nil {
return 0, errors.New("nil value for IntOrString")
@ -153,6 +180,8 @@ func GetValueFromIntOrPercent(intOrPercent *IntOrString, total int, roundUp bool
return value, nil
}
// getIntOrPercentValue is a legacy function and only meant to be called by GetValueFromIntOrPercent
// For a more correct implementation call getIntOrPercentSafely
func getIntOrPercentValue(intOrStr *IntOrString) (int, bool, error) {
switch intOrStr.Type {
case Int:
@ -167,3 +196,25 @@ func getIntOrPercentValue(intOrStr *IntOrString) (int, bool, error) {
}
return 0, false, fmt.Errorf("invalid type: neither int nor percentage")
}
func getIntOrPercentValueSafely(intOrStr *IntOrString) (int, bool, error) {
switch intOrStr.Type {
case Int:
return intOrStr.IntValue(), false, nil
case String:
isPercent := false
s := intOrStr.StrVal
if strings.HasSuffix(s, "%") {
isPercent = true
s = strings.TrimSuffix(intOrStr.StrVal, "%")
} else {
return 0, false, fmt.Errorf("invalid type: string is not a percentage")
}
v, err := strconv.Atoi(s)
if err != nil {
return 0, false, fmt.Errorf("invalid value %q: %v", intOrStr.StrVal, err)
}
return int(v), isPercent, nil
}
return 0, false, fmt.Errorf("invalid type: neither int nor percentage")
}

82
staging/src/k8s.io/apimachinery/pkg/util/intstr/intstr_test.go
View File

@ -110,7 +110,79 @@ func TestIntOrStringMarshalJSONUnmarshalYAML(t *testing.T) {
}
}
func TestGetValueFromIntOrPercent(t *testing.T) {
func TestGetIntFromIntOrString(t *testing.T) {
tests := []struct {
input IntOrString
expectErr bool
expectVal int
expectPerc bool
}{
{
input: FromInt(200),
expectErr: false,
expectVal: 200,
expectPerc: false,
},
{
input: FromString("200"),
expectErr: true,
expectPerc: false,
},
{
input: FromString("30%0"),
expectErr: true,
expectPerc: false,
},
{
input: FromString("40%"),
expectErr: false,
expectVal: 40,
expectPerc: true,
},
{
input: FromString("%"),
expectErr: true,
expectPerc: false,
},
{
input: FromString("a%"),
expectErr: true,
expectPerc: false,
},
{
input: FromString("a"),
expectErr: true,
expectPerc: false,
},
{
input: FromString("40#"),
expectErr: true,
expectPerc: false,
},
{
input: FromString("40%%"),
expectErr: true,
expectPerc: false,
},
}
for _, test := range tests {
t.Run("", func(t *testing.T) {
value, isPercent, err := getIntOrPercentValueSafely(&test.input)
if test.expectVal != value {
t.Fatalf("expected value does not match, expected: %d, got: %d", test.expectVal, value)
}
if test.expectPerc != isPercent {
t.Fatalf("expected percent does not match, expected: %t, got: %t", test.expectPerc, isPercent)
}
if test.expectErr != (err != nil) {
t.Fatalf("expected error does not match, expected error: %v, got: %v", test.expectErr, err)
}
})
}
}
func TestGetIntFromIntOrPercent(t *testing.T) {
tests := []struct {
input IntOrString
total int
@ -156,11 +228,15 @@ func TestGetValueFromIntOrPercent(t *testing.T) {
input: FromString("#%"),
expectErr: true,
},
{
input: FromString("90"),
expectErr: true,
},
}
for i, test := range tests {
t.Logf("test case %d", i)
value, err := GetValueFromIntOrPercent(&test.input, test.total, test.roundUp)
value, err := GetScaledValueFromIntOrPercent(&test.input, test.total, test.roundUp)
if test.expectErr && err == nil {
t.Errorf("expected error, but got none")
continue
@ -176,7 +252,7 @@ func TestGetValueFromIntOrPercent(t *testing.T) {
}
func TestGetValueFromIntOrPercentNil(t *testing.T) {
_, err := GetValueFromIntOrPercent(nil, 0, false)
_, err := GetScaledValueFromIntOrPercent(nil, 0, false)
if err == nil {
t.Errorf("expected error got none")
}

4
staging/src/k8s.io/kubectl/pkg/util/deployment/deployment.go
View File

@ -208,11 +208,11 @@ func findOldReplicaSets(deployment *appsv1.Deployment, rsList []*appsv1.ReplicaS
// 2 desired, max unavailable 0%, surge 1% - should scale new(+1), then old(-1), then new(+1), then old(-1)
// 1 desired, max unavailable 0%, surge 1% - should scale new(+1), then old(-1)
func ResolveFenceposts(maxSurge, maxUnavailable *intstrutil.IntOrString, desired int32) (int32, int32, error) {
surge, err := intstrutil.GetValueFromIntOrPercent(intstrutil.ValueOrDefault(maxSurge, intstrutil.FromInt(0)), int(desired), true)
surge, err := intstrutil.GetScaledValueFromIntOrPercent(intstrutil.ValueOrDefault(maxSurge, intstrutil.FromInt(0)), int(desired), true)
if err != nil {
return 0, 0, err
}
unavailable, err := intstrutil.GetValueFromIntOrPercent(intstrutil.ValueOrDefault(maxUnavailable, intstrutil.FromInt(0)), int(desired), false)
unavailable, err := intstrutil.GetScaledValueFromIntOrPercent(intstrutil.ValueOrDefault(maxUnavailable, intstrutil.FromInt(0)), int(desired), false)
if err != nil {
return 0, 0, err
}

4
test/e2e/apps/deployment.go
View File

@ -755,7 +755,7 @@ func testProportionalScalingDeployment(f *framework.Framework) {
framework.ExpectNoError(err)
// Checking state of first rollout's replicaset.
maxUnavailable, err := intstr.GetValueFromIntOrPercent(deployment.Spec.Strategy.RollingUpdate.MaxUnavailable, int(*(deployment.Spec.Replicas)), false)
maxUnavailable, err := intstr.GetScaledValueFromIntOrPercent(deployment.Spec.Strategy.RollingUpdate.MaxUnavailable, int(*(deployment.Spec.Replicas)), false)
framework.ExpectNoError(err)
// First rollout's replicaset should have Deployment's (replicas - maxUnavailable) = 10 - 2 = 8 available replicas.
@ -780,7 +780,7 @@ func testProportionalScalingDeployment(f *framework.Framework) {
secondRS, err := deploymentutil.GetNewReplicaSet(deployment, c.AppsV1())
framework.ExpectNoError(err)
maxSurge, err := intstr.GetValueFromIntOrPercent(deployment.Spec.Strategy.RollingUpdate.MaxSurge, int(*(deployment.Spec.Replicas)), false)
maxSurge, err := intstr.GetScaledValueFromIntOrPercent(deployment.Spec.Strategy.RollingUpdate.MaxSurge, int(*(deployment.Spec.Replicas)), false)
framework.ExpectNoError(err)
// Second rollout's replicaset should have 0 available replicas.