Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-3.4] etcdctl: fix move-leader for multiple endpoints #14441

Merged
merged 1 commit into from Sep 9, 2022

Conversation

tjungblu
Copy link
Contributor

@tjungblu tjungblu commented Sep 8, 2022

This is a backport of #14434 to 3.4.


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

@tjungblu
Copy link
Contributor Author

tjungblu commented Sep 8, 2022

will take a look at the proxy errors:

2022-09-08T12:26:47.7774077Z     ctl_v3_move_leader_test.go:138: #0: read /dev/ptmx: input/output error (expected "no leader endpoint given at ", got ["2022-09-08 12:26:47.776177 C | pkg/flags: conflicting environment variable \"ETCDCTL_ENDPOINTS\" is shadowed by corresponding command-line flag (either unset environment variable or disable flag)\r\n"])
2022-09-08T12:26:47.7925770Z === RUN   TestCtlV3MoveLeaderScenarios/happy_path_Insecure
2022-09-08T12:26:49.2782536Z     ctl_v3_move_leader_test.go:138: #0: read /dev/ptmx: input/output error (expected "no leader endpoint given at ", got ["2022-09-08 12:26:49.276149 C | pkg/flags: conflicting environment variable \"ETCDCTL_ENDPOINTS\" is shadowed by corresponding command-line flag (either unset environment variable or disable flag)\r\n"])
2022-09-08T12:26:49.2907470Z --- FAIL: TestCtlV3MoveLeaderScenarios (7.06s)
2022-09-08T12:26:49.2908074Z     --- PASS: TestCtlV3MoveLeaderScenarios/with_env_Secure (0.69s)
2022-09-08T12:26:49.2908620Z     --- PASS: TestCtlV3MoveLeaderScenarios/with_env_Insecure (2.28s)
2022-09-08T12:26:49.2909188Z     --- FAIL: TestCtlV3MoveLeaderScenarios/happy_path_Secure (2.59s)
2022-09-08T12:26:49.2909750Z     --- FAIL: TestCtlV3MoveLeaderScenarios/happy_path_Insecure (1.50s)

I saw there were some issues with ordering while debugging it locally, as the test sets the env variables in the test process and not on the forked process.

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>
@tjungblu
Copy link
Contributor Author

tjungblu commented Sep 8, 2022

That one looks bad, but possibly unrelated to my changes:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xab2fa2]

goroutine 3373 [running]:
go.etcd.io/bbolt.(*Tx).Bucket(...)
	/home/runner/go/pkg/mod/go.etcd.io/bbolt@v1.3.6/tx.go:101
go.etcd.io/etcd/mvcc/backend.(*concurrentReadTx).UnsafeRange(0xc000291620, 0x157b2dc, 0x3, 0x3, 0xc0000e2e28, 0x11, 0x12, 0x0, 0x0, 0x0, ...)
	/home/runner/work/etcd/etcd/mvcc/backend/read_tx.go:195 +0x642
go.etcd.io/etcd/mvcc.(*storeTxnRead).rangeKeys(0xc0002916e0, 0xc00048f048, 0x8, 0x8, 0x0, 0x0, 0x0, 0x3, 0x0, 0x2, ...)
	/home/runner/work/etcd/etcd/mvcc/kvstore_txn.go:147 +0x293
go.etcd.io/etcd/mvcc.(*storeTxnRead).Range(0xc0002916e0, 0xc00048f048, 0x8, 0x8, 0x0, 0x0, 0x0, 0x0, 0x2, 0xc00005f000, ...)
	/home/runner/work/etcd/etcd/mvcc/kvstore_txn.go:51 +0xaf
go.etcd.io/etcd/mvcc.(*metricsTxnWrite).Range(0xc000291710, 0xc00048f048, 0x8, 0x8, 0x0, 0x0, 0x0, 0x0, 0x2, 0x40fa00, ...)
	/home/runner/work/etcd/etcd/mvcc/metrics_txn.go:37 +0xaf
go.etcd.io/etcd/mvcc.(*readView).Range(0xc0005fd250, 0xc00048f048, 0x8, 0x8, 0x0, 0x0, 0x0, 0x0, 0x2, 0x0, ...)
	/home/runner/work/etcd/etcd/mvcc/kv_view.go:39 +0x125
go.etcd.io/etcd/etcdserver/api/v3rpc.(*serverWatchStream).sendLoop(0xc0002eb740)
	/home/runner/work/etcd/etcd/etcdserver/api/v3rpc/watch.go:405 +0x1fe
go.etcd.io/etcd/etcdserver/api/v3rpc.(*watchServer).Watch.func1(0xc0002eb740)
	/home/runner/work/etcd/etcd/etcdserver/api/v3rpc/watch.go:180 +0x2b
created by go.etcd.io/etcd/etcdserver/api/v3rpc.(*watchServer).Watch
	/home/runner/work/etcd/etcd/etcdserver/api/v3rpc/watch.go:179 +0x285
FAIL	go.etcd.io/etcd/etcdserver/api/v2store	3.388s
FAIL

@ahrtr could you kindly retry the failures, please? guess the grpcproxy job also got stuck somehow...

@ahrtr
Copy link
Member

ahrtr commented Sep 9, 2022

That one looks bad, but possibly unrelated to my changes:

It is a known issue, FYI. #14256 (comment)

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thank you @tjungblu

@ahrtr ahrtr merged commit a55a9f5 into etcd-io:release-3.4 Sep 9, 2022
@ahrtr
Copy link
Member

ahrtr commented Sep 9, 2022

@tjungblu I see that you fixed this issue in 3.4 and 3.5, but I do not see PR for main ?

@tjungblu tjungblu deleted the bz_1918413_3.4_upstream branch September 9, 2022 07:33
@tjungblu
Copy link
Contributor Author

tjungblu commented Sep 9, 2022

@ahrtr Thanks, that issue doesn't exist in main anymore, which is where the initial confusion came from in #14307.

@ahrtr
Copy link
Member

ahrtr commented Sep 9, 2022

that issue doesn't exist in main anymore

This seems not correct. I just confirmed that the main branch has this issue as well.

wachao-a01:bin wachao$ export ETCDCTL_ENDPOINTS=http://www.google.com:2379

wachao-a01:bin wachao$ ./etcdctl  move-leader --endpoints=http://127.0.0.1:2379,http://127.0.0.1:22379,http://127.0.0.1:32379 8211f1d0f64f3269
{"level":"fatal","ts":"2022-09-09T15:40:57.971+0800","caller":"flags/flag.go:85","msg":"conflicting environment variable is shadowed by corresponding command-line flag (either unset environment variable or disable flag))","environment-variable":"ETCDCTL_ENDPOINTS","stacktrace":"go.etcd.io/etcd/pkg/v3/flags.verifyEnv\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/pkg/flags/flag.go:85\ngo.etcd.io/etcd/pkg/v3/flags.SetPflagsFromEnv\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/pkg/flags/flag.go:63\ngo.etcd.io/etcd/etcdctl/v3/ctlv3/command.clientConfigFromCmd\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/etcdctl/ctlv3/command/global.go:103\ngo.etcd.io/etcd/etcdctl/v3/ctlv3/command.mustClientFromCmd\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/etcdctl/ctlv3/command/global.go:150\ngo.etcd.io/etcd/etcdctl/v3/ctlv3/command.transferLeadershipCommandFunc\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/etcdctl/ctlv3/command/move_leader_command.go:46\ngithub.com/spf13/cobra.(*Command).execute\n\t/Users/wachao/go/gopath/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:860\ngithub.com/spf13/cobra.(*Command).ExecuteC\n\t/Users/wachao/go/gopath/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:974\ngithub.com/spf13/cobra.(*Command).Execute\n\t/Users/wachao/go/gopath/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:902\ngo.etcd.io/etcd/etcdctl/v3/ctlv3.Start\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/etcdctl/ctlv3/ctl.go:113\ngo.etcd.io/etcd/etcdctl/v3/ctlv3.MustStart\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/etcdctl/ctlv3/ctl.go:117\nmain.main\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/etcdctl/main.go:32\nruntime.main\n\t/Users/wachao/software/go/src/runtime/proc.go:255"}

wachao-a01:bin wachao$ ./etcdctl  version
etcdctl version: 3.6.0-alpha.0
API version: 3.6

@ahrtr
Copy link
Member

ahrtr commented Sep 9, 2022

Would you mind deliver a PR or let me fix it in main right now?

@tjungblu
Copy link
Contributor Author

tjungblu commented Sep 9, 2022

@ahrtr the bug I was fixing is slightly different - check out: #14307 (comment)

This happens when you are moving the leader with the env variable set, but without the argument. What you're describing is the correct failure - you're shadowing the env variable with a conflicting value.

@ahrtr
Copy link
Member

ahrtr commented Sep 9, 2022

wachao-a01:bin wachao$ echo $ETCDCTL_ENDPOINTS
http://127.0.0.1:2379,http://127.0.0.1:22379,http://127.0.0.1:32379

wachao-a01:bin wachao$ ./etcdctl  move-leader 8211f1d0f64f3269
{"level":"fatal","ts":"2022-09-09T16:10:16.727+0800","caller":"flags/flag.go:85","msg":"conflicting environment variable is shadowed by corresponding command-line flag (either unset environment variable or disable flag))","environment-variable":"ETCDCTL_ENDPOINTS","stacktrace":"go.etcd.io/etcd/pkg/v3/flags.verifyEnv\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/pkg/flags/flag.go:85\ngo.etcd.io/etcd/pkg/v3/flags.SetPflagsFromEnv\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/pkg/flags/flag.go:63\ngo.etcd.io/etcd/etcdctl/v3/ctlv3/command.clientConfigFromCmd\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/etcdctl/ctlv3/command/global.go:103\ngo.etcd.io/etcd/etcdctl/v3/ctlv3/command.transferLeadershipCommandFunc\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/etcdctl/ctlv3/command/move_leader_command.go:56\ngithub.com/spf13/cobra.(*Command).execute\n\t/Users/wachao/go/gopath/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:860\ngithub.com/spf13/cobra.(*Command).ExecuteC\n\t/Users/wachao/go/gopath/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:974\ngithub.com/spf13/cobra.(*Command).Execute\n\t/Users/wachao/go/gopath/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:902\ngo.etcd.io/etcd/etcdctl/v3/ctlv3.Start\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/etcdctl/ctlv3/ctl.go:113\ngo.etcd.io/etcd/etcdctl/v3/ctlv3.MustStart\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/etcdctl/ctlv3/ctl.go:117\nmain.main\n\t/Users/wachao/go/src/github.com/ahrtr/etcd/etcdctl/main.go:32\nruntime.main\n\t/Users/wachao/software/go/src/runtime/proc.go:255"}
wachao-a01:bin wachao$ 

@tjungblu
Copy link
Contributor Author

tjungblu commented Sep 9, 2022

indeed, apologies - I had a strange build of etcdctl in my path. Will send you the fix for main right away.

@ahrtr
Copy link
Member

ahrtr commented Sep 9, 2022

Sure, thanks.

ahrtr added a commit to ahrtr/etcd that referenced this pull request Sep 15, 2022
There are duplicated items for etcd-io#14441.
Since it's a etcdctl side change, so we should keep the item under etcdctl,
and remove the duplicated item under "etcd server"

Signed-off-by: Benjamin Wang <wachao@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants