fix(controller): prevent negative vsvc weights on a replica scaledown following a canary abort for istio trafficrouting

rollout/trafficrouting.go

@@ -305,7 +305,7 @@ func (c *rolloutContext) calculateDesiredWeightOnAbortOrStableRollback() int32 {
 	}
 	// When using dynamic stable scaling, we must dynamically decreasing the weight to the canary
 	// according to the availability of the stable (whatever it can support).
-	desiredWeight := weightutil.MaxTrafficWeight(c.rollout) - ((weightutil.MaxTrafficWeight(c.rollout) * c.stableRS.Status.AvailableReplicas) / *c.rollout.Spec.Replicas)
+	desiredWeight := maxInt(0, weightutil.MaxTrafficWeight(c.rollout)-((weightutil.MaxTrafficWeight(c.rollout)*c.stableRS.Status.AvailableReplicas) / *c.rollout.Spec.Replicas))
 	if c.rollout.Status.Canary.Weights != nil {
 		// This ensures that if we are already at a lower weight, then we will not
 		// increase the weight because stable availability is flapping (e.g. pod restarts)

test/e2e/istio/istio-subset-split-in-stable-downscale-after-canary-abort.yaml

@@ -0,0 +1,101 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: istio-subset-split-in-stable-downscale-after-canary-abort
+spec:
+  ports:
+  - port: 80
+    targetPort: http
+    protocol: TCP
+    name: http
+  selector:
+    app: istio-subset-split-in-stable-downscale-after-canary-abort
+
+---
+apiVersion: networking.istio.io/v1alpha3
+kind: VirtualService
+metadata:
+  name: istio-subset-split-in-stable-downscale-after-canary-abort-vsvc
+spec:
+  hosts:
+  - istio-subset-split-in-stable-downscale-after-canary-abort
+  http:
+  - route:
+    - destination:
+        host: istio-subset-split-in-stable-downscale-after-canary-abort
+        subset: stable
+      weight: 100
+    - destination:
+        host: istio-subset-split-in-stable-downscale-after-canary-abort
+        subset: canary
+      weight: 0
+
+---
+apiVersion: networking.istio.io/v1alpha3
+kind: DestinationRule
+metadata:
+  name: istio-subset-split-in-stable-downscale-after-canary-abort-destrule
+spec:
+  host: istio-subset-split-in-stable-downscale-after-canary-abort
+  subsets:
+  - name: stable
+    labels:
+      app: istio-subset-split-in-stable-downscale-after-canary-abort
+  - name: canary
+    labels:
+      app: istio-subset-split-in-stable-downscale-after-canary-abort
+
+---
+apiVersion: argoproj.io/v1alpha1
+kind: Rollout
+metadata:
+  name: istio-subset-split-in-stable-downscale-after-canary-abort
+spec:
+  replicas: 4
+  strategy:
+    canary:
+      dynamicStableScale: true
+      trafficRouting:
+        istio:
+          virtualService: 
+            name: istio-subset-split-in-stable-downscale-after-canary-abort-vsvc
+          destinationRule:
+            name: istio-subset-split-in-stable-downscale-after-canary-abort-destrule
+            canarySubsetName: canary
+            stableSubsetName: stable
+      canaryMetadata:
+        labels:
+          role: canary
+      stableMetadata:
+        labels:
+          role: stable
+      maxSurge: "25%"
+      maxUnavailable: "20%"
+      steps:
+        - setWeight: 10
+        - pause: {}
+        - setWeight: 20
+        - pause: {}
+        - setWeight: 30
+        - pause: {}          
+        - setWeight: 50
+        - pause: {}
+  selector:
+    matchLabels:
+      app: istio-subset-split-in-stable-downscale-after-canary-abort
+  template:
+    metadata:
+      labels:
+        app: istio-subset-split-in-stable-downscale-after-canary-abort
+    spec:
+      containers:
+      - name: istio-subset-split-in-stable-downscale-after-canary-abort
+        image: nginx:1.19-alpine
+        ports:
+        - name: http
+          containerPort: 80
+          protocol: TCP
+        resources:
+          requests:
+            memory: 16Mi
+            cpu: 5m

test/e2e/istio_test.go

@@ -492,3 +492,29 @@ func (s *IstioSuite) TestIstioSubsetSplitExperimentStep() {
 
 	s.TearDownSuite()
 }
+
+func (s *IstioSuite) TestIstioSubsetSplitInStableDownscaleAfterCanaryAbort() {
+	s.Given().
+		RolloutObjects("@istio/istio-subset-split-in-stable-downscale-after-canary-abort.yaml").
+		When().
+		ApplyManifests().
+		WaitForRolloutStatus("Healthy").
+		PromoteRolloutFull().
+		UpdateSpec().
+		WaitForRolloutStatus("Paused").
+		AbortRollout().
+		WaitForRolloutStatus("Degraded").
+		ScaleRollout(1).
+		WaitForRolloutReplicas(1).
+		Then().
+		Assert(func(t *fixtures.Then) {
+			vsvc := t.GetVirtualService()
+			stableWeight := vsvc.Spec.HTTP[0].Route[0].Weight
+			canaryWeight := vsvc.Spec.HTTP[0].Route[1].Weight
+
+			assert.Equal(s.T(), int64(0), canaryWeight)
+			assert.Equal(s.T(), int64(100), stableWeight)
+		})
+
+	s.TearDownSuite()
+}