ignore canary-min-replicas if it is > rollout.Spec.Replicas

Signed-off-by: Shlomo Sanders <[email protected]>
argoproj · Nov 29, 2022 · bfb6bac · bfb6bac
1 parent 7f0a23f
commit bfb6bac
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 7 deletions.
diff --git a/utils/replicaset/canary.go b/utils/replicaset/canary.go
@@ -25,8 +25,8 @@ func allDesiredAreCreated(rs *appsv1.ReplicaSet, desired int32) bool {
 	return rs != nil && desired == *rs.Spec.Replicas && desired == rs.Status.Replicas
 }
 
-func checkMinPods(replicas int32) int32 {
-	if replicas > 0 {
+func checkMinPods(rollout *v1alpha1.Rollout, replicas int32) int32 {
+	if replicas > 0 && rollout.Spec.Replicas != nil && *rollout.Spec.Replicas >= defaults.GetDefaultCanaryMinReplicas() {
 		replicas = max(replicas, defaults.GetDefaultCanaryMinReplicas())
 	}
 	return replicas
@@ -149,7 +149,7 @@ func CalculateReplicaCountsForBasicCanary(rollout *v1alpha1.Rollout, newRS *apps
 	if GetReplicaCountForReplicaSets(oldRSs) > 0 {
 		// If any older ReplicaSets exist, we should scale those down first, before even considering
 		// scaling down the newRS or stableRS
-		return checkMinPods(newRSReplicaCount), checkMinPods(stableRSReplicaCount)
+		return checkMinPods(rollout, newRSReplicaCount), checkMinPods(rollout, stableRSReplicaCount)
 	}
 
 	minAvailableReplicaCount := rolloutSpecReplica - MaxUnavailable(rollout)
@@ -161,7 +161,7 @@ func CalculateReplicaCountsForBasicCanary(rollout *v1alpha1.Rollout, newRS *apps
 	replicasToScaleDown := GetReplicasForScaleDown(newRS, !isIncreasing) + GetReplicasForScaleDown(stableRS, isIncreasing)
 	if replicasToScaleDown <= minAvailableReplicaCount {
 		// Cannot scale down stableRS or newRS without going below min available replica count
-		return checkMinPods(newRSReplicaCount), checkMinPods(stableRSReplicaCount)
+		return checkMinPods(rollout, newRSReplicaCount), checkMinPods(rollout, stableRSReplicaCount)
 	}
 
 	scaleDownCount := replicasToScaleDown - minAvailableReplicaCount
@@ -174,7 +174,7 @@ func CalculateReplicaCountsForBasicCanary(rollout *v1alpha1.Rollout, newRS *apps
 		stableRSReplicaCount = calculateScaleDownReplicaCount(stableRS, desiredStableRSReplicaCount, scaleDownCount, stableRSReplicaCount)
 		stableRSReplicaCount, newRSReplicaCount = adjustReplicaWithinLimits(stableRS, newRS, stableRSReplicaCount, newRSReplicaCount, maxReplicaCountAllowed, minAvailableReplicaCount)
 	}
-	return checkMinPods(newRSReplicaCount), checkMinPods(stableRSReplicaCount)
+	return checkMinPods(rollout, newRSReplicaCount), checkMinPods(rollout, stableRSReplicaCount)
 }
 
 // approximateWeightedCanaryStableReplicaCounts approximates the desired canary weight and returns
@@ -336,7 +336,7 @@ func CalculateReplicaCountsForTrafficRoutedCanary(rollout *v1alpha1.Rollout, wei
 	if !rollout.Spec.Strategy.Canary.DynamicStableScale {
 		// Not using dynamic stable scaling. Stable should be left fully scaled (100%), and canary
 		// will be calculated from setWeight
-		return checkMinPods(canaryCount), rolloutSpecReplica
+		return checkMinPods(rollout, canaryCount), rolloutSpecReplica
 	}
 
 	// When using dynamic stable scaling, the stable replica count is calculated from the higher of:
@@ -361,7 +361,7 @@ func CalculateReplicaCountsForTrafficRoutedCanary(rollout *v1alpha1.Rollout, wei
 			canaryCount = max(trafficWeightReplicaCount, canaryCount)
 		}
 	}
-	return checkMinPods(canaryCount), checkMinPods(stableCount)
+	return checkMinPods(rollout, canaryCount), checkMinPods(rollout, stableCount)
 }
 
 // trafficWeightToReplicas returns the appropriate replicas given the full spec.replicas and a weight

diff --git a/utils/replicaset/canary_test.go b/utils/replicaset/canary_test.go
@@ -821,6 +821,14 @@ func TestCalculateReplicaCountsForCanaryWithMinPods(t *testing.T) {
 	newRSReplicaCount, stableRSReplicaCount = CalculateReplicaCountsForTrafficRoutedCanary(rollout, rollout.Status.Canary.Weights)
 	assert.Equal(t, int32(2), newRSReplicaCount)
 	assert.Equal(t, int32(10), stableRSReplicaCount)
+
+	// Test that we don't use minPods when desired replicas < minPods
+	rollout = newRollout(1, 1, intstr.FromInt(0), intstr.FromInt(1), "canary", "stable", nil, nil)
+	rollout.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{}
+	newRSReplicaCount, stableRSReplicaCount = CalculateReplicaCountsForTrafficRoutedCanary(rollout, rollout.Status.Canary.Weights)
+	assert.Equal(t, int32(1), newRSReplicaCount)
+	assert.Equal(t, int32(1), stableRSReplicaCount)
+
 	defaults.SetDefaultCanaryMinReplicas(minPods) // Restore initial value
 }