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

Allow templates in cleanup.schedule #32570

Merged
merged 3 commits into from
Jul 12, 2023
Merged

Allow templates in cleanup.schedule #32570

merged 3 commits into from
Jul 12, 2023

Conversation

danielhoherd
Copy link
Contributor

@danielhoherd danielhoherd commented Jul 12, 2023

This change is quite simple in that it allows helm templates to be used in the value of cleanup.schedule. This also works with strings, which makes the change backwards compatible, so it will have no impact on existing installations. Shout out to @jedcunningham and @dstandish for helping me settle on this very simple syntax change.

There were no tests for cleanup.schedule, so I added tests, including tests for using strings and for using helm template logic.

FWIW, the test logic shows an example of using adler32sum to select a deterministic random-ish number, seeded by .Release.Name, to use as the starting minute for the cleanup cronjob. This mechanism will avoid starting all cleanup jobs across all deployments on minute 0, and was the impetus for changing from string to a template.

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

@dstandish dstandish left a comment

Choose a reason for hiding this comment

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

nice work!

Copy link
Member

@jedcunningham jedcunningham left a comment

Choose a reason for hiding this comment

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

In values.yaml and values.schema.json, can you mention that this is templated. Makes discovery much easier.

@jedcunningham jedcunningham merged commit ae29225 into apache:main Jul 12, 2023
48 checks passed
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Aug 2, 2023
pgvishnuram pushed a commit to astronomer/airflow that referenced this pull request Sep 14, 2023
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 changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants