From a4f85c8dd0aadcf6d49e71cf4b2e9488f26fea11 Mon Sep 17 00:00:00 2001
From: Janet Kuo <chiachenk@google.com>
Date: Tue, 24 Jul 2018 13:20:39 -0700
Subject: [PATCH 1/3] Default extensions/v1beta1 Deployment's
 ProgressDeadlineSeconds to MaxInt32.

1. MaxInt32 has the same meaning as unset, for compatibility
2. Deployment controller treats MaxInt32 the same as unset (nil)
---
 pkg/apis/extensions/v1beta1/defaults.go           |  8 ++++++++
 pkg/apis/extensions/v1beta1/defaults_test.go      | 13 +++++++++----
 pkg/controller/deployment/progress.go             |  6 +++---
 pkg/controller/deployment/progress_test.go        | 12 ++++++++++--
 pkg/controller/deployment/sync.go                 |  8 ++++----
 pkg/controller/deployment/util/deployment_util.go |  7 ++++++-
 .../deployment/util/deployment_util_test.go       | 15 ++++++++++++---
 7 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/pkg/apis/extensions/v1beta1/defaults.go b/pkg/apis/extensions/v1beta1/defaults.go
index d817c2f0f44..3138696a518 100644
--- a/pkg/apis/extensions/v1beta1/defaults.go
+++ b/pkg/apis/extensions/v1beta1/defaults.go
@@ -17,6 +17,8 @@ limitations under the License.
 package v1beta1
 
 import (
+	"math"
+
 	"k8s.io/api/core/v1"
 	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -110,6 +112,12 @@ func SetDefaults_Deployment(obj *extensionsv1beta1.Deployment) {
 			strategy.RollingUpdate.MaxSurge = &maxSurge
 		}
 	}
+	// Set extensionsv1beta1.DeploymentSpec.ProgressDeadlineSeconds to MaxInt,
+	// which has the same meaning as unset.
+	if obj.Spec.ProgressDeadlineSeconds == nil {
+		obj.Spec.ProgressDeadlineSeconds = new(int32)
+		*obj.Spec.ProgressDeadlineSeconds = math.MaxInt32
+	}
 }
 
 func SetDefaults_ReplicaSet(obj *extensionsv1beta1.ReplicaSet) {
diff --git a/pkg/apis/extensions/v1beta1/defaults_test.go b/pkg/apis/extensions/v1beta1/defaults_test.go
index a3baf2f3143..722a16af321 100644
--- a/pkg/apis/extensions/v1beta1/defaults_test.go
+++ b/pkg/apis/extensions/v1beta1/defaults_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package v1beta1_test
 
 import (
+	"math"
 	"reflect"
 	"testing"
 
@@ -188,7 +189,8 @@ func TestSetDefaultDeployment(t *testing.T) {
 							MaxUnavailable: &defaultIntOrString,
 						},
 					},
-					Template: defaultTemplate,
+					Template:                defaultTemplate,
+					ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
 				},
 			},
 		},
@@ -213,7 +215,8 @@ func TestSetDefaultDeployment(t *testing.T) {
 							MaxUnavailable: &defaultIntOrString,
 						},
 					},
-					Template: defaultTemplate,
+					Template:                defaultTemplate,
+					ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
 				},
 			},
 		},
@@ -237,7 +240,8 @@ func TestSetDefaultDeployment(t *testing.T) {
 							MaxUnavailable: &defaultIntOrString,
 						},
 					},
-					Template: defaultTemplate,
+					Template:                defaultTemplate,
+					ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
 				},
 			},
 		},
@@ -256,7 +260,8 @@ func TestSetDefaultDeployment(t *testing.T) {
 					Strategy: extensionsv1beta1.DeploymentStrategy{
 						Type: extensionsv1beta1.RecreateDeploymentStrategyType,
 					},
-					Template: defaultTemplate,
+					Template:                defaultTemplate,
+					ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
 				},
 			},
 		},
diff --git a/pkg/controller/deployment/progress.go b/pkg/controller/deployment/progress.go
index cfe35ebab4d..601359c79c4 100644
--- a/pkg/controller/deployment/progress.go
+++ b/pkg/controller/deployment/progress.go
@@ -36,7 +36,7 @@ func (dc *DeploymentController) syncRolloutStatus(allRSs []*apps.ReplicaSet, new
 	newStatus := calculateStatus(allRSs, newRS, d)
 
 	// If there is no progressDeadlineSeconds set, remove any Progressing condition.
-	if d.Spec.ProgressDeadlineSeconds == nil {
+	if !util.HasProgressDeadline(d) {
 		util.RemoveDeploymentCondition(&newStatus, apps.DeploymentProgressing)
 	}
 
@@ -47,7 +47,7 @@ func (dc *DeploymentController) syncRolloutStatus(allRSs []*apps.ReplicaSet, new
 	isCompleteDeployment := newStatus.Replicas == newStatus.UpdatedReplicas && currentCond != nil && currentCond.Reason == util.NewRSAvailableReason
 	// Check for progress only if there is a progress deadline set and the latest rollout
 	// hasn't completed yet.
-	if d.Spec.ProgressDeadlineSeconds != nil && !isCompleteDeployment {
+	if util.HasProgressDeadline(d) && !isCompleteDeployment {
 		switch {
 		case util.DeploymentComplete(d, &newStatus):
 			// Update the deployment conditions with a message for the new replica set that
@@ -159,7 +159,7 @@ var nowFn = func() time.Time { return time.Now() }
 func (dc *DeploymentController) requeueStuckDeployment(d *apps.Deployment, newStatus apps.DeploymentStatus) time.Duration {
 	currentCond := util.GetDeploymentCondition(d.Status, apps.DeploymentProgressing)
 	// Can't estimate progress if there is no deadline in the spec or progressing condition in the current status.
-	if d.Spec.ProgressDeadlineSeconds == nil || currentCond == nil {
+	if !util.HasProgressDeadline(d) || currentCond == nil {
 		return time.Duration(-1)
 	}
 	// No need to estimate progress if the rollout is complete or already timed out.
diff --git a/pkg/controller/deployment/progress_test.go b/pkg/controller/deployment/progress_test.go
index da0aa2d67df..444bda998e7 100644
--- a/pkg/controller/deployment/progress_test.go
+++ b/pkg/controller/deployment/progress_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package deployment
 
 import (
+	"math"
 	"testing"
 	"time"
 
@@ -67,6 +68,7 @@ func newRSWithAvailable(name string, specReplicas, statusReplicas, availableRepl
 
 func TestRequeueStuckDeployment(t *testing.T) {
 	pds := int32(60)
+	infinite := int32(math.MaxInt32)
 	failed := []apps.DeploymentCondition{
 		{
 			Type:   apps.DeploymentProgressing,
@@ -90,11 +92,17 @@ func TestRequeueStuckDeployment(t *testing.T) {
 		expected time.Duration
 	}{
 		{
-			name:     "no progressDeadlineSeconds specified",
+			name:     "nil progressDeadlineSeconds specified",
 			d:        currentDeployment(nil, 4, 3, 3, 2, nil),
 			status:   newDeploymentStatus(3, 3, 2),
 			expected: time.Duration(-1),
 		},
+		{
+			name:     "infinite progressDeadlineSeconds specified",
+			d:        currentDeployment(&infinite, 4, 3, 3, 2, nil),
+			status:   newDeploymentStatus(3, 3, 2),
+			expected: time.Duration(-1),
+		},
 		{
 			name:     "no progressing condition found",
 			d:        currentDeployment(&pds, 4, 3, 3, 2, nil),
@@ -330,7 +338,7 @@ func TestSyncRolloutStatus(t *testing.T) {
 			newCond := util.GetDeploymentCondition(test.d.Status, test.conditionType)
 			switch {
 			case newCond == nil:
-				if test.d.Spec.ProgressDeadlineSeconds != nil {
+				if test.d.Spec.ProgressDeadlineSeconds != nil && *test.d.Spec.ProgressDeadlineSeconds != math.MaxInt32 {
 					t.Errorf("%s: expected deployment condition: %s", test.name, test.conditionType)
 				}
 			case newCond.Status != test.conditionStatus || newCond.Reason != test.conditionReason:
diff --git a/pkg/controller/deployment/sync.go b/pkg/controller/deployment/sync.go
index 63bd28ab18b..734def26894 100644
--- a/pkg/controller/deployment/sync.go
+++ b/pkg/controller/deployment/sync.go
@@ -71,7 +71,7 @@ func (dc *DeploymentController) sync(d *apps.Deployment, rsList []*apps.ReplicaS
 // These conditions are needed so that we won't accidentally report lack of progress for resumed deployments
 // that were paused for longer than progressDeadlineSeconds.
 func (dc *DeploymentController) checkPausedConditions(d *apps.Deployment) error {
-	if d.Spec.ProgressDeadlineSeconds == nil {
+	if !deploymentutil.HasProgressDeadline(d) {
 		return nil
 	}
 	cond := deploymentutil.GetDeploymentCondition(d.Status, apps.DeploymentProgressing)
@@ -158,7 +158,7 @@ func (dc *DeploymentController) getNewReplicaSet(d *apps.Deployment, rsList, old
 		// of this deployment then it is likely that old users started caring about progress. In that
 		// case we need to take into account the first time we noticed their new replica set.
 		cond := deploymentutil.GetDeploymentCondition(d.Status, apps.DeploymentProgressing)
-		if d.Spec.ProgressDeadlineSeconds != nil && cond == nil {
+		if deploymentutil.HasProgressDeadline(d) && cond == nil {
 			msg := fmt.Sprintf("Found new replica set %q", rsCopy.Name)
 			condition := deploymentutil.NewDeploymentCondition(apps.DeploymentProgressing, v1.ConditionTrue, deploymentutil.FoundNewRSReason, msg)
 			deploymentutil.SetDeploymentCondition(&d.Status, *condition)
@@ -253,7 +253,7 @@ func (dc *DeploymentController) getNewReplicaSet(d *apps.Deployment, rsList, old
 		return nil, err
 	case err != nil:
 		msg := fmt.Sprintf("Failed to create new replica set %q: %v", newRS.Name, err)
-		if d.Spec.ProgressDeadlineSeconds != nil {
+		if deploymentutil.HasProgressDeadline(d) {
 			cond := deploymentutil.NewDeploymentCondition(apps.DeploymentProgressing, v1.ConditionFalse, deploymentutil.FailedRSCreateReason, msg)
 			deploymentutil.SetDeploymentCondition(&d.Status, *cond)
 			// We don't really care about this error at this point, since we have a bigger issue to report.
@@ -269,7 +269,7 @@ func (dc *DeploymentController) getNewReplicaSet(d *apps.Deployment, rsList, old
 	}
 
 	needsUpdate := deploymentutil.SetDeploymentRevision(d, newRevision)
-	if !alreadyExists && d.Spec.ProgressDeadlineSeconds != nil {
+	if !alreadyExists && deploymentutil.HasProgressDeadline(d) {
 		msg := fmt.Sprintf("Created new replica set %q", createdRS.Name)
 		condition := deploymentutil.NewDeploymentCondition(apps.DeploymentProgressing, v1.ConditionTrue, deploymentutil.NewReplicaSetReason, msg)
 		deploymentutil.SetDeploymentCondition(&d.Status, *condition)
diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go
index a435e5eacef..2ea5d22e120 100644
--- a/pkg/controller/deployment/util/deployment_util.go
+++ b/pkg/controller/deployment/util/deployment_util.go
@@ -18,6 +18,7 @@ package util
 
 import (
 	"fmt"
+	"math"
 	"sort"
 	"strconv"
 	"strings"
@@ -773,7 +774,7 @@ var nowFn = func() time.Time { return time.Now() }
 // is older than progressDeadlineSeconds or a Progressing condition with a TimedOutReason reason already
 // exists.
 func DeploymentTimedOut(deployment *apps.Deployment, newStatus *apps.DeploymentStatus) bool {
-	if deployment.Spec.ProgressDeadlineSeconds == nil {
+	if !HasProgressDeadline(deployment) {
 		return false
 	}
 
@@ -918,3 +919,7 @@ func ResolveFenceposts(maxSurge, maxUnavailable *intstrutil.IntOrString, desired
 
 	return int32(surge), int32(unavailable), nil
 }
+
+func HasProgressDeadline(d *apps.Deployment) bool {
+	return d.Spec.ProgressDeadlineSeconds != nil && *d.Spec.ProgressDeadlineSeconds != math.MaxInt32
+}
diff --git a/pkg/controller/deployment/util/deployment_util_test.go b/pkg/controller/deployment/util/deployment_util_test.go
index 1d90e848d0d..5bf5b1486cb 100644
--- a/pkg/controller/deployment/util/deployment_util_test.go
+++ b/pkg/controller/deployment/util/deployment_util_test.go
@@ -18,6 +18,7 @@ package util
 
 import (
 	"fmt"
+	"math"
 	"math/rand"
 	"reflect"
 	"sort"
@@ -1068,8 +1069,9 @@ func TestDeploymentProgressing(t *testing.T) {
 
 func TestDeploymentTimedOut(t *testing.T) {
 	var (
-		null *int32
-		ten  = int32(10)
+		null     *int32
+		ten      = int32(10)
+		infinite = int32(math.MaxInt32)
 	)
 
 	timeFn := func(min, sec int) time.Time {
@@ -1102,12 +1104,19 @@ func TestDeploymentTimedOut(t *testing.T) {
 		expected bool
 	}{
 		{
-			name: "no progressDeadlineSeconds specified - no timeout",
+			name: "nil progressDeadlineSeconds specified - no timeout",
 
 			d:        deployment(apps.DeploymentProgressing, v1.ConditionTrue, "", null, timeFn(1, 9)),
 			nowFn:    func() time.Time { return timeFn(1, 20) },
 			expected: false,
 		},
+		{
+			name: "infinite progressDeadlineSeconds specified - no timeout",
+
+			d:        deployment(apps.DeploymentProgressing, v1.ConditionTrue, "", &infinite, timeFn(1, 9)),
+			nowFn:    func() time.Time { return timeFn(1, 20) },
+			expected: false,
+		},
 		{
 			name: "progressDeadlineSeconds: 10s, now - started => 00:01:20 - 00:01:09 => 11s",
 

From 849c08d1edcc234f07eb2240e65f76cbe24e0ca0 Mon Sep 17 00:00:00 2001
From: Janet Kuo <chiachenk@google.com>
Date: Tue, 24 Jul 2018 13:23:55 -0700
Subject: [PATCH 2/3] Update API doc of ProgressDeadlineSeconds

---
 pkg/apis/extensions/types.go                       | 4 ++--
 staging/src/k8s.io/api/extensions/v1beta1/types.go | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/pkg/apis/extensions/types.go b/pkg/apis/extensions/types.go
index a2330c45dd4..8dd49b46654 100644
--- a/pkg/apis/extensions/types.go
+++ b/pkg/apis/extensions/types.go
@@ -127,8 +127,8 @@ type DeploymentSpec struct {
 	// is considered to be failed. The deployment controller will continue to
 	// process failed deployments and a condition with a ProgressDeadlineExceeded
 	// reason will be surfaced in the deployment status. Note that progress will
-	// not be estimated during the time a deployment is paused. This is not set
-	// by default.
+	// not be estimated during the time a deployment is paused. This is set to
+	// the max value of int32 (i.e. 2147483647) by default, which means "no deadline".
 	// +optional
 	ProgressDeadlineSeconds *int32
 }
diff --git a/staging/src/k8s.io/api/extensions/v1beta1/types.go b/staging/src/k8s.io/api/extensions/v1beta1/types.go
index 3a86ef43a07..a2b17822c29 100644
--- a/staging/src/k8s.io/api/extensions/v1beta1/types.go
+++ b/staging/src/k8s.io/api/extensions/v1beta1/types.go
@@ -168,8 +168,8 @@ type DeploymentSpec struct {
 	// is considered to be failed. The deployment controller will continue to
 	// process failed deployments and a condition with a ProgressDeadlineExceeded
 	// reason will be surfaced in the deployment status. Note that progress will
-	// not be estimated during the time a deployment is paused. This is not set
-	// by default.
+	// not be estimated during the time a deployment is paused. This is set to
+	// the max value of int32 (i.e. 2147483647) by default, which means "no deadline".
 	// +optional
 	ProgressDeadlineSeconds *int32 `json:"progressDeadlineSeconds,omitempty" protobuf:"varint,9,opt,name=progressDeadlineSeconds"`
 }

From 4dadbb531aaf08cec3e26a30049aa00b70a205ae Mon Sep 17 00:00:00 2001
From: Janet Kuo <chiachenk@google.com>
Date: Fri, 27 Jul 2018 10:04:43 -0700
Subject: [PATCH 3/3] Autogen

1. hack/update-generated-protobuf.sh
2. hack/update-generated-swagger-docs.sh
3. hack/update-swagger-spec.sh
4. hack/update-openapi-spec.sh
5. hack/update-api-reference-docs.sh
---
 api/openapi-spec/swagger.json                                 | 2 +-
 api/swagger-spec/extensions_v1beta1.json                      | 2 +-
 docs/api-reference/extensions/v1beta1/definitions.html        | 2 +-
 staging/src/k8s.io/api/extensions/v1beta1/generated.proto     | 4 ++--
 .../api/extensions/v1beta1/types_swagger_doc_generated.go     | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
index 8d9c014cd75..535034ef564 100644
--- a/api/openapi-spec/swagger.json
+++ b/api/openapi-spec/swagger.json
@@ -83779,7 +83779,7 @@
       "type": "boolean"
      },
      "progressDeadlineSeconds": {
-      "description": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is not set by default.",
+      "description": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is set to the max value of int32 (i.e. 2147483647) by default, which means \"no deadline\".",
       "type": "integer",
       "format": "int32"
      },
diff --git a/api/swagger-spec/extensions_v1beta1.json b/api/swagger-spec/extensions_v1beta1.json
index 7cb1b7a42c5..3a75c60e5d8 100644
--- a/api/swagger-spec/extensions_v1beta1.json
+++ b/api/swagger-spec/extensions_v1beta1.json
@@ -9756,7 +9756,7 @@
      "progressDeadlineSeconds": {
       "type": "integer",
       "format": "int32",
-      "description": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is not set by default."
+      "description": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is set to the max value of int32 (i.e. 2147483647) by default, which means \"no deadline\"."
      }
     }
    },
diff --git a/docs/api-reference/extensions/v1beta1/definitions.html b/docs/api-reference/extensions/v1beta1/definitions.html
index 5aac9105ad5..c4fa06d8a1e 100755
--- a/docs/api-reference/extensions/v1beta1/definitions.html
+++ b/docs/api-reference/extensions/v1beta1/definitions.html
@@ -4337,7 +4337,7 @@ When an object is created, the system will populate this list with the current s
 </tr>
 <tr>
 <td class="tableblock halign-left valign-top"><p class="tableblock">progressDeadlineSeconds</p></td>
-<td class="tableblock halign-left valign-top"><p class="tableblock">The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is not set by default.</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is set to the max value of int32 (i.e. 2147483647) by default, which means "no deadline".</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">integer (int32)</p></td>
 <td class="tableblock halign-left valign-top"></td>
diff --git a/staging/src/k8s.io/api/extensions/v1beta1/generated.proto b/staging/src/k8s.io/api/extensions/v1beta1/generated.proto
index 3a87fbe5ec5..07a1d4ee840 100644
--- a/staging/src/k8s.io/api/extensions/v1beta1/generated.proto
+++ b/staging/src/k8s.io/api/extensions/v1beta1/generated.proto
@@ -353,8 +353,8 @@ message DeploymentSpec {
   // is considered to be failed. The deployment controller will continue to
   // process failed deployments and a condition with a ProgressDeadlineExceeded
   // reason will be surfaced in the deployment status. Note that progress will
-  // not be estimated during the time a deployment is paused. This is not set
-  // by default.
+  // not be estimated during the time a deployment is paused. This is set to
+  // the max value of int32 (i.e. 2147483647) by default, which means "no deadline".
   // +optional
   optional int32 progressDeadlineSeconds = 9;
 }
diff --git a/staging/src/k8s.io/api/extensions/v1beta1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/extensions/v1beta1/types_swagger_doc_generated.go
index d261b424797..c9ffadec1e7 100644
--- a/staging/src/k8s.io/api/extensions/v1beta1/types_swagger_doc_generated.go
+++ b/staging/src/k8s.io/api/extensions/v1beta1/types_swagger_doc_generated.go
@@ -196,7 +196,7 @@ var map_DeploymentSpec = map[string]string{
 	"revisionHistoryLimit":    "The number of old ReplicaSets to retain to allow rollback. This is a pointer to distinguish between explicit zero and not specified.",
 	"paused":                  "Indicates that the deployment is paused and will not be processed by the deployment controller.",
 	"rollbackTo":              "DEPRECATED. The config this deployment is rolling back to. Will be cleared after rollback is done.",
-	"progressDeadlineSeconds": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is not set by default.",
+	"progressDeadlineSeconds": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is set to the max value of int32 (i.e. 2147483647) by default, which means \"no deadline\".",
 }
 
 func (DeploymentSpec) SwaggerDoc() map[string]string {