Add Production Helm chart support

[schnie]

This PR adds a default helm chart for Airflow. This chart is based on the one we use at Astronomer to manage hundreds of production deployments.

The chart has been cleaned up to remove any references to the Astronomer platform or anything specific to the surrounding infrastructure.

The bin directory contains some scripts to help run this chart locally or in CI using kind, as well as packaging and releasing it to a helm repository. The scripts reference the Astronomer helm repo right now, but I figured we could use it as a starting point and tweak these scripts, or remove them completely to fit into how we want to manage this going forward. Opening this as a draft PR for now to start some discussion.

---

Make sure to mark the boxes below before creating PR: [x]

• Description above provides context of the change
• Unit tests coverage for changes (not needed for documentation changes)
• Target Github ISSUE in description if exists
• Commits follow "How to write a good git commit message"
• Relevant documentation is updated including usage instructions.
• I will engage committers as explained in Contribution Workflow Example.

---

In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

[thesuperzapper]

I am also working on something similar, which is initially a rewrite of the stable/airflow chart.

[thesuperzapper]

@schnie I have released v7.0.0 of the stable/airflow chart, see here.

There definitely are a few changes from your chart which would improve things in stable/airflow (like how you do health checks), but there are features in the stable/airflow chart which users need for production/enterprise environments, so we should either aim to include them in a new official chart, or start from the stable/airflow one.

Regardless, I think we should decouple the release of airflow from its Helm chart (and potentially keep it in a seperate repo). Also note, the official helm/charts repo is being depreciated at the end of the year, so the community will need an upgrade path.

[Rep1AI]

I want to use the official airflow image, is it just changing this image?
Or changes required across the chart?

[schnie]

Great point. @dimberman has some experience with this. I believe the intention is to follow up this PR with a better integration into the official image as well as Airflow 2.0. Any thoughts on this Daniel?

[dimberman]

@schnie I'd say let's merge what we have and I will then

1. Create a PR making it compatible with the airflow 2.0 official image
2. Create a backport making it compatible with the 1.10 image.

[thesuperzapper]

I really don't understand how the 'offical' helm chart for airflow could use anything but the offical Dockerfile.
For reference, we already use the offical Dockerfile at stable/airflow

[marclamberti]

@thesuperzapper There is an issue in the Helm chart.
redis: enabled: false
produces the following error:
spec.template.spec.containers[0].env[1].valueFrom.secretKeyRef.name: Invalid value: \"\": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
which is related to this in _helpers.tpl :
name: REDIS_PASSWORD valueFrom: secretKeyRef: name: {{ .Values.externalRedis.passwordSecret }} key: {{ .Values.externalRedis.passwordSecretKey }}
where passwordSecret is by default sets to "" and so the error.

Furthermore, I think it should be easier to set it for the KubernetesExecutor as well. It depends too much of the CeleryExecutor whereas both are quite used now.

Hope it helps :)

[thesuperzapper]

@marclamberti, thanks for the heads up, you'll be glad to know that fixes for both issues you raised have made their way into v7.1.0 of the chart.

[thesuperzapper]

@schnie, I can't see a section of this chart which will allow users to sync their DAGs from an external git repo. I think this is probably the single most common deployment config for airflow, you need to support it.

[thesuperzapper]

@schnie, not all K8S clusters support Ingress that are not on the public internet, therefore not all companies be able to use this chart.

You should add support for exposing the webserver/flower as LoadBalancer type Services, (and therefore using Airflow's inbuilt SSL support for HTTPS.

[turbaszek]

Hi @schnie, can we help you somehow with moving this PR forward? 😉

[marclamberti]

Wouldn't be easier to start with the chart of @thesuperzapper ? In the sense that it already include a lot of features ? I'm pretty sure it is widely used too as it is available on the stable Helm charts repo

[streetmapp]

I would definitely agree in terms of basing it from the chart that is in the stable Helm charts repo. There are some interesting components in the Astronomer and Bitnami charts that could make this chart really good. I think another good thing would be increasing the ease of getting going with the Kubernetes executor too.

[dimberman]

@marclamberti we get a lot of questions from users of the community chart who can't seem to get it to work/have all sorts of weird issues. Has this not been your experience?

@streetmapp completely agreed. I'm working on backporting a fix that will allow users to apply arbitrary pod specs via yaml files on the KubernetesExecutor (hopefully in time for 1.10.11), and will then write much more documentation around that.

[marclamberti]

@dimberman Well indeed, I got some issues too, one at this moment actually. I agreed with @streetmapp my point was, there are three charts right now, having some very interesting features that could be nice to merge in some way. Btw, I would be glad to help :)

[schnie]

@schnie, not all K8S clusters support Ingress that are not on the public internet, therefore not all companies be able to use this chart.

You should add support for exposing the webserver/flower as LoadBalancer type Services, (and therefore using Airflow's inbuilt SSL support for HTTPS.

Great point, I meant to fix this before pushing. For now, I've removed the Ingress object and made the Webserver and Flower Service types configurable, defaulting to ClusterIP.

[dimberman]

@schnie can you make the yamllint tests pass please? I think we can merge an initial PR just to get the code in and then start fixing it on master.

[streetmapp]

The ingress components were already capable of being on/off were they not? Seems odd to remove them if that was the case.

[thesuperzapper]

@dimberman, I honestly think we should consider putting any official helm charts in a separate repository, as there is little reason to pin their releases to airflow itself.

[potiuk]

@dimberman, I honestly think we should consider putting any official helm charts in a separate repository, as there is little reason to pin their releases to airflow itself.

@thesuperzapper The main reason is that we're going to make both helm charts and production image used in automated tests of airflow - this way they will both be automatically tested during the tests of airflow. We are basically going to "eat of our dog food" as the term goes - so we are going to use airlfow tests in master to test our Dockerfile and Helm chart. At the same time we use our Helm and Dockerfile to test airflow - it already happens for Dockerfile - we are running Kubernetes Executor tests using production image and we are going to switch to helm charts for that soon.

Even now both Production image and Helm chart from master can be used to install older versions of Airflow - see https://github.com/apache/airflow/blob/master/IMAGES.rst - then you will how to use the current image from master to build any airflow 1.10* version of the image.

You cam treat those Dockerfile and Charts as development versions - same as all other airflow code.

Once this is more stable and proven, both production docker file and help chart will land in their dedicated official repositories (additionally to continuing development in master of Airflow).

This is all described in https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-26+Production-ready+Airflow+Docker+Image+and+helm+chart

and we are approximately half way from what we considered as Done

Helm chart will be here:

• https://github.com/helm/charts/tree/master/stable/airflow

Doickefile will become an official docker image

• https://docs.docker.com/docker-hub/official_images/

[thesuperzapper]

@potiuk I maintain the stable/airflow helm chart, but this chart will need to move before the end of the year, as that repository is being archived.

I think we (Apache Airflow) should have a helm repository, similar to how ArgoProj has their own helm repo, initially this can contain the existing stable/airflow chart.

Please see here: https://github.com/helm/charts#deprecation-timeline

[potiuk]

Great. Then we will have to aadapt our plans and make whatever everyone will be doing if "official" helm chart will no longer be official.

Regardless of that the Dockerfile an Helm Chart will be here to stay and once we stabilise it and make both image and helm robust we will find a good place for both I am sure.

[ashb]

Looking at the yamllint errors, I think we have to make it not check anything under chart/templates/ -- they aren't YAML files as such, but yaml+gotexttemplate, so yamllint is never going to like them.

[kaxil]

Thank you all for the comments and discussions. PRs are now welcome to make any changes to existing code or add new features.

[kaxil]

Big thanks to @schnie 🚀

[marclamberti]

Awesome! Time to use the official chart 🚀🚀

[potiuk]

Fantastic! 🚀 🚀 . I am going to switch to the helm chart for our Kubernetes tests soon and iterate on the prod image @dimberman @schnie @kaxil @ashb -> I guess I am free to do so ? You are not working on this?

[Siddharthk]

Thanks for this. Was waiting for this eagerly. I tested it out quickly and I am getting below error using helm 2. Am I doing something wrong? I just want to get the yaml files and not install via helm.

$ helm template --values ./airflow/chart/values.yaml --output-dir ./flow ./airflow/chart
Error: found in requirements.yaml, but missing in charts/ directory: postgresql

[kaxil]

Thanks for this. Was waiting for this eagerly. I tested it out quickly and I am getting below error using helm 2. Am I doing something wrong? I just want to get the yaml files and not install via helm.

$ helm template --values ./airflow/chart/values.yaml --output-dir ./flow ./airflow/chart
Error: found in requirements.yaml, but missing in charts/ directory: postgresql

You will need to run helm dep update first :)

[streetmapp]

What's the repo URL to be used for this chart? The one listed in the Readme points to the stable Helm repo so if you are following that, you end up pulling down the Airflow chart from there and not this one.

Also of note, later in the Readme the Astronomer repo is listed in the Walkthrough using kind section.

[kaxil]

What's the repo URL to be used for this chart? The one listed in the Readme points to the stable Helm repo so if you are following that, you end up pulling down the Airflow chart from there and not this one.

Also of note, later in the Readme the Astronomer repo is listed in the Walkthrough using kind section.

We haven't published this chart yet, PRs are welcome to improve the chart including documentation and we would let everyone know once we publish it.

[thesuperzapper]

Is the master branch really the correct place to be developing this? Especially as there is currently no release/testing infrastructure in master for this chart.

I am worried that we will end up clogging master's commit history, and confusing users about the maturity/availability of the chart feature.

[potiuk]

Yes. it is good the @thesuperzapper . IMHO Monorepo with all stuff in is one of the best ideas for development for such project size. We are going to use it for our kubernetes tests and we already have unit tests in progress #9371 and we are going to develop it at the same time as developing production image and airflow code itself.

We are definitely going to release - both images and helm charts in an "official" way - the same way as all the code is released for Apache Airflow. Note that there is a very formal process to follow to release any code for Apache Airflow once the code becomes "community managed" http://www.apache.org/legal/release-policy.html - this includes voting by PMCs and at least 72 hours of voting period. So we will likely be releasing Helm chart together with official releases of Apache Airlfow (same with Docker Image). Everything in master is in "development" for Apache Airflow.

[Siddharthk]

I have some questions:

1. I see that the recommended way to store dags is the docker image. But when we deploy new image, the webserver restarts thus causing UI interruption for some time. Is this a bug? Should I run 2 replicas?

airflow-postgresql-0                 1/1     Running           0          25h
airflow-scheduler-5f9c688c54-qxrd8   0/2     PodInitializing   0          14s
airflow-scheduler-76c7949bb6-qwstg   2/2     Running           0          64m
airflow-statsd-7445645ddd-f6m4c      1/1     Running           0          25h
airflow-webserver-5c4bc5d886-7zj5h   0/1     Init:0/1          0          14s

2. By default, the chart does not use persistent volumes for task/pod logs. What is the recommended way for logs? Can pv become default?

3. Getting below error from task pod logs while running a simple tutorial:

$ kubectl logs -f tutorial1sleep300-1ca52dc981174d3daa29e4f606355a3d  -n airflow
Traceback (most recent call last):
  File "/home/airflow/.local/bin/airflow", line 23, in <module>
    import argcomplete
ModuleNotFoundError: No module named 'argcomplete'

@kaxil @ashb @potiuk any idea about this?

[aneesh-joseph]

@Siddharthk , for 3, I added the below config to the airflow.cfg kubernetes section of the configmap

run_as_user = {{ .Values.uid }}
fs_group = {{ .Values.gid }}

so that it runs as the correct airflow user, instead of root

Have added this to the config map in a PR

[ryw]

@schnie see a few questions from @Siddharthk (was chatting w/ him in Airflow Slack)

[marclamberti]

@Siddharthk ,
1 / It's not a bug, it's the way Kubernetes works. If you update your Helm chart with a new image tag, then Kubernetes will update your app to keep the state you desire. Take a look at the document right here: https://kubernetes.io/docs/tutorials/kubernetes-basics/update/update-intro/.
At the bottom you can read:

Similar to application Scaling, if a Deployment is exposed publicly, the Service will load-balance the traffic only to available Pods during the update.
In case your Airflow instance is running in dev/staging, that should not be a problem as it may be ok to have a little downtime between updates. In case you are in production, you should have your webserver in HA (highly available), and so, replicas running with a load balancer in front of them. During the rolling update, while one pod is being updated, the others are still accessible as well as the Airflow UI.

2/ I don't think there is a recommended way to store logs as it depends on your environment and requirements. For instance, I store my logs in S3. S3 is cheap and reliable. I can process the logs later with other tools to make analysis if needed and I can archive them at a defined time. 3

3/ About your error, I got it once with the "unofficial" chart. Sadly I don't remember how I fixed it. Check with kubectl describe your pod if you don't get additional information and keep your workers after completion so that you can debug them.

Hope it helps :)

[Siddharthk]

Thanks @aneesh-joseph @marclamberti

I have made webserver as HA for now and configured S3 for logs. For python module error, I have created below env for now which worked:

 - name: "AIRFLOW__KUBERNETES__RUN_AS_USER"
   value: "50000"

Another issue I am facing is mounting secrets to task pods. Basically I am trying to use ssh_key file for ssh operator. I added additional volumes and volume mounts for the scheduler. Will this reflect on the task pods as well?

Task pod:
airflow@tutorial1sleep300-2f920b4c375b4d54bd15a4535b49d368:/opt/airflow$ ls
airflow.cfg  config  dags  logs  requirements.txt  unittests.cfg  webserver_config.py

Scheduler pod:
airflow@airflow-scheduler-5474b69b67-j5c2c:/opt/airflow$ ls
airflow.cfg  dags  **id_rsa.pub**  logs  requirements.txt  unittests.cfg  webserver_config.py

[marclamberti]

Well I did that too actually but it was with the unofficial helm chart. Are you using the official one here?

[Siddharthk]

Well I did that too actually but it was with the unofficial helm chart. Are you using the official one here?

Yes, using the official one.

[aneesh-joseph]

Another issue I am facing is mounting secrets to task pods. Basically I am trying to use ssh_key file for ssh operator. I added additional volumes and volume mounts for the scheduler. Will this reflect on the task pods as well?

No, I don't think it will mount them automatically to the worker pods created by the KubernetesExecutor. If this is for a specific task, you could probably specify the volumes and volumeMounts via the executor_config param of that task. If you want it to be mounted on all worker pods, you may have to use a pod mutation hook via airflowLocalSettings