[VanderChen]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Upgrading only the Kthena controller can trigger a one-time rollout of already running ModelServing workloads even when the user has not changed the ModelServing Spec. On the first reconciliation after the upgrade, the new controller can calculate a different revision for the same persisted Role/Pod template. Existing Pods still carry the old revision labels, so they are classified as outdated and replaced. The workload stabilizes after every Pod carries the new revision, making the symptom look like a one-off rollout during controller upgrade.

The root cause is that main hashes the in-memory Go representation of spec.template.roles with dump.ForHash. A Kubernetes dependency or CRD Go type update can add an optional nil field to a nested struct. That changes the typed dump and hash even when the serialized Role/Pod template is unchanged.

This PR:

• hashes normalized JSON snapshots, not the Go struct layout; newly added optional nil fields with omitempty no longer change the serialized hash;
• shares one hash-plus-semantic comparator between whole-ModelServing and Role projections. Equal hash plus equal bytes is the fast path; otherwise typed Kubernetes semantic equality is authoritative. Hash collisions do not imply equality, and hash drift does not imply a rollout;
• connects the existing immutable ControllerRevision lifecycle to the controller, including scheduler/plugins. Role hashes are projections of whole-ModelServing history, not separate Role ControllerRevisions;
• migrates legacy Roles-only histories by storing a durable compatibility baseline with historical Roles and current scheduler/plugins. Running Pods retain their UIDs and revision identities; original revision data is never rewritten;
• restores historical templates, worker layout and plugins for missing ordinals below partition, while applying current replica/rollout controls;
• rejects effective scheduler changes in the update webhook because Role rolling does not migrate the shared PodGroup;
• pauses affected template-driven creation/deletion for missing, malformed, foreign or unsupported history; records healthy observed Pods even when history cannot be read after restart; still permits explicit safe replica scale-down;
• preserves observed ServingGroup/Role history references and defers cleanup during partition, rollout, recovery or unresolved history;
• does not modify go.mod, go.sum, APIs, generated code, Helm CRDs, or CRD schemas.

Revision and rollout field scope:

Included in identity	Behavior
Role set	Adding/removing Roles participates; Role order is normalized and does not roll Pods.
spec.template.roles[].name	Role identity changes participate.
entryTemplate.metadata and entryTemplate.spec	Serialized labels, annotations and PodSpec fields participate, except controller-overwritten inputs listed below.
workerReplicas and active workerTemplate	Worker layout participates. An unused worker template is retained in snapshots but does not roll entry-only Roles.
plugins	Ordered plugin name/type/config and effective Entry/Worker scope participate only for targeted Roles. Expanding the Role whitelist does not re-roll existing targets. Worker-only plugin changes do not roll entry-only Roles.
schedulerName	Stored in snapshots. The update webhook rejects effective scheduler switches; omission defaults to volcano at admission, whereas explicit empty means Kubernetes default-scheduler.

Excluded from identity	Behavior
Role replicas	Replica-only changes remain scaling operations.
Role maxUnavailable, maxSurge, partition	Applied as current rollout controls, not Pod-template identity.
ModelServing spec.replicas and spec.rolloutStrategy	Scaling and rollout execution controls.
recoveryPolicy and revisionHistoryLimit	Controller behavior/history policy.
ServingGroup restartGracePeriodSeconds, gangPolicy, networkTopology	Outside the existing Role-template hash scope.
ModelServing metadata/status	Not part of template identity.
Role list order, plugin scope ordering/duplicates, empty metadata and omitted default plugin type/target	Canonicalized; no rollout for representation-only changes. Plugin execution order is still significant.
Nested Pod schedulerName; GROUP_SIZE, ENTRY_ADDRESS, WORKER_INDEX environment entries	Normalized because the controller overwrites them before creating Pods.

Plugin comparison uses configuration inputs, not hook output. Arbitrary Pod API defaults are not stripped: hooks can observe pre-admission input.

Migration boundary: legacy snapshots have no old plugin configuration. The first migration uses current plugins as the compatibility baseline and does not force a rollout. A plugin edit simultaneous with that migration cannot be detected. Later plugin edits are detected against the durable baseline, including after restart. An annotation on the original revision records the established baseline; losing that baseline blocks reconciliation instead of silently rebuilding it from current plugins.

Which issue(s) this PR fixes:
Fixes #1803

Bug evidence (required for bug-related PRs):

Trigger:

1. Run a ModelServing using a controller whose Kubernetes API dependency is k8s.io/api v0.34.2.
2. Upgrade only the controller to a build using k8s.io/api v0.35.3; do not change the ModelServing Spec.
3. The newer PodSpec contains the optional nil WorkloadRef field. Serialized Role JSON remains identical, but dump.ForHash produces a different revision.
4. syncModelServing passes the new revision into rolling-update handling. Existing Pods have the legacy revision label and are treated as outdated, triggering one unnecessary rollout.

Representative workload:

apiVersion: workload.serving.volcano.sh/v1alpha1
kind: ModelServing
metadata:
  name: revision-upgrade-repro
spec:
  replicas: 1
  schedulerName: volcano
  rolloutStrategy:
    type: ServingGroupRollingUpdate
  template:
    roles:
    - name: decode
      replicas: 1
      workerReplicas: 0
      entryTemplate:
        spec:
          schedulerName: volcano
          containers:
          - name: decode
            image: busybox:1.36
            command: ["sh", "-c", "sleep 86400"]

Cross-version hash probe using the same Role JSON:

dependency	Role JSON	typed dump	revision
k8s.io/api v0.34.2	identical	no WorkloadRef	655f5cbc6f
k8s.io/api v0.35.3	identical	contains WorkloadRef: nil	7d97c697b8

Production path at base commit d5f99e10a1996714bb29228cfa5af451c5305639:

• Revision hashes typed values through dump.ForHash.
• ModelServingRevision and CalRoleTemplateHash pass Role structs directly to that function.
• syncModelServing calculates the revision on every reconciliation and passes it to rolling-update handling.
• Role rollout compares stored and newly calculated template hashes.
• ServingGroup rollout treats a Pod with a different revision label as outdated.

This issue was found through source review and reproduced with the isolated dependency probe and Kind controller-upgrade path; no production incident log is available.

Verification:

• go test ./pkg/model-serving-controller/... -count=1
• go test $(go list ./... | grep -v /e2e)
• make -o generate test (coverage gate; generation intentionally omitted because no API/schema changes are made and generate runs go mod tidy)
• go vet ./pkg/model-serving-controller/...
• focused go test -race coverage for revision equality, entry points, baseline migration, scheduler admission, missing-history observation and partition recovery;
• unit tests verify that an added nil omitempty field changes the legacy typed hash but not the serialized hash;
• unit tests pin the expected ModelServingRevision, RoleRevisionHash and legacy CalRoleTemplateHash, repeat calls, and verify the input Spec is unchanged;
• unit tests verify a legacy dump.ForHash revision is reused after switching to serialized hashing;
• isolated Kind v1.34.0 / linux-arm64 with Volcano 1.14.1: upgrade from the Roles-only controller at 884928de retained all 11 original Pod UIDs, status revision identities and original ControllerRevision UID/Data across both rollout strategies; additional baseline objects are expected;
• plugin-only edits rolled the whole ServingGroup or only the applicable Role as configured; expanding the plugin whitelist did not re-roll the existing target;
• both partition strategies kept ordinal 0 unchanged; deleting its Pod recreated it with the old command, plugin annotation and revision; controller restart preserved the recovered Pods;
• changing a Worker-only plugin on entry-only workloads did not roll either strategy or change its persisted revision identity;
• scheduler changes were rejected by admission without changing running Pods;
• deterministic history-loss injection across controller restart preserved Pods under both strategies; restoring history resumed the intended update; deleting/restoring a legacy baseline confirmed that it is not silently re-created.

Representative Kind evidence from the unchanged-spec upgrade (same UID before/after):

Pod	UID before = after	Revision before = after
migrate-sg-0-decode-0-0	7ff0cefc-1900-4c56-95e4-f103c9ff1ed8	68c7587c89
migrate-role-0-decode-0-0	45683189-81a0-4e2a-a8ea-7c60670c153c	68c7587c89

During deliberate baseline-loss injection, the controller retried with:

resolve desired revision: establish legacy revision baseline: previously established legacy baseline partition-role-68c7587c89-baseline is missing

Pod UIDs stayed unchanged until the saved baseline was restored, after which the pending real plugin edit proceeded only outside partition.

Verification caveats (not reported as passing):

• make lint is blocked by the repository-pinned golangci-lint v1.64.8 export-data reader under host Go 1.27.1 (export data version 4 is greater than maximum supported version 2). No dependency or lint configuration workaround is included.
• Full controller-package -race detects the existing unsynchronized initialSync read in model_serving_controller_harness_test.go:95 versus the write in syncAll. Reproduced on unmodified upstream/main at 4f293e39; focused revision tests pass with -race. This unrelated race is not changed here.

Special notes for your reviewer:

Kubernetes ControllerRevision users hash serialized revision data and use semantic comparison of reconstructed inputs where appropriate. This PR applies that two-layer approach to the same canonical projections used for ModelServing and Role decisions.

The hash algorithm intentionally changes from typed dump.ForHash to serialized bytes. The semantic history fallback is therefore required to absorb this migration without rolling existing workloads. If the referenced ControllerRevision is missing, malformed, or owned by another resource, template-driven rollout remains blocked and retried rather than assuming equality.

Parts of the full-snapshot recovery and Role projection follow #1767 (reviewed at b8da6b8868f62a1fd52953dd20aaf7f1af1a92f8). This is not a wholesale import:

• legacy/missing history must not be classified as outdated to force replacement;
• first migration must not roll workloads just to establish plugin snapshots;
• Role hash mismatches require semantic comparison;
• unrelated plugin scope changes and inactive worker inputs must not roll unaffected Roles;
• scheduler switching is rejected instead of expanding PodGroup migration logic;
• no status.revisionReferences or other CRD/API additions are introduced.

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

Does this PR introduce a user-facing change?:

Prevent controller upgrades from rolling unchanged ModelServing workloads by sharing serialized hash and semantic revision comparison. Track applicable plugins and restore complete historical templates for partition recovery. Legacy plugin snapshots migrate without a first-upgrade rollout (simultaneous first-migration plugin edits cannot be detected). The validation webhook now rejects effective schedulerName changes.

[VanderChen]

/kind bug

[acsoto]

How should this compose with #1767, which switches to scheduler/plugin-aware revisions and intentionally rolls legacy workloads on upgrade?

[LiZhenCheng9527]

/cherry-pick release-1.1

[volcano-sh-bot]

@LiZhenCheng9527: once the present PR merges, I will cherry-pick it on top of release-1.1 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-1.1

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[LiZhenCheng9527]

/lgtm

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LiZhenCheng9527

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• docs/OWNERS [LiZhenCheng9527]
• pkg/OWNERS [LiZhenCheng9527]

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

[volcano-sh-bot]

@LiZhenCheng9527: new pull request created: #1968

Details

In response to this:

/cherry-pick release-1.1

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.