Skip to content

model-serving: keep CurrentRevision until the target is ready - #1960

Open
kube-gopher wants to merge 1 commit into
volcano-sh:mainfrom
kube-gopher:fix/modelserving-current-revision-promotion
Open

kube-gopher wants to merge 1 commit into
volcano-sh:mainfrom
kube-gopher:fix/modelserving-current-revision-promotion

Conversation

@kube-gopher

Copy link
Copy Markdown
Member

What type of PR is this?
/kind bug

What this PR does / why we need it:
status.currentRevision could 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 currentRevision is 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?:


@volcano-sh-bot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign git-malu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kube-gopher kube-gopher changed the title modelserving: keep CurrentRevision until the target is ready model-serving: keep CurrentRevision until the target is ready Nov 5, 2026
@kube-gopher

Copy link
Copy Markdown
Member Author

/cc @acsoto

@volcano-sh-bot
volcano-sh-bot requested a review from acsoto November 5, 2026 03:59
if rolloutComplete {
currentRevision = updateRevision
} else if currentRevision == "" && rolloutActive {
} else if currentRevision == "" && (rolloutActive || (len(revisionCount) == 0 && available < replicas)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

available includes surge replicas in the count.
If you want to ensure that all replicas are ready, you should limit the order to < replicas

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this conditional statement a little complicated? 😆

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can’t think of any other ideas 😆

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@kube-gopher
kube-gopher force-pushed the fix/modelserving-current-revision-promotion branch from f8ff5fe to 95659ca Compare November 5, 2026 09:50
Signed-off-by: kube-gopher <jzlyy68@gmail.com>
@kube-gopher
kube-gopher force-pushed the fix/modelserving-current-revision-promotion branch from 95659ca to d1de9e6 Compare November 6, 2026 08:31

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Accepted

Development

Successfully merging this pull request may close these issues.

4 participants