add volcano gang-scheduler pg min resource calculation #566
Conversation
Signed-off-by: lowang_bh <lhui_wang@163.com>
pkg/controller/podgroup.go
Outdated
| // calculatePGMinResources calculate minResources for volcano podGroup | ||
| // default to total replicas | ||
| // ret: https://github.com/volcano-sh/volcano/blob/1933d46bdc4434772518ebb74c4281671ddeffa1/pkg/webhooks/admission/jobs/mutate/mutate_job.go#L168 | ||
| // ref: https://github.com/volcano-sh/volcano/blob/1933d46bdc4434772518ebb74c4281671ddeffa1/pkg/controllers/job/job_controller_actions.go#L761 | ||
| func (v *VolcanoCtrl) calculatePGMinResources(minMember *int32, mpiJob *kubeflow.MPIJob) *corev1.ResourceList { |
There was a problem hiding this comment.
If we don't set minResources, does the volcano automatically calculate resources and fill out the PodGroup?
There was a problem hiding this comment.
I test this it will not set minResources when it is empty. it will automatically calculate minResources just in a vcJob.
There was a problem hiding this comment.
I have raised an issue a few months ago about this to keep correspding with vcjob when handle other podgroups. volcano-sh/volcano#2791
There was a problem hiding this comment.
I see. Thanks for the clarifications.
Can you update this comment?
pkg/controller/podgroup.go
Outdated
| // calculatePGMinResources calculate minResources for volcano podGroup | ||
| // default to total replicas | ||
| // ret: https://github.com/volcano-sh/volcano/blob/1933d46bdc4434772518ebb74c4281671ddeffa1/pkg/webhooks/admission/jobs/mutate/mutate_job.go#L168 | ||
| // ref: https://github.com/volcano-sh/volcano/blob/1933d46bdc4434772518ebb74c4281671ddeffa1/pkg/controllers/job/job_controller_actions.go#L761 | ||
| func (v *VolcanoCtrl) calculatePGMinResources(minMember *int32, mpiJob *kubeflow.MPIJob) *corev1.ResourceList { |
There was a problem hiding this comment.
I see. Thanks for the clarifications.
Can you update this comment?
|
@lowang-bh Also, can you update the PR description? |
Signed-off-by: lowang_bh <lhui_wang@163.com>
Done |
|
@terrytangyuan Can you approve CI? |
Co-authored-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
There was a problem hiding this comment.
Thanks! @lowang-bh Overall LGTM
/lgtm
/assign @alculquicondor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alculquicondor The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fix #535