Cluster-api: [KCP] Provide all endpoint when getting ETCD client

Created on 1 Apr 2020  路  12Comments  路  Source: kubernetes-sigs/cluster-api

Instead of trying to get ETCD client for a specific node, provide all control plane nodes as endpoints and let clientv3.New() handle connecting to an available endpoint. This will simplify some of the logic in the code.
Kubeadm is using a similar logic:
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/util/etcd/etcd.go

/kind cleanup
/area control-plane

arecontrol-plane kincleanup lifecyclactive

Most helpful comment

Jason suggested using WithRequireLeader in another issue, but it just seems to require that a leader exists.

All 12 comments

cc @randomvariable

/milestone v0.3.x

/assign
/lifecycle active

@gab-satchi let's hold on to this one until we have #2821 merged

I think the dialler would need to be refactored to do something like this:

  • Change the etcd dialler to receive every etcd pod as a URI in the format:

    • portforward://<name>.<namespace>.<type>:<port>

  • Refactor the Dialler so that instead of taking a pod as input, it can parse portforward URLs sent in via .Dial() and create a new tunnel on demand. Or wrap the existing Dialler to do the same.

If we go down this path, how can we get a client to the leader directly? And how can we effectively write tests for these changes?

Jason suggested using WithRequireLeader in another issue, but it just seems to require that a leader exists.

What do you think about making a POC / small PR to see if the idea works out? Then we can discuss the design there, I want to make we have a test plan in place for any changes related to etcd.

Agree with that approach. On top of having a test plan, I wanted to know if there's something I can reproduce locally to get etcd connections in a flaky state. Then see if the changes are improving that in any way.

Constantly scaling up and down comes to mind, but most of our connections go through the leader now. And I think the newest machine gets assigned the leader while the oldest one is always the one to get deleted in a scale down. So unsure if that plan would work

Yeah, a POC probably makes sense.

Was going to mention WithRequireLeader, but we can add that in post, and is more applicable to watches than anything else.

@gab-satchi . Using tc or iptables to knock out / delay packets should work. Over 1s delay causes heartbeats to start failing.

/close

@gab-satchi: Closing this issue.

In response to this:

/close

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.

Was this page helpful?
0 / 5 - 0 ratings