Support helm install --wait for the chart

[FloChehab]

Hello,

We are used to using helm install / helm upgrade with the --wait option ; which is nice because the command returns only if install / upgrade is kind of done and not just "commited".

Unfortunately it doesn't currently play really nice with the chart as:

• The migrate database job is set to run "post-install" & "post-upgrade" (https://github.com/apache/airflow/blob/master/chart/templates/migrate-database-job.yaml#L35)
• The init containers actually wait for the migrations to be applied on install.

So if the database hasn't been migrated yet, the install / upgrade fails waiting for the migrations to be applied with the --wait option.

One option would be to switch to run the migrations with pre-install and pre-upgrade hooks instead.
I'd enable this behavior only if we have an external database (in a more production friendly env. where we are more likely to use the --wait option too -?-). If we use a database deployed by the chart, I don't think this could play nice.

What do you think ?

PS: another small question related to this ; on airflow versions bump (with schema migrations to perform), are we expecting the RollingUpdate of pods to work (ie are the schemas backward compatible for one airflow version bump).

[aquam8]

Hi,

First of all, let me thank you for this project and your involvement in this awesome project.

Now onto this issue.. I have come to realise that this is also the problem I am encountering.
This is also the problem reported in this issue (see the use of --wait): #15340 where it's reported that the webserver fails reporting no migrations ran.

I could not understand why the Job resources would not get created, I could see the ServiceAccount resources but not the jobs.
I then created YAML files manually from the helm get values command, and they got created and ran successfully.
After looking more into it I have realised that they are Helm Hooks and indeed will not be installed when Helm is used in conjunction with --wait flag in an install/upgrade command.

I would argue that you cannot make assumption on how helm update is run - usually it's as part of a CI/CD pipeline and applies to potentially many other helm releases. I don't think you can force people to forgot '--wait' flag especially because of its nature. As the OP explains, that flag is particularly critical as it waits for the upgrade to be successful or it rolls back.

I think the solution is to not make these jobs as Hooks - but I'm not sure of the reason. In the past I have switched from Hooks to normal job to run my app migrations as part of normal app deployment. Applying migrations multiple time as no-op is a regular use-case.

I would say that this is a serious issue and should be prioritised to allow a smooth install and upgrade. At the moment, fresh install for people using --wait will fail.

Some more info on the problem:
https://helm.sh/docs/topics/charts_hooks/ says:

Note that if the --wait flag is set, the library will wait until all resources are in a ready state and will not run the post-install hook until they are ready.

At the same time, the webserver & scheduler wait for the migrations to have run, so we have no READY pod, and it's a dead-end. Eventually the Helm --wait will time out and roll back.

Thank you for your consideration

[drdenzy]

I will like to look into this @kaxil

[thesuperzapper]

In the airflow-helm chart, we implemented a value to fix this called helmWait, it just removes the post-install hook from our db migration job.

However this can result in an issue with "field is immutable", as described our issue: airflow-helm/charts#230

[vsimon]

I just got bit by this. Was running it part as part of a CI/CD pipeline using the same arguments as my other helm releases (with --wait). It was confusing why this was happening, noticed that no Jobs were being deployed, started removing arguments one by one until removing --wait worked. Then found this issue :)

[vsimon]

This also precludes the use of the --atomic option as well since that internally sets the wait flag.

[potiuk]

@kaxil @jedcunningham -> i do agre we have to fix it in the official chart, it's kinda unexpected behaviour and I am sure we can do better than post-install :D

[kaxil]

Yeah this is something we have on our list to fix sooner, we hate post-install !!!

This will be either Helm Chart 1.3.0 or 2.0.0