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 <[email protected]>
openshift · Sep 8, 2022 · 2cfc6dc · 2cfc6dc
1 parent 31b6b2d
commit 2cfc6dc
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 8 deletions.
diff --git 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
@@ -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)