Add support for remote logging to be injected into the airflow.cfg configmap in helm chart #9695
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
|
Datkros
changed the title
|
How do you benefit from changing your environment configuration? I am afraid that in a moment we will have in Helm copies of all options, but with a different interface and worse documentation. Lines 94 to 97 in 40af9f0 An additional option in Helm Chart is useful when changing options in Airflow also requires changing elsewhere in Helm. An example here: Configuring logging would be useful if the bucket were automatically configured at the same time. You can do it using the product below for GCP. |
It benefits me because we'd be managing multiple airflow instances with different logging folders and credentials, this is specially important if it's across multiple teams / use cases, as some of them would have different isolated environment configuration (prod v staging i.e). I could always overwrite it as an env var as you mentioned, but we'd lose visibility on what's the current configuration running by just looking at the source. In this particular use case, everything is in AWS, so it'd be preferable to use terraform / cloudformation (or maybe someone else would prefer Puppet, Chef, or ansible) to better manage those bucket and credential resources instead of locking you into a solution through having the helm release create the buckets as well. This is what happens right now by having the helm chart depend on the k8s secret creation, as you cannot choose somewhere else to store DB DNS secrets, locking you into that solution. Essentially, one big issue of having both and the configmap is that your entire configuration becomes disjointed, being partially loaded through a .cfg, and from environment variables. It becomes hard to track what gets changed where, what is being already provided to you. Not having a single entry source makes it difficult for end users to see the full picture of their airflow configuration without having to dig through the code. This is why I chose to modify the configmap, as I believe that maybe it should be exposed through here since airflow already has templates for how this configuration would look like. |
|
To clarify on the above, would this env be the entry source for any configuration that needs to be passed to Airflow itself? (is this the consensus?) As far as I understood, it seems to be more about k8s pods, than isolated to airflow only. |
In Airflow 1.101.11, I added new command - I am not involved in the development of the Helm Chart, so it is difficult for me to address the other topics, but maybe @potiuk or @potiuk will be able to help you. I don't know the full vision of how we want to develop it. I will only add that when it comes to logger configurations it is sometimes helpful to be configured by environment variables, because secrets are easier to provide - https://airflow.readthedocs.io/en/latest/configurations-ref.html#stackdriver-key-path |
Not really, these commands are meant for adhoc configurations, not direct injections or automations, right? I |
|
Closed in favor of #9816 |
2 participants
Added support to inject the remote logging configuration into the
airflow.cfgconfigmap. I kept it backwards compatible with the current version of the chart by not dropping theelasticsearch.enabledvalue.Make sure to mark the boxes below before creating PR: [x]
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.