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. Signed-off-by: Thomas Jungblut <[email protected]>
openshift · Sep 8, 2022 · 7c81485 · 7c81485
1 parent 3cc60d0
commit 7c81485
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 11 deletions.
diff --git a/etcdctl/ctlv3/command/move_leader_command.go b/etcdctl/ctlv3/command/move_leader_command.go
@@ -42,7 +42,8 @@ func transferLeadershipCommandFunc(cmd *cobra.Command, args []string) {
 		ExitWithError(ExitBadArgs, err)
 	}
 
-	c := mustClientFromCmd(cmd)
+	cfg := clientConfigFromCmd(cmd)
+	c := cfg.mustClient()
 	eps := c.Endpoints()
 	c.Close()
 
@@ -52,7 +53,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
@@ -28,15 +28,31 @@ import (
 	"go.etcd.io/etcd/pkg/types"
 )
 
-func TestCtlV3MoveLeaderSecure(t *testing.T) {
-	testCtlV3MoveLeader(t, configTLS)
-}
+func TestCtlV3MoveLeaderScenarios(t *testing.T) {
+	security := map[string]struct {
+		cfg etcdProcessClusterConfig
+	}{
+		"Secure":   {cfg: configTLS},
+		"Insecure": {cfg: configNoTLS},
+	}
 
-func TestCtlV3MoveLeaderInsecure(t *testing.T) {
-	testCtlV3MoveLeader(t, configNoTLS)
+	tests := map[string]struct {
+		env map[string]struct{}
+	}{
+		"happy path": {env: nil},
+		"with env":   {env: map[string]struct{}{}},
+	}
+
+	for testName, tx := range tests {
+		for subTestName, tc := range security {
+			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]struct{}) {
 	defer testutil.AfterTest(t)
 
 	epc := setupEtcdctlTest(t, &cfg, true)
@@ -95,23 +111,29 @@ func testCtlV3MoveLeader(t *testing.T, cfg etcdProcessClusterConfig) {
 		cfg:         configNoTLS,
 		dialTimeout: 7 * time.Second,
 		epc:         epc,
+		envMap:      envVars,
 	}
 
 	tests := []struct {
 		prefixes []string
 		expect   string
 	}{
 		{ // request to non-leader
-			cx.prefixArgs([]string{cx.epc.EndpointsV3()[(leadIdx+1)%3]}),
+			[]string{cx.epc.EndpointsV3()[(leadIdx+1)%3]},
 			"no leader endpoint given at ",
 		},
 		{ // request to leader
-			cx.prefixArgs([]string{cx.epc.EndpointsV3()[leadIdx]}),
+			[]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 {
-		cmdArgs := append(tc.prefixes, "move-leader", types.ID(transferee).String())
+		pf := cx.prefixArgs(tc.prefixes)
+		cmdArgs := append(pf, "move-leader", types.ID(transferee).String())
 		if err := spawnWithExpect(cmdArgs, tc.expect); err != nil {
 			t.Fatalf("#%d: %v", i, err)
 		}