Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PyTorchJob: OnFailure Policy won't handle pod failure gracefully #1570

Closed
georgkaleido opened this issue Apr 6, 2022 · 0 comments · Fixed by #1572
Closed

PyTorchJob: OnFailure Policy won't handle pod failure gracefully #1570

georgkaleido opened this issue Apr 6, 2022 · 0 comments · Fixed by #1572

Comments

@georgkaleido
Copy link
Contributor

When a PyTorchJob with restartPolicy=OnFailure is created, it won't handle all errors correctly.

In case the worker pod status is in Status=Failed due to a graceful node shutdown it won't get recreated, but instead the the PytorchJob while fail in this block:

training-operator/pkg/controller.v1/pytorch/pytorchjob_controller.go

Lines 431 to 443 in 8c43231

} else {
msg := fmt.Sprintf("PyTorchJob %s is failed because %d %s replica(s) failed.", pytorchjob.Name, failed, rtype)
r.Recorder.Event(pytorchjob, corev1.EventTypeNormal, commonutil.JobFailedReason, msg)
if pytorchjob.Status.CompletionTime == nil {
now := metav1.Now()
pytorchjob.Status.CompletionTime = &now
}
err := commonutil.UpdateJobConditions(jobStatus, commonv1.JobFailed, commonutil.JobFailedReason, msg)
if err != nil {
commonutil.LoggerForJob(pytorchjob).Infof("Append job condition error: %v", err)
return err
}
trainingoperatorcommon.FailedJobsCounterInc(pytorchjob.Namespace, pytorchv1.FrameworkName)

I think a fix would be to recreate the pod if policy is OnFailure here:
https://github.com/kubeflow/common/blob/2b40c8f8991e302920ee5536c0ad49dec6724c66/pkg/controller.v1/common/pod.go#L350-L360

I would go ahead an create a PR for this unless there is another way to go about this.

Sample job:

apiVersion: "kubeflow.org/v1"
kind: PyTorchJob
metadata:
  name: restart-failure
spec:
  pytorchReplicaSpecs:
    Worker:
      replicas: 1
      restartPolicy: OnFailure
      template:
        spec:
          containers:
            - name: pytorch
              image: python
              tty: true
              stdin: true

Can be tested with preemptible nodes on GKE
georgkaleido added a commit to georgkaleido/training-operator that referenced this issue Apr 8, 2022
@georgkaleido
Restart job on failure for Always,OnFailure Policy
7dec61f 
Fixes kubeflow#1570
Together with kubeflow/common#189

There can be pod level failures caused by the system, which would perviously caused the entire job to fail on all policies except ExitCode.
@georgkaleido georgkaleido mentioned this issue Apr 8, 2022
Merged
1 task
@yoanisgil yoanisgil mentioned this issue Jun 8, 2022
Closed
1 task
georgkaleido added a commit to georgkaleido/training-operator that referenced this issue Jun 9, 2022
@georgkaleido
Restart job on failure for Always,OnFailure Policy
574efbb 
Fixes kubeflow#1570
Together with kubeflow/common#189

There can be pod level failures caused by the system, which would perviously caused the entire job to fail on all policies except ExitCode.
google-oss-prow bot pushed a commit that referenced this issue Jun 9, 2022
@georgkaleido
Restart job on failure for Always,OnFailure Policy (#1572)
58cc3a0 
Fixes #1570
Together with kubeflow/common#189

There can be pod level failures caused by the system, which would perviously caused the entire job to fail on all policies except ExitCode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Restart job on failure for Always,OnFailure Policy georgkaleido/training-operator
1 participant
@georgkaleido