feat: KEP-2437 - PodGroup Creation for Volcano Scheduler #2729
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ import ( | |
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/util/intstr" | ||
| jobsetv1alpha2 "sigs.k8s.io/jobset/api/jobset/v1alpha2" | ||
| volcanov1beta1 "volcano.sh/apis/pkg/apis/scheduling/v1beta1" | ||
| ) | ||
|
|
||
| const ( | ||
|
|
@@ -134,7 +135,8 @@ type PodGroupPolicySource struct { | |
| // Coscheduling plugin from the Kubernetes scheduler-plugins for gang-scheduling. | ||
| Coscheduling *CoschedulingPodGroupPolicySource `json:"coscheduling,omitempty"` | ||
|
|
||
| // Volcano plugin for gang-scheduling. | ||
| Volcano *VolcanoPodGroupPolicySource `json:"volcano,omitempty"` | ||
| } | ||
|
|
||
| // CoschedulingPodGroupPolicySource represents configuration for coscheduling plugin. | ||
|
|
@@ -147,6 +149,25 @@ type CoschedulingPodGroupPolicySource struct { | |
| ScheduleTimeoutSeconds *int32 `json:"scheduleTimeoutSeconds,omitempty"` | ||
| } | ||
|
|
||
| // VolcanoPodGroupPolicySource represents configuration for the Volcano gang-scheduler. | ||
| type VolcanoPodGroupPolicySource struct { | ||
|
There was a problem hiding this comment. Hi, would like to why other fields are not present, like queue, minMember, minResource?
Contributor
Author
There was a problem hiding this comment. As introduced here, we'll use annotation to inject trainer/docs/proposals/2437-volcano-scheduler/README.md Lines 183 to 184 in d239d6a For trainer/docs/proposals/2437-volcano-scheduler/README.md Lines 179 to 180 in d239d6a There was a problem hiding this comment. Great, so users cannot set a custom minmember, it will always be equal to the number of replicas? There was a problem hiding this comment. @Doris-xm currently volcano uses 'scheduling.k8s.io/group-name' as group name annotation, not 'scheduling.volcano.sh/group-name', as can be seen in https://github.com/volcano-sh/volcano/blob/5007c6b010a44518ec2d946c6d126f2dfdadc980/pkg/controllers/podgroup/pg_controller_handler.go#L189 cc @Monokaix There was a problem hiding this comment. Autually also want to know why not put all these attrs in VolcanoPodGroupPolicySource?
Member
There was a problem hiding this comment. As we discussed in the KEP, we want to make it consistent with other schedulers like Coscheduling or Kueue: #2672 (comment) |
||
| // Queue is the name of the Volcano queue to which the PodGroup will be submitted. | ||
| // Defaults to the “default” queue, which has the lowest weight. | ||
| // +kubebuilder:default=default | ||
| Queue *string `json:"queue,omitempty"` | ||
|
|
||
| // PriorityClassName is the name of the Kubernetes PriorityClass to use for the PodGroup. | ||
| // e.g. system-node-critical, system-cluster-critical. | ||
| // This field is optional. | ||
| PriorityClassName *string `json:"priorityClassName,omitempty"` | ||
|
Doris-xm marked this conversation as resolved.
Outdated
|
||
|
|
||
| // NetworkTopology defines the NetworkTopology config, this field works in conjunction with network topology feature and hyperNode CRD. | ||
| // +kubebuilder:validation:EmbeddedResource | ||
| // +kubebuilder:pruning:PreserveUnknownFields | ||
| // +optional | ||
| NetworkTopology *volcanov1beta1.NetworkTopologySpec `json:"networkTopology,omitempty"` | ||
| } | ||
|
|
||
| // MLPolicy represents configuration for the model trining with ML-specific parameters. | ||
| // +kubebuilder:validation:XValidation:rule="!(has(self.numNodes) && (has(self.torch) && has(self.torch.elasticPolicy)))", message="numNodes should not be set if torch.elasticPolicy is configured" | ||
| // +kubebuilder:validation:XValidation:rule="!(has(self.torch) && has(self.mpi))", message="Only one of the policy can be configured" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| /* | ||
| Copyright 2024 The Kubeflow Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package volcano | ||
|
|
||
| import ( | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
|
|
||
| trainer "github.com/kubeflow/trainer/v2/pkg/apis/trainer/v1alpha1" | ||
| ) | ||
|
|
||
|
Member
There was a problem hiding this comment. @Doris-xm @Electronic-Waste @tenzen-y @astefanutti Can we share indexer across all gang-scheduling plugins ?
Contributor
Author
There was a problem hiding this comment. We'll meet indexer conflict panic if we use the same index key. The root cause is that he framework initializes all registered plugins. Both the trainer/pkg/runtime/framework/core/framework.go Lines 54 to 55 in c6997fc Currently, I use different keys to avoid conflicts. trainer/pkg/runtime/framework/plugins/volcano/indexer.go Lines 26 to 27 in 8497e6b Is there a better solution? @andreyvelich @Electronic-Waste @rudeigerc
Member
There was a problem hiding this comment. @Doris-xm Please can you explore if we can create a single indexer that we can re-use across Volcano and Coscheduling plugin ?
Contributor
Author
There was a problem hiding this comment. Yes, I will do that. But still I think the problem is the conflict index key.
Contributor
Author
There was a problem hiding this comment. I've created a single indexer: But I keep two different keys to avoid conflicts as I stated above. trainer/pkg/runtime/framework/plugins/indexer/indexer.go Lines 26 to 29 in 167a595 Still wonder if there's a better solution. |
||
| var ( | ||
| TrainingRuntimeContainerRuntimeClassKey = ".trainingRuntimeSpec.jobSetTemplateSpec.replicatedJobs.podTemplateSpec.runtimeClassName" | ||
| ClusterTrainingRuntimeContainerRuntimeClassKey = ".clusterTrainingRuntimeSpec.jobSetTemplateSpec.replicatedJobs.podTemplateSpec.runtimeClassName" | ||
| ) | ||
|
|
||
| func IndexTrainingRuntimeContainerRuntimeClass(obj client.Object) []string { | ||
| runtime, ok := obj.(*trainer.TrainingRuntime) | ||
| if !ok { | ||
| return nil | ||
| } | ||
| var runtimeClasses []string | ||
| for _, rJob := range runtime.Spec.Template.Spec.ReplicatedJobs { | ||
| if rJob.Template.Spec.Template.Spec.RuntimeClassName != nil { | ||
| runtimeClasses = append(runtimeClasses, *rJob.Template.Spec.Template.Spec.RuntimeClassName) | ||
| } | ||
| } | ||
| return runtimeClasses | ||
| } | ||
|
|
||
| func IndexClusterTrainingRuntimeContainerRuntimeClass(obj client.Object) []string { | ||
| clRuntime, ok := obj.(*trainer.ClusterTrainingRuntime) | ||
| if !ok { | ||
| return nil | ||
| } | ||
| var runtimeClasses []string | ||
| for _, rJob := range clRuntime.Spec.Template.Spec.ReplicatedJobs { | ||
| if rJob.Template.Spec.Template.Spec.RuntimeClassName != nil { | ||
| runtimeClasses = append(runtimeClasses, *rJob.Template.Spec.Template.Spec.RuntimeClassName) | ||
| } | ||
| } | ||
| return runtimeClasses | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.