[Issue#21344]: avoid migration hook serviceaccount dependency cycle

[devarakondasrikanth]

Relevant issues

Fixes #21344

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

• I have Added testing in the tests/litellm/ directory, Adding at least 1 test is a hard requirement - see details
• My PR passes all unit tests on make test-unit
• My PR's scope is as isolated as possible, it only solves 1 specific problem
• I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

CI (LiteLLM team)

CI status guideline:

• 50-55 passing tests: main is stable with minor issues.
• 45-49 passing tests: acceptable but needs attention
• <= 40 passing tests: unstable; be careful with your merges and assess the risk.

• Branch creation CI run
  Link:

• CI run for the last commit
  Link:

• Merge / cherry-pick CI run
  Links:

Type

🐛 Bug Fix
✅ Test

Changes

• Fixed Helm chart cyclic dependency when migrationJob.hooks.helm.enabled=true and serviceAccount.create=true.
• Added a migration-specific helper to resolve service account safely for Helm pre-install/pre-upgrade hooks.
• Updated migration job template to use the new helper.
• Added migrationJob.serviceAccountName value for explicit override in hook mode.
• Added Helm unit tests for:
  - default fallback to default SA in hook mode.
  - explicit migration SA override in hook mode
• Maintained existing behavior when Helm hooks are disabled.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
litellm	Ready	Preview, Comment	Feb 17, 2026 8:39pm

[devarakondasrikanth]

@greptileai

[greptile-apps]

Greptile Summary

This PR fixes a Helm chart cyclic dependency (Issue #21344) that occurred when migrationJob.hooks.helm.enabled=true and serviceAccount.create=true. The migration job, annotated as a pre-install/pre-upgrade Helm hook, would reference a ServiceAccount that hadn't been created yet (since hooks run before normal resources), causing deployment failures.

• Added a new litellm.migrationServiceAccountName helper in _helpers.tpl that falls back to the Kubernetes default service account when both Helm hooks and SA creation are enabled
• Added migrationJob.serviceAccountName in values.yaml for explicit override in hook mode
• Updated migrations-job.yaml to use the new helper (single-line change)
• Added three Helm unit tests covering default fallback, explicit override, and non-hook behavior preservation
• Existing behavior is fully preserved when Helm hooks are disabled

Confidence Score: 5/5

• This PR is safe to merge — it's a targeted fix for a well-understood Helm hook ordering issue with correct logic and adequate test coverage.
• The changes are minimal and well-scoped: a new template helper, a single-line template update, a new values.yaml field, and three unit tests. The logic correctly handles all edge cases (helm hooks enabled/disabled, SA create true/false, explicit override). The fix preserves backward compatibility — when Helm hooks are disabled, the behavior is unchanged. The cyclic dependency root cause is well-understood and the solution is the standard Helm pattern for this class of problem.
• No files require special attention

Important Files Changed

Filename	Overview
deploy/charts/litellm-helm/templates/_helpers.tpl	New litellm.migrationServiceAccountName helper added to resolve service account for migration jobs, avoiding cyclic dependency when Helm hooks are enabled alongside serviceAccount.create=true. Logic is correct and well-documented with an inline comment.
deploy/charts/litellm-helm/templates/migrations-job.yaml	Single-line change replacing litellm.serviceAccountName with litellm.migrationServiceAccountName. Clean and minimal change that correctly delegates to the new helper.
deploy/charts/litellm-helm/tests/migrations-job_tests.yaml	Three new Helm unit tests covering: default fallback to default SA, explicit override, and existing behavior preservation. Also fixes a missing newline at end of file. Could benefit from one additional test for helm.enabled=true with serviceAccount.create=false.
deploy/charts/litellm-helm/values.yaml	New migrationJob.serviceAccountName field with clear documentation about when it is used. Default empty string correctly triggers fallback to "default" in the helper template.

Flowchart

flowchart TD
    A[migrations-job.yaml] -->|calls| B["litellm.migrationServiceAccountName"]
    B --> C{"helm.hooks.enabled AND\nserviceAccount.create?"}
    C -->|Yes| D{"migrationJob.serviceAccountName\nset?"}
    D -->|Yes| E["Use migrationJob.serviceAccountName"]
    D -->|No / empty| F["Use 'default' SA"]
    C -->|No| G["litellm.serviceAccountName"]
    G --> H{"serviceAccount.create?"}
    H -->|Yes| I["Use chart fullname SA"]
    H -->|No| J{"serviceAccount.name set?"}
    J -->|Yes| K["Use serviceAccount.name"]
    J -->|No| L["Use 'default' SA"]

Loading

_{Last reviewed commit: 7644217}

[greptile-apps]

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Consider adding a test for pre-existing SA with hooks

The tests cover the three main scenarios well. One additional edge case worth testing: helm.enabled=true with serviceAccount.create=false and an explicit serviceAccount.name. In this case, the SA already exists (not managed by the chart), so there's no cyclic dependency and the helper should pass through the pre-existing SA name rather than falling back to default. This would confirm the else branch of the new helper behaves correctly when hooks are active but the SA isn't chart-managed.

  - it: should use pre-existing service account when helm hooks are enabled but serviceAccount.create is false
    template: migrations-job.yaml
    set:
      migrationJob:
        enabled: true
        hooks:
          helm:
            enabled: true
      serviceAccount:
        create: false
        name: pre-existing-sa
    asserts:
      - equal:
          path: spec.template.spec.serviceAccountName
          value: pre-existing-sa

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

[devarakondasrikanth]

Updated the tests to handle above mentioned issue.