fix(controller): caps canary replicas for an aborted rollout to its Traffic Weight - #4974
fix(controller): caps canary replicas for an aborted rollout to its Traffic Weight#4974Ghost-B0t wants to merge 2 commits into
Conversation
…t Distribution Signed-off-by: Rahul <93730014+Ghost-B0t@users.noreply.github.com>
Signed-off-by: Rahul <93730014+Ghost-B0t@users.noreply.github.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4974 +/- ##
==========================================
- Coverage 85.17% 85.17% -0.01%
==========================================
Files 166 166
Lines 19453 19457 +4
==========================================
+ Hits 16570 16572 +2
- Misses 2030 2031 +1
- Partials 853 854 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Published E2E Test Results 4 files 4 suites 4h 8m 36s ⏱️ For more details on these failures, see this check. Results for commit 87b95a8. |
Published Unit Test Results2 632 tests 2 632 ✅ 3m 30s ⏱️ Results for commit 87b95a8. |
|
@zachaller We are facing this issue in our production system due to frequent HPA triggers. As we are using service mesh the impact is getting multiplied for the same due to very frequent endpoints update choking the service mesh control plane. |



Fixes: #4973
Checklist:
"fix(controller): Updates such and such. Fixes #1234".Summary
An aborted rollout that uses
dynamicStableScale: trueprematurely scales up the canary replicaset on any scaleup of rollout.Spec.Replicas(scaled due to HPA/Keda, or manually).Root cause
GetDesiredCanaryWeightin utils/replicaset/canary.go infers the canary's size from the stable ReplicaSet's shortfall:This assumes the only reason stable is below spec.replicas is that the canary still holds those replicas. That assumption breaks whenever the shortfall has any other cause. The resulting non-zero weight is converted back to a replica count and the aborted ReplicaSet is scaled to it.
Worked example, spec.replicas 10 → 12, steps [5, 10, 20, 50, 100], stable at 10 available, canary at 0:
expectedCanaryReplicas = 12 - 10 = 2
canaryReplicas = max(2, 0) = 2
reverse scan: 100 -> 12 , 50 -> 6, 20 -> 3, 10 -> 2, 5 -> 1 < 2 -> weight 5
canary replicas = ceil(5% × 12) = 1
It reverts once stable reports 11 available: the gap becomes 1, no step qualifies, weight returns to 0, canary back to 0.
A large scale-up or a significant availability dip can therefore resurrect most of an aborted ReplicaSet.
Fix
utils/replicaset/canary.go: InCalculateReplicaCountsForTrafficRoutedCanary, whendynamicStableScaleis enabled, add a ceiling cap of its assigned traffic weight and add checks to skip the cases where we need to scale the canary above its traffic boundry, such as full-promote, rollback & whenabortScaleDownDelaySecondsis set to 0.Compared to the progressive abort behaviour (#4035): Abort with
dynamicStableScalereverse scans the canary steps finding the canary replica count for the previous step and progressively making it to 0. But it also leads to re-evaluation of these steps for an aborted rollout(canary: 0) resulting in the scaleup of aborted canary. This PR instead caps the canary scale up to its traffic distribution and preserves the progressive scale down of canary pods during an abort.Testing
TestAbortedCanaryNotScaledUpOnStableAvailabilityGap: fails on master withthe scaleup of canary replicas, passes with the fix. No-delay and zero-delay behavior pinned.
go test ./rollout/... ./utils/replicaset/...passes unchanged.