Skip to content

Conversation

@guan404ming
Copy link
Member

@guan404ming guan404ming commented Apr 14, 2025

Related Issue

Why

The Airflow Helm chart currently uses the break function in chart/templates/rbac/pod-launcher-rolebinding.yaml which lead to error since break is not defined

How

The PR replaces the break function usage with alternative control flow mechanisms in pod-launcher-rolebinding.yaml


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added the area:helm-chart Airflow Helm Chart label Apr 14, 2025
@guan404ming guan404ming changed the title fix: replace break function in pod-launcher-rolebinding template fix: replace break function in pod-launcher-rolebinding.yaml Apr 14, 2025
@eladkal eladkal added this to the Airflow Helm Chart 1.17.0 milestone Apr 14, 2025
@eladkal
Copy link
Contributor

eladkal commented Apr 14, 2025

Is there a way to test this to avoid regression?

@guan404ming
Copy link
Member Author

guan404ming commented Apr 14, 2025

These are a few of my current thoughts:

  • adding a pre-commit check in the CI to detect incompatible features (e.g., the break function) in all Helm templates?
  • to mark features in the documentation or code that require a specific Helm version, so developers and users are clear on compatibility requirements?

Do you have any hints or suggestions?

@guan404ming guan404ming force-pushed the fix-helm branch 3 times, most recently from 08e5ec6 to 0b3bac3 Compare April 19, 2025 12:03
@eladkal
Copy link
Contributor

eladkal commented Apr 22, 2025

These are a few of my current thoughts:

  • adding a pre-commit check in the CI to detect incompatible features (e.g., the break function) in all Helm templates?
  • to mark features in the documentation or code that require a specific Helm version, so developers and users are clear on compatibility requirements?

Do you have any hints or suggestions?

Don't we have a test that tries to upgrade the helm chart version? Such test would have caught the issue as the upgrade would fail

@guan404ming
Copy link
Member Author

It seems like there's currently no test for this, but I feel like my Helm knowledge is a bit limited at the moment. Would it be okay to open another issue to address the test later?

@eladkal eladkal merged commit e836a41 into apache:main Apr 28, 2025
64 checks passed
@guan404ming guan404ming deleted the fix-helm branch April 28, 2025 03:46
@guan404ming
Copy link
Member Author

Thanks for the review!

jroachgolf84 pushed a commit to jroachgolf84/airflow that referenced this pull request Apr 30, 2025
@guan404ming guan404ming changed the title fix: replace break function in pod-launcher-rolebinding.yaml Replace break function in pod-launcher-rolebinding.yaml Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:helm-chart Airflow Helm Chart

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error: UPGRADE FAILED: parse error at (airflow/templates/rbac/pod-launcher-rolebinding.yaml:66): function "break" not defined

3 participants