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

* helm cyclic dependency fix

* updating test case for handling edge cases

deploy/charts/litellm-helm/templates/_helpers.tpl

@@ -61,6 +61,20 @@ Create the name of the service account to use
 {{- end }}
 {{- end }}
 
+{{/*
+Create the service account name used by migration jobs.
+When Helm hooks are enabled, pre-install/pre-upgrade hooks run before normal resources.
+If this chart is creating the ServiceAccount, it is not yet available for the hook job,
+so fall back to "default" (or an explicit override) to avoid a cyclic dependency.
+*/}}
+{{- define "litellm.migrationServiceAccountName" -}}
+{{- if and .Values.migrationJob.hooks.helm.enabled .Values.serviceAccount.create }}
+{{- default "default" .Values.migrationJob.serviceAccountName }}
+{{- else }}
+{{- include "litellm.serviceAccountName" . }}
+{{- end }}
+{{- end }}
+
 {{/*
 Get redis service name
 */}}

deploy/charts/litellm-helm/templates/migrations-job.yaml

@@ -34,7 +34,7 @@ spec:
       imagePullSecrets:
         {{- toYaml . | nindent 8 }}
       {{- end }}
-      serviceAccountName: {{ include "litellm.serviceAccountName" . }}
+      serviceAccountName: {{ include "litellm.migrationServiceAccountName" . }}
       {{- with .Values.migrationJob.extraInitContainers }}
       initContainers:
         {{- toYaml . | nindent 8 }}

deploy/charts/litellm-helm/tests/migrations-job_tests.yaml

@@ -124,4 +124,67 @@ tests:
       - notContains:
           path: spec.template.spec.containers[0].env
           content:
-            name: DATABASE_URL
+            name: DATABASE_URL
+
+  - it: should use default service account for helm hooks when serviceAccount.create is true
+    template: migrations-job.yaml
+    set:
+      migrationJob:
+        enabled: true
+        hooks:
+          helm:
+            enabled: true
+      serviceAccount:
+        create: true
+    asserts:
+      - equal:
+          path: spec.template.spec.serviceAccountName
+          value: default
+
+  - it: should use migrationJob.serviceAccountName override for helm hooks when serviceAccount.create is true
+    template: migrations-job.yaml
+    set:
+      migrationJob:
+        enabled: true
+        serviceAccountName: migration-sa
+        hooks:
+          helm:
+            enabled: true
+      serviceAccount:
+        create: true
+    asserts:
+      - equal:
+          path: spec.template.spec.serviceAccountName
+          value: migration-sa
+
+  - it: should use chart service account when helm hooks are disabled
+    template: migrations-job.yaml
+    set:
+      migrationJob:
+        enabled: true
+        hooks:
+          helm:
+            enabled: false
+      serviceAccount:
+        create: true
+        name: my-custom-sa
+    asserts:
+      - equal:
+          path: spec.template.spec.serviceAccountName
+          value: my-custom-sa
+
+  - 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

deploy/charts/litellm-helm/values.yaml

@@ -299,6 +299,10 @@ migrationJob:
   retries: 3 # Number of retries for the Job in case of failure
   backoffLimit: 4 # Backoff limit for Job restarts
   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".
+  serviceAccountName: ""
   annotations: {}
   ttlSecondsAfterFinished: 120
   resources: {}