Use field AbortScaleDownDelaySeconds

Signed-off-by: Hui Kang <[email protected]>

manifests/crds/rollout-crd.yaml

@@ -64,8 +64,6 @@ spec:
               revisionHistoryLimit:
                 format: int32
                 type: integer
-              scaleDownOnAbort:
-                type: boolean
               selector:
                 properties:
                   matchExpressions:
@@ -93,6 +91,9 @@ spec:
                 properties:
                   blueGreen:
                     properties:
+                      abortScaleDownDelaySeconds:
+                        format: int32
+                        type: integer
                       activeMetadata:
                         properties:
                           annotations:
@@ -226,6 +227,9 @@ spec:
                     type: object
                   canary:
                     properties:
+                      abortScaleDownDelaySeconds:
+                        format: int32
+                        type: integer
                       analysis:
                         properties:
                           args:

manifests/install.yaml

@@ -9753,8 +9753,6 @@ spec:
               revisionHistoryLimit:
                 format: int32
                 type: integer
-              scaleDownOnAbort:
-                type: boolean
               selector:
                 properties:
                   matchExpressions:
@@ -9782,6 +9780,9 @@ spec:
                 properties:
                   blueGreen:
                     properties:
+                      abortScaleDownDelaySeconds:
+                        format: int32
+                        type: integer
                       activeMetadata:
                         properties:
                           annotations:
@@ -9915,6 +9916,9 @@ spec:
                     type: object
                   canary:
                     properties:
+                      abortScaleDownDelaySeconds:
+                        format: int32
+                        type: integer
                       analysis:
                         properties:
                           args:

manifests/namespace-install.yaml

@@ -9753,8 +9753,6 @@ spec:
               revisionHistoryLimit:
                 format: int32
                 type: integer
-              scaleDownOnAbort:
-                type: boolean
               selector:
                 properties:
                   matchExpressions:
@@ -9782,6 +9780,9 @@ spec:
                 properties:
                   blueGreen:
                     properties:
+                      abortScaleDownDelaySeconds:
+                        format: int32
+                        type: integer
                       activeMetadata:
                         properties:
                           annotations:
@@ -9915,6 +9916,9 @@ spec:
                     type: object
                   canary:
                     properties:
+                      abortScaleDownDelaySeconds:
+                        format: int32
+                        type: integer
                       analysis:
                         properties:
                           args:

pkg/apiclient/rollout/rollout.swagger.json

@@ -662,6 +662,11 @@
         "activeMetadata": {
           "$ref": "#/definitions/github.jparrowsec.cn.argoproj.argo_rollouts.pkg.apis.rollouts.v1alpha1.PodTemplateMetadata",
           "title": "ActiveMetadata specify labels and annotations which will be attached to the active pods for\nthe duration which they act as a active pod, and will be removed after"
+        },
+        "abortScaleDownDelaySeconds": {
+          "type": "integer",
+          "format": "int32",
+          "title": "AbortScaleDownDelaySeconds\n+optional"
         }
       },
       "title": "BlueGreenStrategy defines parameters for Blue Green deployment"
@@ -766,6 +771,11 @@
           "type": "integer",
           "format": "int32",
           "title": "ScaleDownDelayRevisionLimit limits the number of old RS that can run at one time before getting scaled down\n+optional"
+        },
+        "abortScaleDownDelaySeconds": {
+          "type": "integer",
+          "format": "int32",
+          "title": "AbortScaleDownDelaySeconds speficifes\n+optional"
         }
       },
       "title": "CanaryStrategy defines parameters for a Replica Based Canary"
@@ -1155,10 +1165,6 @@
         "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"

pkg/apis/rollouts/v1alpha1/types.go

@@ -71,8 +71,6 @@ 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) {
@@ -194,6 +192,9 @@ type BlueGreenStrategy struct {
 	// ActiveMetadata specify labels and annotations which will be attached to the active pods for
 	// the duration which they act as a active pod, and will be removed after
 	ActiveMetadata *PodTemplateMetadata `json:"activeMetadata,omitempty" protobuf:"bytes,13,opt,name=activeMetadata"`
+	// AbortScaleDownDelaySeconds
+	// +optional
+	AbortScaleDownDelaySeconds *int32 `json:"abortScaleDownDelaySeconds,omitempty" protobuf:"varint,14,opt,name=abortScaleDownDelaySeconds"`
 }
 
 // AntiAffinity defines which inter-pod scheduling rule to use for anti-affinity injection
@@ -274,6 +275,9 @@ type CanaryStrategy struct {
 	// ScaleDownDelayRevisionLimit limits the number of old RS that can run at one time before getting scaled down
 	// +optional
 	ScaleDownDelayRevisionLimit *int32 `json:"scaleDownDelayRevisionLimit,omitempty" protobuf:"varint,12,opt,name=scaleDownDelayRevisionLimit"`
+	// AbortScaleDownDelaySeconds speficifes
+	// +optional
+	AbortScaleDownDelaySeconds *int32 `json:"abortScaleDownDelaySeconds,omitempty" protobuf:"varint,13,opt,name=abortScaleDownDelaySeconds"`
 }
 
 // ALBTrafficRouting configuration for ALB ingress controller to control traffic routing

rollout/replicaset.go

@@ -38,6 +38,31 @@ func (c *rolloutContext) removeScaleDownDelay(rs *appsv1.ReplicaSet) error {
 	return err
 }
 
+// addScaleDownDelay injects the `scale-down-deadline` annotation to the ReplicaSet, or if
+// scaleDownDelaySeconds is zero, removes it if it exists
+func (c *rolloutContext) addScaleDownDelay2(rs *appsv1.ReplicaSet, scaleDownDelaySeconds time.Duration) error {
+	if rs == nil {
+		return nil
+	}
+	ctx := context.TODO()
+	// scaleDownDelaySeconds := time.Duration(defaults.GetScaleDownDelaySecondsOrDefault(c.rollout))
+	if scaleDownDelaySeconds == 0 {
+		// If scaledown deadline is zero, it means we need to remove any replicasets with the delay
+		// This might happen if we switch from canary with traffic routing to basic canary
+		if replicasetutil.HasScaleDownDeadline(rs) {
+			return c.removeScaleDownDelay(rs)
+		}
+		return nil
+	}
+	deadline := metav1.Now().Add(scaleDownDelaySeconds * time.Second).UTC().Format(time.RFC3339)
+	patch := fmt.Sprintf(addScaleDownAtAnnotationsPatch, v1alpha1.DefaultReplicaSetScaleDownDeadlineAnnotationKey, deadline)
+	_, err := c.kubeclientset.AppsV1().ReplicaSets(rs.Namespace).Patch(ctx, rs.Name, patchtypes.JSONPatchType, []byte(patch), metav1.PatchOptions{})
+	if err == nil {
+		c.log.Infof("Set '%s' annotation on '%s' to %s (%ds)", v1alpha1.DefaultReplicaSetScaleDownDeadlineAnnotationKey, rs.Name, deadline, scaleDownDelaySeconds)
+	}
+	return err
+}
+
 // addScaleDownDelay injects the `scale-down-deadline` annotation to the ReplicaSet, or if
 // scaleDownDelaySeconds is zero, removes it if it exists
 func (c *rolloutContext) addScaleDownDelay(rs *appsv1.ReplicaSet) error {
@@ -91,11 +116,14 @@ func (c *Controller) getReplicaSetsForRollouts(r *v1alpha1.Rollout) ([]*appsv1.R
 	return cm.ClaimReplicaSets(rsList)
 }
 
-// removeScaleDownDelays removes the scale-down-deadline annotation from the new/stable ReplicaSets,
+// removeScaleDownDeadlines removes the scale-down-deadline annotation from the new/stable ReplicaSets,
 // in the event that we moved back to an older revision that is still within its scaleDownDelay.
 func (c *rolloutContext) removeScaleDownDeadlines() error {
 	var toRemove []*appsv1.ReplicaSet
-	if c.newRS != nil {
+
+	abortScaleDownDelaySeconds := time.Duration(defaults.GetAbortScaleDownDelaySecondsOrDefault(c.rollout))
+	if c.newRS != nil && (c.pauseContext == nil || !c.pauseContext.IsAborted() || abortScaleDownDelaySeconds == 0) {
+		c.log.Info("hkang removeScaleDownDeadlines")
 		toRemove = append(toRemove, c.newRS)
 	}
 	if c.stableRS != nil {
@@ -121,12 +149,47 @@ func (c *rolloutContext) reconcileNewReplicaSet() (bool, error) {
 		return false, err
 	}
 
-	if c.pauseContext != nil && c.pauseContext.IsAborted() && c.rollout.Spec.ScaleDownOnAbort {
-		c.log.Info("scale down on abort")
-		newReplicasCount = int32(0)
+	abortScaleDownDelaySeconds := time.Duration(defaults.GetAbortScaleDownDelaySecondsOrDefault(c.rollout))
+	if c.pauseContext != nil && c.pauseContext.IsAborted() && abortScaleDownDelaySeconds > 0 {
+		c.log.Info("scale down on abort for the newRS")
+		// newReplicasCount = int32(0)
+
+		// if the newRS has scale down annotation and to be scaled down now, set
+		// Update newReplicasCount = int32(0)
+		if scaleDownAtStr, ok := c.newRS.Annotations[v1alpha1.DefaultReplicaSetScaleDownDeadlineAnnotationKey]; ok {
+			c.log.Info("scaledown annotation exists: ", scaleDownAtStr)
+
+			c.log.Infof("New rs '%s' has scaledown delay annotation", c.newRS.Name)
+			scaleDownAtTime, err := time.Parse(time.RFC3339, scaleDownAtStr)
+			if err != nil {
+				c.log.Warnf("Unable to read scaleDownAt label on rs '%s'", c.newRS.Name)
+			} else {
+				now := metav1.Now()
+				scaleDownAt := metav1.NewTime(scaleDownAtTime)
+				if scaleDownAt.After(now.Time) {
+					c.log.Infof("RS '%s' has not reached the scaleDownTime", c.newRS.Name)
+					remainingTime := scaleDownAt.Sub(now.Time)
+					if remainingTime < c.resyncPeriod {
+						c.enqueueRolloutAfter(c.rollout, remainingTime)
+						return false, nil
+					}
+				} else {
+					c.log.Infof("RS '%s' has reached the scaleDownTime", c.newRS.Name)
+					newReplicasCount = int32(0)
+				}
+			}
+		} else {
+			c.log.Info("Add scale down annotation")
+			err = c.addScaleDownDelay2(c.newRS, abortScaleDownDelaySeconds)
+			if err != nil {
+				return false, err
+			}
+		}
 	}
+	c.log.Info("hkang newReplicasCount:", newReplicasCount)
 
 	scaled, _, err := c.scaleReplicaSetAndRecordEvent(c.newRS, newReplicasCount)
+	c.log.Info("hkang after scaleReplicaSetAndRecordEvent, scaled:", scaled)
 	return scaled, err
 }
 

rollout/replicaset_test.go

@@ -121,40 +121,55 @@ func TestGetReplicaSetsForRollouts(t *testing.T) {
 
 func TestReconcileNewReplicaSet(t *testing.T) {
 	tests := []struct {
-		name                string
-		rolloutReplicas     int
-		newReplicas         int
-		scaleExpected       bool
-		ScaleDownOnAbort    bool
-		expectedNewReplicas int
+		name            string
+		rolloutReplicas int
+		newReplicas     int
+		scaleExpected   bool
+		// ScaleDownOnAbort    bool
+		abortScaleDownDelaySeconds int32
+		abortScaleDownTrigger      bool
+		expectedNewReplicas        int
 	}{
+		/*
+			{
+				name:            "New Replica Set matches rollout replica: No scale",
+				rolloutReplicas: 10,
+				newReplicas:     10,
+				scaleExpected:   false,
+			},
+			{
+				name:                "New Replica Set higher than rollout replica: Scale down",
+				rolloutReplicas:     10,
+				newReplicas:         12,
+				scaleExpected:       true,
+				expectedNewReplicas: 10,
+			},
+			{
+				name:                "New Replica Set lower than rollout replica: Scale up",
+				rolloutReplicas:     10,
+				newReplicas:         8,
+				scaleExpected:       true,
+				expectedNewReplicas: 10,
+			},
+		*/
 		{
-			name:            "New Replica Set matches rollout replica: No scale",
+			name:            "New Replica scaled down to 0: scale down on abort - triggered",
 			rolloutReplicas: 10,
 			newReplicas:     10,
-			scaleExpected:   false,
+			// ScaleDownOnAbort:           true,
+			abortScaleDownDelaySeconds: 5,
+			abortScaleDownTrigger:      true,
+			scaleExpected:              true,
+			expectedNewReplicas:        0,
 		},
 		{
-			name:                "New Replica Set higher than rollout replica: Scale down",
-			rolloutReplicas:     10,
-			newReplicas:         12,
-			scaleExpected:       true,
-			expectedNewReplicas: 10,
-		},
-		{
-			name:                "New Replica Set lower than rollout replica: Scale up",
-			rolloutReplicas:     10,
-			newReplicas:         8,
-			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,
+			name:                       "New Replica scaled down to 0: scale down on abort - add annotation",
+			rolloutReplicas:            10,
+			newReplicas:                10,
+			abortScaleDownDelaySeconds: 5,
+			abortScaleDownTrigger:      false,
+			scaleExpected:              false,
+			expectedNewReplicas:        0,
 		},
 	}
 	for i := range tests {
@@ -175,12 +190,22 @@ func TestReconcileNewReplicaSet(t *testing.T) {
 					recorder:          record.NewFakeEventRecorder(),
 				},
 			}
-			if test.ScaleDownOnAbort {
+			if test.abortScaleDownDelaySeconds > 0 {
 				roCtx.pauseContext = &pauseContext{
 					rollout: rollout,
 				}
 				rollout.Status.Abort = true
-				rollout.Spec.ScaleDownOnAbort = true
+				// rollout.Spec.ScaleDownOnAbort = true
+				rollout.Spec.Strategy = v1alpha1.RolloutStrategy{
+					BlueGreen: &v1alpha1.BlueGreenStrategy{
+						AbortScaleDownDelaySeconds: &test.abortScaleDownDelaySeconds,
+					},
+				}
+
+				if test.abortScaleDownTrigger {
+					deadline := metav1.Now().Add(-time.Duration(test.abortScaleDownDelaySeconds) * time.Second).UTC().Format(time.RFC3339)
+					roCtx.newRS.Annotations[v1alpha1.DefaultReplicaSetScaleDownDeadlineAnnotationKey] = deadline
+				}
 			}
 
 			scaled, err := roCtx.reconcileNewReplicaSet()