Copilot commented on code in PR #68074:
URL: https://github.com/apache/airflow/pull/68074#discussion_r3376369384
##########
chart/templates/jobs/migrate-database-job.yaml:
##########
@@ -43,7 +43,7 @@ metadata:
{{- end }}
{{- $annotations := dict }}
{{- if .Values.migrateDatabaseJob.useHelmHooks }}
- {{- $_ := set $annotations "helm.sh/hook" "post-install,post-upgrade" }}
+ {{- $_ := set $annotations "helm.sh/hook" "pre-install,pre-upgrade" }}
{{- $_ := set $annotations "helm.sh/hook-weight" "1" }}
{{- $_ := set $annotations "helm.sh/hook-delete-policy"
"before-hook-creation,hook-succeeded" }}
{{- end }}
Review Comment:
The migrate-database Job is now a `pre-install,pre-upgrade` hook, but it
still runs under `serviceAccountName: {{ include
"migrateDatabaseJob.serviceAccountName" . }}`. Helm does not create non-hook
resources (including the migrate-database-job ServiceAccount and these new
Role/RoleBindings) before running pre-install/pre-upgrade hooks, so on a fresh
install the Job will start before its ServiceAccount exists, and on the first
downgrade after adopting this chart version the Job may run before the new RBAC
is created. To make the hook reliable, the ServiceAccount (and any required
Role/RoleBinding) need to be created as hook resources with a lower hook weight
than the Job, or the Job hook timing needs to be adjusted so required RBAC
exists before it runs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
