-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Memberlist gossip implementation #7521
Conversation
Hi @jacksontj. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
^^ That is an implementation of the flags -- so barring the upstream PR and any review notes this should be ready to go (I'll do another round of testing on my dev cluster to verify) |
13c994f
to
7d5b736
Compare
Upstream AM decided that I should just fork the library, which I have done. Since I had the fork already I changed some of the deps to reduce the vendor changes in this PR as well. |
5f44c2b
to
66a73f5
Compare
/ok-to-test |
I have identified most of the issues, I have another branch (based on 1.12) which is working -- I have to port those changes forward (should be able to do that monday). In those changes I have to configure the primary+secondary gossip for both protokube and dns-controller. Unfortunately dns-controller is assuming the port for the gossip seed -- so right now I have it using the protokube gossip config to determine the port -- which means you have to not apply the change from 2 -> 1 gossip on the dns-controller until nothing is remaining on the old gossip. As of now the controller isn't auto-deploying on kops edit anyways -- so maybe this is okay? If not then I'll need to add a Thoughts? |
f350bc4
to
c5f38e3
Compare
c5f38e3
to
e0c42e7
Compare
Just updated this PR with the final changes required to get it working on my test cluster. The upgrade path is:
Last questions:
|
83d83ce
to
83b150d
Compare
@justinsb friendly reminder bump :) |
/retest At least the bazel logs were missing, indicating a bad run. |
OK - as we want to get this into 1.16 (I believe), going to merge! /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jacksontj, justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
As discussed in office hours last wee, this PR implements #7436 to swap gossip from mesh to memberlist (primarily due to scale issues with mesh -- #7427).
At this point it is functional (with some manual hacking to get the one dep PR in) -- and I have tested this on a cluster of ~800 nodes with no noticeable CPU usage increase from a ~5 node cluster.
TODO:
cc @justinsb