Set up JobSet failure policy on workloads #308
Conversation
IrvingMg
force-pushed
the
feature/set-up-jobset-failure-policy
branch
from
e5f75ed to
63738ec
Compare
| @@ -445,6 +453,27 @@ def workload_create(args) -> None: | |||
| if return_code != 0: | |||
| xpk_exit(return_code) | |||
|
|
|||
| failure_policy_rules = '' | |||
| pod_failure_policy = '' | |||
| if args.restart_on_user_code_failure: | |||
There was a problem hiding this comment.
This is great to use failure policy!! Looking forward to this change.
I named this argument poorly -- its intent is to allow for the workload to restart additionally on user code failures when set. The range of exit codes this should specify is anything that a user would specify like 1,2,...
Some system level exit codes that should always restart the job are 42, 143. Probably should ask GKE folks what other system level exit codes exist. These should always restart the job and should be set / or assumed in the pod failure policy to restart by default.
For background some customers depend on xpk to restart the job in system failure cases by default and avoid user errors (like code typos) from restarting their workload. So this is the default case.
There was a problem hiding this comment.
To be honest, I don't think anyone actually wants a user error to restart the job so the ideal choice in my opinion is to remove the "args.restart_on_user_code_failure" argument and just "restart the job in system failure cases by default and avoid user errors (like code typos) from restarting their workload". Since now with the failure policy support we can do this.
There was a problem hiding this comment.
Thanks for the information. Then, to sum up:
- Default behavior should be: restart on exit codes 42 and 143.
- Remove
restart-on-user-code-failureflag but allow users to pass additional exit codes for restarting workload.
Is that correct? And how is the user going to pass these additional exit codes, should we have another flag for that?
There was a problem hiding this comment.
@Obliviour can you confirm those are the changes needed?
IrvingMg
force-pushed
the
feature/set-up-jobset-failure-policy
branch
3 times, most recently
from
d8052eb to
7299390
Compare
IrvingMg
requested review from
44past4,
sharabiani,
pawloch00 and
BluValor
as code owners
IrvingMg
force-pushed
the
feature/set-up-jobset-failure-policy
branch
from
7299390 to
f6da574
Compare
Fixes / Features
Testing / Documentation
Testing details.