vidispine / hull

The incredible HULL - Helm Uniform Layer Library - is a Helm library chart to improve Helm chart based workflows
https://github.com/vidispine/hull
Apache License 2.0
231 stars 13 forks source link

Cronjob jobspec not mapping several fields #313

Closed YoannMa closed 5 months ago

YoannMa commented 5 months ago

Hello,

First I wanted to thank the devs behind this lib, really enjoying using it :+1:

As for this issue, I think fields in jobspec-v1-batch are not being populated in the resulting YAML

I made a small reproduction with this configuration :

hull:
    cronjob:
      _HULL_OBJECT_TYPE_DEFAULT_:
        staticName: true
        schedule: "0 0 * * *"
        successfulJobsHistoryLimit: 1
        failedJobsHistoryLimit: 2
        concurrencyPolicy: Forbid
        job:
          activeDeadlineSeconds: 28800
          backoffLimit: 2
          parallelism: 1
          pod:
            restartPolicy: OnFailure
            containers:
              cron:
                image: 
                  repository: busybox
      foo:
        schedule: "0 0 * * *"
      bar:
        schedule: "0 0 * * *"
        job:
          activeDeadlineSeconds: 28800
          backoffLimit: 2
          parallelism: 1

This would generate this using helm template (removed useless labels/annotations and containers fields) :

---
apiVersion: batch/v1
kind: CronJob
metadata:
  name: bar
spec:
  schedule: 0 0 * * *
  successfulJobsHistoryLimit: 1
  concurrencyPolicy: Forbid
  failedJobsHistoryLimit: 2
  jobTemplate:
    metadata:
      name: bar
    spec:
      template:
        spec:
          restartPolicy: OnFailure
          containers:
          - image: busybox
            name: cron
---
apiVersion: batch/v1
kind: CronJob
metadata:
  name: foo
spec:
  schedule: 0 0 * * *
  successfulJobsHistoryLimit: 1
  concurrencyPolicy: Forbid
  failedJobsHistoryLimit: 2
  jobTemplate:
    metadata:
      name: foo
    spec:
      template:
        spec:
          restartPolicy: OnFailure
          containers:
          - image: busybox
            name: cron

You can see that activeDeadlineSeconds, backoffLimit and parallelism were not populated.

I also noticed the cronjob schedule field is not pulled from _HULL_OBJECT_TYPE_DEFAULT_ as well, you need to explicitly set it for each cronjob, not sure if it's intended to be that way.

YoannMa commented 5 months ago

Never mind, it seems to have been fixed in a later release (I was using v1.26.2)

gre9ory commented 5 months ago

Hello @YoannMa,

yes, 1.26.2 is already pretty old and a lot has been improved since then. Particularly, with the CronJob implementation, there was some severe flaw I found a while ago. Nowadays I think it should work as expected after the creation of CronJobs was aligned with the code for the other pod-based objects.

Please let us know if you find other problems or room for improvement. And thanks for the heads up!