Repository navigation
fix(controller): fall back to default scheduleTimeoutSeconds when unset - #4106
DsThakurRawat wants to merge 1 commit into
Conversation
…nset In the Coscheduling plugin Build method, ScheduleTimeoutSeconds was unconditionally dereferenced. Because scheduleTimeoutSeconds is an optional field in CoschedulingPodGroupPolicySource with +kubebuilder:default=60, configuring coscheduling without specifying scheduleTimeoutSeconds caused a nil pointer dereference panic during reconciliation. Safely dereference ScheduleTimeoutSeconds using ptr.Deref with the documented default of 60 seconds, and add unit test coverage for the unset case. Fixes kubeflow#4085 Assisted-by: Claude Code Signed-off-by: Divyansh Rawat <divyanshrawatofficial@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
🎉 Welcome to the Kubeflow Trainer! 🎉 Thanks for opening your first PR! We're happy to have you as part of our community 🚀 Here's what happens next:
Join the community:
Feel free to ask questions in the comments if you need any help or clarification! |
|
hi can any one take a look at this |
|
Thanks @DsThakurRawat. Flagging that this overlaps with #4086, which @yush-1018 opened on 21 September against the same issue, #4085: same constant, same One thing yours does nicely is build |
|
Thanks @abhishekKokadwar for the review and notice. Since #4086 was opened first and already adopted the empty struct test shape, closing this in favor of #4086 to keep the review stream consolidated. |
What this PR does
In the Coscheduling plugin
Buildmethod (pkg/runtime/framework/plugins/coscheduling/coscheduling.go),ScheduleTimeoutSecondswas unconditionally dereferenced.Because
scheduleTimeoutSecondsis an optional field inCoschedulingPodGroupPolicySource(+optional,+kubebuilder:default=60), any runtime configured withpodGroupPolicy.coscheduling: {}(or created programmatically withoutScheduleTimeoutSecondsset) causes the controller worker to panic with a nil pointer dereference during reconciliation.This PR:
ScheduleTimeoutSecondsusingptr.Derefwith a default of 60 seconds (defaultScheduleTimeoutSeconds).coscheduling_test.goverifying that building a PodGroup whenScheduleTimeoutSecondsis unset successfully assigns the default 60-second scheduling timeout without panicking.Fixes #4085
How has this been tested?
go test -v ./pkg/runtime/framework/plugins/coscheduling/...passing.go test ./pkg/runtime/framework/...passing.make fmt vet verify-boilerplatepassing.