Closed rllin closed 1 year ago
extra information:
Chief
works as expected:
restartPolicy
of Chief
to ExitCode
Worker
however does not work either like Evaluator
/cc @zw0610 @kubeflow/wg-training-leads
Hi @rllin,
For busybox, I use /bin/sh to make it run, otherwise pod will get a containerCannotRun error.
It is unrelated to the main issue.
@cheimu sorry, that's my bad, i had to replace an internal command with the sleep
so had not doublechecked the command properly.
Well, it's a very tricky problem. It took me a very long time to figure it out.
call chain is following:
restarting
condition.We got a for loop here, for @rllin cases, we will iterate 2 times, chief
first, and then evaluator
: at loop 1, conds are: creating
and restarting
, and rtype is chief
, so it will "append" a new running
cond to job as shown in figure. HOWEVER, for this case, it is not append at all.
creating
and restarting
, and the new one is running
, which will be continued (as shown in figure), so restarting
cond will be lost. And the new job conds become creating
and running
, then at loop 2 (when for evaluator
), because there is a failed
pod with running
cond, so jobstatus will become failed
as well
Here the job is set to failed
.
https://github.com/kubeflow/training-operator/blob/master/pkg/controller.v1/tensorflow/tfjob_controller.go#L526, and the whole job is failed....
@zw0610 @gaocegege Hi experts, am I correct about filterOutCondition()
? I think it is confusing...
This is inline with what we think is the issue. But we weren't sure if the bugfix was to be in the UpdateJobConditions or if line 504 in the second loop needs to check for JobRunning
.
This is inline with what we think is the issue. But we weren't sure if the bugfix was to be in the UpdateJobConditions or if line 504 in the second loop needs to check for
JobRunning
.
yeah, probably.
I've tried to record the restarting
cond first, then append it back, it passed the unit tests and worked for this situation. Please discuss to see if there is a better way to handle it elegantly! : D
/cc @jian-he
Seems that the code is from @jian-he, would you mind having a look at this issue?
@cheimu thanks for the quick turnaround!
Expectations: if a replica has
restartPolicy=ExitCode
, then a pod deletion (triggers137
) should cause that pod to restart without triggering TFJob failure. Reality: The entire TFJob fails.However, the correct behavior happens for
OnFailure
where the pod is properly restarted without the entire job failing.How to reproduce:
restartPolicy
withOnFailure
, repeat steps 1,2 and see that the pod restarts without failing the job.Suspected issue (may not be core issue):
[0] TFJob spec.