fix(controller): honor scaleUpPreviewCheckPoint reset on blueGreen pod template change. Fixes #4728#4729
Open
abizer wants to merge 1 commit into
Open
Conversation
Contributor
Published E2E Test Results 4 files 4 suites 3h 54m 8s ⏱️ For more details on these failures, see this check. Results for commit 28c1d69. ♻️ This comment has been updated with latest results. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4729 +/- ##
=======================================
Coverage 85.03% 85.04%
=======================================
Files 164 164
Lines 18989 18989
=======================================
+ Hits 16148 16149 +1
+ Misses 1993 1992 -1
Partials 848 848 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Published Unit Test Results2 470 tests 2 470 ✅ 3m 21s ⏱️ Results for commit 28c1d69. ♻️ This comment has been updated with latest results. |
…d template change When a new pod template is rolled out while the previous revision had already completed prePromotionAnalysis (and thus set status.blueGreen.scaleUpPreviewCheckPoint=true), the new revision inherits that value and Argo Rollouts skips creating an AnalysisRun for it entirely -- including the prePromotionAnalysis gate. syncRolloutStatusBlueGreen calls resetRolloutStatus(&newStatus) on pod template change, which clears newStatus.BlueGreen.ScaleUpPreviewCheckPoint. The subsequent call to calculateScaleUpPreviewCheckPoint read prevValue from c.rollout.Status (the pre-reset value) rather than the newStatus parameter that was just reset, immediately clobbering the reset. Read prevValue from newStatus so the reset applied earlier in the same reconcile is honored, matching the documented behavior: // ... gets reset to false when the pod template changes, or the // rollout fully promotes (stableRS == newRS) Add a regression test that fails on the old behavior and passes with the fix. Signed-off-by: Abizer Lokhandwala <abizer@abizer.me>
abizer
force-pushed
the
fix-bluegreen-preview-status-check
branch
from
b7f948b to
28c1d69
Compare
|
Author
|
cc @argoproj/argo-rollouts-approvers |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Read
prevValuefromnewStatusinstead ofc.rollout.StatusincalculateScaleUpPreviewCheckPoint. WhensyncRolloutStatusBlueGreencallsresetRolloutStatuson pod template change, the reset mutatesnewStatus.BlueGreen.ScaleUpPreviewCheckPoint = false; reading fromc.rollout.Statusimmediately afterwards picks up the pre-reset value, clobbering the reset.Why
Matches the function's documented behavior:
Without this fix, a new revision started while the previous one was in the scale-up /
autoPromotionSecondswindow inheritsscaleUpPreviewCheckPoint: trueand skipsprePromotionAnalysisentirely — the new preview RS scales straight toreplicas(bypassingpreviewReplicaCount), no AnalysisRun is created, and cutover proceeds untested.Full repro + root cause + live-cluster evidence in #4728.
Tests
TestCalculateScaleUpPreviewCheckPointHonorsResetinrollout/bluegreen_test.go. Verified it fails on the unfixed code (Should be false) and passes with the fix.TestPreviewReplicaCountHandleScaleUpPreviewCheckPointsub-tests (TrueAfterMeetingMinAvailable,FalseAfterActiveServiceSwitch,TrueWhenScalingUpPreview) continue to pass../rollout/...package suite passes locally (make testequivalent).Checklist
fix(controller): ...), states what changed, and suffixes the related issue number (Fixes #4728).blueGreen+previewReplicaCount+prePromotionAnalysis: previously a back-to-back revision would skip the gate; now it correctly runsprePromotionAnalysisfor the new revision. No API or config change.