Skip to content

feat(flagsmith): Allow custom annotations on migrate jobs - #542

Merged
germangarces merged 2 commits into
Flagsmith:mainfrom
ronenl1:feat/migrate-job-annotations
Jun 4, 2026
Merged

feat(flagsmith): Allow custom annotations on migrate jobs#542
germangarces merged 2 commits into
Flagsmith:mainfrom
ronenl1:feat/migrate-job-annotations

Conversation

@ronenl1

@ronenl1 ronenl1 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Thanks for submitting a PR! Please check the boxes below:

  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?

Changes

Add a new annotations value to jobs.migrateDb and jobs.migrateAnalyticsData that is rendered onto the Job resource's own metadata.annotations.

Why

jobs.migrateDb.jobAnnotations looks like it should do this, but it is wired into the pod template (spec.template.metadata.annotations), so it never reaches the Job's metadata. Today, the only way to set Job-level annotations is common.annotations, which is applied to every resource the chart renders (Deployments, Services, Secrets, Ingresses, …).

That makes it impossible — without extraObjects or post-render kustomize — to apply per-resource concerns to just the migrate Job, including:

  • Helm hooks (helm.sh/hook: pre-upgrade, helm.sh/hook-weight, helm.sh/hook-delete-policy). Helm only recognises these on a resource's top-level metadata.annotations (see https://helm.sh/docs/topics/charts_hooks/#writing-a-hook). Promoting the migrate-db Job into a pre-upgrade hook is the standard way to gate a rollout on DB migrations and abort the upgrade cleanly on failure — particularly valuable when running >1 API replica so the new pods don't start serving against an unmigrated schema.
  • ArgoCD sync waves (argocd.argoproj.io/sync-wave).
  • Kyverno / OPA Gatekeeper policy exclusions on specific resources.

Design

  • New value: jobs.migrateDb.annotations (default {}), merged with common.annotations into the Job's metadata.annotations. Job-specific keys win on conflict, matching "more specific wins" convention.
  • Same treatment for jobs.migrateAnalyticsData.annotations for symmetry.
  • The existing jobs.migrateDb.jobAnnotations (pod-template annotations) is left untouched to preserve backward compatibility — despite its misleading name.
  • Default {} means rendered output is identical to today for existing users.

Files changed

  • charts/flagsmith/templates/jobs-migrate-db.yaml
  • charts/flagsmith/templates/jobs-migrate-analytics-data.yaml
  • charts/flagsmith/values.yaml (new value + docstring with a pre-upgrade hook example)

How did you test this code?

helm template against three values configurations:

  1. Default (annotations unset) — output is byte-identical to main; no metadata.annotations block emitted. Backward compatibility verified.

  2. Hook annotations set:

    jobs:
      migrateDb:
        enabled: true
        annotations:
          helm.sh/hook: pre-upgrade
          helm.sh/hook-weight: "-5"
          helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded
      migrateAnalyticsData:
        enabled: true
        annotations:
          argocd.argoproj.io/sync-wave: "-1"

    Renders:

    kind: Job
    metadata:
      name: t-flagsmith-migrate-db-1-xxxxx
      annotations:
        helm.sh/hook: pre-upgrade
        helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded
        helm.sh/hook-weight: "-5"

    and:

    kind: Job
    metadata:
      name: t-flagsmith-migrate-analytics-data-1-xxxxx
      annotations:
        argocd.argoproj.io/sync-wave: "-1"
  3. Conflict with common.annotations — given:

    common:
      annotations:
        company.io/owner: platform
    jobs:
      migrateDb:
        enabled: true
        annotations:
          helm.sh/hook: pre-upgrade
          company.io/owner: migrations-team

    the migrate-db Job correctly merges both sources and the Job-specific company.io/owner: migrations-team wins over the common value, while the analytics Job (no override) still inherits platform.

Verified with helm lint charts/flagsmith (passes, same warnings as main).

The migrate-db and migrate-analytics-data Jobs currently expose
`jobAnnotations`, but those are wired into the pod template
(`spec.template.metadata.annotations`), not the Job's own
`metadata.annotations`.

The only existing source of Job-level annotations is
`common.annotations`, which is applied to every resource the chart
renders. That makes it unusable for per-resource concerns like Helm
hooks (`helm.sh/hook`, `helm.sh/hook-weight`,
`helm.sh/hook-delete-policy`), ArgoCD sync waves, or Kyverno policy
exceptions, all of which must live on the resource's own metadata.

Add a new `jobs.migrateDb.annotations` (and matching
`jobs.migrateAnalyticsData.annotations`) value that is merged into
the Job's `metadata.annotations` on top of `common.annotations`,
with Job-specific keys taking precedence. Defaults to {}, so existing
renders are unchanged.
@ronenl1 ronenl1 changed the title feat(flagsmith): allow custom annotations on migrate jobs feat(flagsmith): Allow custom annotations on migrate jobs Jun 4, 2026
@alexmalev

Copy link
Copy Markdown

good stuff. need this

@germangarces germangarces left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the contribution. Is solid overall. Two small nits to avoid possible headaches in the future.

Comment thread charts/flagsmith/templates/jobs-migrate-analytics-data.yaml Outdated
Comment thread charts/flagsmith/templates/jobs-migrate-db.yaml Outdated
@ronenl1
ronenl1 requested a review from germangarces June 4, 2026 12:42

@germangarces germangarces left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 LGTM
Thanks!

@germangarces
germangarces merged commit a377ab7 into Flagsmith:main Jun 4, 2026
1 check passed
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.

3 participants