etcdctl: find leader when promoting learner

[jingyih]

Background:
In #15, we decided to have client only send member promote request to leader in order to simply the implementation of learner progress check on server side.

What this PR does:
This PR partially addresses the issue of "only send member promote request to leader". etcdctl member promote will scan over the provided endpoints to find leader, then send promote request to leader endpoint.

Remaining issue:
User can still create their own clientv3 client with random endpoint in cluster and use it to send member promote request. If the endpoint is not a leader, request will fail with error ErrNotLeader according to logic added in #19. But at least etcdctl member promote command will try to find leader endpoint.

cc @gyuho @WIZARD-CXY @jpbetz

[WIZARD-CXY]

hmm, I don't have better idea to solve all the problem. I think it might just work for now.

[WIZARD-CXY]

maybe @jpbetz has better idea? for example add a helper function in clientv3 that can make client talk to the leader directly?

[jpbetz]

If ctx has a deadline, loop multiple times over the endpoints until the deadline expires or a leader is found? This would reduce the odd of a leader change causing this operation to fail.

[jpbetz]

Ignore if we can do a server side request foward like mentioned in #28 (comment)

[jpbetz]

EtcdServer.LeaseTimeToLive() appears to have forward to leader logic:

etcd/etcdserver/v3_server.go

Lines 289 to 294 in 636bf1c

	// forward to leader
	for cctx.Err() == nil {
	leader, err := s.waitLeader(cctx)
	if err != nil {
	return nil, err
	}

Can we use the same approach for PromoteLeader (and MoveLeader)? And avoid having the logic in the client?

[jpbetz]

If we can forwarding use waitLeader() we should be able to check for ErrGRPCNotLeader when deciding if an error from the forward request should be retired in the for cctx.Err() == nil loop.

Maybe introduce an forwardToLeader(ctx context.Context, forwardFn func() error) error function that manages the retries and is shared by LeaseTimeToLive, PromoteLeader, MoveLeader and any other calls that need to be handled by the leader? It could keep attempting to find the leader and send the request to it, retrying only on ErrGRPCNotLeader error, and halting if the context is Done.

[jingyih]

Thanks @jpbetz. I'll take a closer look on server side forwarding. We might want to keep the client side logic (at least for move leader) for backward compatibility reasons (when new etcdctl send move leader request to old server).

[jingyih]

We decided to implement server side forwarding of the member promote request. Closing this PR.

[WIZARD-CXY]

agreed