-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Extend init_containers defined in pod_override #17537
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/main/CONTRIBUTING.rst)
|
Thanks @repocho! Can you add a test for this in tests/kubernetes/test_pod_generator.py? |
Thanks for the time reviewing this PR @jedcunningham ! I have added a test for this. Let me know if you need anything more. |
Thanks @jedcunningham, I have merged your suggestions and launched the tests again with success.
|
Oh, looks like we have a static check failure in that test file. Can you reformat that when you get the chance? |
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
492e6e3
to
7e40cc6
Compare
…sync and similar init_containers will stay) With that change if you define a pod_override and specifies the "init_containers" attribute it will extend the base init_containers with the new ones instead of overwriting them. That is to keep the dag git sync init_containers or other pre-defined init_containers based on the template or the configuration.
Improves readability and removes unused attributes from the spec. Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com>
I have rebased the branch with the last changes in branch main, to be able to launch all the tests |
Awesome work, congrats on your first merged pull request! |
Thanks for the bugfix @repocho and congrats on your first commit 🎉! |
Thank you for all your help !! 😉 |
If you add an init_container in pod_override the specified init_containers in the pod-template-file for kubernetes are ovewritten instead of extended as it is the case of the volumes for example.
So, for example, if you have a git sync ini_container but you define another init container, the task won't be executed because airflow won't find the dags as the init_container is replaced.
This change is just extending the init_container defined in the pod_override with the existing ones defined in the pod template file for kubernetes. In that sense you can define new extra init_containers.