From 5e0c0779a11946e5b2ae28e4f1518a64143534c5 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Fri, 24 Jan 2020 02:08:48 +0200 Subject: [PATCH 1/2] kubeadm: handle multiple members without names during concurrent join For the etcd client, amend AddMember() to handle a very rare bug when multiple members can end up with the same name. Match the member peer address and assign it the name of the member we are adding. For the rest of the members with missing names use their member IDs as name. The etcd node is not disrupted by the unknown names. The important aspects are: - The number of members of the initial cluster must match the members in the cluster. - The member we are current adding is present in the initial cluster. --- cmd/kubeadm/app/util/etcd/etcd.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/cmd/kubeadm/app/util/etcd/etcd.go b/cmd/kubeadm/app/util/etcd/etcd.go index 09ada277530..0584c4dcecf 100644 --- a/cmd/kubeadm/app/util/etcd/etcd.go +++ b/cmd/kubeadm/app/util/etcd/etcd.go @@ -303,12 +303,20 @@ func (c *Client) AddMember(name string, peerAddrs string) ([]Member, error) { // Returns the updated list of etcd members ret := []Member{} for _, m := range resp.Members { - // fixes the entry for the joining member (that doesn't have a name set in the initialCluster returned by etcd) - if m.Name == "" { - ret = append(ret, Member{Name: name, PeerURL: m.PeerURLs[0]}) - } else { - ret = append(ret, Member{Name: m.Name, PeerURL: m.PeerURLs[0]}) + // If the peer address matches, this is the member we are adding. + // Use the name we passed to the function. + if peerAddrs == m.PeerURLs[0] { + ret = append(ret, Member{Name: name, PeerURL: peerAddrs}) + continue } + // Otherwise, we are processing other existing etcd members returned by AddMembers. + memberName := m.Name + // In some cases during concurrent join, some members can end up without a name. + // Use the member ID as name for those. + if len(memberName) == 0 { + memberName = strconv.FormatUint(m.ID, 16) + } + ret = append(ret, Member{Name: memberName, PeerURL: m.PeerURLs[0]}) } // Add the new member client address to the list of endpoints From a027c379f7e4f74717f7896b96b037857e171b45 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Fri, 24 Jan 2020 02:13:18 +0200 Subject: [PATCH 2/2] kubeadm: increase timeouts in the etcd client - Extend the exponential backoff for add/remove/... retry to 11 steps ~=106 seconds. From experiments for 3 and more members the race can take more that ~=26 seconds. - Increase the dialTimeout for client creation to 40 seconds. 20 seconds seems racy for 3 and more members. --- cmd/kubeadm/app/util/etcd/etcd.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/kubeadm/app/util/etcd/etcd.go b/cmd/kubeadm/app/util/etcd/etcd.go index 0584c4dcecf..461ce8477d5 100644 --- a/cmd/kubeadm/app/util/etcd/etcd.go +++ b/cmd/kubeadm/app/util/etcd/etcd.go @@ -42,7 +42,7 @@ const etcdTimeout = 2 * time.Second // Exponential backoff for etcd operations var etcdBackoff = wait.Backoff{ - Steps: 9, + Steps: 11, Duration: 50 * time.Millisecond, Factor: 2.0, Jitter: 0.1, @@ -128,9 +128,9 @@ func NewFromCluster(client clientset.Interface, certificatesDir string) (*Client } // dialTimeout is the timeout for failing to establish a connection. -// It is set to 20 seconds as times shorter than that will cause TLS connections to fail +// It is set to >20 seconds as times shorter than that will cause TLS connections to fail // on heavily loaded arm64 CPUs (issue #64649) -const dialTimeout = 20 * time.Second +const dialTimeout = 40 * time.Second // Sync synchronizes client's endpoints with the known endpoints from the etcd membership. func (c *Client) Sync() error {