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

netutil: add url comparison without resolver to URLStringsEqual #13224

Merged
merged 3 commits into from
Oct 7, 2021
Merged

netutil: add url comparison without resolver to URLStringsEqual #13224

merged 3 commits into from
Oct 7, 2021

Conversation

sakateka
Copy link
Contributor

If one of the nodes in the cluster has lost a dns record,
restarting the second node will break it.
This PR makes an attempt to add a comparison without using a resolver,
which allows to protect cluster from dns errors and does not break
the current logic of comparing urls in the URLStringsEqual function.
You can read more in the issue #7798

Fixes #7798

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

If one of the nodes in the cluster has lost a dns record,
restarting the second node will break it.
This PR makes an attempt to add a comparison without using a resolver,
which allows to protect cluster from dns errors and does not break
the current logic of comparing urls in the URLStringsEqual function.
You can read more in the issue #7798

Fixes #7798
@sakateka
Copy link
Contributor Author

@gyuho @xiang90 @heyitsanthony @ptabor PTAL

Copy link
Contributor

@lilic lilic 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 PR, few comments.

pkg/netutil/netutil.go Outdated Show resolved Hide resolved
pkg/netutil/netutil.go Outdated Show resolved Hide resolved
pkg/netutil/netutil.go Outdated Show resolved Hide resolved
Co-authored-by: Lili Cosic <cosiclili@gmail.com>
pkg/netutil/netutil.go Outdated Show resolved Hide resolved
@sakateka
Copy link
Contributor Author

sakateka commented Sep 26, 2021

I found a bug in the test TestURLsEqual, it was impossible to take urls from preva, prevb by index, because after the resolving, the order of the indexes will not match the order before resolving (because sorting is performed after the resolving)

@sakateka
Copy link
Contributor Author

sakateka commented Oct 7, 2021

@ptabor please take a look.

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.

@ptabor ptabor merged commit 33623c3 into etcd-io:main Oct 7, 2021
@sakateka sakateka deleted the issue-7798-netutil-fix branch October 7, 2021 17:51
ahrtr added a commit that referenced this pull request Oct 12, 2022
pchan added a commit to pchan/etcd that referenced this pull request Oct 12, 2022
@pchan pchan mentioned this pull request Oct 12, 2022
ahrtr added a commit that referenced this pull request Oct 12, 2022
@serathius serathius mentioned this pull request Nov 14, 2022
22 tasks
tjungblu added a commit to tjungblu/etcd that referenced this pull request Jan 5, 2023
PR etcd-io#13224 introduced an early exit when the names match. This change
restores the previous behaviour by doing an explicit DNS resolution
before checking the URLs for equality.

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.

etcd pod stuck at bootstrap and kept "failed resolving host"
5 participants