Feature/add args cronjob spec by fernandoataoldotcom · Pull Request #101 · GlueOps/project-template-helm-chart-app

fernandoataoldotcom · 2026-03-10T16:43:39Z

No description provided.

…mplates - job.yaml: use chart.commonLabels instead of chart.appLabels to match all other templates (adds managed-by, helm.sh/chart, teamowner labels) - job.yaml: fix annotation reference from $.Values.cronJob.annotations to $.Values.job.annotations - cronjob.yaml: replace merge with set for suffixName context to fix label rendering when multiple cronjobs are defined

Introduce chart.jobSpec named template in _jobSpec.tpl that both job.yaml and cronjob.yaml now share. CronJob jobTemplate.spec gains all Job-level fields it was previously missing: - parallelism - completions - backoffLimit - activeDeadlineSeconds - ttlSecondsAfterFinished - podFailurePolicy All new fields are optional — existing values files render identically (verified via baseline diff across all examples and testcases). Includes warning comments in the helper about: - CronJob spec.suspend vs Job spec.suspend having different semantics (scheduling vs pod execution); CronJob keeps suspend at its own spec level, not inside jobTemplate.spec - activeDeadlineSeconds terminating a Job even if backoffLimit retries remain Each field supports both per-job values and global overrides via globalSpec (e.g. cronJob.backoffLimit or job.backoffLimit), consistent with the existing Job pattern.

…t mutation Replace `set $ "suffixName"` with `merge (deepCopy $) (dict "suffixName" $name)` in both cronjob.yaml and job.yaml. `set` mutates the root context ($) in place, causing the last job/cronjob suffixName to leak into templates rendered afterward (e.g., Deployment, Service), incorrectly appending the suffix to their app.kubernetes.io/name label.

Assert that chart.commonLabels renders managed-by and helm.sh/chart in Job metadata, and that job.annotations (not cronJob.annotations) appears in Job metadata.annotations.

Align with the assertion style used throughout the rest of the test suite. isNotEmpty may not be supported by all helm-unittest versions.

Render two cronjobs and assert each document gets a distinct app.kubernetes.io/name label, covering the deepCopy fix that prevents suffixName from leaking between loop iterations.

Map iteration order is non-deterministic, so documentIndex-based assertions can be flaky. Use documentSelector to match documents by metadata.name before asserting labels.

After replacing set $ with deepCopy, the pod template's .Root no longer carries suffixName, causing pod app.kubernetes.io/name labels to lose the job suffix. Pass $newContext (which includes suffixName) as Root so Job/CronJob metadata and pod labels stay consistent.

Extract common Job spec fields into a reusable chart.jobSpec helper (templates/_jobSpec.tpl) used by both job.yaml and cronjob.yaml. New CronJob job spec fields: - parallelism - completions - backoffLimit - activeDeadlineSeconds - ttlSecondsAfterFinished - podFailurePolicy Each field supports both per-job and global override values. CronJob suspend remains at CronJob spec level; Job suspend renders at Job spec level. Added 8 new cronjob unit tests covering all new fields, absent field checks, suspend placement, and multi-cronjob label correctness. Merge branch 'fix/label-and-annotation-inconsistencies' into feature/add-args-cronjob-spec

Copilot

Pull request overview

This PR refactors the Helm chart’s Job and CronJob rendering to share a common Job spec helper, while expanding CronJob support for additional Job-level spec fields and improving labeling consistency.

Changes:

Introduces a shared chart.jobSpec helper to render Job/CronJob jobTemplate spec fields plus the pod template.
Updates Job/CronJob templates to use a deep-copied context for suffix labeling and to centralize spec rendering via the helper.
Expands/updates helm-unittest coverage for CronJob Job-spec fields and for Job commonLabels/annotations behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`values.yaml`	Adds commented examples for additional CronJob jobTemplate Job-spec fields.
`templates/job.yaml`	Switches to `chart.commonLabels`, fixes job-level annotations source, and delegates spec rendering to `chart.jobSpec`.
`templates/cronjob.yaml`	Uses deep-copied context for suffix labels and delegates jobTemplate spec rendering to `chart.jobSpec`.
`templates/_jobSpec.tpl`	New shared helper that renders Job/CronJob Job-level spec fields and the pod template.
`tests/job_test.yaml`	Adds assertions for commonLabels and job annotations on Jobs.
`tests/cronjob_test.yaml`	Adds assertions for newly supported Job-spec fields and validates multi-cronjob label suffixing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Switch jobSpec helper from truthiness to hasKey checks so explicit zero values (e.g. backoffLimit: 0) are honored - Add cronjob test for explicit zero values on all numeric fields - Document parallelism, completions, activeDeadlineSeconds, and podFailurePolicy in the job section of values.yaml

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-10T17:03:11Z

    {{- range $job.labels }}
    {{ .key }}: {{ .value | quote }}
    {{- end }}


Per-job labels are iterated as range $job.labels and then accessed via .key/.value. In values/examples, labels is a map (e.g. labels: {} / labels: {foo: bar}), so rendering non-empty labels will error (can't evaluate field key on a string) and/or produce invalid YAML. Iterate as a map (range $k, $v := $job.labels) to render key/value pairs.

Copilot · 2026-03-10T17:03:11Z

    {{- range $job.annotations }}
    {{.key}}: {{.value | quote}}
    {{- end }}


Per-job annotations are iterated as range $job.annotations and then accessed via .key/.value. Since annotations is documented/used as a map in values/examples, setting any per-job annotations will break template rendering. Iterate over the map keys/values instead.

Copilot · 2026-03-10T17:03:11Z

+    asserts:
+      - equal:
+          path: metadata.annotations["example.com/owner"]
+          value: "my-team"


chart.jobSpec is now used by both Job and CronJob templates, but the new/updated unittest coverage only validates the CronJob rendering of backoffLimit/ttlSecondsAfterFinished/parallelism/completions/activeDeadlineSeconds (including zero values). Add equivalent assertions for the Job template to avoid regressions in Job rendering (especially around the hasKey/zero-value behavior).

Suggested change

value: "my-team"

value: "my-team"

- it: should render jobSpec fields on Job

set:

job:

enabled: true

jobSpec:

backoffLimit: 5

ttlSecondsAfterFinished: 3600

parallelism: 3

completions: 3

activeDeadlineSeconds: 600

jobs:

db-migration:

serviceAccount:

enabled: false

asserts:

- equal:

path: spec.backoffLimit

value: 5

- equal:

path: spec.ttlSecondsAfterFinished

value: 3600

- equal:

path: spec.parallelism

value: 3

- equal:

path: spec.completions

value: 3

- equal:

path: spec.activeDeadlineSeconds

value: 600

- it: should render zero values for jobSpec fields on Job

set:

job:

enabled: true

jobSpec:

backoffLimit: 0

ttlSecondsAfterFinished: 0

parallelism: 0

completions: 0

activeDeadlineSeconds: 0

jobs:

db-migration:

serviceAccount:

enabled: false

asserts:

- equal:

path: spec.backoffLimit

value: 0

- equal:

path: spec.ttlSecondsAfterFinished

value: 0

- equal:

path: spec.parallelism

value: 0

- equal:

path: spec.completions

value: 0

- equal:

path: spec.activeDeadlineSeconds

value: 0

Copilot · 2026-03-10T17:03:12Z

+    {{- $newContext := merge (deepCopy $) (dict "suffixName" $name) }}
+    {{- include "chart.commonLabels" $newContext | indent 4 }}
    {{- if $.Values.cronJob.labels }}
    {{- range $key, $value := $.Values.cronJob.labels }}
    {{$key}}: {{$value | quote}}


CronJob metadata renders per-job labels/annotations using range $job.labels / range $job.annotations with .key/.value. In values.yaml and examples these fields are maps (e.g. labels: {foo: bar}), so setting them will cause template evaluation errors. Update those loops to iterate map key/value pairs (range $k, $v := ...) when rendering CronJobs.

fernandoataoldotcom added 10 commits March 10, 2026 13:43

test: add job metadata label and annotation coverage

f4e3fff

Assert that chart.commonLabels renders managed-by and helm.sh/chart in Job metadata, and that job.annotations (not cronJob.annotations) appears in Job metadata.annotations.

fix: use isNotNullOrEmpty assertion for helm.sh/chart label test

651e473

Align with the assertion style used throughout the rest of the test suite. isNotEmpty may not be supported by all helm-unittest versions.

test: add multi-cronjob label regression test

5a65e7a

Render two cronjobs and assert each document gets a distinct app.kubernetes.io/name label, covering the deepCopy fix that prevents suffixName from leaking between loop iterations.

fix: use documentSelector instead of documentIndex in multi-cronjob test

1a683d0

Map iteration order is non-deterministic, so documentIndex-based assertions can be flaky. Use documentSelector to match documents by metadata.name before asserting labels.

style(job_test): newline

80afa70

Copilot AI review requested due to automatic review settings March 10, 2026 16:43

github-actions Bot added include-in-release-notes enhancement New feature or request patch minor labels Mar 10, 2026

Copilot started reviewing on behalf of fernandoataoldotcom March 10, 2026 16:44 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Comment thread templates/_jobSpec.tpl Outdated

Comment thread tests/cronjob_test.yaml

Comment thread values.yaml

fernandoataoldotcom requested a review from Copilot March 10, 2026 16:57

Copilot started reviewing on behalf of fernandoataoldotcom March 10, 2026 16:57 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature/add args cronjob spec#101

Feature/add args cronjob spec#101
fernandoataoldotcom wants to merge 11 commits into
mainfrom
feature/add-args-cronjob-spec

fernandoataoldotcom commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-          value: "my-team"
+          value: "my-team"
+  - it: should render jobSpec fields on Job
+    set:
+      job:
+        enabled: true
+        jobSpec:
+          backoffLimit: 5
+          ttlSecondsAfterFinished: 3600
+          parallelism: 3
+          completions: 3
+          activeDeadlineSeconds: 600
+        jobs:
+          db-migration:
+            serviceAccount:
+              enabled: false
+    asserts:
+      - equal:
+          path: spec.backoffLimit
+          value: 5
+      - equal:
+          path: spec.ttlSecondsAfterFinished
+          value: 3600
+      - equal:
+          path: spec.parallelism
+          value: 3
+      - equal:
+          path: spec.completions
+          value: 3
+      - equal:
+          path: spec.activeDeadlineSeconds
+          value: 600
+  - it: should render zero values for jobSpec fields on Job
+    set:
+      job:
+        enabled: true
+        jobSpec:
+          backoffLimit: 0
+          ttlSecondsAfterFinished: 0
+          parallelism: 0
+          completions: 0
+          activeDeadlineSeconds: 0
+        jobs:
+          db-migration:
+            serviceAccount:
+              enabled: false
+    asserts:
+      - equal:
+          path: spec.backoffLimit
+          value: 0
+      - equal:
+          path: spec.ttlSecondsAfterFinished
+          value: 0
+      - equal:
+          path: spec.parallelism
+          value: 0
+      - equal:
+          path: spec.completions
+          value: 0
+      - equal:
+          path: spec.activeDeadlineSeconds
+          value: 0

Uh oh!

Conversation

fernandoataoldotcom commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants