airflow-helm / charts

The User-Community Airflow Helm Chart is the standard way to deploy Apache Airflow on Kubernetes with Helm. Originally created in 2017, it has since helped thousands of companies create production-ready deployments of Airflow on Kubernetes.
https://github.com/airflow-helm/charts/tree/main/charts/airflow
Apache License 2.0
631 stars 473 forks source link

Support renamed Kubernetes configuration in Airflow 2.5.0 #677

Closed emartgu closed 1 year ago

emartgu commented 1 year ago

Checks

Motivation

Airflow config section kubernetes renamed to kubernetes_executor in 2.5.0: https://airflow.apache.org/docs/apache-airflow/stable/release_notes.html#airflow-config-section-kubernetes-renamed-to-kubernetes-executor-26873

This is causing warnings for now and might not work for later versions.

Implementation

Introducing an if statement to pick the correct env variable names based on Airflow version in this section: https://github.com/airflow-helm/charts/blob/2268f2416a580cbd3db2050160a29226913473ca/charts/airflow/templates/config/secret-config-envs.yaml#L182

Example (with introduced airflow.version value):

  {{- if semverCompare "<2.5" .Values.airflow.version }}
    {{- if not .Values.airflow.config.AIRFLOW__KUBERNETES__NAMESPACE }}
  AIRFLOW__KUBERNETES__NAMESPACE: {{ .Release.Namespace | toString | b64enc | quote }}
    {{- end }}
  {{- else -}}
    {{- if not .Values.airflow.config.AIRFLOW__KUBERNETES_EXECUTOR__NAMESPACE }}
  AIRFLOW__KUBERNETES_EXECUTOR__NAMESPACE: {{ .Release.Namespace | toString | b64enc | quote }}
    {{- end }}
  {{- end }}

Are you willing & able to help?

thesuperzapper commented 1 year ago

@emartgu I want to avoid the need to explicitly set an airflow.version value, we can probably do something similar to what we did for logging.worker_log_server_port replacing celery.worker_log_server_port in airflow 2.2.0, and just include both configs.

This should suppress the warning logs, and future-proof us in case they remove the old one.