[yush-1018]

What this PR does / why we need it:

In \pkg/runtime/framework/plugins/coscheduling/coscheduling.go, \Build\ unconditionally dereferences *info.RuntimePolicy.PodGroupPolicy.Coscheduling.ScheduleTimeoutSeconds.

Because \scheduleTimeoutSeconds\ is an optional pointer field in \CoschedulingPodGroupPolicySource\ (+optional, defaulting to 60s), any path that constructs
untime.Info\ without API server defaulting (such as programmatic construction, fake clients, or unit test harnesses) leaves \ScheduleTimeoutSeconds\ as
il, which causes a nil pointer dereference panic.

This PR uses \ptr.Deref\ as defensive hardening to safely fall back to the documented default (\60\ seconds) when \ScheduleTimeoutSeconds\ is nil.

Fixes #4085

Special notes for your reviewer:

• Added unit test case \succeeded to build PodGroup when ScheduleTimeoutSeconds is nil\ using an empty \CoschedulingPodGroupPolicySource{}\ to \TestCoScheduling.
• Verified with \go test -v ./pkg/runtime/framework/plugins/coscheduling/...\ and \go vet.

Checklist:

• Unit tests added and passing.

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

[abhishekKokadwar]

Thanks @yush-1018. The fix looks correct to me, and leaving defaultScheduleTimeoutSeconds untyped is the right call, since ptr.Deref infers the type from the pointer argument.

Overlap with #4106. @DsThakurRawat opened #4106 two days later with the same change, also against #4085. Flagging it only so it's visible; which one goes forward is up to the maintainers and the two of you. I've put my comments here because this one came first, and they apply to #4106 just as much.

One detail from #4106 that might be worth using: its test builds CoschedulingPodGroupPolicySource{} empty rather than setting ScheduleTimeoutSeconds: nil explicitly. Same value, but the empty struct is closer to what a stored object without the field looks like.

On reachability: the explanation I originally gave here was wrong; see my correction below.

Last note: the same unguarded dereference pattern exists in the Volcano plugin at volcano.go:242 for NetworkTopology.HighestTierAllowed, which #4101 is fixing separately. Nothing to do here, just noting it is a class of bug in this area rather than a one-off.

[yush-1018]

Thanks for the review , @abhishekKokadwar!

I've addressed both suggestions:

1. Test improvement: Updated the test case to use an empty CoschedulingPodGroupPolicySource{} instead of explicitly specifying nil, which better mirrors stored objects without the field.
2. PR description: Updated the description to explicitly explain how nil is reachable in practice (runtimes created prior to commit 69a29c2 without subsequent updates, objects restored from older backups/snapshots, and programmatically constructed runtimes).

[abhishekKokadwar]

Correction to my comment above: the reachability reason I gave is wrong.

I read the history from a shallow clone, so 69a29c2 is just where my local history started, not where the default was added. The +kubebuilder:default=60 marker actually came in with #2313 in February 2025, and v2.0.0, the first release to ship TrainingRuntime, already had it. CRD defaults are also applied when an object is read back from etcd, not only on write, so even an older stored runtime would come back with the field set.

So I don't think the nil is reachable through the API server. The guard is defensive hardening for paths that build Info without going through it, like unit tests or fake clients. Still worth having, just please don't use my earlier explanation in the PR description. Sorry for the noise.

[yush-1018]

Thanks, @abhishekKokadwar!

I've updated the PR description to remove the earlier commit reference and frame this purely as defensive hardening for paths that construct runtime.Info outside of API server defaulting (such as programmatic construction, fake clients, and test harnesses).