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.

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

implement crashLoopBackOff for failed processes #64

Merged
merged 7 commits into from
Aug 10, 2023

Conversation

jkmw
Copy link
Contributor

@jkmw jkmw commented Aug 8, 2023

  • maxAttemps = -1 will restart the job forever

fixes #63

+ `maxAttemps = -1` will restart the job forever
Comment on lines 61 to 63
if maxAttempts == 0 {
maxAttempts = 3
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also (I know from the diff that that's not new behaviour, but still) it's come to my attention that it's annoying for some users (see #62) that you cannot actually set maxAttempts to 0 (because that counts as "not set", and will default to 3). Should we maybe think about solving this differently?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, there are some possibilities that make sense in my opinion. But I can't decide which one is the "best":

  1. the current method from this pr since by default I still have 3 tries if maxAttempts is not set. With -1 you can set infinite.

  2. maxAttempts = 0 means infinite. Against this, however, is that the behavior changes to before and that could possibly be unexpected as well. In addition, you cannot set the job to be executed only once.

  3. maxAttempts = ' actually means 0 attempts. Then infinity would still mean -1. I think this is the nicest solution, but it is also not backward compatible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also lean towards option 3; one thing I could (possibly) think of would be to change the maxAttempts type in the configuration *int (if supported by the HCL parser), so that we could differentiate between "not set" (maxAttempts == nil) and "explicitly set to 0" (*maxAttempts == 0)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! Done.

pkg/proc/job_common.go Outdated Show resolved Hide resolved
@martin-helmich martin-helmich merged commit 0257a36 into master Aug 10, 2023
1 check passed
@martin-helmich martin-helmich deleted the crashLoopBackOff branch August 10, 2023 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Imitate CrashLoopBackoff behaviour of Kubernetes
2 participants