fix: verify the weight of the alb at the end of the rollout #3627
Conversation
This file contains 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
…uto set max weight Signed-off-by: Zach Aller <[email protected]>
Go Published Test Results2 163 tests 2 163 ✅ 2m 54s ⏱️ Results for commit 68c6f6f. ♻️ This comment has been updated with latest results. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3627 +/- ##
=======================================
Coverage 80.26% 80.26%
=======================================
Files 156 156
Lines 17969 17970 +1
=======================================
+ Hits 14423 14424 +1
Misses 2634 2634
Partials 912 912 ☔ View full report in Codecov by Sentry. |
E2E Tests Published Test Results 4 files 4 suites 3h 29m 12s ⏱️ For more details on these failures, see this check. Results for commit 68c6f6f. ♻️ This comment has been updated with latest results. |
Signed-off-by: Zach Aller <[email protected]>
Signed-off-by: Zach Aller <[email protected]>
Signed-off-by: Zach Aller <[email protected]>
Signed-off-by: Zach Aller <[email protected]>
Signed-off-by: Zach Aller <[email protected]>
Signed-off-by: Zach Aller <[email protected]>
leoluz
reviewed
Jun 12, 2024
rollout/trafficrouting.go
Outdated
| // At the end of the rollout we need to verify the weight is correct, and return an error if not because we don't want the rest of the | ||
| // reconcile process to continue. We don't need to do this if we are in the middle of the rollout because the rest of the reconcile | ||
| // process won't scale down the old replicasets yet due to being in the middle of some steps. | ||
| if *weightVerified == false && desiredWeight == weightutil.MaxTrafficWeight(c.rollout) && len(c.rollout.Spec.Strategy.Canary.Steps) >= int(*c.rollout.Status.CurrentStepIndex) { |
There was a problem hiding this comment.
We already know that *weightVerified is false given the if condition above. I think it is safe to remove this condition.
Signed-off-by: Zach Aller <[email protected]>
|
zachaller
added a commit
that referenced
this pull request
Jun 13, 2024
* fix: verify the weight of the alb at the end of the rollout when we auto set max weight Signed-off-by: Zach Aller <[email protected]> * add unit test Signed-off-by: Zach Aller <[email protected]> * refactor add unit test Signed-off-by: Zach Aller <[email protected]> * Trigger Build Signed-off-by: Zach Aller <[email protected]> * Trigger Build Signed-off-by: Zach Aller <[email protected]> * block reconcile Signed-off-by: Zach Aller <[email protected]> * add tests Signed-off-by: Zach Aller <[email protected]> * rename test and cleanup un-need logic check Signed-off-by: Zach Aller <[email protected]> --------- Signed-off-by: Zach Aller <[email protected]>
zachaller
added a commit
that referenced
this pull request
Jun 13, 2024
* fix: verify the weight of the alb at the end of the rollout when we auto set max weight Signed-off-by: Zach Aller <[email protected]> * add unit test Signed-off-by: Zach Aller <[email protected]> * refactor add unit test Signed-off-by: Zach Aller <[email protected]> * Trigger Build Signed-off-by: Zach Aller <[email protected]> * Trigger Build Signed-off-by: Zach Aller <[email protected]> * block reconcile Signed-off-by: Zach Aller <[email protected]> * add tests Signed-off-by: Zach Aller <[email protected]> * rename test and cleanup un-need logic check Signed-off-by: Zach Aller <[email protected]> --------- Signed-off-by: Zach Aller <[email protected]>
zachaller
added
the
cherry-pick-completed
anandf
added a commit
to anandf/argo-rollouts
that referenced
this pull request
Jun 27, 2024
…rgoproj#3627)" This reverts commit d28e75f.
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
At the end of the rollout when we auto set the weight to the max (100%) etc. we should verify the weight. If we don't this can actually cause issues even with target group verification at the end of the rollout. The reason is the ALB controller can be slow to update for many reasons, which means we can actually verify target groups successfully and start tearing down old pods before we have shifted the weight over to the new service.