From 2cfc6dc397823be73736a4ac14cc9138df56bfb5 Mon Sep 17 00:00:00 2001
From: Thomas Jungblut <tjungblu@redhat.com>
Date: Thu, 8 Sep 2022 11:20:15 +0200
Subject: [PATCH] UPSTREAM <carry>: etcdctl: fix move-leader for multiple
 endpoints

Due to a duplicate call of clientConfigFromCmd, the move-leader command
would fail with "conflicting environment variable is shadowed by corresponding command-line flag".
Also in scenarios where no command-line flag was supplied.

This can be removed on a branch rebased to 3.6 safely.

Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
---
 etcdctl/ctlv3/command/move_leader_command.go |  4 +--
 tests/e2e/ctl_v3_move_leader_test.go         | 33 ++++++++++++++++----
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/etcdctl/ctlv3/command/move_leader_command.go b/etcdctl/ctlv3/command/move_leader_command.go
index 098c897cd7a..b70fabdee3c 100644
--- a/etcdctl/ctlv3/command/move_leader_command.go
+++ b/etcdctl/ctlv3/command/move_leader_command.go
@@ -43,7 +43,8 @@ func transferLeadershipCommandFunc(cmd *cobra.Command, args []string) {
 		cobrautl.ExitWithError(cobrautl.ExitBadArgs, err)
 	}
 
-	c := mustClientFromCmd(cmd)
+	cfg := clientConfigFromCmd(cmd)
+	c := cfg.mustClient()
 	eps := c.Endpoints()
 	c.Close()
 
@@ -53,7 +54,6 @@ func transferLeadershipCommandFunc(cmd *cobra.Command, args []string) {
 	var leaderCli *clientv3.Client
 	var leaderID uint64
 	for _, ep := range eps {
-		cfg := clientConfigFromCmd(cmd)
 		cfg.endpoints = []string{ep}
 		cli := cfg.mustClient()
 		resp, serr := cli.Status(ctx, ep)
diff --git a/tests/e2e/ctl_v3_move_leader_test.go b/tests/e2e/ctl_v3_move_leader_test.go
index 05dc4993999..70bc50ac445 100644
--- a/tests/e2e/ctl_v3_move_leader_test.go
+++ b/tests/e2e/ctl_v3_move_leader_test.go
@@ -27,15 +27,31 @@ import (
 	"go.etcd.io/etcd/client/v3"
 )
 
-func TestCtlV3MoveLeaderSecure(t *testing.T) {
-	testCtlV3MoveLeader(t, *newConfigTLS())
-}
+func TestCtlV3MoveLeaderScenarios(t *testing.T) {
+	securityParent := map[string]struct {
+		cfg etcdProcessClusterConfig
+	}{
+		"Secure":   {cfg: *newConfigTLS()},
+		"Insecure": {cfg: *newConfigNoTLS()},
+	}
 
-func TestCtlV3MoveLeaderInsecure(t *testing.T) {
-	testCtlV3MoveLeader(t, *newConfigNoTLS())
+	tests := map[string]struct {
+		env map[string]string
+	}{
+		"happy path": {env: map[string]string{}},
+		"with env":   {env: map[string]string{"ETCDCTL_ENDPOINTS": "something-else-is-set"}},
+	}
+
+	for testName, tc := range securityParent {
+		for subTestName, tx := range tests {
+			t.Run(testName+" "+subTestName, func(t *testing.T) {
+				testCtlV3MoveLeader(t, tc.cfg, tx.env)
+			})
+		}
+	}
 }
 
-func testCtlV3MoveLeader(t *testing.T, cfg etcdProcessClusterConfig) {
+func testCtlV3MoveLeader(t *testing.T, cfg etcdProcessClusterConfig, envVars map[string]string) {
 	BeforeTest(t)
 
 	epc := setupEtcdctlTest(t, &cfg, true)
@@ -94,6 +110,7 @@ func testCtlV3MoveLeader(t *testing.T, cfg etcdProcessClusterConfig) {
 		cfg:         *newConfigNoTLS(),
 		dialTimeout: 7 * time.Second,
 		epc:         epc,
+		envMap:      envVars,
 	}
 
 	tests := []struct {
@@ -108,6 +125,10 @@ func testCtlV3MoveLeader(t *testing.T, cfg etcdProcessClusterConfig) {
 			[]string{cx.epc.EndpointsV3()[leadIdx]},
 			fmt.Sprintf("Leadership transferred from %s to %s", types.ID(leaderID), types.ID(transferee)),
 		},
+		{ // request to all endpoints
+			cx.epc.EndpointsV3(),
+			fmt.Sprintf("Leadership transferred"),
+		},
 	}
 	for i, tc := range tests {
 		prefix := cx.prefixArgs(tc.eps)