[BUGFIX] podMutations are now deployed correctly to airflow_local_settings.py

[alikhtag]

There is a BUG in helm chart where, setting anything in podMutation in values.yaml does not seem to affect anything. Currently these podMutations will not apply during deployment to airflow_local_settings.py, hence they will not be used by Kubernetes Executor and Pod Operator to correctly set taints and affinities in pod configuration.

This PR should fix this, but only if .Values.scheduler.airflowLocalSettings were not set but it seems it was by design originally.

If you like to reproduce the bug:

• Firstly use KubernetesExecutor for the Airflow deployment.
• Simply add desired toleration and/or affinity in podMutation.
• Deploy some dag/task etc.
• Check that newly created pods do not have either tolerations and affinities set in their pod configuration.

[boring-cyborg]

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)
Here are some useful points:

• Pay attention to the quality of your code (flake8, pylint and type annotations). Our pre-commits will help you with that.
• In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
• Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
• Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
  Apache Airflow is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: dev@airflow.apache.org
  Slack: https://apache-airflow-slack.herokuapp.com/

[potiuk]

@dimberman - it looks cool, but I want your opinion here as well :)

[dimberman]

Why are we not just having podMutation as a string and allowing people to write whatever function they want? I don't want to start prescribing values or we'll maintain it forever.

[alikhtag]

Why are we not just having podMutation as a string and allowing people to write whatever function they want? I don't want to start prescribing values or we'll maintain it forever.

Hmm this yaml configuration of podMutation is very useful, since it will transform toleration and affinity to json (or python dict), which is harder to write as string in this yaml, in my opinion. But podMutation can have a templating string ( using tpl .Values.podMutiation.podMutationHookTpl . ) as value as well. So if one decides to write function themselves they can overwrite it or use the yaml k:v for templating. I will add this idea soon.

[alikhtag]

I don't understand why postgres and mysql CI failed, current changes I would think would've not affected that section?

[potiuk]

I'd say those were transient errors. I re-run all jobs now to see if it helps

[alikhtag]

@dimberman does current implementation satisfy the maintainability requirement?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[dimberman]

Hi @alikhtag, would you like to rebase this PR so we can merge it? If not I can take this on and tag you as the author.

[dimberman]

Also you're gonna need to make the pod_mutation_hook function compliant with 2.0 :)

[alikhtag]

Also you're gonna need to make the pod_mutation_hook function compliant with 2.0 :)

Hi @dimberman. What do you refer to 2.0? Helm 2.0? If yes I can probably do that during this week.

[ashb]

@alikhtag Airflow 2.0 -- we changed the format of pod mutation hooks quite a lot -- it now takes the Kube API Model object directly rather than Airflow's own classes.
See #10393 and #9903 for example

[dimberman]

@alikhtag can you please get the tests to pass? Would be glad to merge once they are passing :).

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

[dimberman]

@alikhtag are you interested in finishing this?

[jedcunningham]

I'm going to close this since all podMutation parameters have been removed from the chart at this point. This functionality can still be achieved with airflowLocalSettings without being too cumbersome.