fix: standardize upgrade cordon-drain timeout#298
fix: standardize upgrade cordon-drain timeout#298jackfrancis wants to merge 2 commits intoAzure:masterfrom
Conversation
| retry = time.Second * 5 | ||
| interval = time.Second * 1 | ||
| retry = time.Second * 5 | ||
| cordonDrainTimeout = time.Minute * 10 |
There was a problem hiding this comment.
why 10 minutes? if scale is 1 hour
There was a problem hiding this comment.
This is just a placeholder to demonstrate the surface area that needs changing. No concrete requirement yet.
Codecov Report
@@ Coverage Diff @@
## master #298 +/- ##
=======================================
Coverage 53.22% 53.22%
=======================================
Files 95 95
Lines 14224 14224
=======================================
Hits 7571 7571
Misses 5988 5988
Partials 665 665 |
|
any blockers for moving this forward? |
jackfrancis
force-pushed
the
cordon-drain-timeout
branch
from
afea064 to
97c4172
Compare
|
@CecileRobertMichon I reverted the functional change from 1 minute to 10 minutes. Now the scope of this PR is just to move the timeout value to be more maintainable as a discrete const. |
CecileRobertMichon
left a comment
CecileRobertMichon
left a comment
There was a problem hiding this comment.
lgtm as-is but do we not want to increase the timeout? 1 minute seems to be too short for some VMSS upgrades
|
I think we want to have definitive repro cases where increasing the timeout causes a demonstrable improvement (with no tradeoffs). |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CecileRobertMichon, jackfrancis The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Reason for Change:
Enable future maintenance of cordon/drain timeout by making it a discrete const.
Issue Fixed:
Requirements:
Notes: