refactor PodGroup control with more generic design #185
Conversation
|
test pr with kubeflow training-operator for this common pr: kubeflow/training-operator#1526 |
pkg/controller.v1/common/pod.go
Outdated
| @@ -469,7 +456,7 @@ func (jc *JobController) createNewPod(job interface{}, rt string, index int, spe | |||
| return nil | |||
| } | |||
|
|
|||
| func isNonGangSchedulerSet(replicas map[apiv1.ReplicaType]*apiv1.ReplicaSpec) bool { | |||
| func isAnotherGangSchedulerSet(replicas map[apiv1.ReplicaType]*apiv1.ReplicaSpec, gangSchedulerName string) bool { | |||
There was a problem hiding this comment.
Perhaps something like isCustomSchedulerSet?
| "k8s.io/apimachinery/pkg/util/intstr" | ||
| ) | ||
|
|
||
| type PGFillSpecFunc func(object metav1.Object) error |
There was a problem hiding this comment.
PodGroup. Maybe FillPodGroupSpecFunc will be more illustrative.
| @@ -191,7 +193,7 @@ func (jc *JobController) ReconcileJobs( | |||
| return err | |||
| } | |||
|
|
|||
| if jc.Config.EnableGangScheduling { | |||
| if jc.Config.EnableGangScheduling() { | |||
There was a problem hiding this comment.
Is this typo or intentional?
There was a problem hiding this comment.
This is intentional. Because this PR is expanding the gang-scheduler options, instead of adding another field for the name of gang-scheduler, Config only stores the name of gang-scheduler. The EnableGangScheduling() method returns false if None is set for the name of the gang-scheduler, otherwise it returns true.
|
[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.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
I take over this PR. |
|
/close
…On Sat, Jan 21, 2023 at 23:02 Yuki Iwai ***@***.***> wrote:
@zw0610 <https://github.com/zw0610> We can close this PR since #203
<#203> is merged.
—
Reply to this email directly, view it on GitHub
<#185 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AK7V6IW7FWRX7JWUCUOTPDDWTP3ATANCNFSM5M3H4B3A>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@zw0610: Closed this PR. In response to this:
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. |
To support scheduler-plugins/coscheduling, this pr refactors the podgroup-related code.
Addressing: kubeflow/training-operator#1518
Test PR: kubeflow/training-operator#1526