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.

Sign up for GitHub

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

Jump to bottom

Chart: add support for container lifecycle hooks #32349

Merged
merged 10 commits into from
Aug 18, 2023
Merged

Chart: add support for container lifecycle hooks #32349

merged 10 commits into from
Aug 18, 2023

Conversation

rzjfr
Copy link
Contributor

@rzjfr rzjfr commented Jul 4, 2023 •
edited
Loading

This PR attempts to add possibility to add lifecycle to containers. There are some use cases which in which PostStart and PreStop could be utilized. The new containerLifecycleHooks (templated) can be configured on global and service/node level not dissimilar to the new securityContexts variable.

closes: #32402

@boring-cyborg boring-cyborg bot added the area:helm-chart Airflow Helm Chart label Jul 4, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Jul 4, 2023

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

  • Pay attention to the quality of your code (ruff, mypy 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: [email protected]
    Slack: https://s.apache.org/airflow-slack

@rzjfr rzjfr force-pushed the add-container-lifecycle branch 9 times, most recently from b39f4b0 to b5aa8ff Compare July 5, 2023 17:53
@rzjfr rzjfr changed the title Chart: add container lifecycle hooks Chart: add support for container lifecycle hooks Jul 5, 2023
@rzjfr rzjfr force-pushed the add-container-lifecycle branch 4 times, most recently from 7a0e00b to aebea1a Compare July 6, 2023 12:07
@jscheffl jscheffl added kind:feature Feature Requests type:new-feature Changelog: New Features labels Jul 6, 2023
@rzjfr rzjfr force-pushed the add-container-lifecycle branch 2 times, most recently from 64880c8 to ee49042 Compare July 7, 2023 16:12
@rzjfr rzjfr requested review from potiuk and mik-laj as code owners July 7, 2023 16:12
@rzjfr rzjfr force-pushed the add-container-lifecycle branch 4 times, most recently from 95bb900 to 506dcb3 Compare July 11, 2023 08:25
jedcunningham
jedcunningham reviewed Jul 28, 2023
View reviewed changes
chart/files/pod-template-file.kubernetes-helm-yaml Outdated Show resolved Hide resolved
chart/files/pod-template-file.kubernetes-helm-yaml Outdated Show resolved Hide resolved
chart/values.yaml Outdated Show resolved Hide resolved
helm_tests/airflow_aux/test_airflow_common.py Outdated Show resolved Hide resolved
helm_tests/airflow_aux/test_airflow_common.py Outdated Show resolved Hide resolved
@rzjfr
Copy link
Contributor Author

rzjfr commented Jul 29, 2023 •
edited
Loading

@jedcunningham Thank you for the review, good points, I should have applied all of the suggested changes on my last commit, please let me know in case I missed something.
I also consolidated the tests in a single file, if it the structure is fine I can try adding more test cases.

potiuk
potiuk approved these changes Aug 10, 2023
View reviewed changes
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@potiuk
Copy link
Member

potiuk commented Aug 10, 2023

Rebased to see if everything works fine with latest main. @jedcunningham ?

@rzjfr
Copy link
Contributor Author

rzjfr commented Aug 11, 2023

@jedcunningham thanks again for the review, I should have applied both of the suggested changes on my last commit, please let me know in case I missed something.

@jedcunningham jedcunningham merged commit 5fef1db into apache:main Aug 18, 2023
48 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Aug 18, 2023

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@jedcunningham
Copy link
Member

Thanks @rzjfr! Congrats on your first commit 🎉

lucasfcnunes added a commit to lucasfcnunes/airflow that referenced this pull request Aug 25, 2023
@lucasfcnunes
Chart: add support for container lifecycle hooks (apache#32349)
e298079
@jedcunningham jedcunningham mentioned this pull request Sep 27, 2023
Closed
24 tasks
@ephraimbuddy ephraimbuddy added changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) and removed type:new-feature Changelog: New Features labels Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@potiuk potiuk potiuk approved these changes

@jedcunningham jedcunningham jedcunningham approved these changes

@dstandish dstandish Awaiting requested review from dstandish dstandish is a code owner

@mik-laj mik-laj Awaiting requested review from mik-laj

@hussein-awala hussein-awala Awaiting requested review from hussein-awala hussein-awala is a code owner

Assignees
No one assigned
Labels
area:helm-chart Airflow Helm Chart changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) kind:feature Feature Requests
Projects
None yet
Milestone
Airflow Helm Chart 1.11.0
Development

Successfully merging this pull request may close these issues.

[Chart] Add support for container lifecycle
6 participants
@rzjfr @potiuk @jedcunningham @ephraimbuddy @eladkal @jscheffl