fix(controller): don't timeout rollout when still waiting for scale down delay #3417
Conversation
yohanb
force-pushed
the
fix_progress_deadline_exceeded_with_scale_down_delay
branch
from
c248418
to
6c2f787
Compare
yohanb
changed the title
|
Fixes #3414 |
yohanb
changed the title
yohanb
force-pushed
the
fix_progress_deadline_exceeded_with_scale_down_delay
branch
from
6c2f787
to
45b07a9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3417 +/- ##
==========================================
+ Coverage 81.83% 82.90% +1.06%
==========================================
Files 135 135
Lines 20688 16910 -3778
==========================================
- Hits 16931 14020 -2911
+ Misses 2883 2013 -870
- Partials 874 877 +3 ☔ View full report in Codecov by Sentry. |
Go Published Test Results2 130 tests 2 130 ✅ 2m 50s ⏱️ Results for commit 6460c18. ♻️ This comment has been updated with latest results. |
E2E Tests Published Test Results 4 files 4 suites 3h 43m 58s ⏱️ For more details on these failures, see this check. Results for commit 6460c18. ♻️ This comment has been updated with latest results. |
…own delay Signed-off-by: Yohan Belval <ybelval@turo.com>
yohanb
force-pushed
the
fix_progress_deadline_exceeded_with_scale_down_delay
branch
from
45b07a9
to
9bf3b55
Compare
| @@ -578,6 +578,19 @@ func isIndefiniteStep(r *v1alpha1.Rollout) bool { | |||
| return false | |||
| } | |||
|
|
|||
| // isWaitingForReplicaSetScaleDown returns whether or not the rollout still has other replica sets with a scale down deadline annotation | |||
| func isWaitingForReplicaSetScaleDown(r *v1alpha1.Rollout, newRS, stableRS *appsv1.ReplicaSet, allRSs []*appsv1.ReplicaSet) bool { | |||
There was a problem hiding this comment.
During this check, do you know if the r.status.currentPodHash == r.status.stableRS if so that means that we are finished or are not in the middle of the rollout wondering if it would make sense to add this check?
this would then become isWaitingForReplicaSetScaleDownAndRolloutCompleted however my guess is that the status is probably not updated yet till the very end though and so they are not equal yet, it would be nice to confirm due to https://cloud-native.slack.com/archives/C01U781DW2E/p1709327328941959
There was a problem hiding this comment.
I can check if that's the case
There was a problem hiding this comment.
I did a bit of checking around deployment behavior and what you are doing here seems logical. Deployments can throw progressDeadline not during a "deployment" on a few cases and so I don't think we would want to add a check that limits adding a progressDeadline if we are not in the middle of a rollout.
There was a problem hiding this comment.
This was an interesting thread around k8s deployment behavior, kubernetes/kubernetes#106054
There was a problem hiding this comment.
@zachaller thanks for looking at this. Looking forward to v1.7 :)
let me know if I can do anything else.
|
Fixes #3414
Checklist:
"fix(controller): Updates such and such. Fixes #1234".