Kubeadm issue #3152 ControlPlane node setup failing with "etcdserver: can only promote a learner member" #130782
Conversation
k8s-ci-robot
added
release-note
|
|
k8s-ci-robot
added
cncf-cla: no
|
Welcome @BernardMC! |
|
Hi @BernardMC. 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-sigs/prow repository. |
k8s-ci-robot
added
needs-priority
k8s-ci-robot
added
the
do-not-merge/contains-merge-commits
|
/release-note-edit |
|
@BernardMC please keep the commits squashed to 1. |
|
please change this in the pr description to be |
There was a problem hiding this comment.
/cc @pacoxu
ptal if possible.
@BernardMC please message here once the cla is approved.
cmd/kubeadm/app/util/etcd/etcd.go
Outdated
| @@ -586,6 +595,11 @@ func (c *Client) MemberPromote(learnerID uint64) error { | |||
| return false, nil | |||
| }) | |||
| if err != nil { | |||
| klog.V(5).Infof("[etcd] Failed to promote the learner %s before timeout. Attempting to remove learner member", strconv.FormatUint(learnerID, 16)) | |||
| _, err = cli.MemberRemove(context.Background(), learnerID) | |||
There was a problem hiding this comment.
I understand that if promoting an etcd learner fails, we should remove the unstable member. But why don't we move the member removal action outside the Client.MemberPromote method, and when the Client.MemberPromote call fails, we invoke Client.RemoveMember to delete the member?
Since Client.RemoveMember contains retry mechanisms, it might better handle member removal failures. For example:
err = etcdClient.MemberPromote(learnerID)
if err != nil {
klog.V(5).Infof("[etcd] Failed to promote the learner %s before timeout. Attempting to remove learner member", strconv.FormatUint(learnerID, 16))
if _, removeErr := etcdClient.RemoveMember(learnerID); removeErr != nil {
klog.V(5).Infof("[etcd] Removing the learner %s failed: %v", strconv.FormatUint(learnerID, 16), removeErr)
}
return err
}
If you believe that removing the unstable member after a failed promote etcd learner is just a best-effort action and does not require retries, please add comments to clarify this behavior.
There was a problem hiding this comment.
Yeah, I think it makes sense to make the remove call an explicit call from outside rather than inside the member promote
cmd/kubeadm/app/util/etcd/etcd.go
Outdated
| } | ||
| if !isLearner { | ||
| klog.V(1).Infof("[etcd] Member %s already promoted.", strconv.FormatUint(learnerID, 16)) | ||
| return false, nil |
k8s-ci-robot
added
size/XS
Yeah my global commit email was @vmware.com and I did not update it to @broadcom.com |
k8s-ci-robot
added
cncf-cla: yes
|
@HirazawaUi @pacoxu I got my CLA resolved! |
|
Did you miss the content mentioned comment? |
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
I'll do a separate PR for that, since leaving a zombie etcd member issue isn't unique to this "learner already promoted" issue. I'm also not as familiar with that part of code yet 😊 |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 7c728138cdf1259d0b6b83c4a92759982bccd768
|
|
Since we missed the code freeze of v1.33, should we cherry-pick it after v1.33.0 release? @neolit123 |
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BernardMC, SataQiu 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 |
k8s-ci-robot
added
the
approved
sgtm |
|
|
|
since it's not a critical bug we should ideally wait for 1.33.0 to first release. then this pr will auto-merge in 1.34-pre.
versions in support: schedule for 1.33: |
|
/triage accepted |
k8s-ci-robot
added
triage/accepted
What type of PR is this?
/kind bug
What this PR does / why we need it:
Fixes issues where kubeadm tries to promote a learner member that is already promoted
This can happen when a previous promote does not return a success but the promotion actually succeeded on
the backend. This caused the script to eventually timeout and node bring up to fail
Also added an additional call to remove the member we failed to add if member promotion failed entirely
Which issue(s) this PR fixes:
kubernetes/kubeadm#3152
Fixes kubernetes/kubeadm#3152
Special notes for your reviewer:
Does this PR introduce a user-facing change?
NONE
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: