Skip to content

Commit

Permalink
etcdctl: fix move-leader for multiple endpoints
Browse files Browse the repository at this point in the history
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 <tjungblu@redhat.com>
  • Loading branch information
tjungblu committed Sep 8, 2022
1 parent dd743ee commit a3539ad
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 11 deletions.
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/move_leader_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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)
Expand Down
40 changes: 31 additions & 9 deletions tests/e2e/ctl_v3_move_leader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit a3539ad

Please sign in to comment.