feat: integrate ModelServing revision history by acsoto · Pull Request #1767 · volcano-sh/kthena

acsoto · 2026-09-12T03:16:22Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Extend the ModelServing revision core from #1643 with immutable ControllerRevision lifecycle handling, partition-protected historical recovery, durable revision retention during recovery and cleanup, and deterministic plugin rendering for historical workloads.

Which issue(s) this PR fixes:

Refs #1643 #1607

Bug evidence (required for bug-related PRs):

N/A

Special notes for your reviewer:

This is the second part of the implementation.

This PR was written in part with the assistance of generative AI.

Tested with:

make test
go test ./pkg/model-serving-controller/...
go vet ./pkg/model-serving-controller/...
GOTOOLCHAIN=go1.26.0 bin/golangci-lint run ./pkg/model-serving-controller/...

Does this PR introduce a user-facing change?:

Upgrading to this version changes ModelServing revision tracking and triggers a one-time rolling replacement of existing workloads, even when the ModelServing spec is unchanged. This affects both ServingGroupRollingUpdate and RoleRollingUpdate and respects the configured rollout partitions and rolling update settings. Plan the controller upgrade accordingly, as workload replacement may temporarily reduce serving capacity.

volcano-sh-bot · 2026-09-12T03:16:27Z

[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 yaozengzeng 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

acsoto · 2026-09-15T03:21:35Z

@LiZhenCheng9527 @YaoZengzeng @VanderChen @haoeeeee

YaoZengzeng · 2026-09-16T09:25:51Z

+	if err != nil {
+		return fmt.Errorf("build ModelServing revision data: %w", err)
+	}
+	controllerRevision, collisionCount, err := utils.RecordModelServingRevision(ctx, c.kubeClientSet, ms, revisionData)


This switches the revision identity from ModelServingRevision(ms) (hash of roles only) to RevisionDataHash over the full revision data, so an unchanged spec gets a different revision label after the controller upgrade (verified with the same fixture: 5c8649477c → 6bc4bb5bc9). For workloads created by a released controller (e.g. v1.0.0):

Under the default ServingGroupRollingUpdate, isServingGroupOutdated compares pod revision labels against the new value, so every existing serving group is treated as outdated and rolled even though nothing in its spec changed.

Under RoleRollingUpdate, newRoleUpdateCheck unconditionally returns outdated for pre-V1 ControllerRevisions (no revision-data-version annotation), so all roles are restarted as well.

If a one-time full restart on upgrade is intended, could this be called out in a release note / upgrade doc (the PR is currently marked release-note NONE)? Otherwise, consider keeping a legacy comparison path that hashes the roles stored in pre-V1 revisions so unchanged workloads are not restarted.

I think for this part, restart.
Before the release, if this is unacceptable, we can create a patch
if it’s acceptable, we’ll include it in the release notes.

Signed-off-by: Zhou Zihang <z@mcac.cc>

acsoto · 2026-09-22T02:34:00Z

@LiZhenCheng9527

acsoto · 2026-09-22T02:34:20Z

@VanderChen

kube-gopher · 2026-10-11T12:54:13Z

Annotate: within the inplace range.

LiZhenCheng9527 · 2026-10-12T09:08:11Z

+			if err := c.CreatePodsByRole(ctx, *roleToApply.DeepCopy(), workloadToApply, roleIndex, servingGroupOrdinal, replicaRevision, hashToUse); err != nil {
+				return fmt.Errorf("recreate pods for role %s/%s in ServingGroup %s: %w", targetRole.Name, roleObj.Name, groupName, err)


Increasing workerReplicas (for example, from 0 to 1) makes an existing, unprotected replica enter this missing-Pod branch. CreatePodsByRole then tries to create its entry Pod using the desired revision, but the same-named entry Pod still belongs to the previous revision, so createPod returns an identity-mismatch error.

#1799 now covers most of this work, so I suggest consolidating there

LiZhenCheng9527 · 2026-10-12T09:09:00Z

 		klog.V(4).Infof("scaleUpServingGroups: creating/updating PodGroup for ServingGroup=%s", groupName)
 		// Ensure a PodGroup exists for the new ServingGroup when gang scheduling is enabled.
-		if err := c.createOrUpdatePodGroupByServingGroup(ctx, ms, groupName); err != nil {
+		if err := c.createOrUpdatePodGroupByServingGroup(ctx, workload, groupName); err != nil {


Passing the historical workload here correctly initializes the recovered group's PodGroup, but syncServingGroupReplicas subsequently iterates over every non-deleting group and calls createOrUpdatePodGroupByServingGroup(ctx, ms, servingGroup.Name) with the current spec. Once the PodGroup is observed, that path overwrites its historical requirements.

acsoto · 2026-10-13T13:23:50Z

Closing in favor of #1799 to consolidate the revision work into one implementation. The outstanding review findings and recovery coverage from this PR should be carried over there.

github-project-automation Bot added this to Kthena Release Tracking Sep 12, 2026

github-project-automation Bot moved this to Accepted in Kthena Release Tracking Sep 12, 2026

volcano-sh-bot added the kind/feature label Sep 12, 2026

volcano-sh-bot requested review from LiZhenCheng9527, YaoZengzeng, git-malu, hzxuzhonghu and katara-Jayprakash September 12, 2026 03:16

acsoto mentioned this pull request Sep 12, 2026

feat: integrate ModelServing revision history acsoto/kthena#3

Closed

volcano-sh-bot added the size/XXL label Sep 12, 2026

acsoto force-pushed the feature/modelserving-revision-controller-review branch 4 times, most recently from fa5bd6f to 3cc7520 Compare September 12, 2026 09:23

volcano-sh-bot added the do-not-merge/contains-merge-commits label Sep 14, 2026

acsoto force-pushed the feature/modelserving-revision-controller-review branch from 4ee987c to 49b2f7c Compare September 14, 2026 07:59

volcano-sh-bot removed the do-not-merge/contains-merge-commits label Sep 14, 2026

LiZhenCheng9527 moved this from Accepted to Planned in Release v1.1 in Kthena Release Tracking Sep 16, 2026

YaoZengzeng reviewed Sep 16, 2026

View reviewed changes

acsoto added 5 commits September 22, 2026 10:29

fix: defer historical revision recovery

97e1c7b

Signed-off-by: Zhou Zihang <z@mcac.cc>

fix: align historical recovery readiness

521a259

Signed-off-by: Zhou Zihang <z@mcac.cc>

test: tighten historical recovery coverage

372b613

Signed-off-by: Zhou Zihang <z@mcac.cc>

modelserving: fix partitioned role recovery and reuse revision checks

90105fc

Signed-off-by: Zhou Zihang <z@mcac.cc>

test: consolidate historical recovery coverage

b8da6b8

Signed-off-by: Zhou Zihang <z@mcac.cc>

acsoto force-pushed the feature/modelserving-revision-controller-review branch from 5487641 to b8da6b8 Compare September 22, 2026 02:30

acsoto mentioned this pull request Sep 22, 2026

fix(modelserving): preserve semantic revisions across upgrades #1799

Merged

acsoto mentioned this pull request Sep 22, 2026

feat: manage headless services through ModelServing plugin hooks #1801

Merged

VanderChen mentioned this pull request Sep 22, 2026

Include applicable ModelServing plugins in rollout revision identity #1813

Open

LiZhenCheng9527 reviewed Oct 12, 2026

View reviewed changes

acsoto closed this Oct 13, 2026

github-project-automation Bot moved this from Planned in Release v1.1 to Done in Kthena Release Tracking Oct 13, 2026

Navigation Menu

Repository navigation

feat: integrate ModelServing revision history - #1767

feat: integrate ModelServing revision history#1767
acsoto wants to merge 5 commits into
volcano-sh:mainfrom
acsoto:feature/modelserving-revision-controller-review

acsoto commented Sep 12, 2026 •

edited

Loading

Uh oh!

volcano-sh-bot commented Sep 12, 2026

Uh oh!

acsoto commented Sep 15, 2026 •

edited

Loading

Uh oh!

YaoZengzeng Sep 16, 2026

Uh oh!

acsoto Sep 17, 2026

Uh oh!

acsoto commented Sep 22, 2026

Uh oh!

acsoto commented Sep 22, 2026

Uh oh!

kube-gopher commented Oct 11, 2026

Uh oh!

LiZhenCheng9527 Oct 12, 2026

Uh oh!

acsoto Oct 13, 2026

Uh oh!

LiZhenCheng9527 Oct 12, 2026

Uh oh!

acsoto Oct 13, 2026

Uh oh!

acsoto commented Oct 13, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		if err := c.CreatePodsByRole(ctx, *roleToApply.DeepCopy(), workloadToApply, roleIndex, servingGroupOrdinal, replicaRevision, hashToUse); err != nil {
		return fmt.Errorf("recreate pods for role %s/%s in ServingGroup %s: %w", targetRole.Name, roleObj.Name, groupName, err)

Conversation

acsoto commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

volcano-sh-bot commented Sep 12, 2026

Uh oh!

acsoto commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YaoZengzeng Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

acsoto Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

acsoto commented Sep 22, 2026

Uh oh!

acsoto commented Sep 22, 2026

Uh oh!

kube-gopher commented Oct 11, 2026

Uh oh!

LiZhenCheng9527 Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

acsoto Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

LiZhenCheng9527 Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

acsoto Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

acsoto commented Oct 13, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

acsoto commented Sep 12, 2026 •

edited

Loading

acsoto commented Sep 15, 2026 •

edited

Loading