diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go index f2099d7c87953..42640c46a9f38 100644 --- a/pkg/controller/deployment/util/deployment_util.go +++ b/pkg/controller/deployment/util/deployment_util.go @@ -863,11 +863,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(maxSurge, int(desired), true) + surge, err := intstrutil.GetValueFromIntOrPercent(intstrutil.ValueOrDefault(maxSurge, intstrutil.FromInt(0)), int(desired), true) if err != nil { return 0, 0, err } - unavailable, err := intstrutil.GetValueFromIntOrPercent(maxUnavailable, int(desired), false) + unavailable, err := intstrutil.GetValueFromIntOrPercent(intstrutil.ValueOrDefault(maxUnavailable, intstrutil.FromInt(0)), int(desired), false) if err != nil { return 0, 0, err } diff --git a/pkg/controller/deployment/util/deployment_util_test.go b/pkg/controller/deployment/util/deployment_util_test.go index 5bf5b1486cb45..fd1b83abc58d0 100644 --- a/pkg/controller/deployment/util/deployment_util_test.go +++ b/pkg/controller/deployment/util/deployment_util_test.go @@ -618,52 +618,83 @@ func TestGetReplicaCountForReplicaSets(t *testing.T) { func TestResolveFenceposts(t *testing.T) { tests := []struct { - maxSurge string - maxUnavailable string + maxSurge *string + maxUnavailable *string desired int32 expectSurge int32 expectUnavailable int32 expectError bool }{ { - maxSurge: "0%", - maxUnavailable: "0%", + maxSurge: newString("0%"), + maxUnavailable: newString("0%"), desired: 0, expectSurge: 0, expectUnavailable: 1, expectError: false, }, { - maxSurge: "39%", - maxUnavailable: "39%", + maxSurge: newString("39%"), + maxUnavailable: newString("39%"), desired: 10, expectSurge: 4, expectUnavailable: 3, expectError: false, }, { - maxSurge: "oops", - maxUnavailable: "39%", + maxSurge: newString("oops"), + maxUnavailable: newString("39%"), desired: 10, expectSurge: 0, expectUnavailable: 0, expectError: true, }, { - maxSurge: "55%", - maxUnavailable: "urg", + maxSurge: newString("55%"), + maxUnavailable: newString("urg"), desired: 10, expectSurge: 0, expectUnavailable: 0, expectError: true, }, + { + maxSurge: nil, + maxUnavailable: newString("39%"), + desired: 10, + expectSurge: 0, + expectUnavailable: 3, + expectError: false, + }, + { + maxSurge: newString("39%"), + maxUnavailable: nil, + desired: 10, + expectSurge: 4, + expectUnavailable: 0, + expectError: false, + }, + { + maxSurge: nil, + maxUnavailable: nil, + desired: 10, + expectSurge: 0, + expectUnavailable: 1, + expectError: false, + }, } for num, test := range tests { - t.Run("maxSurge="+test.maxSurge, func(t *testing.T) { - maxSurge := intstr.FromString(test.maxSurge) - maxUnavail := intstr.FromString(test.maxUnavailable) - surge, unavail, err := ResolveFenceposts(&maxSurge, &maxUnavail, test.desired) + t.Run(fmt.Sprintf("%d", num), func(t *testing.T) { + var maxSurge, maxUnavail *intstr.IntOrString + if test.maxSurge != nil { + surge := intstr.FromString(*test.maxSurge) + maxSurge = &surge + } + if test.maxUnavailable != nil { + unavail := intstr.FromString(*test.maxUnavailable) + maxUnavail = &unavail + } + surge, unavail, err := ResolveFenceposts(maxSurge, maxUnavail, test.desired) if err != nil && !test.expectError { t.Errorf("unexpected error %v", err) } @@ -677,6 +708,10 @@ func TestResolveFenceposts(t *testing.T) { } } +func newString(s string) *string { + return &s +} + func TestNewRSNewReplicas(t *testing.T) { tests := []struct { Name string diff --git a/staging/src/k8s.io/apimachinery/pkg/util/intstr/intstr.go b/staging/src/k8s.io/apimachinery/pkg/util/intstr/intstr.go index 231498ca03246..642b83cec2173 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/intstr/intstr.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/intstr/intstr.go @@ -18,6 +18,7 @@ package intstr import ( "encoding/json" + "errors" "fmt" "math" "runtime/debug" @@ -142,7 +143,17 @@ func (intstr *IntOrString) Fuzz(c fuzz.Continue) { } } +func ValueOrDefault(intOrPercent *IntOrString, defaultValue IntOrString) *IntOrString { + if intOrPercent == nil { + return &defaultValue + } + return intOrPercent +} + func GetValueFromIntOrPercent(intOrPercent *IntOrString, total int, roundUp bool) (int, error) { + if intOrPercent == nil { + return 0, errors.New("nil value for IntOrString") + } value, isPercent, err := getIntOrPercentValue(intOrPercent) if err != nil { return 0, fmt.Errorf("invalid value for IntOrString: %v", err) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/intstr/intstr_test.go b/staging/src/k8s.io/apimachinery/pkg/util/intstr/intstr_test.go index 4faba46f8d09b..690fe2d5331bd 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/intstr/intstr_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/intstr/intstr_test.go @@ -174,3 +174,10 @@ func TestGetValueFromIntOrPercent(t *testing.T) { } } } + +func TestGetValueFromIntOrPercentNil(t *testing.T) { + _, err := GetValueFromIntOrPercent(nil, 0, false) + if err == nil { + t.Errorf("expected error got none") + } +}