Merge pull request #67493 from soltysh/nil_int_percent

Automatic merge from submit-queue (batch tested with PRs 67493, 67617, 67582, 67337). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Tolerate nil input in GetValueFromIntOrPercent

**What this PR does / why we need it**:
`GetValueFromIntOrPercent` accepts pointer argument but does not validate it. This PR fixes that problem preventing from panics.

/assign @deads2k @sttts 

**Release note**:
```release-note
NONE
```

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
 	}

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

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)

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