[release-3.5]etcdctl: allow move-leader to connect to multiple endpoints #14434
Conversation
| @@ -19,6 +19,7 @@ import ( | |||
| "strconv" | |||
|
|
|||
| "github.com/spf13/cobra" | |||
|
|
|||
There was a problem hiding this comment.
minor comment: Did you intentionally add this blank line or it's added by IDE automatically? I would suggest to keep it as it's. Of course, it isn't a big deal.
There was a problem hiding this comment.
yep IDE, let me remove it again
There was a problem hiding this comment.
@tjungblu thanks! lgtm but I think the PR/commit description is not correct, so I would suggest to at least edit PR description for future reference if someone look at it. First, there is a typo in mustClientForCmd, it's mustClientCfgFromCmd. Secondly, it's not the mustClientCfgFromCmd getting called multiple times resulting into flag related error as the description says. It's the clientConfigFromCmd that's getting called twice (directly and through mustClientCfgFromCmd) which the PR provides fix for.
| @@ -19,6 +19,7 @@ import ( | |||
| "strconv" | |||
|
|
|||
| "github.com/spf13/cobra" | |||
|
|
|||
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
force-pushed
the
bz_1918413_3.5
branch
from
2b14b2a
to
243b7a1
Compare
|
Thanks for the reviews, fixed the commit message and description and removed that newline. |
|
@tjungblu Could you please backport this PR to 3.4 as well? |
|
sure, can do. |
ahrtr
changed the title
Re-opening closed PR #11775 which was originaly authored by benmoss. Then again opened PR #12757 which was authored by zerodayz. And then a continuation of #14307.
The mustClientForCmd function is responsible for parsing environment variables and flags into configuration data. A change was made in #9382 to call Fatal if a flag is provided multiple times. This means that we cannot call the mustClientForCmd function more than once, since it will think that flags parsed the first time are now being redefined and error out.
Some people have commented about this in #8380 but I don't think there's an open issue for it.
Signed-off-by: Thomas Jungblut tjungblu@redhat.com