fix(jobset): omit container images from SSA apply config when resuming suspended TrainJob

[hrathina]

What this does

Fixes #3387

When a suspended TrainJob is resumed after a TrainingRuntime image update, the Trainer controller includes the updated image in its SSA patch. The JobSet webhook rejects this because spec.replicatedJobs container images are immutable — even on suspended JobSets.

Fix: In Build(), when a resume is detected (oldJobSet.Suspend=true, trainJob.Suspend=false), nil out all container and initContainer images in the apply configuration before the SSA call. SSA then omits those fields from the patch, the webhook sees no image change, and the JobSet resumes with the image it was originally created with.

Note: This is an interim fix addressing the specific symptom. The approach is still under discussion.

How it works

The JobSet webhook (vjobset.kb.io) allows only annotations, labels, nodeSelector, tolerations, and schedulingGates to change on a suspended JobSet. Container images are unconditionally immutable. By omitting images from the SSA apply config, we respect this constraint: the field manager releases ownership of the image field and the value in etcd is unchanged.

Tests

• Added TestBuild_ResumeOmitsContainerImages with 4 cases:
  • Suspended JobSet being resumed → images are nil in apply config ✅
  • No existing JobSet (creation path) → images present ✅
  • Both running (existing early-exit guard) → nil result ✅
  • TrainJob suspended (not a resume) → images present ✅
• All existing unit tests pass (pkg/...)
• Integration tests pass

[google-oss-prow]

[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 gaocegege 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:

• OWNERS

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

[Copilot]

Pull request overview

This PR addresses a JobSet admission webhook immutability constraint by preventing Server-Side Apply (SSA) from attempting to patch spec.replicatedJobs[*] container images when resuming a previously suspended TrainJob.

Changes:

• Detect the “resume from suspended JobSet” scenario in JobSet.Build() and nil out container/initContainer image fields in the SSA apply configuration so those fields are omitted from the apply patch.
• Add a new unit test covering resume vs. non-resume behavior and the existing “both running” early-exit guard.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
pkg/runtime/framework/plugins/jobset/jobset.go	Omits container/initContainer images from SSA apply config on resume to avoid immutable-field webhook rejections.
pkg/runtime/framework/plugins/jobset/jobset_test.go	Adds unit coverage for the resume scenario and related Build() paths.

[Copilot]

The inner loops use for j := range ... which shadows the method receiver j and makes this block harder to read/grep; rename the loop indices (e.g., cIdx, icIdx) to avoid shadowing.

Suggested change

	for j := range podSpec.Containers {
	podSpec.Containers[j].Image = nil
	}
	for j := range podSpec.InitContainers {
	podSpec.InitContainers[j].Image = nil
	for cIdx := range podSpec.Containers {
	podSpec.Containers[cIdx].Image = nil
	}
	for icIdx := range podSpec.InitContainers {
	podSpec.InitContainers[icIdx].Image = nil

[Copilot]

This test intends to verify both containers and initContainers have images omitted on resume, but the apply template only defines WithContainers(...) and never sets any InitContainers, so the initContainer assertions never exercise the new behavior; add an initContainer with an image to the template (and possibly the PodSets) so the resume/non-resume cases actually validate initContainer image handling.