From f541d8df1eb6b0884e430bb276f6c05a9cb11c05 Mon Sep 17 00:00:00 2001 From: Jesse Suen Date: Thu, 16 May 2019 16:06:23 -0700 Subject: [PATCH] Rename AutoPromoteActiveServiceDelaySeconds to autoPromotionSeconds Improve and fix documentation for various fields --- controller/bluegreen.go | 2 +- controller/bluegreen_test.go | 4 ++-- controller/pause.go | 4 ++-- manifests/crds/rollout-crd.yaml | 21 +++++++++------- manifests/install.yaml | 21 +++++++++------- manifests/namespace-install.yaml | 21 +++++++++------- .../rollouts/v1alpha1/openapi_generated.go | 10 ++++---- pkg/apis/rollouts/v1alpha1/types.go | 24 +++++++++++-------- .../v1alpha1/zz_generated.deepcopy.go | 4 ++-- 9 files changed, 62 insertions(+), 49 deletions(-) diff --git a/controller/bluegreen.go b/controller/bluegreen.go index beb7c71acd..cddfb714c2 100644 --- a/controller/bluegreen.go +++ b/controller/bluegreen.go @@ -121,7 +121,7 @@ func (c *Controller) reconcileBlueGreenPause(activeSvc *corev1.Service, rollout return false } pauseStartTime := rollout.Status.PauseStartTime - autoPromoteActiveServiceDelaySeconds := rollout.Spec.Strategy.BlueGreenStrategy.AutoPromoteActiveServiceDelaySeconds + autoPromoteActiveServiceDelaySeconds := rollout.Spec.Strategy.BlueGreenStrategy.AutoPromotionSeconds if autoPromoteActiveServiceDelaySeconds != nil && pauseStartTime != nil { c.checkEnqueueRolloutDuringWait(rollout, *pauseStartTime, *autoPromoteActiveServiceDelaySeconds) } diff --git a/controller/bluegreen_test.go b/controller/bluegreen_test.go index cb9fc10bd2..c16c7560cd 100644 --- a/controller/bluegreen_test.go +++ b/controller/bluegreen_test.go @@ -200,7 +200,7 @@ func TestBlueGreenHandlePause(t *testing.T) { r1 := newBlueGreenRollout("foo", 1, nil, "active", "preview") r2 := bumpVersion(r1) - r2.Spec.Strategy.BlueGreenStrategy.AutoPromoteActiveServiceDelaySeconds = pointer.Int32Ptr(10) + r2.Spec.Strategy.BlueGreenStrategy.AutoPromotionSeconds = pointer.Int32Ptr(10) rs1 := newReplicaSetWithStatus(r1, "foo-895c6c4f9", 1, 1) rs2 := newReplicaSetWithStatus(r2, "foo-5f79b78d7f", 1, 1) @@ -234,7 +234,7 @@ func TestBlueGreenHandlePause(t *testing.T) { r1 := newBlueGreenRollout("foo", 1, nil, "active", "preview") r2 := bumpVersion(r1) - r2.Spec.Strategy.BlueGreenStrategy.AutoPromoteActiveServiceDelaySeconds = pointer.Int32Ptr(10) + r2.Spec.Strategy.BlueGreenStrategy.AutoPromotionSeconds = pointer.Int32Ptr(10) rs1 := newReplicaSetWithStatus(r1, "foo-895c6c4f9", 1, 1) rs2 := newReplicaSetWithStatus(r2, "foo-5f79b78d7f", 1, 1) diff --git a/controller/pause.go b/controller/pause.go index 6e8a5e2ac2..b943924b51 100644 --- a/controller/pause.go +++ b/controller/pause.go @@ -61,9 +61,9 @@ func calculatePauseStatus(rollout *v1alpha1.Rollout, addPause bool) (*metav1.Tim } if paused && rollout.Spec.Strategy.BlueGreenStrategy != nil { - if pauseStartTime != nil && rollout.Spec.Strategy.BlueGreenStrategy.AutoPromoteActiveServiceDelaySeconds != nil { + if pauseStartTime != nil && rollout.Spec.Strategy.BlueGreenStrategy.AutoPromotionSeconds != nil { now := metav1.Now() - autoPromoteActiveServiceDelaySeconds := *rollout.Spec.Strategy.BlueGreenStrategy.AutoPromoteActiveServiceDelaySeconds + autoPromoteActiveServiceDelaySeconds := *rollout.Spec.Strategy.BlueGreenStrategy.AutoPromotionSeconds switchDeadline := pauseStartTime.Add(time.Duration(autoPromoteActiveServiceDelaySeconds) * time.Second) if now.After(switchDeadline) { return nil, false diff --git a/manifests/crds/rollout-crd.yaml b/manifests/crds/rollout-crd.yaml index 53cea5008a..3a4842c0b7 100644 --- a/manifests/crds/rollout-crd.yaml +++ b/manifests/crds/rollout-crd.yaml @@ -57,10 +57,8 @@ spec: format: int32 type: integer revisionHistoryLimit: - description: The number of old ReplicaSets to retain. 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". + description: The number of old ReplicaSets to retain. If unspecified, + will retain 10 old ReplicaSets format: int32 type: integer selector: {} @@ -75,10 +73,12 @@ spec: description: Name of the service that the rollout modifies as the active service. type: string - autoPromoteActiveServiceDelaySeconds: - description: AutoPromoteActiveServiceDelaySeconds add a delay - before automatically promoting the ReplicaSet under the preview - service to the active service. + autoPromotionSeconds: + description: AutoPromotionSeconds automatically promotes the + current ReplicaSet to active after the specified pause delay + in seconds after the ReplicaSet becomes ready. If omitted, + the Rollout enters and remains in a paused state until manually + resumed by resetting spec.Paused to false. format: int32 type: integer previewReplicaCount: @@ -94,7 +94,10 @@ spec: type: string scaleDownDelaySeconds: description: ScaleDownDelaySeconds adds a delay before scaling - down the previous replicaset. See https://github.com/argoproj/argo-rollouts/issues/19#issuecomment-476329960 + down the previous replicaset. If omitted, the Rollout waits + 30 seconds before scaling down the previous ReplicaSet. A + minimum of 30 seconds is recommended to ensure IP table propagation + across the nodes in a cluster. See https://github.com/argoproj/argo-rollouts/issues/19#issuecomment-476329960 for more information format: int32 type: integer diff --git a/manifests/install.yaml b/manifests/install.yaml index c6b630277d..5a5106733b 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -58,10 +58,8 @@ spec: format: int32 type: integer revisionHistoryLimit: - description: The number of old ReplicaSets to retain. 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". + description: The number of old ReplicaSets to retain. If unspecified, + will retain 10 old ReplicaSets format: int32 type: integer selector: {} @@ -76,10 +74,12 @@ spec: description: Name of the service that the rollout modifies as the active service. type: string - autoPromoteActiveServiceDelaySeconds: - description: AutoPromoteActiveServiceDelaySeconds add a delay - before automatically promoting the ReplicaSet under the preview - service to the active service. + autoPromotionSeconds: + description: AutoPromotionSeconds automatically promotes the + current ReplicaSet to active after the specified pause delay + in seconds after the ReplicaSet becomes ready. If omitted, + the Rollout enters and remains in a paused state until manually + resumed by resetting spec.Paused to false. format: int32 type: integer previewReplicaCount: @@ -95,7 +95,10 @@ spec: type: string scaleDownDelaySeconds: description: ScaleDownDelaySeconds adds a delay before scaling - down the previous replicaset. See https://github.com/argoproj/argo-rollouts/issues/19#issuecomment-476329960 + down the previous replicaset. If omitted, the Rollout waits + 30 seconds before scaling down the previous ReplicaSet. A + minimum of 30 seconds is recommended to ensure IP table propagation + across the nodes in a cluster. See https://github.com/argoproj/argo-rollouts/issues/19#issuecomment-476329960 for more information format: int32 type: integer diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index f04424b8df..a74e98842c 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -58,10 +58,8 @@ spec: format: int32 type: integer revisionHistoryLimit: - description: The number of old ReplicaSets to retain. 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". + description: The number of old ReplicaSets to retain. If unspecified, + will retain 10 old ReplicaSets format: int32 type: integer selector: {} @@ -76,10 +74,12 @@ spec: description: Name of the service that the rollout modifies as the active service. type: string - autoPromoteActiveServiceDelaySeconds: - description: AutoPromoteActiveServiceDelaySeconds add a delay - before automatically promoting the ReplicaSet under the preview - service to the active service. + autoPromotionSeconds: + description: AutoPromotionSeconds automatically promotes the + current ReplicaSet to active after the specified pause delay + in seconds after the ReplicaSet becomes ready. If omitted, + the Rollout enters and remains in a paused state until manually + resumed by resetting spec.Paused to false. format: int32 type: integer previewReplicaCount: @@ -95,7 +95,10 @@ spec: type: string scaleDownDelaySeconds: description: ScaleDownDelaySeconds adds a delay before scaling - down the previous replicaset. See https://github.com/argoproj/argo-rollouts/issues/19#issuecomment-476329960 + down the previous replicaset. If omitted, the Rollout waits + 30 seconds before scaling down the previous ReplicaSet. A + minimum of 30 seconds is recommended to ensure IP table propagation + across the nodes in a cluster. See https://github.com/argoproj/argo-rollouts/issues/19#issuecomment-476329960 for more information format: int32 type: integer diff --git a/pkg/apis/rollouts/v1alpha1/openapi_generated.go b/pkg/apis/rollouts/v1alpha1/openapi_generated.go index ac9c28cfb8..387e9984c3 100644 --- a/pkg/apis/rollouts/v1alpha1/openapi_generated.go +++ b/pkg/apis/rollouts/v1alpha1/openapi_generated.go @@ -112,16 +112,16 @@ func schema_pkg_apis_rollouts_v1alpha1_BlueGreenStrategy(ref common.ReferenceCal Format: "int32", }, }, - "autoPromoteActiveServiceDelaySeconds": { + "autoPromotionSeconds": { SchemaProps: spec.SchemaProps{ - Description: "AutoPromoteActiveServiceDelaySeconds add a delay before automatically promoting the ReplicaSet under the preview service to the active service.", + Description: "AutoPromotionSeconds automatically promotes the current ReplicaSet to active after the specified pause delay in seconds after the ReplicaSet becomes ready. If omitted, the Rollout enters and remains in a paused state until manually resumed by resetting spec.Paused to false.", Type: []string{"integer"}, Format: "int32", }, }, "scaleDownDelaySeconds": { SchemaProps: spec.SchemaProps{ - Description: "ScaleDownDelaySeconds adds a delay before scaling down the previous replicaset. See https://github.com/argoproj/argo-rollouts/issues/19#issuecomment-476329960 for more information", + Description: "ScaleDownDelaySeconds adds a delay before scaling down the previous replicaset. If omitted, the Rollout waits 30 seconds before scaling down the previous ReplicaSet. A minimum of 30 seconds is recommended to ensure IP table propagation across the nodes in a cluster. See https://github.com/argoproj/argo-rollouts/issues/19#issuecomment-476329960 for more information", Type: []string{"integer"}, Format: "int32", }, @@ -168,7 +168,7 @@ func schema_pkg_apis_rollouts_v1alpha1_CanaryStep(ref common.ReferenceCallback) }, "pause": { SchemaProps: spec.SchemaProps{ - Description: "Pause freezes the rollout. If an empty struct is provided, it will freeze until a user sets the spec.Pause to false", + Description: "Pause freezes the rollout by setting spec.Paused to true. A Rollout will resume when spec.Paused is reset to false.", Ref: ref("github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutPause"), }, }, @@ -424,7 +424,7 @@ func schema_pkg_apis_rollouts_v1alpha1_RolloutSpec(ref common.ReferenceCallback) }, "revisionHistoryLimit": { SchemaProps: spec.SchemaProps{ - Description: "The number of old ReplicaSets to retain. 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\".", + Description: "The number of old ReplicaSets to retain. If unspecified, will retain 10 old ReplicaSets", Type: []string{"integer"}, Format: "int32", }, diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go index f5aea5a94e..0faa7f878b 100644 --- a/pkg/apis/rollouts/v1alpha1/types.go +++ b/pkg/apis/rollouts/v1alpha1/types.go @@ -39,10 +39,7 @@ type RolloutSpec struct { // The deployment strategy to use to replace existing pods with new ones. // +optional Strategy RolloutStrategy `json:"strategy"` - // The number of old ReplicaSets to retain. - // 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". + // The number of old ReplicaSets to retain. If unspecified, will retain 10 old ReplicaSets RevisionHistoryLimit *int32 `json:"revisionHistoryLimit,omitempty"` // Paused pauses the rollout at its current step. Paused bool `json:"paused,omitempty"` @@ -81,11 +78,17 @@ type BlueGreenStrategy struct { // resumed the new replicaset will be full scaled up before the switch occurs // +optional PreviewReplicaCount *int32 `json:"previewReplicaCount,omitempty"` - // AutoPromoteActiveServiceDelaySeconds add a delay before automatically promoting the ReplicaSet under the preview - // service to the active service. - AutoPromoteActiveServiceDelaySeconds *int32 `json:"autoPromoteActiveServiceDelaySeconds,omitempty"` - // ScaleDownDelaySeconds adds a delay before scaling down the previous replicaset. See - // https://github.com/argoproj/argo-rollouts/issues/19#issuecomment-476329960 for more information + // AutoPromotionSeconds automatically promotes the current ReplicaSet to active after the + // specified pause delay in seconds after the ReplicaSet becomes ready. + // If omitted, the Rollout enters and remains in a paused state until manually resumed by + // resetting spec.Paused to false. + // +optional + AutoPromotionSeconds *int32 `json:"autoPromotionSeconds,omitempty"` + // ScaleDownDelaySeconds adds a delay before scaling down the previous replicaset. + // If omitted, the Rollout waits 30 seconds before scaling down the previous ReplicaSet. + // A minimum of 30 seconds is recommended to ensure IP table propagation across the nodes in + // a cluster. See https://github.com/argoproj/argo-rollouts/issues/19#issuecomment-476329960 for + // more information // +optional ScaleDownDelaySeconds *int32 `json:"scaleDownDelaySeconds,omitempty"` } @@ -126,7 +129,8 @@ type CanaryStrategy struct { type CanaryStep struct { // SetWeight sets what percentage of the newRS should receive SetWeight *int32 `json:"setWeight,omitempty"` - // Pause freezes the rollout. If an empty struct is provided, it will freeze until a user sets the spec.Pause to false + // Pause freezes the rollout by setting spec.Paused to true. + // A Rollout will resume when spec.Paused is reset to false. // +optional Pause *RolloutPause `json:"pause,omitempty"` } diff --git a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go index defe3afc37..26d1c8d870 100644 --- a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go @@ -54,8 +54,8 @@ func (in *BlueGreenStrategy) DeepCopyInto(out *BlueGreenStrategy) { *out = new(int32) **out = **in } - if in.AutoPromoteActiveServiceDelaySeconds != nil { - in, out := &in.AutoPromoteActiveServiceDelaySeconds, &out.AutoPromoteActiveServiceDelaySeconds + if in.AutoPromotionSeconds != nil { + in, out := &in.AutoPromotionSeconds, &out.AutoPromotionSeconds *out = new(int32) **out = **in }