Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AIRFLOW-6504] Allow specifying configmap for Airflow Local Setting #7097

Merged
merged 3 commits into from
Jan 8, 2020

Conversation

kaxil
Copy link
Member

@kaxil kaxil commented Jan 8, 2020

Currently airflow.cfg file can be passed via ConfigMap using airflow_configmap under [kubernetes] section setting.

We want to be able to specify airflow_local_settings.py via configmap too so that the Kubernetes Worker has this file. We need airflow_local_settings.py to specify pod_mutation_hook.

Without that currently, if we are using KubernetesExecutor and have a task with KubernetesPodOperator, it will inherit and change Pod using pod_mutation_hook


Issue link: AIRFLOW-6504

  • Description above provides context of the change
  • Commit message/PR title starts with [AIRFLOW-NNNN]. AIRFLOW-NNNN = JIRA ID*
  • Unit tests coverage for changes (not needed for documentation changes)
  • 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.

* For document-only changes commit message can start with [AIRFLOW-XXXX].


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.

@kaxil kaxil requested a review from dimberman January 8, 2020 00:40
@boring-cyborg boring-cyborg bot added area:Scheduler including HA (high availability) scheduler k8s labels Jan 8, 2020
@codecov-io
Copy link

codecov-io commented Jan 8, 2020

Codecov Report

Merging #7097 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7097      +/-   ##
==========================================
+ Coverage   85.15%   85.15%   +<.01%     
==========================================
  Files         680      680              
  Lines       38824    38832       +8     
==========================================
+ Hits        33061    33069       +8     
  Misses       5763     5763
Impacted Files Coverage Δ
airflow/kubernetes/worker_configuration.py 99.29% <100%> (+0.03%) ⬆️
airflow/executors/kubernetes_executor.py 57.57% <100%> (+0.09%) ⬆️
airflow/operators/postgres_operator.py 100% <0%> (ø) ⬆️
airflow/utils/sqlalchemy.py 96.66% <0%> (ø) ⬆️
airflow/hooks/dbapi_hook.py 91.73% <0%> (ø) ⬆️
airflow/hooks/postgres_hook.py 94.36% <0%> (ø) ⬆️
airflow/jobs/backfill_job.py 91.88% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe20ef6...a4f961b. Read the comment docs.

@kaxil kaxil merged commit 4748a51 into apache:master Jan 8, 2020
@kaxil kaxil deleted the add-local-settings-configmap branch January 8, 2020 14:41
galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020
@kristof-mattei
Copy link

Hello @kaxil

Can you elaborate on why you would want to call pod_mutating_hook inside of a newly launched pod?

@kaxil
Copy link
Member Author

kaxil commented Jul 29, 2020

Hello @kaxil

Can you elaborate on why you would want to call pod_mutating_hook inside of a newly launched pod?

We set tolerations and affinity inside the pod_mutation_hook for the PODs launched by KubernetesPodOperator and KuberneteExecutor. Without this PR, if we are using KubernetesExecutor and have a task with KubernetesPodOperator, the k8s worker pod won't have airflow_local_settings file and so tolerations and affinity won't be applied there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Scheduler including HA (high availability) scheduler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants