Skip to content

[Feature] Add opt-in post-upgrade job to trigger MLRun DB migrations - #325

Merged
GiladShapira94 merged 8 commits into
mlrun:developmentfrom
GiladShapira94:mlrun-trigger
Oct 29, 2026
Merged

GiladShapira94 merged 8 commits into
mlrun:developmentfrom
GiladShapira94:mlrun-trigger

Conversation

@GiladShapira94

Copy link
Copy Markdown
Collaborator

📝 Description

Adds an optional Job that triggers MLRun DB migrations automatically after helm upgrade, so users no longer have to run mlrun.get_run_db().trigger_migrations() by hand.

The Job is rendered only when mlrunMigrations.enabled=true, mlrun.enabled=true and the release is an upgrade (.Release.IsUpgrade).

Before sending POST /api/v1/operations/migrations, it makes sure the new API chief is up: it waits for the mlrun-api-chief rollout, for every chief pod to run the expected mlrun.api.image, and for /api/v1/client-spec to report the expected version.

It is a regular release resource, not a Helm hook, so long migrations can't make helm upgrade time out (also with --wait, unless --wait-for-jobs is set).

It is disabled by default, so existing installations are not affected.


🛠️ Changes Made

  • charts/mlrun-ce/templates/mlrun-migrations/migrations-job.yaml (new): Job <release>-mlrun-migrations-<revision>.
    • Guarded by {{- if and .Values.mlrun.enabled .Values.mlrunMigrations.enabled .Release.IsUpgrade }}.
    • Has no helm.sh/hook annotations. The revision suffix creates a fresh Job on every upgrade, because Job specs are immutable. Helm deletes the previous revision's Job because it is no longer part of the release.
    • Single container (alpine/kubectl:1.33.4, sh + curl):
      1. Waits for kubectl rollout status deployment/mlrun-api-chief.
      2. Waits until every chief pod, including terminating ones, runs exactly mlrun.api.image.
      3. Polls GET /api/v1/client-spec until the chief reports the expected version.
      4. Sends POST /api/v1/operations/migrations. A 200 means no migrations were needed and the Job succeeds. A 202 returns a background task, which the Job polls at GET /api/v1/background-tasks/<name> until it is succeeded or failed. Any other status fails the Job.
    • restartPolicy: Never, backoffLimit and activeDeadlineSeconds come from values.
    • There is no ttlSecondsAfterFinished, so the Job and its pods stay for inspection until the next upgrade replaces them.
  • charts/mlrun-ce/templates/mlrun-migrations/rbac.yaml (new): ServiceAccount, Role and RoleBinding <release>-mlrun-migrations, with read-only get/list/watch on deployments and replicasets and get/list on pods. They are rendered whenever mlrun.enabled and mlrunMigrations.enabled are true, including on install.
  • charts/mlrun-ce/templates/NOTES.txt: on upgrades with the feature enabled, prints the kubectl logs -f job/<release>-mlrun-migrations-<revision> command.
  • charts/mlrun-ce/templates/_helpers.tpl: new helpers mlrun-ce.mlrun.api.chief.fullname, mlrun-ce.mlrun.api.image, mlrun-ce.mlrunMigrations.labels and mlrun-ce.mlrunMigrations.selectorLabels.
  • charts/mlrun-ce/values.yaml: new top-level mlrunMigrations block with enabled: false, expectedVersion: "" (defaults to mlrun.api.image.tag), pollIntervalSeconds: 5, timeoutSeconds: 3600, backoffLimit: 2, kubectlImage (alpine/kubectl:1.33.4) and resources.
  • admin_installation_values.yaml, non_admin_installation_values.yaml and non_admin_cluster_ip_installation_values.yaml: added mlrunMigrations.enabled: false.
  • charts/mlrun-ce/README.md: the "Triggering DB Migrations" section now documents the automatic option and how to check the Job result.
  • AGENTS.md: added the mlrun-migrations/ template directory and a row in the Component Dependency Map.
  • charts/mlrun-ce/Chart.yaml: version bumped from 0.12.1-rc.4 to 0.12.1-rc.5.

✅ Checklist

  • I have tested the changes in this PR
  • I confirmed whether my changes require a change in documentation and if so, I created another PR in MLRun for the relevant documentation.
  • I confirmed whether my changes require changes in QA tests, for example: credentials changes, resources naming change and if so, I updated the relevant Jira ticket for QA.
  • I increased the Chart version in charts/mlrun-ce/Chart.yaml.
  • I confirmed that the installation works both on a local Docker Desktop environment and on a real cluster when using the required prerequisites.
    • If installation issues were found, I updated the relevant Jira ticket with the issue and steps to reproduce, or updated the prerequisites documentation if the issue is related to missing or outdated prerequisites.
  • If needed, update https://github.com/mlrun/ce/blob/development/charts/mlrun-ce/README.md with the relevant installation instructions and version Matrix.
  • If needed, update the following values files for multi namespace support:

🧪 Testing

  • helm lint charts/mlrun-ce passes. ct lint was not run because ct is not installed locally.
  • helm template --set mlrunMigrations.enabled=true renders only the RBAC on install. With --is-upgrade it also renders the Job mlrun-mlrun-migrations-<revision> without hook annotations. Nothing renders in admin mode (mlrun.enabled: false) or when the flag is off.
  • The rendered values resolve from the mlrun sub-chart defaults: EXPECTED_IMAGE=quay.io/mlrun/mlrun-api:1.12.0, EXPECTED_VERSION=1.12.0 and MLRUN_CHIEF_URL=http://mlrun-api-chief.<namespace>.svc.cluster.local:8080. Overriding mlrun.api.image.tag updates both the image and the version.
  • sh -n syntax check of the rendered script passes.
  • Confirmed that alpine/kubectl:1.33.4 ships sh and curl but not bash or jq.
  • --wait behaviour was checked against the Helm and kstatus source. In both the default and the legacy wait modes, Helm does not wait for a running Job unless --wait-for-jobs is set.
  • The API contract was checked against the mlrun source (1.12.x). After an upgrade that needs migrations, the chief starts in waiting_for_migrations and only allows healthz, client-spec, migrations and background-tasks. POST /operations/migrations returns 200, 202 with a background task, or 412 when migrations already failed.
  • Not yet tested: the sh/curl script end to end, and a real upgrade that requires migrations.

🔗 References

  • Ticket link:
  • External links: MLRun migrations endpoint server/py/services/api/api/endpoints/operations.py (POST /operations/migrations)
  • Design docs links (Optional):

🚨 Breaking Changes?

  • Yes (explain below)
  • No

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The migration Job can exceed Kubernetes name limits for valid Helm releases, while the documented value-reuse approach and hardcoded naming assumptions can cause rendering failures or prevent migrations.

5 open issues
What changed in this PR

Adds opt-in database migrations to the MLRun CE Helm chart, replacing the manual trigger with an upgrade-only Job. The feature is disabled by default.

Changes:

  • Adds a migration Job that checks chief readiness, image, and version, then triggers migrations and monitors completion.
  • Adds read-only permissions, naming helpers, and configuration across installation modes.
  • Documents usage and log inspection, and bumps the chart to 0.12.1-rc.5.
File Description
charts/​mlrun-ce/​values.yaml Defines migration settings, image, timeouts, and resources.
charts/​mlrun-ce/​templates/​NOTES.txt Prints the migration Job logs command on enabled upgrades.
charts/​mlrun-ce/​templates/​mlrun-migrations/​rbac.yaml Adds the ServiceAccount and read-only permissions.
charts/​mlrun-ce/​templates/​mlrun-migrations/​migrations-job.yaml Implements readiness checks, migration triggering, and task polling.
charts/​mlrun-ce/​templates/​_helpers.tpl Adds chief-name, image, and migration-label helpers.
charts/​mlrun-ce/​README.md Documents automatic migrations and result inspection.
charts/​mlrun-ce/​non_admin_installation_values.yaml Keeps migrations disabled in NodePort mode.
charts/​mlrun-ce/​non_admin_cluster_ip_installation_values.yaml Keeps migrations disabled in ClusterIP mode.
charts/​mlrun-ce/​Chart.yaml Bumps the chart version to 0.12.1-rc.5.
charts/​mlrun-ce/​admin_installation_values.yaml Keeps migrations disabled in admin mode.
AGENTS.md Documents the migration component and its dependencies.

Review effort: Balanced

Comment thread charts/mlrun-ce/README.md Resolved
Comment thread charts/mlrun-ce/templates/_helpers.tpl Resolved

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Existing-release upgrades can fail to render when migration values are absent, and incorrect chief resource naming can prevent migrations from running.

1 open issue
5 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The migration job must write parsing diagnostics to stderr because command substitution captures stdout and set -e exits without logging the error or response body.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Write JSON parsing errors to stderr for job diagnostics

charts/​mlrun-ce/​templates/​mlrun-migrations/​migrations-job.yaml:66

Write failure diagnostics to stderr. Calls such as version=$(json_string version) capture stdout, including this error message when a JSON key is missing; set -e then exits the script without printing the captured output. The Job therefore fails without logging the parsing error or response body, making malformed or changed API responses difficult to diagnose.

1 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Human assessment is needed because the migrations Job sends no credentials to protected routes, causing HTTP 401 failures and leaving migrations untriggered when authentication is enabled.

No open issues 🎉

Review effort: Balanced

@royischoss royischoss left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@GiladShapira94
GiladShapira94 merged commit efa1815 into mlrun:development Oct 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants