feat: scaleDownOnAbort for rollout strategies

- if an update is aborted, the preview RS and canary RS will be aborted. - scaleDownOnAbort is false by default - updated doc - added e2e test Signed-off-by: Hui Kang <[email protected]>
argoproj · May 21, 2021 · b33ed06 · b33ed06
1 parent 60092c3
commit b33ed06
Show file tree

Hide file tree

Showing 10 changed files with 62 additions and 0 deletions.
diff --git a/docs/features/specification.md b/docs/features/specification.md
@@ -58,6 +58,11 @@ spec:
   # than or equal to this value.
   restartAt: "2020-03-30T21:19:35Z"
 
+  # Scale down the preview replicasets or canary replicasets (if
+  # canary's setCanaryScale.replicas is set) on aborted update
+  # Defaults to false
+  scaleDownOnAbort: false
+
   strategy:
 
     # Blue-green update strategy

diff --git a/manifests/crds/rollout-crd.yaml b/manifests/crds/rollout-crd.yaml
@@ -64,6 +64,8 @@ spec:
               revisionHistoryLimit:
                 format: int32
                 type: integer
+              scaleDownOnAbort:
+                type: boolean
               selector:
                 properties:
                   matchExpressions:

diff --git a/manifests/install.yaml b/manifests/install.yaml
@@ -9753,6 +9753,8 @@ spec:
               revisionHistoryLimit:
                 format: int32
                 type: integer
+              scaleDownOnAbort:
+                type: boolean
               selector:
                 properties:
                   matchExpressions:

diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml
@@ -9753,6 +9753,8 @@ spec:
               revisionHistoryLimit:
                 format: int32
                 type: integer
+              scaleDownOnAbort:
+                type: boolean
               selector:
                 properties:
                   matchExpressions:

diff --git a/pkg/apiclient/rollout/rollout.swagger.json b/pkg/apiclient/rollout/rollout.swagger.json
@@ -1155,6 +1155,10 @@
         "restartAt": {
           "$ref": "#/definitions/k8s.io.apimachinery.pkg.apis.meta.v1.Time",
           "title": "RestartAt indicates when all the pods of a Rollout should be restarted"
+        },
+        "scaleDownOnAbort": {
+          "type": "boolean",
+          "title": "ScaleDownOnAbort scales down"
         }
       },
       "title": "RolloutSpec is the spec for a Rollout resource"

diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go
@@ -71,6 +71,8 @@ type RolloutSpec struct {
 	ProgressDeadlineSeconds *int32 `json:"progressDeadlineSeconds,omitempty" protobuf:"varint,8,opt,name=progressDeadlineSeconds"`
 	// RestartAt indicates when all the pods of a Rollout should be restarted
 	RestartAt *metav1.Time `json:"restartAt,omitempty" protobuf:"bytes,9,opt,name=restartAt"`
+	// ScaleDownOnAbort scales down
+	ScaleDownOnAbort bool `json:"scaleDownOnAbort,omitempty" protobuf:"varint,11,opt,name=scaleDownOnAbort"`
 }
 
 func (s *RolloutSpec) SetResolvedSelector(selector *metav1.LabelSelector) {

diff --git a/rollout/replicaset.go b/rollout/replicaset.go
@@ -120,6 +120,12 @@ func (c *rolloutContext) reconcileNewReplicaSet() (bool, error) {
 	if err != nil {
 		return false, err
 	}
+
+	if c.pauseContext != nil && c.pauseContext.IsAborted() && c.rollout.Spec.ScaleDownOnAbort {
+		c.log.Info("scale down on abort")
+		newReplicasCount = int32(0)
+	}
+
 	scaled, _, err := c.scaleReplicaSetAndRecordEvent(c.newRS, newReplicasCount)
 	return scaled, err
 }

diff --git a/rollout/replicaset_test.go b/rollout/replicaset_test.go
@@ -125,6 +125,7 @@ func TestReconcileNewReplicaSet(t *testing.T) {
 		rolloutReplicas     int
 		newReplicas         int
 		scaleExpected       bool
+		ScaleDownOnAbort    bool
 		expectedNewReplicas int
 	}{
 		{
@@ -147,6 +148,14 @@ func TestReconcileNewReplicaSet(t *testing.T) {
 			scaleExpected:       true,
 			expectedNewReplicas: 10,
 		},
+		{
+			name:                "New Replica scaled down to 0: scale down on abort",
+			rolloutReplicas:     10,
+			newReplicas:         10,
+			ScaleDownOnAbort:    true,
+			scaleExpected:       true,
+			expectedNewReplicas: 0,
+		},
 	}
 	for i := range tests {
 		test := tests[i]
@@ -166,6 +175,14 @@ func TestReconcileNewReplicaSet(t *testing.T) {
 					recorder:          record.NewFakeEventRecorder(),
 				},
 			}
+			if test.ScaleDownOnAbort {
+				roCtx.pauseContext = &pauseContext{
+					rollout: rollout,
+				}
+				rollout.Status.Abort = true
+				rollout.Spec.ScaleDownOnAbort = true
+			}
+
 			scaled, err := roCtx.reconcileNewReplicaSet()
 			if err != nil {
 				t.Errorf("unexpected error: %v", err)

diff --git a/utils/replicaset/canary.go b/utils/replicaset/canary.go
@@ -297,6 +297,10 @@ func GetCanaryReplicasOrWeight(rollout *v1alpha1.Rollout) (*int32, int32) {
 		return nil, 100
 	}
 	if scs := UseSetCanaryScale(rollout); scs != nil {
+		if rollout.Status.Abort && rollout.Spec.ScaleDownOnAbort {
+			return nil, 0
+		}
+
 		if scs.Replicas != nil {
 			return scs.Replicas, 0
 		} else if scs.Weight != nil {

diff --git a/utils/replicaset/canary_test.go b/utils/replicaset/canary_test.go
@@ -80,6 +80,7 @@ func TestCalculateReplicaCountsForCanary(t *testing.T) {
 		maxSurge            intstr.IntOrString
 		maxUnavailable      intstr.IntOrString
 		promoteFull         bool
+		scaleDownOnAbort    bool
 
 		stableSpecReplica      int32
 		stableAvailableReplica int32
@@ -571,12 +572,29 @@ func TestCalculateReplicaCountsForCanary(t *testing.T) {
 			expectedStableReplicaCount: 4,
 			expectedCanaryReplicaCount: 3,
 		},
+		{
+			name:                "Ignore setCanaryScale when scaleDownonAbort is true and rollout is aborted",
+			rolloutSpecReplicas: 10,
+			setWeight:           20,
+			scaleDownOnAbort:    true,
+
+			stableSpecReplica:      10,
+			stableAvailableReplica: 10,
+
+			setCanaryScale: newSetCanaryScale(intPnt(3), intPnt(25), false),
+			trafficRouting: &v1alpha1.RolloutTrafficRouting{},
+
+			expectedStableReplicaCount: 10,
+			expectedCanaryReplicaCount: 0,
+		},
 	}
 	for i := range tests {
 		test := tests[i]
 		t.Run(test.name, func(t *testing.T) {
 			rollout := newRollout(test.rolloutSpecReplicas, test.setWeight, test.maxSurge, test.maxUnavailable, "canary", "stable", test.setCanaryScale, test.trafficRouting)
 			rollout.Status.PromoteFull = test.promoteFull
+			rollout.Spec.ScaleDownOnAbort = test.scaleDownOnAbort
+			rollout.Status.Abort = test.scaleDownOnAbort
 			stableRS := newRS("stable", test.stableSpecReplica, test.stableAvailableReplica)
 			canaryRS := newRS("canary", test.canarySpecReplica, test.canaryAvailableReplica)
 			newRSReplicaCount, stableRSReplicaCount := CalculateReplicaCountsForCanary(rollout, canaryRS, stableRS, []*appsv1.ReplicaSet{test.olderRS})