From 1f921760d687fd0483320e0fe32ce81eaf71aac2 Mon Sep 17 00:00:00 2001
From: Weibin Lin <islinwb@gmail.com>
Date: Fri, 10 Aug 2018 10:12:43 +0800
Subject: [PATCH] Default extensions/v1beta1 Deployment's RevisionHistoryLimit
 to MaxInt32

---
 pkg/apis/extensions/v1beta1/defaults.go           |  6 ++++++
 pkg/apis/extensions/v1beta1/defaults_test.go      |  5 +++++
 pkg/controller/deployment/sync.go                 |  2 +-
 pkg/controller/deployment/sync_test.go            | 12 ++++++++++++
 pkg/controller/deployment/util/deployment_util.go |  3 +++
 5 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/pkg/apis/extensions/v1beta1/defaults.go b/pkg/apis/extensions/v1beta1/defaults.go
index 3138696a518..4bb885567b7 100644
--- a/pkg/apis/extensions/v1beta1/defaults.go
+++ b/pkg/apis/extensions/v1beta1/defaults.go
@@ -118,6 +118,12 @@ func SetDefaults_Deployment(obj *extensionsv1beta1.Deployment) {
 		obj.Spec.ProgressDeadlineSeconds = new(int32)
 		*obj.Spec.ProgressDeadlineSeconds = math.MaxInt32
 	}
+	// Set extensionsv1beta1.DeploymentSpec.RevisionHistoryLimit to MaxInt32,
+	// which has the same meaning as unset.
+	if obj.Spec.RevisionHistoryLimit == nil {
+		obj.Spec.RevisionHistoryLimit = new(int32)
+		*obj.Spec.RevisionHistoryLimit = 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 2d0de110370..dcab1e2aacc 100644
--- a/pkg/apis/extensions/v1beta1/defaults_test.go
+++ b/pkg/apis/extensions/v1beta1/defaults_test.go
@@ -191,6 +191,7 @@ func TestSetDefaultDeployment(t *testing.T) {
 					},
 					Template:                defaultTemplate,
 					ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
+					RevisionHistoryLimit:    utilpointer.Int32Ptr(math.MaxInt32),
 				},
 			},
 		},
@@ -217,6 +218,7 @@ func TestSetDefaultDeployment(t *testing.T) {
 					},
 					Template:                defaultTemplate,
 					ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
+					RevisionHistoryLimit:    utilpointer.Int32Ptr(math.MaxInt32),
 				},
 			},
 		},
@@ -242,6 +244,7 @@ func TestSetDefaultDeployment(t *testing.T) {
 					},
 					Template:                defaultTemplate,
 					ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
+					RevisionHistoryLimit:    utilpointer.Int32Ptr(math.MaxInt32),
 				},
 			},
 		},
@@ -262,6 +265,7 @@ func TestSetDefaultDeployment(t *testing.T) {
 					},
 					Template:                defaultTemplate,
 					ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
+					RevisionHistoryLimit:    utilpointer.Int32Ptr(math.MaxInt32),
 				},
 			},
 		},
@@ -283,6 +287,7 @@ func TestSetDefaultDeployment(t *testing.T) {
 					},
 					Template:                defaultTemplate,
 					ProgressDeadlineSeconds: utilpointer.Int32Ptr(30),
+					RevisionHistoryLimit:    utilpointer.Int32Ptr(math.MaxInt32),
 				},
 			},
 		},
diff --git a/pkg/controller/deployment/sync.go b/pkg/controller/deployment/sync.go
index 734def26894..7f2ed0d6016 100644
--- a/pkg/controller/deployment/sync.go
+++ b/pkg/controller/deployment/sync.go
@@ -424,7 +424,7 @@ func (dc *DeploymentController) scaleReplicaSet(rs *apps.ReplicaSet, newScale in
 // where N=d.Spec.RevisionHistoryLimit. Old replica sets are older versions of the podtemplate of a deployment kept
 // around by default 1) for historical reasons and 2) for the ability to rollback a deployment.
 func (dc *DeploymentController) cleanupDeployment(oldRSs []*apps.ReplicaSet, deployment *apps.Deployment) error {
-	if deployment.Spec.RevisionHistoryLimit == nil {
+	if !deploymentutil.HasRevisionHistoryLimit(deployment) {
 		return nil
 	}
 
diff --git a/pkg/controller/deployment/sync_test.go b/pkg/controller/deployment/sync_test.go
index 4cd4c0ab2d9..79389a01cd3 100644
--- a/pkg/controller/deployment/sync_test.go
+++ b/pkg/controller/deployment/sync_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package deployment
 
 import (
+	"math"
 	"testing"
 	"time"
 
@@ -393,6 +394,16 @@ func TestDeploymentController_cleanupDeployment(t *testing.T) {
 			revisionHistoryLimit: 0,
 			expectedDeletions:    0,
 		},
+		{
+			// with unlimited revisionHistoryLimit
+			oldRSs: []*apps.ReplicaSet{
+				newRSWithStatus("foo-1", 0, 0, selector),
+				newRSWithStatus("foo-2", 0, 0, selector),
+				newRSWithStatus("foo-3", 0, 0, selector),
+			},
+			revisionHistoryLimit: math.MaxInt32,
+			expectedDeletions:    0,
+		},
 	}
 
 	for i := range tests {
@@ -418,6 +429,7 @@ func TestDeploymentController_cleanupDeployment(t *testing.T) {
 		defer close(stopCh)
 		informers.Start(stopCh)
 
+		t.Logf(" &test.revisionHistoryLimit: %d", test.revisionHistoryLimit)
 		d := newDeployment("foo", 1, &test.revisionHistoryLimit, nil, nil, map[string]string{"foo": "bar"})
 		controller.cleanupDeployment(test.oldRSs, d)
 
diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go
index f2099d7c879..a677d3a4d47 100644
--- a/pkg/controller/deployment/util/deployment_util.go
+++ b/pkg/controller/deployment/util/deployment_util.go
@@ -886,3 +886,6 @@ func ResolveFenceposts(maxSurge, maxUnavailable *intstrutil.IntOrString, desired
 func HasProgressDeadline(d *apps.Deployment) bool {
 	return d.Spec.ProgressDeadlineSeconds != nil && *d.Spec.ProgressDeadlineSeconds != math.MaxInt32
 }
+func HasRevisionHistoryLimit(d *apps.Deployment) bool {
+	return d.Spec.RevisionHistoryLimit != nil && *d.Spec.RevisionHistoryLimit != math.MaxInt32
+}