From 1f921760d687fd0483320e0fe32ce81eaf71aac2 Mon Sep 17 00:00:00 2001 From: Weibin Lin Date: Fri, 10 Aug 2018 10:12:43 +0800 Subject: [PATCH 1/3] 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 3138696a51..4bb885567b 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 2d0de11037..dcab1e2aac 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 734def2689..7f2ed0d601 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 4cd4c0ab2d..79389a01cd 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 f2099d7c87..a677d3a4d4 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 +} From 935fc2c715c5d1f0f1b14699ebbaad24ce739874 Mon Sep 17 00:00:00 2001 From: Weibin Lin Date: Fri, 10 Aug 2018 10:19:01 +0800 Subject: [PATCH 2/3] Update API doc of extensions/v1beta1 Deployment's RevisionHistoryLimit --- pkg/apis/extensions/types.go | 2 ++ staging/src/k8s.io/api/extensions/v1beta1/types.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/pkg/apis/extensions/types.go b/pkg/apis/extensions/types.go index 8dd49b4665..8ac06701da 100644 --- a/pkg/apis/extensions/types.go +++ b/pkg/apis/extensions/types.go @@ -110,6 +110,8 @@ type DeploymentSpec struct { // The number of old ReplicaSets to retain to allow rollback. // This is a pointer to distinguish between explicit zero and not specified. + // This is set to the max value of int32 (i.e. 2147483647) by default, which means + // "retaining all old ReplicaSets". // +optional RevisionHistoryLimit *int32 diff --git a/staging/src/k8s.io/api/extensions/v1beta1/types.go b/staging/src/k8s.io/api/extensions/v1beta1/types.go index a2b17822c2..b7b9e2a17c 100644 --- a/staging/src/k8s.io/api/extensions/v1beta1/types.go +++ b/staging/src/k8s.io/api/extensions/v1beta1/types.go @@ -151,6 +151,8 @@ type DeploymentSpec struct { // The number of old ReplicaSets to retain to allow rollback. // This is a pointer to distinguish between explicit zero and not specified. + // This is set to the max value of int32 (i.e. 2147483647) by default, which + // means "retaining all old RelicaSets". // +optional RevisionHistoryLimit *int32 `json:"revisionHistoryLimit,omitempty" protobuf:"varint,6,opt,name=revisionHistoryLimit"` From 84ff6c36b023d2bb95872824bee3774e183528e7 Mon Sep 17 00:00:00 2001 From: Weibin Lin Date: Fri, 10 Aug 2018 10:31:21 +0800 Subject: [PATCH 3/3] Autogen --- 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 | 2 ++ .../api/extensions/v1beta1/types_swagger_doc_generated.go | 2 +- 5 files changed, 6 insertions(+), 4 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index eaa62f40d9..da4b89617d 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -83789,7 +83789,7 @@ "format": "int32" }, "revisionHistoryLimit": { - "description": "The number of old ReplicaSets to retain to allow rollback. This is a pointer to distinguish between explicit zero and not specified.", + "description": "The number of old ReplicaSets to retain to allow rollback. This is a pointer to distinguish between explicit zero and not specified. This is set to the max value of int32 (i.e. 2147483647) by default, which means \"retaining all old RelicaSets\".", "type": "integer", "format": "int32" }, diff --git a/api/swagger-spec/extensions_v1beta1.json b/api/swagger-spec/extensions_v1beta1.json index c4284fe139..43b9d43739 100644 --- a/api/swagger-spec/extensions_v1beta1.json +++ b/api/swagger-spec/extensions_v1beta1.json @@ -9743,7 +9743,7 @@ "revisionHistoryLimit": { "type": "integer", "format": "int32", - "description": "The number of old ReplicaSets to retain to allow rollback. This is a pointer to distinguish between explicit zero and not specified." + "description": "The number of old ReplicaSets to retain to allow rollback. This is a pointer to distinguish between explicit zero and not specified. This is set to the max value of int32 (i.e. 2147483647) by default, which means \"retaining all old RelicaSets\"." }, "paused": { "type": "boolean", diff --git a/docs/api-reference/extensions/v1beta1/definitions.html b/docs/api-reference/extensions/v1beta1/definitions.html index a2e9e85751..960dfa04f5 100755 --- a/docs/api-reference/extensions/v1beta1/definitions.html +++ b/docs/api-reference/extensions/v1beta1/definitions.html @@ -4316,7 +4316,7 @@ When an object is created, the system will populate this list with the current s

revisionHistoryLimit

-

The number of old ReplicaSets to retain to allow rollback. This is a pointer to distinguish between explicit zero and not specified.

+

The number of old ReplicaSets to retain to allow rollback. This is a pointer to distinguish between explicit zero and not specified. This is set to the max value of int32 (i.e. 2147483647) by default, which means "retaining all old RelicaSets".

false

integer (int32)

diff --git a/staging/src/k8s.io/api/extensions/v1beta1/generated.proto b/staging/src/k8s.io/api/extensions/v1beta1/generated.proto index 07a1d4ee84..bd59dd72f9 100644 --- a/staging/src/k8s.io/api/extensions/v1beta1/generated.proto +++ b/staging/src/k8s.io/api/extensions/v1beta1/generated.proto @@ -336,6 +336,8 @@ message DeploymentSpec { // The number of old ReplicaSets to retain to allow rollback. // This is a pointer to distinguish between explicit zero and not specified. + // This is set to the max value of int32 (i.e. 2147483647) by default, which + // means "retaining all old RelicaSets". // +optional optional int32 revisionHistoryLimit = 6; 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 c9ffadec1e..b6c7a5ac70 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 @@ -193,7 +193,7 @@ var map_DeploymentSpec = map[string]string{ "template": "Template describes the pods that will be created.", "strategy": "The deployment strategy to use to replace existing pods with new ones.", "minReadySeconds": "Minimum number of seconds for which a newly created pod should be ready without any of its container crashing, for it to be considered available. Defaults to 0 (pod will be considered available as soon as it is ready)", - "revisionHistoryLimit": "The number of old ReplicaSets to retain to allow rollback. This is a pointer to distinguish between explicit zero and not specified.", + "revisionHistoryLimit": "The number of old ReplicaSets to retain to allow rollback. This is a pointer to distinguish between explicit zero and not specified. This is set to the max value of int32 (i.e. 2147483647) by default, which means \"retaining all old RelicaSets\".", "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 set to the max value of int32 (i.e. 2147483647) by default, which means \"no deadline\".",