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

etcdctl: allow move-leader to connect to multiple endpoints with TLS #11775

Closed
wants to merge 1 commit into from

Conversation

benmoss
Copy link

@benmoss benmoss commented Apr 9, 2020

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.

The mustClientForCmd function is responsible for parsing environment
variables and flags into configuration data. A change was made in etcd-io#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.
@benmoss benmoss changed the title etcdctl: allow move-leader to connect to multiple endpoints with TLS … etcdctl: allow move-leader to connect to multiple endpoints with TLS Apr 9, 2020
@codecov-io
Copy link

Codecov Report

Merging #11775 into master will decrease coverage by 0.45%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11775      +/-   ##
==========================================
- Coverage   66.92%   66.47%   -0.46%     
==========================================
  Files         403      403              
  Lines       36868    36868              
==========================================
- Hits        24675    24508     -167     
- Misses      10706    10855     +149     
- Partials     1487     1505      +18     
Impacted Files Coverage Δ
etcdctl/ctlv3/command/move_leader_command.go 15.78% <0.00%> (ø)
pkg/transport/timeout_conn.go 60.00% <0.00%> (-20.00%) ⬇️
etcdserver/api/v3rpc/util.go 51.61% <0.00%> (-16.13%) ⬇️
pkg/netutil/netutil.go 61.47% <0.00%> (-7.38%) ⬇️
auth/store.go 71.62% <0.00%> (-6.75%) ⬇️
clientv3/namespace/watch.go 87.87% <0.00%> (-6.07%) ⬇️
client/client.go 78.43% <0.00%> (-5.56%) ⬇️
pkg/adt/interval_tree.go 82.70% <0.00%> (-4.52%) ⬇️
clientv3/leasing/cache.go 87.22% <0.00%> (-4.45%) ⬇️
proxy/grpcproxy/watcher.go 89.79% <0.00%> (-4.09%) ⬇️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7eae024...561b3bd. Read the comment docs.

@stale
Copy link

stale bot commented Jul 8, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 8, 2020
@stale stale bot closed this Jul 29, 2020
@benmoss
Copy link
Author

benmoss commented Jul 30, 2020

dang where's the etcd maintainers at

@spzala spzala reopened this Jul 31, 2020
@stale stale bot removed the stale label Jul 31, 2020
@renan
Copy link

renan commented Aug 26, 2020

Will this be moving forward? This would help with maintaining v3.4 clusters.

@stale
Copy link

stale bot commented Nov 24, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 24, 2020
@renan
Copy link

renan commented Nov 24, 2020

Not so fast, bot! We still need a reply here.

@stale stale bot removed the stale label Nov 24, 2020
Copy link
Contributor

@ptabor ptabor left a comment

Choose a reason for hiding this comment

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

Thank you for the fix.

Could you, please, add a regression test.
Probably this file is a convenient place and a template:

./tests/e2e/ctl_v3_move_leader_test.go

Copy link
Contributor

@ptabor ptabor left a comment

Choose a reason for hiding this comment

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

[deleted]

@benmoss benmoss closed this Feb 3, 2021
@zerodayz
Copy link

zerodayz commented Mar 8, 2021

/cc @benmoss

We hit this problem in https://bugzilla.redhat.com/show_bug.cgi?id=1918413

Is this that this PR only needs regression tests? Are you still fine to work on it @benmoss ?

@benmoss
Copy link
Author

benmoss commented Mar 8, 2021

I don't plan to work on this, while waiting for feedback since April 9th 2020 I think I lost my motivation for seeing this bugfix through to the end 😄

@zerodayz
Copy link

zerodayz commented Mar 8, 2021

@benmoss Do you mind If I pick your PR from where you left?

zerodayz added a commit to zerodayz/etcd that referenced this pull request Mar 9, 2021
Re-opening closed PR etcd-io#11775 which was originaly authored by benmoss.

The mustClientForCmd function is responsible for parsing environment
variables and flags into configuration data. A change was made in etcd-io#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 etcd-io#8380 but I don't think
there's an open issue for it.
zerodayz added a commit to zerodayz/etcd that referenced this pull request Mar 10, 2021
Re-opening closed PR etcd-io#11775 which was originaly authored by benmoss.

The mustClientForCmd function is responsible for parsing environment
variables and flags into configuration data. A change was made in etcd-io#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 etcd-io#8380 but I don't think
there's an open issue for it.
zerodayz added a commit to zerodayz/etcd that referenced this pull request Mar 10, 2021
Re-opening closed PR etcd-io#11775 which was originaly authored by benmoss.

The mustClientForCmd function is responsible for parsing environment
variables and flags into configuration data. A change was made in etcd-io#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 etcd-io#8380 but I don't think
there's an open issue for it.
zerodayz added a commit to zerodayz/etcd that referenced this pull request Mar 11, 2021
Re-opening closed PR etcd-io#11775 which was originaly authored by benmoss.

The mustClientForCmd function is responsible for parsing environment
variables and flags into configuration data. A change was made in etcd-io#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 etcd-io#8380 but I don't think
there's an open issue for it.
zerodayz added a commit to zerodayz/etcd that referenced this pull request Jun 14, 2021
Re-opening closed PR etcd-io#11775 which was originaly authored by benmoss.

The mustClientForCmd function is responsible for parsing environment
variables and flags into configuration data. A change was made in etcd-io#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 etcd-io#8380 but I don't think
there's an open issue for it.
zerodayz added a commit to zerodayz/etcd that referenced this pull request Jun 14, 2021
Re-opening closed PR etcd-io#11775 which was originaly authored by benmoss.

The mustClientForCmd function is responsible for parsing environment
variables and flags into configuration data. A change was made in etcd-io#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 etcd-io#8380 but I don't think
there's an open issue for it.
tjungblu added a commit to tjungblu/etcd that referenced this pull request Aug 3, 2022
Re-opening closed PR etcd-io#11775 which was originaly authored by benmoss.
Then again opened PR etcd-io#12757 which was authored by zerodayz.

The mustClientForCmd function is responsible for parsing environment
variables and flags into configuration data. A change was made in etcd-io#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 etcd-io#8380 but I don't think
there's an open issue for it.

Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
tjungblu added a commit to tjungblu/etcd that referenced this pull request Sep 7, 2022
Re-opening closed PR etcd-io#11775 which was originaly authored by benmoss.
Then again opened PR etcd-io#12757 which was authored by zerodayz.

The mustClientForCmd function is responsible for parsing environment
variables and flags into configuration data. A change was made in etcd-io#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 etcd-io#8380 but I don't think
there's an open issue for it.

Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
tjungblu added a commit to tjungblu/etcd that referenced this pull request Sep 7, 2022
…ndpoints

Re-opening closed PR etcd-io#11775 which was originaly authored by benmoss.
Then again opened PR etcd-io#12757 which was authored by zerodayz.

The mustClientForCmd function is responsible for parsing environment
variables and flags into configuration data. A change was made in etcd-io#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 etcd-io#8380 but I don't think
there's an open issue for it.

Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants