-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
[bitnami/thanos] Receive, ruler & storegateway statefulsets persistentVolumeClaimRetentionPolicy support #25676
[bitnami/thanos] Receive, ruler & storegateway statefulsets persistentVolumeClaimRetentionPolicy support #25676
Conversation
Thank you for initiating this pull request. We appreciate your effort. Just a friendly reminder that it's important to sign your commits. Adding your signature certifies that you either authored the patch or have the necessary rights to contribute the changes. You can find detailed information on how to do this in the “Sign your work” section of our contributing guidelines. Feel free to reach out if you have any questions or need assistance with the signing process. |
Just signed an empty commit in my fork's branch, is that enough !? I'm sorry about that, I thought that having my commits signed on Github directly was it/ was enough. |
It seems it's not enough since the DCO action is still failing. Here you can see the details and how to fix the issue. |
cca14c3
to
4517d7a
Compare
Thank you for pointing me to that! Done, signed off every commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this great contribution! Please ensure you merge latest changes from main
branch and resolve the conflicts.
Also, please note the PersistentVolumeClaim retention was included in Kubernetes 1.23 and therefore, we should include take it into account introducing the safeguard below:
{{- if semverCompare ">=1.23-0" (include "common.capabilities.kubeVersion" .) -}}
This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution. |
@clarifai-fmarceau did you see my last comment? |
…tnami#25672) Signed-off-by: Bitnami Containers <[email protected]> Signed-off-by: Francois Marceau <[email protected]>
…tVolumeClaimRetentionPolicy support Signed-off-by: Francois Marceau <[email protected]>
Signed-off-by: Francois Marceau <[email protected]>
Signed-off-by: Francois Marceau <[email protected]>
09f6fa7
to
cb12141
Compare
…imRetentionPolicy
Signed-off-by: Francois Marceau <[email protected]>
Signed-off-by: François Marceau <[email protected]>
Fixed ! Hopefully we can now merge this ! On a side note, support for persistentVolumeClaimRetentionPolicy is already in a few of your charts and there's no safeguard around it. I didn't push back based on that because it's a fair comment/ change request but just wanted to point that out. |
Signed-off-by: Bitnami Containers <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT! Thanks so much for the contribution
Description of the change
These changes add persistentVolumeClaimRetentionPolicy support to statefulsets of the Thanos Helm chart.
Benefits
We can now set persistentVolumeClaimRetentionPolicy.whenScaled & persistentVolumeClaimRetentionPolicy.whenDeleted for Ruler, Receive & Store Gateway individually.
Possible drawbacks
None, the change is feature flagged and if enabled the defaults are aligned with the Kubernetes ones.
Applicable issues
Checklist
Chart.yaml
according to semver. This is not necessary when the changes only affect README.md files.README.md
using readme-generator-for-helm