Repository navigation
model-serving: keep CurrentRevision until the target is ready - #1960
kube-gopher wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/cc @acsoto |
| if rolloutComplete { | ||
| currentRevision = updateRevision | ||
| } else if currentRevision == "" && rolloutActive { | ||
| } else if currentRevision == "" && (rolloutActive || (len(revisionCount) == 0 && available < replicas)) { |
There was a problem hiding this comment.
available includes surge replicas in the count.
If you want to ensure that all replicas are ready, you should limit the order to < replicas
There was a problem hiding this comment.
is this conditional statement a little complicated? 😆
There was a problem hiding this comment.
I can’t think of any other ideas 😆
There was a problem hiding this comment.
available includes surge replicas in the count.
If you want to ensure that all replicas are ready, you should limit the order to < replicas
okay, thank you.
f8ff5fe to
95659ca
Compare
Signed-off-by: kube-gopher <jzlyy68@gmail.com>
95659ca to
d1de9e6
Compare
What type of PR is this?
/kind bug
What this PR does / why we need it:
status.currentRevisioncould move to a revision that had never become Ready.Once every ServingGroup is on the target revision, no old group remains to keep the previous value, so the status logic fell through to
updateRevision. This happens as soon as the last old group is replaced, which makes a single ServingGroup the most exposed setup.Promotion also triggers revision cleanup, so the previous ControllerRevision was deleted once no Pods referenced it. After a failed update, the last completed revision was neither recorded nor recoverable from the cluster.
With this PR, the previous
currentRevisionis kept until the desired replicas are Ready. Promotion while Ready surge capacity drains is unchanged.Which issue(s) this PR fixes:
Fixes #
Bug evidence (required for bug-related PRs):
Special notes for your reviewer:
Does this PR introduce a user-facing change?: