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

making .ReleaseIsInstall optional for init jobs #480

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

doug-ba
Copy link
Contributor

@doug-ba doug-ba commented Mar 22, 2024

Fixes #479

Motivation

In solutions like ArgoCD which don't actually track helm state, .ReleaseIsInstall always evaluates to true. We should be able to optionally not use .ReleaseIsInstall to determine whether init jobs should be created/removed

Modifications

makes a new value useReleaseStatus which defaults to true.
When useReleaseStatus is set to false, the chart relies only on the value of the initialize value to determine whether the init jobs should be created.

This PR also changes an or in the pulsar-manager-cluster-initialize template to an and. With the or in place, the value passed at .Values.job.ttl.enabled was ignored for all clusters newer than Kubernetes version 1.23.0. This causes the job to be constantly recreated in argocd.

Verifying this change

  • Make sure that the change passes the CI checks.

@doug-ba doug-ba changed the title making .ReleasIsInstall optional for init jobs making .ReleaseIsInstall optional for init jobs Mar 22, 2024
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Really helpful.

I made a suggestion to simplify the condition. Please check if it makes sense.

charts/pulsar/templates/bookkeeper-cluster-initialize.yaml Outdated Show resolved Hide resolved
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@lhotari lhotari merged commit 3dee8df into apache:master Mar 26, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make it possible to optionally override .ReleaseIsInstall logic in init jobs
2 participants