Skip to content

fix: skip canary steps and fast-promote when spec.replicas is 0 - #4950

Draft
zachaller wants to merge 2 commits into
argoproj:masterfrom
zachaller:scale0-fix
Draft

fix: skip canary steps and fast-promote when spec.replicas is 0#4950
zachaller wants to merge 2 commits into
argoproj:masterfrom
zachaller:scale0-fix

Conversation

@zachaller

Copy link
Copy Markdown
Collaborator

Summary

When a canary Rollout has spec.replicas: 0 and a new version is deployed, the controller now skips canary steps and immediately promotes the new ReplicaSet as stable. Previously, percentage-based steps were no-ops but explicit steps still ran — notably pause could suspend the rollout indefinitely, and setCanaryScale.replicas could spawn unexpected pods (e.g. scaling to 1 while the rollout was scaled down).
This matches the expected behavior for scaled-down workloads: with no pods to validate, canary progression adds no value. The new version becomes stable so that traffic routing (VirtualServices, Services, etc.) is already pointed at the correct hash when replicas are scaled back up.

Changes

  • shouldFullPromote: when spec.replicas == 0 and an update is in progress, promote immediately with reason "Zero replicas - skipping canary steps"
  • reconcileCanaryPause: skip pause steps at zero replicas
  • CalculateReplicaCountsForTrafficRoutedCanary: ignore explicit setCanaryScale.replicas when spec.replicas == 0
  • Template-change path: promote in the same reconcile when the pod-template-change fast path fires at zero replicas
    Only applies when spec.replicas is explicitly set to 0 (not nil/default).

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional with a list of types and scopes found here, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I've signed my commits with DCO
  • My builds are green. Try syncing with master if they are not.
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • I have run all tests locally (including the flaky ones) and they pass on my workstation
  • I have used LLM/AI/Agent tools for this PR but I am responsible for all code of this PR
  • I understand what the code does and WHY/HOW it works in several scenarios
  • I know if my code is just adding new functionality or changing old functionality for existing users
  • My organization is added to USERS.md.

Signed-off-by: Zach Aller <zachaller@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Published E2E Test Results

  4 files    4 suites   4h 12m 17s ⏱️
149 tests 140 ✅  7 💤 2 ❌
604 runs  568 ✅ 28 💤 8 ❌

For more details on these failures, see this check.

Results for commit f208dd5.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Published Unit Test Results

2 627 tests   2 627 ✅  3m 31s ⏱️
  131 suites      0 💤
    1 files        0 ❌

Results for commit f208dd5.

♻️ This comment has been updated with latest results.

Signed-off-by: Zach Aller <zachaller@users.noreply.github.com>
@sonarqubecloud

sonarqubecloud Bot commented Aug 4, 2026

Copy link
Copy Markdown

@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.56522% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.14%. Comparing base (fdce048) to head (f208dd5).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
rollout/canary.go 28.57% 4 Missing and 1 partial ⚠️
rollout/sync.go 81.81% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4950      +/-   ##
==========================================
- Coverage   85.15%   85.14%   -0.01%     
==========================================
  Files         166      166              
  Lines       19437    19458      +21     
==========================================
+ Hits        16551    16567      +16     
- Misses       2031     2035       +4     
- Partials      855      856       +1     
Flag Coverage Δ
e2e 52.97% <43.47%> (-0.11%) ⬇️
unit-tests 81.62% <69.56%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread rollout/context.go
}

if isScalingEvent {
if isScalingEvent && !c.needsZeroReplicaFastTrackReconcile() {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a better way to do this that does not block scaling only reconciles

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant