fix(controller): re-drain workloadRef Deployment on later revisions. Fixes #4935

[michaelgriskified]

Fixes #4935.

The bug

With spec.workloadRef.scaleDown: progressively, the drain of the referenced Deployment was gated on revision == 1. If anything scales the Deployment back up after the initial migration, nothing ever drains it again:

1. Revision 1 works as documented — Rollout scales to N, controller drains the Deployment to 0.
2. A GitOps controller re-applies the Deployment manifest (which still declares replicas: N in git) on the next sync, or an operator scales it by hand. spec.replicas field ownership moves away from the rollouts controller.
3. Every later revision reconciles with revision >= 2, so the gate never matches. Rollout stable RS at N and Deployment RS at N, permanently, for a service sized at N.

Nothing reports unhealthy: the Rollout is Healthy, the pods are Ready, the GitOps app is Synced. The only symptom is doubled capacity. Recovery requires deleting the Rollout and re-migrating, or patching the Deployment by hand.

For Argo CD users specifically, ignoreDifferences on the Deployment's replicas is not enough — it only suppresses the diff, while the apply still writes git's value. Avoiding step 2 requires the separate RespectIgnoreDifferences=true sync option (or omitting replicas from the manifest under server-side apply), and there is no in-cluster signal that you got it wrong until you notice the bill.

The change

Drain to 0 whenever the Rollout is Healthy, on any revision, instead of only on revision 1. The controller then converges on the documented end state (as the Rollout is scaled up the Deployment is scaled down) rather than getting exactly one chance at it. This also resolves the case the removed NOTE described — a failed revision 1 whose fix lands in revision 2+.

The availability branch (scaling the Deployment to spec.replicas - status.readyReplicas while the Rollout is not yet Healthy) stays scoped to revision == 1. That branch exists to hold total capacity steady during the initial handover; on a later revision the stable ReplicaSet already serves that capacity, so running it there would add capacity — and would resurrect the Deployment mid-canary on every update. There's a new test case pinning that.

scaleDeployment already no-ops when the Deployment is at the target replica count, so a steady-state Healthy Rollout costs one extra GET per reconcile and no writes — the same cost revision-1 Rollouts already pay today.

Behavior change for existing users

This changes existing behavior, deliberately: a Deployment currently sitting at full size behind a Healthy Rollout with scaleDown: progressively will be scaled to 0 on the first reconcile after upgrade. That is the documented intent of the setting, and for anyone in the state described above it is the recovery they'd otherwise have to perform by hand. Worth a release note.

The pre-existing unit case named Do not scale deployment asserted the old gate (revision 2 + Healthy + Deployment at 5 → stays 5); it's been renamed and flipped to expect 0, which is the crux of the fix.

Tests

rollout/replicaset_test.go, in TestScaleDownProgressively:

• Rollout healthy on a later revision - Deployment scaled to 0 — the regression. Verified it fails on unmodified master (expected: 0, actual: 5) and passes with the change.
• Rollout progressing on a later revision - Deployment untouched — guards the scoping of the availability branch, so this fix can't turn into a mid-canary scale-up.

go test ./rollout/... passes locally, as does go build ./....

I did not add an e2e case: reproducing this needs an external actor scaling the referenced Deployment back up between revisions, and I'd rather not push e2e code I can't run end-to-end here. The existing TestScaleDownProgressivelyCompleted* suites pass unchanged. Happy to add one if you'd like it, or to hand over the exact repro.

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.
• 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.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.20%. Comparing base (5112dba) to head (274cd5f).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4940   +/-   ##
=======================================
  Coverage   85.20%   85.20%           
=======================================
  Files         166      166           
  Lines       19437    19437           
=======================================
  Hits        16561    16561           
  Misses       2026     2026           
  Partials      850      850           

Flag	Coverage Δ
e2e	53.06% <100.00%> (-0.01%)	⬇️
unit-tests	81.64% <100.00%> (ø)

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.

[github-actions]

Published E2E Test Results

  4 files    4 suites   4h 10m 30s ⏱️
149 tests 142 ✅  7 💤 0 ❌
596 runs  568 ✅ 28 💤 0 ❌

Results for commit 274cd5f.

♻️ This comment has been updated with latest results.

[github-actions]

Published Unit Test Results

2 624 tests   2 624 ✅  3m 30s ⏱️
  131 suites      0 💤
    1 files        0 ❌

Results for commit 274cd5f.

♻️ This comment has been updated with latest results.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud