You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "calebwoofenden (via GitHub)" <gi...@apache.org> on 2023/02/13 08:27:51 UTC
[GitHub] [airflow] calebwoofenden opened a new issue, #27561: Helm chart tries to patch immutable Job resources on helm upgrade
calebwoofenden opened a new issue, #27561:
URL: https://github.com/apache/airflow/issues/27561
### Apache Airflow version
2.4.2
### What happened
Running `helm upgrade` with helm hooks disabled in a namespace that already has the chart installed will fail because it's trying to patch a `Job` resource, which is immutable:
```
cannot patch "<release_name>-run-airflow-migrations" with kind Job: Job.batch "<release_name>-run-airflow-migrations" is invalid: spec.template: Invalid value: core.PodTemplateSpec {<...entire json spec of Job resource...>}: field is immutable
```
This happens when helm hooks are disabled on the db migrations job:
```
migrateDatabaseJob:
useHelmHooks: false
```
For more detail on why a user would want to do this, see https://github.com/apache/airflow/issues/11979
Copying this from my comment on that issue:
> Kube does offer a parameter called .spec.ttlSecondsAfterFinished that, when specified, will delete the Job after the specified number of seconds after completion (see https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/). This is a pretty good solution, though installs done in a period of time shorter than this value might still run into the same problem.
>
> The solution I propose here is to set a helm pre-install hook on the database deployment as well as the db migrations, and set the hook weight on the database lower than that of the migrations, so the database will be created first, then the migrations will run, then the webserver and etc. pods will come up. Then I think we could safely remove the wait-for-db-migrations initContainer on the deployments.
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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: commits-unsubscribe@airflow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] alexandermalyga commented on issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by "alexandermalyga (via GitHub)" <gi...@apache.org>.
alexandermalyga commented on issue #27561:
URL: https://github.com/apache/airflow/issues/27561#issuecomment-1439792777
Yes, it is fixed, but we discussed a follow-up improvement: https://github.com/apache/airflow/pull/29314#pullrequestreview-1295134749
We agreed with @potiuk that it would be done in a separate PR, but I don't have much time to do it right now.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Throne3d commented on issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by "Throne3d (via GitHub)" <gi...@apache.org>.
Throne3d commented on issue #27561:
URL: https://github.com/apache/airflow/issues/27561#issuecomment-1732030327
Now ttlSecondsAfterFinished has been added, is the recommended approach still as documented? Reading the thread, it sounds like instead of (as in [the documentation](https://airflow.apache.org/docs/helm-chart/stable/index.html#installing-the-chart-with-argo-cd-flux-rancher-or-terraform)):
```yaml
createUserJob:
useHelmHooks: false
applyCustomEnv: false
migrateDatabaseJob:
useHelmHooks: false
applyCustomEnv: false
```
the recommendation is instead again:
```yaml
createUserJob:
useHelmHooks: false
migrateDatabaseJob:
useHelmHooks: false
```
making use of the implicit `ttlSecondsAfterFinished` to drop the jobs between deploys before the next patch comes along and tries to mutate immutable fields.
Should I create a follow-up to update the documentation with this recommendation, and a caveat that deploys < ttlSecondsAfterFinished apart may still trigger the underlying Job mutation issue?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27561:
URL: https://github.com/apache/airflow/issues/27561#issuecomment-1357699942
Marking as fixed then.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] alexandermalyga commented on issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by "alexandermalyga (via GitHub)" <gi...@apache.org>.
alexandermalyga commented on issue #27561:
URL: https://github.com/apache/airflow/issues/27561#issuecomment-1427644243
Just to clarify, #27148 can indeed fix this issue, but it comes with the effect of removing env vars secrets from the migration job, which the job may use to connect to the database. #29314 should be a better fix for this issue.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] sunghospark-calm commented on issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by GitBox <gi...@apache.org>.
sunghospark-calm commented on issue #27561:
URL: https://github.com/apache/airflow/issues/27561#issuecomment-1329870939
Running into the same problem here.
Had to disable the hooks because migration was not running properly during upgrade, without hooks the followup deployment fails because of the job spec conflict.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on issue #27561:
URL: https://github.com/apache/airflow/issues/27561#issuecomment-1439788045
@alexandermalyga with PR merged this issue is now resolved right?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] alexandermalyga commented on issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by "alexandermalyga (via GitHub)" <gi...@apache.org>.
alexandermalyga commented on issue #27561:
URL: https://github.com/apache/airflow/issues/27561#issuecomment-1439793241
> @alexandermalyga with PR merged this issue is now resolved right?
Yes, it is fixed, but we discussed a follow-up improvement: https://github.com/apache/airflow/pull/29314#pullrequestreview-1295134749
We agreed with @potiuk that it would be done in a separate PR, but I don't have much time to do it right now.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal closed issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal closed issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
URL: https://github.com/apache/airflow/issues/27561
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] alexandermalyga commented on issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by "alexandermalyga (via GitHub)" <gi...@apache.org>.
alexandermalyga commented on issue #27561:
URL: https://github.com/apache/airflow/issues/27561#issuecomment-1412500470
Just tried `applyCustomEnv: false` with `1.8.0rc1` and I'm still getting `cannot patch "run-airflow-migrations" with kind Job ... field is immutable`, so https://github.com/apache/airflow/pull/27148 does not fix this issue.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mconigliaro commented on issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by GitBox <gi...@apache.org>.
mconigliaro commented on issue #27561:
URL: https://github.com/apache/airflow/issues/27561#issuecomment-1343412885
I think https://github.com/apache/airflow/pull/27148 might be the fix for this. It adds `createUserJob.applyCustomEnv` and `migrateDatabaseJob.applyCustomEnv` options which we should set to `false`. We're just waiting on a new release of the helm chart.
Related: https://github.com/apache/airflow/issues/21943
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by GitBox <gi...@apache.org>.
potiuk closed issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
URL: https://github.com/apache/airflow/issues/27561
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #27561: Helm chart tries to patch immutable Job resources on helm upgrade
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on issue #27561:
URL: https://github.com/apache/airflow/issues/27561#issuecomment-1439797374
OK so I'm closing this issue as resolved. Please open new issue dedicated to the followup task
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org