Add job suspend semantics by xiaoxubeii · Pull Request #196 · kubeflow/common

xiaoxubeii · 2022-07-04T09:27:39Z

To support job suspend semantics like Kubernetes batch job: https://kubernetes.io/docs/concepts/workloads/controllers/job/#suspending-a-job

google-oss-prow · 2022-07-04T09:27:46Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign gaocegege after the PR has been reviewed.
You can assign the PR to them by writing /assign @gaocegege in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

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

gaocegege · 2022-07-04T09:46:34Z

/ok-to-test

gaocegege · 2022-07-04T09:50:09Z

Thanks for the PR, is it ready to review?

xiaoxubeii · 2022-07-14T00:20:09Z

Thanks for the PR, is it ready to review?

@gaocegege Ready for review. Thanks :)

kerthcet · 2022-07-14T05:07:03Z

pkg/controller.v1/common/job.go

+	if err != nil {
+		return err
+	}
+	if commonutil.IsSucceeded(jobStatus) || commonutil.IsFailed(jobStatus) || (jobSuspended != nil && *jobSuspended) {


My concern here is Suspend is just a transition state, should we delete all the pods or just the active ones, leaving the completed pods(succeeded/failed) as they are.

If anything, it should have the same semantics as kubernetes Job, where we delete the running pods.

Yes, the current implementation is consistent with batch/job.

gaocegege · 2022-07-14T05:37:56Z

pkg/controller.v1/common/job.go

 }
+
+func (jc *JobController) JobSuspended(job interface{}) (*bool, error) {
+	log.Infof("Not implemented.")


I am wondering if we should merge this since the feature is not completed.

This is a base class default function in case Job subclasses(TFJob, MPIJob, etc.) do not implement this method.

Actually it will be override in Job subclass which supports job suspend.

If DeletePodsAndServices has an implementation, I don't see why this function wouldn't have one.

ggaaooppeenngg · 2022-10-17T08:57:28Z

How is this PR going now?

alculquicondor · 2023-01-05T13:27:08Z

Is this actively being worked on? Or will we get rid of the common repo first?

tenzen-y · 2023-01-06T19:53:42Z

Is this actively being worked on? Or will we get rid of the common repo first?

@alculquicondor Maybe, we will work on the Job suspend feature in the next kubeflow release cycle (maybe kubeflow v1.8?). Since we didn't push this feature to the enhancement lists for the next kubeflow release (v1.7) and the feature freeze for the next kubeflow version (v1.7) is coming up.

kubeflow/trainer#1683

Wed Jan 25th 2023 Week 18 Release Team Feature Freeze

https://github.com/kubeflow/community/blob/6ba2e0e754166989d2f0d06aae827ceafdb65b29/releases/release-1.7/README.md

johnugeorge · 2023-01-10T16:29:04Z

Agree. we will take this up in next release after our merging kubeflow/common as planned in kubeflow/trainer#1714 (comment)

alculquicondor · 2023-01-20T19:54:29Z

@tenzen-y how do you feel about starting with the integration for mpi-operator v2 and follow through with training-operator later?
It might give us a better chance to iterate faster and learn.

tenzen-y · 2023-01-20T20:02:05Z

@tenzen-y how do you feel about starting with the integration for mpi-operator v2 and follow through with training-operator later? It might give us a better chance to iterate faster and learn.

@alculquicondor Yes. that is a good idea. I was thinking of the same.
Although, we need to move forward kubernetes-sigs/kueue#369 before we adapt mpi-operator to Kueue.

alculquicondor · 2023-01-20T20:12:08Z

Excellent!
We can work on the kueue side in parallel, while we add support for suspend in the mpi-operator.

tenzen-y · 2023-01-20T20:23:15Z

Excellent! We can work on the kueue side in parallel, while we add support for suspend in the mpi-operator.

You are right. I will work on the following steps after kubeflow feature freeze date (1/25) since I have no enough bandwidth for mpi-operator v2, now:

Upgrade Kubernetes dependencies mpi-operator#502
Support coscheduling plugin mpi-operator#500
Support suspend in mpi-operator

Although, other anyone can take step 3 after step 1 is completed.

alculquicondor · 2023-01-24T16:06:45Z

@mimowo will help with suspend in mpi-operator kubeflow/mpi-operator#504

tenzen-y · 2023-01-24T16:08:44Z

Great! Thanks to @mimowo!

xiaoxubeii · 2023-03-14T02:16:08Z

Is this actively being worked on? Or will we get rid of the common repo first?

@alculquicondor Maybe, we will work on the Job suspend feature in the next kubeflow release cycle (maybe kubeflow v1.8?). Since we didn't push this feature to the enhancement lists for the next kubeflow release (v1.7) and the feature freeze for the next kubeflow version (v1.7) is coming up.

kubeflow/training-operator#1683

Wed Jan 25th 2023 Week 18 Release Team Feature Freeze

https://github.com/kubeflow/community/blob/6ba2e0e754166989d2f0d06aae827ceafdb65b29/releases/release-1.7/README.md

Agreed. We could try to work on Job suspend feature for kubeflow v1.8.

alculquicondor · 2023-03-29T15:36:03Z

@johnugeorge how are we doing with the branch creation?
Can we proceed with this PR or move it to training-operator?

Add job suspend semantics.

11c1e3d

google-oss-prow bot requested review from gaocegege and terrytangyuan July 4, 2022 09:27

google-oss-prow bot added the size/S label Jul 4, 2022

xiaoxubeii mentioned this pull request Jul 14, 2022

Support kubeflow operator kubernetes-sigs/kueue#297

Closed

2 tasks

kerthcet reviewed Jul 14, 2022

View reviewed changes

gaocegege reviewed Jul 14, 2022

View reviewed changes

tenzen-y mentioned this pull request Jan 5, 2023

Support queue-related logic with kube-queue kubeflow/trainer#1519

Closed

tenzen-y mentioned this pull request Jan 10, 2023

Introduce batch/v1 Job with Indexed completion mode kubeflow/trainer#1718

Closed

alculquicondor mentioned this pull request Mar 20, 2023

Can we use tfjob or pytorchjob in kueue? kubernetes-sigs/kueue#652

Closed

mwielgus mentioned this pull request Mar 31, 2023

☂️ Requirements for v0.4 kubernetes-sigs/kueue#636

Closed

Conversation

xiaoxubeii commented Jul 4, 2022

Uh oh!

google-oss-prow bot commented Jul 4, 2022

Uh oh!

gaocegege commented Jul 4, 2022

Uh oh!

gaocegege commented Jul 4, 2022

Uh oh!

xiaoxubeii commented Jul 14, 2022

Uh oh!

kerthcet Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

alculquicondor Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

xiaoxubeii Jul 27, 2022

Choose a reason for hiding this comment

Uh oh!

gaocegege Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

xiaoxubeii Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

xiaoxubeii Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

alculquicondor Jul 27, 2022

Choose a reason for hiding this comment

Uh oh!

ggaaooppeenngg commented Oct 17, 2022

Uh oh!

alculquicondor commented Jan 5, 2023

Uh oh!

tenzen-y commented Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnugeorge commented Jan 10, 2023

Uh oh!

alculquicondor commented Jan 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tenzen-y commented Jan 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alculquicondor commented Jan 20, 2023

Uh oh!

tenzen-y commented Jan 20, 2023

Uh oh!

alculquicondor commented Jan 24, 2023

Uh oh!

tenzen-y commented Jan 24, 2023

Uh oh!

xiaoxubeii commented Mar 14, 2023

Uh oh!

alculquicondor commented Mar 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tenzen-y commented Jan 6, 2023 •

edited

Loading

alculquicondor commented Jan 20, 2023 •

edited

Loading

tenzen-y commented Jan 20, 2023 •

edited

Loading