Merge pull request #11090 from kargakis/remove-updatePercent

Merged by openshift-bot

UPGRADE.md

@@ -25,10 +25,10 @@ when that change will happen.
 Origin 1.y / OSE 3.y, support for `/ns/namespace-name/subjectaccessreview` will be removed.
 At that time, the openshift docker registry image must be upgraded in order to continue functioning.
 
-1. The `deploymentConfig.rollingParams.updatePercent` field is deprecated in
-  favor of `deploymentConfig.rollingParams.maxUnavailable` and
-  `deploymentConfig.rollingParams.maxSurge`. The `updatePercent` field will be
-  removed  in Origin 1.1 (OSE 3.1).
+1. The `deploymentConfig.spec.strategy.rollingParams.updatePercent` field is deprecated in
+  favor of `deploymentConfig.spec.strategy.rollingParams.maxUnavailable` and
+  `deploymentConfig.spec.strategy.rollingParams.maxSurge`. The `updatePercent` field is
+  removed in Origin 1.4 (OSE 3.4).
 
 1. The `volume.metadata` field is deprecated as of Origin 1.0.6 in favor of `volume.downwardAPI`.
 
@@ -95,4 +95,11 @@ references:
 ## Origin 1.3.x / OSE 3.3.x
 
 1.  `oc tag -d` now matches `oc delete imagestreamtag` behavior instead of removing the spec tag and leaving the status tag.
-    The old behavior can be achieved using `oc edit` or if you just want to disabling scheduled imports, `oc tag --scheduled=false`
+    The old behavior can be achieved using `oc edit` or if you just want to disabling scheduled imports, `oc tag --scheduled=false`
+
+
+## Origin 1.4.x / OSE 3.4.x
+
+1. The `deploymentConfig.spec.strategy.rollingParams.updatePercent` field is removed in
+  favor of `deploymentConfig.spec.strategy.rollingParams.maxUnavailable` and
+  `deploymentConfig.spec.strategy.rollingParams.maxSurge`.

api/swagger-spec/oapi-v1.json

@@ -22936,11 +22936,6 @@
       "type": "string",
       "description": "MaxSurge is the maximum number of pods that can be scheduled above the original number of pods. Value can be an absolute number (ex: 5) or a percentage of total pods at the start of the update (ex: 10%). Absolute number is calculated from percentage by rounding up.\n\nThis cannot be 0 if MaxUnavailable is 0. By default, 25% is used.\n\nExample: when this is set to 30%, the new RC can be scaled up by 30% immediately when the rolling update starts. Once old pods have been killed, new RC can be scaled up further, ensuring that total number of pods running at any time during the update is atmost 130% of original pods."
      },
-     "updatePercent": {
-      "type": "integer",
-      "format": "int32",
-      "description": "UpdatePercent is the percentage of replicas to scale up or down each interval. If nil, one replica will be scaled up and down each interval. If negative, the scale order will be down/up instead of up/down. DEPRECATED: Use MaxUnavailable/MaxSurge instead."
-     },
      "pre": {
       "$ref": "v1.LifecycleHook",
       "description": "Pre is a lifecycle hook which is executed before the deployment process begins. All LifecycleHookFailurePolicy values are supported."

pkg/cmd/admin/router/router.go

@@ -724,7 +724,6 @@ func RunCmdRouter(f *clientcmd.Factory, cmd *cobra.Command, out io.Writer, cfg *
 			},
 		)
 	}
-	updatePercent := int32(-25)
 	objects = append(objects, &deployapi.DeploymentConfig{
 		ObjectMeta: kapi.ObjectMeta{
 			Name:   name,
@@ -733,7 +732,7 @@ func RunCmdRouter(f *clientcmd.Factory, cmd *cobra.Command, out io.Writer, cfg *
 		Spec: deployapi.DeploymentConfigSpec{
 			Strategy: deployapi.DeploymentStrategy{
 				Type:          deployapi.DeploymentStrategyTypeRolling,
-				RollingParams: &deployapi.RollingDeploymentStrategyParams{UpdatePercent: &updatePercent},
+				RollingParams: &deployapi.RollingDeploymentStrategyParams{MaxUnavailable: intstr.FromString("25%")},
 			},
 			Replicas: cfg.Replicas,
 			Selector: label,

pkg/deploy/api/types.go

@@ -168,11 +168,6 @@ type RollingDeploymentStrategyParams struct {
 	// new RC can be scaled up further, ensuring that total number of pods running
 	// at any time during the update is atmost 130% of original pods.
 	MaxSurge intstr.IntOrString
-	// UpdatePercent is the percentage of replicas to scale up or down each
-	// interval. If nil, one replica will be scaled up and down each interval.
-	// If negative, the scale order will be down/up instead of up/down.
-	// DEPRECATED: Use MaxUnavailable/MaxSurge instead.
-	UpdatePercent *int32
 	// Pre is a lifecycle hook which is executed before the deployment process
 	// begins. All LifecycleHookFailurePolicy values are supported.
 	Pre *LifecycleHook

pkg/deploy/api/v1/conversion.go

@@ -1,8 +1,6 @@
 package v1
 
 import (
-	"fmt"
-	"math"
 	"reflect"
 	"strings"
 
@@ -60,7 +58,6 @@ func Convert_v1_RollingDeploymentStrategyParams_To_api_RollingDeploymentStrategy
 	out.UpdatePeriodSeconds = in.UpdatePeriodSeconds
 	out.IntervalSeconds = in.IntervalSeconds
 	out.TimeoutSeconds = in.TimeoutSeconds
-	out.UpdatePercent = in.UpdatePercent
 
 	if in.Pre != nil {
 		if err := s.Convert(&in.Pre, &out.Pre, 0); err != nil {
@@ -72,18 +69,12 @@ func Convert_v1_RollingDeploymentStrategyParams_To_api_RollingDeploymentStrategy
 			return err
 		}
 	}
-
-	if in.UpdatePercent != nil {
-		pct := intstr.FromString(fmt.Sprintf("%d%%", int(math.Abs(float64(*in.UpdatePercent)))))
-		if *in.UpdatePercent > 0 {
-			out.MaxSurge = pct
-		} else {
-			out.MaxUnavailable = pct
-		}
-	} else {
+	if in.MaxUnavailable != nil {
 		if err := s.Convert(in.MaxUnavailable, &out.MaxUnavailable, 0); err != nil {
 			return err
 		}
+	}
+	if in.MaxSurge != nil {
 		if err := s.Convert(in.MaxSurge, &out.MaxSurge, 0); err != nil {
 			return err
 		}
@@ -95,7 +86,6 @@ func Convert_api_RollingDeploymentStrategyParams_To_v1_RollingDeploymentStrategy
 	out.UpdatePeriodSeconds = in.UpdatePeriodSeconds
 	out.IntervalSeconds = in.IntervalSeconds
 	out.TimeoutSeconds = in.TimeoutSeconds
-	out.UpdatePercent = in.UpdatePercent
 
 	if in.Pre != nil {
 		if err := s.Convert(&in.Pre, &out.Pre, 0); err != nil {
@@ -114,20 +104,11 @@ func Convert_api_RollingDeploymentStrategyParams_To_v1_RollingDeploymentStrategy
 	if out.MaxSurge == nil {
 		out.MaxSurge = &intstr.IntOrString{}
 	}
-	if in.UpdatePercent != nil {
-		pct := intstr.FromString(fmt.Sprintf("%d%%", int(math.Abs(float64(*in.UpdatePercent)))))
-		if *in.UpdatePercent > 0 {
-			out.MaxSurge = &pct
-		} else {
-			out.MaxUnavailable = &pct
-		}
-	} else {
-		if err := s.Convert(&in.MaxUnavailable, out.MaxUnavailable, 0); err != nil {
-			return err
-		}
-		if err := s.Convert(&in.MaxSurge, out.MaxSurge, 0); err != nil {
-			return err
-		}
+	if err := s.Convert(&in.MaxUnavailable, out.MaxUnavailable, 0); err != nil {
+		return err
+	}
+	if err := s.Convert(&in.MaxSurge, out.MaxSurge, 0); err != nil {
+		return err
 	}
 	return nil
 }

pkg/deploy/api/v1/conversion_test.go

@@ -65,7 +65,7 @@ func Test_convert_v1_RollingDeploymentStrategyParams_To_api_RollingDeploymentStr
 				UpdatePeriodSeconds: newInt64(5),
 				IntervalSeconds:     newInt64(6),
 				TimeoutSeconds:      newInt64(7),
-				UpdatePercent:       newInt32(-25),
+				MaxUnavailable:      newIntOrString(intstr.FromString("25%")),
 				Pre: &LifecycleHook{
 					FailurePolicy: LifecycleHookFailurePolicyIgnore,
 				},
@@ -77,7 +77,6 @@ func Test_convert_v1_RollingDeploymentStrategyParams_To_api_RollingDeploymentStr
 				UpdatePeriodSeconds: newInt64(5),
 				IntervalSeconds:     newInt64(6),
 				TimeoutSeconds:      newInt64(7),
-				UpdatePercent:       newInt32(-25),
 				MaxSurge:            intstr.FromInt(0),
 				MaxUnavailable:      intstr.FromString("25%"),
 				Pre: &newer.LifecycleHook{
@@ -93,13 +92,12 @@ func Test_convert_v1_RollingDeploymentStrategyParams_To_api_RollingDeploymentStr
 				UpdatePeriodSeconds: newInt64(5),
 				IntervalSeconds:     newInt64(6),
 				TimeoutSeconds:      newInt64(7),
-				UpdatePercent:       newInt32(25),
+				MaxSurge:            newIntOrString(intstr.FromString("25%")),
 			},
 			out: &newer.RollingDeploymentStrategyParams{
 				UpdatePeriodSeconds: newInt64(5),
 				IntervalSeconds:     newInt64(6),
 				TimeoutSeconds:      newInt64(7),
-				UpdatePercent:       newInt32(25),
 				MaxSurge:            intstr.FromString("25%"),
 				MaxUnavailable:      intstr.FromInt(0),
 			},
@@ -120,9 +118,24 @@ func Test_convert_v1_RollingDeploymentStrategyParams_To_api_RollingDeploymentStr
 				MaxUnavailable:      intstr.FromInt(20),
 			},
 		},
+		{
+			in: &RollingDeploymentStrategyParams{
+				UpdatePeriodSeconds: newInt64(5),
+				IntervalSeconds:     newInt64(6),
+				TimeoutSeconds:      newInt64(7),
+			},
+			out: &newer.RollingDeploymentStrategyParams{
+				UpdatePeriodSeconds: newInt64(5),
+				IntervalSeconds:     newInt64(6),
+				TimeoutSeconds:      newInt64(7),
+				MaxSurge:            intstr.FromString("25%"),
+				MaxUnavailable:      intstr.FromString("25%"),
+			},
+		},
 	}
 
-	for _, test := range tests {
+	for i, test := range tests {
+		t.Logf("running test case #%d", i)
 		out := &newer.RollingDeploymentStrategyParams{}
 		if err := kapi.Scheme.Convert(test.in, out, nil); err != nil {
 			t.Errorf("unexpected error: %v", err)
@@ -143,15 +156,13 @@ func Test_convert_api_RollingDeploymentStrategyParams_To_v1_RollingDeploymentStr
 				UpdatePeriodSeconds: newInt64(5),
 				IntervalSeconds:     newInt64(6),
 				TimeoutSeconds:      newInt64(7),
-				UpdatePercent:       newInt32(-25),
 				MaxSurge:            intstr.FromInt(0),
 				MaxUnavailable:      intstr.FromString("25%"),
 			},
 			out: &RollingDeploymentStrategyParams{
 				UpdatePeriodSeconds: newInt64(5),
 				IntervalSeconds:     newInt64(6),
 				TimeoutSeconds:      newInt64(7),
-				UpdatePercent:       newInt32(-25),
 				MaxSurge:            newIntOrString(intstr.FromInt(0)),
 				MaxUnavailable:      newIntOrString(intstr.FromString("25%")),
 			},
@@ -161,15 +172,13 @@ func Test_convert_api_RollingDeploymentStrategyParams_To_v1_RollingDeploymentStr
 				UpdatePeriodSeconds: newInt64(5),
 				IntervalSeconds:     newInt64(6),
 				TimeoutSeconds:      newInt64(7),
-				UpdatePercent:       newInt32(25),
 				MaxSurge:            intstr.FromString("25%"),
 				MaxUnavailable:      intstr.FromInt(0),
 			},
 			out: &RollingDeploymentStrategyParams{
 				UpdatePeriodSeconds: newInt64(5),
 				IntervalSeconds:     newInt64(6),
 				TimeoutSeconds:      newInt64(7),
-				UpdatePercent:       newInt32(25),
 				MaxSurge:            newIntOrString(intstr.FromString("25%")),
 				MaxUnavailable:      newIntOrString(intstr.FromInt(0)),
 			},

pkg/deploy/api/v1/defaults.go

@@ -84,16 +84,12 @@ func SetDefaults_RollingDeploymentStrategyParams(obj *RollingDeploymentStrategyP
 		obj.TimeoutSeconds = mkintp(deployapi.DefaultRollingTimeoutSeconds)
 	}
 
-	if obj.UpdatePercent == nil {
-		// Apply defaults.
-		if obj.MaxUnavailable == nil {
-			maxUnavailable := intstr.FromString("25%")
-			obj.MaxUnavailable = &maxUnavailable
-		}
-		if obj.MaxSurge == nil {
-			maxSurge := intstr.FromString("25%")
-			obj.MaxSurge = &maxSurge
-		}
+	if obj.MaxUnavailable == nil && obj.MaxSurge == nil {
+		maxUnavailable := intstr.FromString("25%")
+		obj.MaxUnavailable = &maxUnavailable
+
+		maxSurge := intstr.FromString("25%")
+		obj.MaxSurge = &maxSurge
 	}
 }
 

pkg/deploy/api/v1/defaults_test.go

@@ -163,7 +163,7 @@ func TestDefaults(t *testing.T) {
 							UpdatePeriodSeconds: newInt64(5),
 							IntervalSeconds:     newInt64(6),
 							TimeoutSeconds:      newInt64(7),
-							UpdatePercent:       newInt32(50),
+							MaxSurge:            newIntOrString(intstr.FromString("50%")),
 						},
 					},
 					Triggers: []deployv1.DeploymentTriggerPolicy{
@@ -181,7 +181,6 @@ func TestDefaults(t *testing.T) {
 							UpdatePeriodSeconds: newInt64(5),
 							IntervalSeconds:     newInt64(6),
 							TimeoutSeconds:      newInt64(7),
-							UpdatePercent:       newInt32(50),
 							MaxSurge:            newIntOrString(intstr.FromString("50%")),
 							MaxUnavailable:      newIntOrString(intstr.FromInt(0)),
 						},
@@ -203,7 +202,7 @@ func TestDefaults(t *testing.T) {
 							UpdatePeriodSeconds: newInt64(5),
 							IntervalSeconds:     newInt64(6),
 							TimeoutSeconds:      newInt64(7),
-							UpdatePercent:       newInt32(-25),
+							MaxUnavailable:      newIntOrString(intstr.FromString("25%")),
 						},
 					},
 					Triggers: []deployv1.DeploymentTriggerPolicy{
@@ -221,7 +220,6 @@ func TestDefaults(t *testing.T) {
 							UpdatePeriodSeconds: newInt64(5),
 							IntervalSeconds:     newInt64(6),
 							TimeoutSeconds:      newInt64(7),
-							UpdatePercent:       newInt32(-25),
 							MaxSurge:            newIntOrString(intstr.FromInt(0)),
 							MaxUnavailable:      newIntOrString(intstr.FromString("25%")),
 						},
@@ -311,6 +309,10 @@ func TestDeepDefaults(t *testing.T) {
 				if *obj.Spec.Strategy.RollingParams.TimeoutSeconds != deployapi.DefaultRollingTimeoutSeconds {
 					return false
 				}
+				if obj.Spec.Strategy.RollingParams.MaxSurge.String() != "25%" ||
+					obj.Spec.Strategy.RollingParams.MaxUnavailable.String() != "25%" {
+					return false
+				}
 				return true
 			},
 		},