From ebfec956d26b8eab3269cbae86f148187282e488 Mon Sep 17 00:00:00 2001 From: Bernedotcom2312 <88426124+Bernedotcom2312@users.noreply.github.com> Date: Fri, 2 Oct 2026 19:13:52 +0200 Subject: [PATCH] chore(helm): drop migrationJob values the chart never reads (#42141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `migrationJob.retries` and `migrationJob.disableSchemaUpdate` are declared in values.yaml but referenced by no template, no test and no README row. Setting either changes nothing about the rendered Job. `disableSchemaUpdate` is the misleading one: its comment promises "the job will exit with code 0", but the Job hardcodes DISABLE_SCHEMA_UPDATE=false and renders it after envVars/extraEnvVars precisely so nothing can turn the migration off — that ordering is what #12809 fixed. An operator who reads values.yaml, sets the flag and watches migrations run anyway has no way to tell the knob is inert. `migrationJob.enabled: false` is the supported way to skip the Job, and the componentized chart in helm/litellm already ships a migrationJob block with neither key. `retries` is simply dead: Jobs retry through `backoffLimit`, which the chart does render. Removing values keys is backward compatible — Helm ignores user values that no template consumes, so existing releases setting either key keep working. Adds a test pinning the override: with envVars.DISABLE_SCHEMA_UPDATE="true" the Job's last env entry is still DISABLE_SCHEMA_UPDATE=false, so the last-wins ordering cannot regress and the key cannot quietly come back as a chart value. Verified by mutation: flipping the hardcoded value and moving the entry above the envVars loop each fail the suite. Co-authored-by: Claude Opus 5 Co-authored-by: ryan-crabbe-berri --- .../tests/migrations-job_tests.yaml | 18 ++++++++++++++++++ helm/litellm-helm/values.yaml | 2 -- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/helm/litellm-helm/tests/migrations-job_tests.yaml b/helm/litellm-helm/tests/migrations-job_tests.yaml index 1fe545636d4..dd4276ac60f 100644 --- a/helm/litellm-helm/tests/migrations-job_tests.yaml +++ b/helm/litellm-helm/tests/migrations-job_tests.yaml @@ -112,6 +112,24 @@ tests: name: CUSTOM_VAR value: "custom_value" + - it: should override a user-supplied DISABLE_SCHEMA_UPDATE so the Job always migrates + template: migrations-job.yaml + set: + envVars: + DISABLE_SCHEMA_UPDATE: "true" + migrationJob: + enabled: true + asserts: + # The Job is what owns the schema, so it renders its own + # DISABLE_SCHEMA_UPDATE=false after envVars and extraEnvVars. Kubernetes + # takes the last value for a duplicated name, so the user's "true" cannot + # leave the schema unmigrated. Skipping migrations is migrationJob.enabled. + - equal: + path: spec.template.spec.containers[0].env[-1] + value: + name: DISABLE_SCHEMA_UPDATE + value: "false" + - it: should not include DATABASE_URL when deployStandalone is false template: migrations-job.yaml set: diff --git a/helm/litellm-helm/values.yaml b/helm/litellm-helm/values.yaml index fcee331a5aa..03d2a66a2b5 100644 --- a/helm/litellm-helm/values.yaml +++ b/helm/litellm-helm/values.yaml @@ -545,7 +545,6 @@ redis: # Prisma migration job settings migrationJob: enabled: true # Enable or disable the schema migration Job - retries: 3 # Number of retries for the Job in case of failure backoffLimit: 4 # Backoff limit for Job restarts # Wall-clock budget for the whole Job, shared across every `backoffLimit` # retry rather than granted per attempt. Without it a migration that blocks @@ -554,7 +553,6 @@ migrationJob: # stop reconciling the whole chart until someone deletes the Job by hand. # Set to null to opt out and restore the unbounded behaviour. activeDeadlineSeconds: 1800 - disableSchemaUpdate: false # Skip schema migrations for specific environments. When True, the job will exit with code 0. # Optional service account for the migration job. # Only used when migrationJob.hooks.helm.enabled=true and serviceAccount.create=true. # In that case, pre-install/pre-upgrade hooks run before normal resources, so this defaults to "default".