fix: Abort rollout doesn't remove all canary pods for setCanaryScale (argoproj#1352)

Signed-off-by: Chinmoy Samant <[email protected]>

Author: csamant-salesforce

test/e2e/istio/istio-rollout-abort-delete-all-canary-pods.yaml

@@ -0,0 +1,90 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: istio-host-split-canary
+spec:
+  ports:
+    - port: 80
+      targetPort: http
+      protocol: TCP
+      name: http
+  selector:
+    app: istio-host-split
+
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: istio-host-split-stable
+spec:
+  ports:
+    - port: 80
+      targetPort: http
+      protocol: TCP
+      name: http
+  selector:
+    app: istio-host-split
+
+---
+apiVersion: networking.istio.io/v1alpha3
+kind: VirtualService
+metadata:
+  name: istio-host-split-vsvc
+spec:
+  hosts:
+    - istio-host-split
+  http:
+    - name: primary
+      route:
+        - destination:
+            host: istio-host-split-stable
+          weight: 100
+        - destination:
+            host: istio-host-split-canary
+          weight: 0
+
+---
+apiVersion: argoproj.io/v1alpha1
+kind: Rollout
+metadata:
+  name: istio-host-split
+spec:
+  replicas: 5
+  strategy:
+    canary:
+      canaryService: istio-host-split-canary
+      stableService: istio-host-split-stable
+      trafficRouting:
+        istio:
+          virtualService:
+            name: istio-host-split-vsvc
+            routes:
+              - primary
+      steps:
+        - setCanaryScale:
+            replicas: 2
+        - setWeight: 20
+        - pause: {}
+        - setCanaryScale:
+            replicas: 4
+        - setWeight: 40
+        - pause: {}
+  selector:
+    matchLabels:
+      app: istio-host-split
+  template:
+    metadata:
+      labels:
+        app: istio-host-split
+    spec:
+      containers:
+        - name: istio-host-split
+          image: nginx:1.19-alpine
+          ports:
+            - name: http
+              containerPort: 80
+              protocol: TCP
+          resources:
+            requests:
+              memory: 16Mi
+              cpu: 5m

test/e2e/istio_test.go

@@ -197,3 +197,32 @@ func (s *IstioSuite) TestIstioSubsetSplitSingleRoute() {
 		}).
 		ExpectRevisionPodCount("1", 1) // don't scale down old replicaset since it will be within scaleDownDelay
 }
+
+func (s *IstioSuite) TestIstioAbortUpdateDeleteAllCanaryPods() {
+	s.Given().
+		RolloutObjects("@istio/istio-rollout-abort-delete-all-canary-pods.yaml").
+		When().
+		ApplyManifests().
+		WaitForRolloutStatus("Healthy").
+		Then().
+		When().
+		UpdateSpec().
+		WaitForRolloutStatus("Paused").
+		Then().
+		ExpectRevisionPodCount("2", 2).
+		When().
+		PromoteRollout().
+		WaitForRolloutStatus("Paused").
+		Then().
+		When().
+		PromoteRollout().
+		WaitForRolloutStatus("Paused").
+		Then().
+		ExpectRevisionPodCount("2", 4).
+		When().
+		AbortRollout().
+		WaitForRolloutStatus("Degraded").
+		Then().
+		ExpectRevisionPodCount("2", 0).
+		ExpectRevisionPodCount("1", 5)
+}

utils/replicaset/canary.go

@@ -331,6 +331,10 @@ func GetCurrentSetWeight(rollout *v1alpha1.Rollout) int32 {
 // TrafficRouting is required to be set for SetCanaryScale to be applicable.
 // If MatchTrafficWeight is set after a previous SetCanaryScale step, it will likewise be ignored.
 func UseSetCanaryScale(rollout *v1alpha1.Rollout) *v1alpha1.SetCanaryScale {
+	// Return nil when rollout is aborted
+	if rollout.Status.Abort {
+		return nil
+	}
 	currentStep, currentStepIndex := GetCurrentCanaryStep(rollout)
 	if currentStep == nil {
 		return nil