Make scheduler-plugins the default gang scheduler. #1747

Syulin7 · 2023-01-29T09:59:34Z

What this PR does / why we need it:
Training Operator now supports many gang schedulers(volcano, scheduler-plugins), and now we can easily add koordinator gang scheduler.

Related: #1746

Reviewers can check the koordinator gang schedule feature with the koordinator in the following steps:

# install koordinator
$ helm repo add koordinator-sh https://koordinator-sh.github.io/charts/
$ helm repo update
$ helm install koordinator koordinator-sh/koordinator --version 1.1.0
# pull this pr and run training-operator with --gang-scheduler-name=koord-scheduler
$ cat <<EOF | kubectl apply -f -
apiVersion: "kubeflow.org/v1"
kind: TFJob
metadata:
  name: tfjob-simple
  namespace: kubeflow
spec:
  runPolicy:
    schedulingPolicy:
      minAvailable: 10
      scheduleTimeoutSeconds: 120
  tfReplicaSpecs:
    Worker:
      replicas: 2
      restartPolicy: OnFailure
      template:
        spec:
          schedulerName: koord-scheduler
          containers:
          - name: tensorflow
            image: gcr.io/kubeflow-ci/tf-mnist-with-summaries:1.0
            command:
            - "python"
            - "/var/tf_mnist/mnist_with_summaries.py"
EOF
$ kubectl get pods -n kubeflow
NAME                    READY   STATUS    RESTARTS   AGE
tfjob-simple-worker-0   0/1     Pending   0          82s
tfjob-simple-worker-1   0/1     Pending   0          82s
$ kubectl describe pods -n kubeflow tfjob-simple-worker-0 | grep -A10  Events
Events:
  Type     Reason            Age   From             Message
  ----     ------            ----  ----             -------
  Warning  FailedScheduling  15s   koord-scheduler  running PreFilter plugin "Coscheduling": gang child pod not collect enough, gangName: kubeflow/tfjob-simple, podName: kubeflow/tfjob-simple-worker-0

Which issue(s) this PR fixes (optional, in Fixes #<issue number>, #<issue number>, ... format, will close the issue(s) when PR gets merged):
Fixes #1746

Checklist:

Docs included if any changes are user facing

Syulin7 · 2023-01-29T10:03:05Z

@tenzen-y @johnugeorge PTAL, Related: kubeflow/common#209

tenzen-y · 2023-01-29T15:32:28Z

/hold

tenzen-y

LGTM

tenzen-y · 2023-02-05T11:13:30Z

cmd/training-operator.v1/main.go

@@ -73,7 +73,8 @@ func main() {
 	flag.StringVar(&leaderElectionID, "leader-election-id", "1ca428e5.training-operator.kubeflow.org", "The ID for leader election.")
 	flag.Var(&enabledSchemes, "enable-scheme", "Enable scheme(s) as --enable-scheme=tfjob --enable-scheme=pytorchjob, case insensitive."+
 		" Now supporting TFJob, PyTorchJob, MXNetJob, XGBoostJob, PaddleJob. By default, all supported schemes will be enabled.")
-	flag.StringVar(&gangSchedulerName, "gang-scheduler-name", "none", "The scheduler to gang-schedule kubeflow jobs, defaults to none")
+	flag.StringVar(&gangSchedulerName, "gang-scheduler-name", "none", "The scheduler to gang-schedule kubeflow jobs, defaults to none."+
+		" Now supporting node, volcano, scheduler-plugins, koord-scheduler.")


Suggested change

" Now supporting node, volcano, scheduler-plugins, koord-scheduler.")

" Now supporting none, volcano, scheduler-plugins, koord-scheduler.")

coveralls · 2023-03-13T12:46:53Z

Pull Request Test Coverage Report for Build 4412816813

0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.2%) to 39.555%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/apis/kubeflow.org/v1/openapi_generated.go	0	1	0.0%

Files with Coverage Reduction	New Missed Lines	%
pkg/controller.v1/mpi/mpijob_controller.go	2	77.16%

Totals
Change from base Build 4405001546:	0.2%
Covered Lines:	2740
Relevant Lines:	6927

💛 - Coveralls

johnugeorge · 2023-05-17T11:30:51Z

@tenzen-y Are there any blockers here?

tenzen-y · 2023-05-17T17:28:56Z

@johnugeorge We need to cut a release on the common repository.

kubeflow/common#209 (comment)

johnugeorge · 2023-05-17T20:20:36Z

@tenzen-y created 0.4.7 release https://github.com/kubeflow/common/releases/tag/v0.4.7

tenzen-y · 2023-05-17T20:23:05Z

@johnugeorge Thank you!

@Syulin7 Can you update this PR with a new common library version?

Syulin7 · 2023-05-18T03:17:51Z

/assign
Thanks, I will update it this week.

tenzen-y

@Syulin7 Thank you for the updates!
Mostly, LGTM. I left a comment for a nit.

tenzen-y · 2023-05-19T22:24:09Z

cmd/training-operator.v1/main.go

@@ -73,7 +73,8 @@ func main() {
 	flag.StringVar(&leaderElectionID, "leader-election-id", "1ca428e5.training-operator.kubeflow.org", "The ID for leader election.")
 	flag.Var(&enabledSchemes, "enable-scheme", "Enable scheme(s) as --enable-scheme=tfjob --enable-scheme=pytorchjob, case insensitive."+
 		" Now supporting TFJob, PyTorchJob, MXNetJob, XGBoostJob, PaddleJob. By default, all supported schemes will be enabled.")
-	flag.StringVar(&gangSchedulerName, "gang-scheduler-name", "none", "The scheduler to gang-schedule kubeflow jobs, defaults to none")
+	flag.StringVar(&gangSchedulerName, "gang-scheduler-name", "", "The scheduler to gang-schedule kubeflow jobs."+
+		" Now supporting volcano, default-scheduler, scheduler-plugins, koord-scheduler.")


Suggested change

" Now supporting volcano, default-scheduler, scheduler-plugins, koord-scheduler.")

" Now Supporting volcano and scheduler-plugins. Note: If you set another scheduler name, the training-operator assumes it's the scheduler-plugins.")

Signed-off-by: Syulin7 <735122171@qq.com>

tenzen-y

@Syulin7 Great! Thank you for the awesome contribution!

/lgtm
/assign @johnugeorge

tenzen-y · 2023-05-20T17:15:47Z

/hold cancel

johnugeorge · 2023-05-23T11:09:27Z

Thanks @Syulin7

Need to update docs( https://www.kubeflow.org/docs/components/training/job-scheduling/#running-jobs-with-gang-scheduling) regarding this.

/approve

google-oss-prow · 2023-05-23T11:09:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnugeorge, Syulin7

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:

~~OWNERS~~ [johnugeorge]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* Support for k8s v1.25 in CI * Support for k8s v1.25 in CI * Change k8s api to v1.25 * Upgrade golangci-lint version * Add changelog * Update CHANGELOG.md * Update Changelog * Merge common repo * Avoid to depend on local env when installing the code-generators (#1810) Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com> * Make scheduler-plugins the default gang scheduler. (#1747) Signed-off-by: Syulin7 <735122171@qq.com> * Fix tests * Fix merge conflicts * Fix CI issues * Fix CI issues * Fix review comments * Add contributors in Readme file * Fix review comments * Fix review comments --------- Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com> Signed-off-by: Syulin7 <735122171@qq.com> Co-authored-by: Yuki Iwai <yuki.iwai.tz@gmail.com> Co-authored-by: yu lin <37265556+Syulin7@users.noreply.github.com>

google-oss-prow bot requested review from jinchihe and tenzen-y January 29, 2023 09:59

google-oss-prow bot added the size/S label Jan 29, 2023

google-oss-prow bot added the do-not-merge/hold label Jan 29, 2023

Syulin7 mentioned this pull request Jan 31, 2023

Support koordinator gang scheduler plugin #1746

Closed

tenzen-y reviewed Jan 31, 2023

View reviewed changes

tenzen-y reviewed Feb 5, 2023

View reviewed changes

Syulin7 force-pushed the koordinator branch from fe66559 to 8c0e348 Compare February 5, 2023 12:27

Syulin7 force-pushed the koordinator branch from 8c0e348 to b6215ce Compare March 13, 2023 12:43

Syulin7 force-pushed the koordinator branch 3 times, most recently from 67452d5 to e08164d Compare March 14, 2023 06:17

google-oss-prow bot added size/M and removed size/S labels Mar 14, 2023

tenzen-y mentioned this pull request Mar 28, 2023

Make scheduler-plugins the default gang scheduler kubeflow/common#209

Merged

google-oss-prow bot assigned Syulin7 May 18, 2023

Syulin7 force-pushed the koordinator branch from e08164d to 37a0051 Compare May 19, 2023 08:27

tenzen-y reviewed May 19, 2023

View reviewed changes

Set SchedulerPluginsControl as the default implementation

b94ea76

Signed-off-by: Syulin7 <735122171@qq.com>

Syulin7 force-pushed the koordinator branch from 37a0051 to b94ea76 Compare May 20, 2023 17:10

Syulin7 changed the title ~~Support Koordinator Gang Scheduler Plugin~~ Make scheduler-plugins the default gang scheduler. May 20, 2023

tenzen-y reviewed May 20, 2023

View reviewed changes

google-oss-prow bot assigned johnugeorge and tenzen-y May 20, 2023

google-oss-prow bot added the lgtm label May 20, 2023

google-oss-prow bot removed the do-not-merge/hold label May 20, 2023

google-oss-prow bot added the approved label May 23, 2023

google-oss-prow bot merged commit 6d6d83d into kubeflow:master May 23, 2023

tenzen-y mentioned this pull request May 23, 2023

Consider the way to inform users that the specified gang scheduler isn't supported #1811

Closed

johnugeorge mentioned this pull request Aug 5, 2023

[Release] Training operator 1.7.0 release #1809

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make scheduler-plugins the default gang scheduler. #1747

Make scheduler-plugins the default gang scheduler. #1747

Syulin7 commented Jan 29, 2023

Syulin7 commented Jan 29, 2023

tenzen-y commented Jan 29, 2023

tenzen-y left a comment

tenzen-y Feb 5, 2023

Syulin7 Feb 5, 2023

coveralls commented Mar 13, 2023 •

edited

Loading

johnugeorge commented May 17, 2023

tenzen-y commented May 17, 2023

johnugeorge commented May 17, 2023

tenzen-y commented May 17, 2023

Syulin7 commented May 18, 2023

tenzen-y left a comment

tenzen-y May 19, 2023

Syulin7 May 20, 2023

tenzen-y left a comment

tenzen-y commented May 20, 2023

johnugeorge commented May 23, 2023

google-oss-prow bot commented May 23, 2023

	" Now supporting node, volcano, scheduler-plugins, koord-scheduler.")
	" Now supporting none, volcano, scheduler-plugins, koord-scheduler.")

	" Now supporting volcano, default-scheduler, scheduler-plugins, koord-scheduler.")
	" Now Supporting volcano and scheduler-plugins. Note: If you set another scheduler name, the training-operator assumes it's the scheduler-plugins.")

Make scheduler-plugins the default gang scheduler. #1747

Make scheduler-plugins the default gang scheduler. #1747

Conversation

Syulin7 commented Jan 29, 2023

Syulin7 commented Jan 29, 2023

tenzen-y commented Jan 29, 2023

tenzen-y left a comment

Choose a reason for hiding this comment

tenzen-y Feb 5, 2023

Choose a reason for hiding this comment

Syulin7 Feb 5, 2023

Choose a reason for hiding this comment

coveralls commented Mar 13, 2023 • edited Loading

Pull Request Test Coverage Report for Build 4412816813

💛 - Coveralls

johnugeorge commented May 17, 2023

tenzen-y commented May 17, 2023

johnugeorge commented May 17, 2023

tenzen-y commented May 17, 2023

Syulin7 commented May 18, 2023

tenzen-y left a comment

Choose a reason for hiding this comment

tenzen-y May 19, 2023

Choose a reason for hiding this comment

Syulin7 May 20, 2023

Choose a reason for hiding this comment

tenzen-y left a comment

Choose a reason for hiding this comment

tenzen-y commented May 20, 2023

johnugeorge commented May 23, 2023

google-oss-prow bot commented May 23, 2023

coveralls commented Mar 13, 2023 •

edited

Loading