-
Notifications
You must be signed in to change notification settings - Fork 19
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
Enable bootstrapping statefulsets #12
Enable bootstrapping statefulsets #12
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: abdurrehman107 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Thanks @abdurrehman107 for the PR. Based on our discussion in last WG meeting, @ Gaurav Dasson is supposed to deliver a PR for this. I am not sure what's his progress so far. Let me ping him in slack. Also I'd like to see a complete PR to transfer all the implementation of syncHandler (around 300 -400 lines of code). |
@abdurrehman107 since there is no response from Gaurav Dasson, please feel free to continue to work on this. Could you deliver a complete PR to transfer all the implementation of syncHandler (around 300 -400 lines of code)? |
The goal is to implement the initial Reconcile workflow based on syncHandler . Please ensure that you verify it in a K8s cluster. thx |
Yes I can. I'll ask for help in the wg slack channel.
Aighty. |
@abdurrehman107 , @ahrtr : Sorry, I had to be away from work as I had some viral symptoms. I'll start looking at this work from today onwards. Thanks. |
I hope you're well @gdasson . I have made a lil progress. I dont mind getting on a call later today or tomorrow briefing you about it. Please lemme know over slack of when we can schedule a syncup. |
Thanks both @gdasson and @abdurrehman107 for the feedback. We definitely need more contributors. But in the very first step, I'd like to we have a skeleton implementation for the @abdurrehman107 Since you have already started working on this for 1+ weeks, please continue to work on it. Are you able to deliver a PR per #12 (comment) before our next working group meeting (Jan. 7th)? |
Yes we can have the sync handler function implementation by 7th Jan. |
@abdurrehman107 @ahrtr : I worked on this over this weekend and I tried moving most of the logic from syncHandler under the Reconcile loop along with a base/inital draft type code. I have kept my code in a forked branch. Please review here: If this looks fine for a starting point/skeleton, we can then start to refine it further. If you guys feel fine after the review, I can either add this in Abdur's PR or open a new one. @abdurrehman107 : I can try to meet this coming week over a call to discuss and syncup? |
Thanks @gdasson. It is almost identical to the existing implementation, and it's exactly what I want. I just expect a base/foundation asap for anyone to continue to work on it. So please raise a PR, thanks. As a new team, we need to work together more to get used to each other and avoid overlapping tasks. I’m hopeful that our teamwork will be much smoother going forward. |
You can send a draft PR first. |
Yes, I am working on one. I'll raise a PR tomorrow. |
What this PR does:
This PR enables bootstrapping StatefulSets. The controller:
EtcdCluster
resource.Spec.Size > 0
, creates a StatefulSet with replicas = 0.SetControllerReference
for proper garbage collection.It follows the workflow mentioned here.