hiddeco / cronjobber

Cronjobber is a cronjob controller for Kubernetes with support for time zones
Apache License 2.0
244 stars 38 forks source link

Implement default values for job history limits #39

Open hikhvar opened 3 years ago

hikhvar commented 3 years ago

According to the documentation of both the cronjob and tzcronjob the defaults for the job limits are either 1 for failed jobs and 3 for successful jobs. Currently tzCronjobber ignores those defaults completely.

obitech commented 3 years ago

@hiddeco any opinions on this PR? This could really help us since there are a lot of completed jobs in our clusters. It would make it much easier to adjust this on the controller-side, other than edit each TZCronJob.

hiddeco commented 3 years ago

:wave:, sorry it took me so long to get to this.

This is indeed a mistake from my side, and thanks for noticing this and creating a (working) patch. The more idiomatic way of doing this, and also how this is done in the Kubernetes cronjob controller, is by defining default values in the v1alpha1 API package.

This approach has my preference as it does not create "buried" default values in the orchestrating controller code.

hikhvar commented 3 years ago

Hey sorry for the long delay, was also busy and forgot this PR. I tried the defaulters from the upstream controller in #41. However in my test setup, this does not work. I'm not that familiar with the kubernetes API machinery. Can you maybe give me a hint what I did wrong?