test: add unit tests for maxSurge=0, replicas=1 (#3375)

* tests: add unit tests for maxSurge=0, replicas=1 Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * update doc Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * update test name Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> --------- Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
argoproj · Feb 15, 2024 · 36c8009 · 36c8009
1 parent 47deb7a
commit 36c8009
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 11 deletions.
diff --git a/docs/features/canary.md b/docs/features/canary.md
@@ -1,13 +1,13 @@
 # Canary Deployment Strategy
-A canary rollout is a deployment strategy where the operator releases a new version of their application to a small percentage of the production traffic. 
+A canary rollout is a deployment strategy where the operator releases a new version of their application to a small percentage of the production traffic.
 
 ## Overview
 Since there is no agreed upon standard for a canary deployment, the rollouts controller allows users to outline how they want to run their canary deployment. Users can define a list of steps the controller uses to manipulate the ReplicaSets when there is a change to the `.spec.template`. Each step will be evaluated before the new ReplicaSet is promoted to the stable version, and the old version is completely scaled down.
 
 Each step can have one of two fields. The `setWeight` field dictates the percentage of traffic that should be sent to the canary, and the `pause` struct instructs the rollout to pause.  When the controller reaches a `pause` step for a rollout, it will add a `PauseCondition` struct to the `.status.PauseConditions` field. If the `duration` field within the `pause` struct is set, the rollout will not progress to the next step until it has waited for the value of the `duration` field. Otherwise, the rollout will wait indefinitely until that Pause condition is removed. By using the `setWeight` and the `pause` fields, a user can declaratively describe how they want to progress to the new version. Below is an example of a canary strategy.
 
 !!! important
-    If the canary Rollout does not use [traffic management](traffic-management/index.md), the Rollout makes a best effort attempt to achieve the percentage listed in the last `setWeight` step between the new and old version. For example, if a Rollout has 10 Replicas and 10% for the first `setWeight` step, the controller will scale the new desired ReplicaSet to 1 replicas and the old stable ReplicaSet to 9. In the case where the setWeight is 15%, the Rollout attempts to get there by rounding up the calculation (i.e. the new ReplicaSet has 2 pods since 15% of 10 rounds up to 2 and the old ReplicaSet has 9 pods since 85% of 10 rounds up to 9). If a user wants to have more fine-grained control of the percentages without a large number of Replicas, that user should use the  [traffic management](#trafficrouting) functionality.
+    If the canary Rollout does not use [traffic management](traffic-management/index.md), the Rollout makes a best effort attempt to achieve the percentage listed in the last `setWeight` step between the new and old version. For example, if a Rollout has 10 Replicas and 10% for the first `setWeight` step, the controller will scale the new desired ReplicaSet to 1 replicas and the old stable ReplicaSet to 9. In the case where the setWeight is 41%, the Rollout attempts to get there by finding the whole number with the smallest delta, rounding up the calculation if the deltas are equals (i.e. the new ReplicaSet has 4 pods since 41% of 10 is closer to 4/10 than 5/10, and the old ReplicaSet has 6 pods). If a user wants to have more fine-grained control of the percentages without a large number of Replicas, that user should use the  [traffic management](#trafficrouting) functionality.
 
 ## Example
 ```yaml
@@ -59,7 +59,7 @@ spec:
         - pause: {}                # pause indefinitely
 ```
 
-If no `duration` is specified for a pause step, the rollout will be paused indefinitely. To unpause, use the [argo kubectl plugin](kubectl-plugin.md) `promote` command. 
+If no `duration` is specified for a pause step, the rollout will be paused indefinitely. To unpause, use the [argo kubectl plugin](kubectl-plugin.md) `promote` command.
 
 ```shell
 # promote to the next step

diff --git a/utils/replicaset/canary.go b/utils/replicaset/canary.go
@@ -56,11 +56,12 @@ func AtDesiredReplicaCountsForCanary(ro *v1alpha1.Rollout, newRS, stableRS *apps
 // when using the basic canary strategy. The function calculates the desired number of replicas for
 // the new and stable RS using the following equations:
 //
-// newRS Replica count = spec.Replica * (setweight / 100)
-// stableRS Replica count = spec.Replica * (1 - setweight / 100)
+// desired newRS Replica count = spec.Replica * (setweight / 100)
+// desired stableRS Replica count = spec.Replica - newRS
+//
+// The function for newRS finds the closest whole number of replicas based on the weight percentage
+// and rounds up the desired replica count in case of a tie.
 //
-// In both equations, the function rounds the desired replica count up if the math does not divide into whole numbers
-// because the rollout guarantees at least one replica for both the stable and new RS when the setWeight is not 0 or 100.
 // Then, the function finds the number of replicas it can scale up using the following equation:
 //
 // scaleUpCount := (maxSurge + rollout.Spec.Replica) - sum of rollout's RSs spec.Replica

diff --git a/utils/replicaset/canary_test.go b/utils/replicaset/canary_test.go
@@ -276,7 +276,7 @@ func TestCalculateReplicaCountsForCanary(t *testing.T) {
 			expectedCanaryReplicaCount: 10,
 		},
 		{
-			name:                "Do not scale newRS down to zero on non-zero weight",
+			name:                "Do not scale canaryRS down to zero on non-zero weight",
 			rolloutSpecReplicas: 1,
 			setWeight:           20,
 			maxSurge:            intstr.FromInt(1),
@@ -308,7 +308,55 @@ func TestCalculateReplicaCountsForCanary(t *testing.T) {
 			expectedCanaryReplicaCount: 1,
 		},
 		{
-			name:                "Scale up Stable before newRS",
+			name:                "Scale canaryRS to zero on <50 weight without surge",
+			rolloutSpecReplicas: 1,
+			setWeight:           49,
+			maxSurge:            intstr.FromInt(0),
+			maxUnavailable:      intstr.FromInt(1),
+
+			stableSpecReplica:      1,
+			stableAvailableReplica: 1,
+
+			canarySpecReplica:      0,
+			canaryAvailableReplica: 0,
+
+			expectedStableReplicaCount: 1,
+			expectedCanaryReplicaCount: 0,
+		},
+		{
+			name:                "Scale stableRS down to zero on >=50 weight without surge",
+			rolloutSpecReplicas: 1,
+			setWeight:           51,
+			maxSurge:            intstr.FromInt(0),
+			maxUnavailable:      intstr.FromInt(1),
+
+			stableSpecReplica:      1,
+			stableAvailableReplica: 1,
+
+			canarySpecReplica:      0,
+			canaryAvailableReplica: 0,
+
+			expectedStableReplicaCount: 0,
+			expectedCanaryReplicaCount: 0,
+		},
+		{
+			name:                "Scale canaryRS to one on >=50 weight without surge and stable replicas",
+			rolloutSpecReplicas: 1,
+			setWeight:           51,
+			maxSurge:            intstr.FromInt(0),
+			maxUnavailable:      intstr.FromInt(1),
+
+			stableSpecReplica:      0,
+			stableAvailableReplica: 0,
+
+			canarySpecReplica:      0,
+			canaryAvailableReplica: 0,
+
+			expectedStableReplicaCount: 0,
+			expectedCanaryReplicaCount: 1,
+		},
+		{
+			name:                "Scale up Stable before canaryRS",
 			rolloutSpecReplicas: 10,
 			setWeight:           30,
 			maxSurge:            intstr.FromInt(1),
@@ -326,7 +374,7 @@ func TestCalculateReplicaCountsForCanary(t *testing.T) {
 			olderRS: newRS("older", 3, 3),
 		},
 		{
-			name:                "Scale down newRS and stable",
+			name:                "Scale down canaryRS and stable",
 			rolloutSpecReplicas: 10,
 			setWeight:           30,
 			maxSurge:            intstr.FromInt(0),
@@ -358,7 +406,7 @@ func TestCalculateReplicaCountsForCanary(t *testing.T) {
 			expectedCanaryReplicaCount: 9,
 		},
 		{
-			name:                "Do not scale down newRS or stable when older RS count >= scaleDownCount",
+			name:                "Do not scale down canaryRS or stable when older RS count >= scaleDownCount",
 			rolloutSpecReplicas: 10,
 			setWeight:           30,
 			maxSurge:            intstr.FromInt(0),