-
Notifications
You must be signed in to change notification settings - Fork 14
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
Chart refactor #3
Conversation
Sorry for the mega-delay here @FedeBev, thank you very much for the work! I had not prioritized the K8s work until now, going to test this and merge if it works. |
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.
Aside from the nitpicks, it LGTM but I have been unable to do a rolling restart without breaking the cluster, what's your strategy?
What kind of problem are you facing @Victorcoder? I just tried a rolling update from version v3.1.8 to v3.1.10 and everything went good except for a slight leader election delay. Do you mind sharing your values and some details about your cluster? I've just fixed the missing newlines and cleaned up the sample values file |
I just do a Are you tweeking some other options? |
No additional tweek on my configuration. This also happens to me, but after a few seconds after the end of the rollout a new leader is elected again and everything goes fine. We can try to tune the rollout strategy, but taking into account the issue about the kubernetes discovery, I don't think we can achieve something better with helm. An operator would solve the issue, but it's a huge effort. |
@FedeBev what about a liveness, readiness check? |
@vcastellm in the issue about the kubernetes discovery I've mentioned before, you can find why I wasn't able to use those. I don't see any other possible way right now. I guess we should change something about how dkron starts or request a PR to the kubernetes discovery library. |
Understood, thanks, I will take a look on how we can improve the /health endpoint |
Keep me posted, I'd be glad to help. |
I'm also very interested in this fix. @vcastellm @FedeBev anything that I can do to make this happen? Also, is this not better than the currently available chart? |
@Espina2 the chart is working on my cluster in a dev environment, but it's still very young and there's still a long way to go before being production ready. |
* Add Server Pod Disruption Budget * Add missing newline at EOF * Add preStop command to mitigate raft peers total caos in Server StatefulSet Co-authored-by: Alberto Pinardi <alberto.pinardi@criptalia.it>
hello! also highly interested in this. Has anyone come up with a way to work around the discovery issue? any thoughts on how to proceed? |
Merged this PR and added some changes, I'll take over from here. |
Since there's still an open issue in the main repo and I've the feeling this chart has been dropped, I've decided to rework it completely.
The main differences are:
Known issues:
Let me know what are your thoughts, I'm open to feedback available to make any changes