Skip to content

Commit

Permalink
netutil: add url comparison without resolver to URLStringsEqual
Browse files Browse the repository at this point in the history
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 etcd-io#7798

Fixes etcd-io#7798
  • Loading branch information
sakateka authored and pchan committed Oct 11, 2022
1 parent 07c7a98 commit 0924bab
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 19 deletions.
32 changes: 18 additions & 14 deletions pkg/netutil/netutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,21 +174,13 @@ func URLStringsEqual(ctx context.Context, lg *zap.Logger, a []string, b []string
if len(a) != len(b) {
return false, fmt.Errorf("len(%q) != len(%q)", a, b)
}
urlsA := make([]url.URL, 0)
for _, str := range a {
u, err := url.Parse(str)
if err != nil {
return false, fmt.Errorf("failed to parse %q", str)
}
urlsA = append(urlsA, *u)
urlsA, err := stringsToURLs(a)
if err != nil {
return false, err
}
urlsB := make([]url.URL, 0)
for _, str := range b {
u, err := url.Parse(str)
if err != nil {
return false, fmt.Errorf("failed to parse %q", str)
}
urlsB = append(urlsB, *u)
urlsB, err := stringsToURLs(b)
if err != nil {
return false, err
}
return urlsEqual(ctx, lg, urlsA, urlsB)
}
Expand All @@ -201,6 +193,18 @@ func urlsToStrings(us []url.URL) []string {
return rs
}

func stringsToURLs(us []string) ([]url.URL, error) {
urls := make([]url.URL, 0, len(us))
for _, str := range us {
u, err := url.Parse(str)
if err != nil {
return nil, fmt.Errorf("failed to parse %q", str)
}
urls = append(urls, *u)
}
return urls, nil
}

func IsNetworkTimeoutError(err error) bool {
nerr, ok := err.(net.Error)
return ok && nerr.Timeout()
Expand Down
33 changes: 28 additions & 5 deletions pkg/netutil/netutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package netutil
import (
"context"
"errors"
"fmt"
"net"
"net/url"
"reflect"
Expand Down Expand Up @@ -292,11 +293,33 @@ func TestURLsEqual(t *testing.T) {
}
}
func TestURLStringsEqual(t *testing.T) {
result, err := URLStringsEqual(context.TODO(), zap.NewExample(), []string{"http://127.0.0.1:8080"}, []string{"http://127.0.0.1:8080"})
if !result {
t.Errorf("unexpected result %v", result)
defer func() { resolveTCPAddr = resolveTCPAddrDefault }()
errOnResolve := func(ctx context.Context, addr string) (*net.TCPAddr, error) {
return nil, fmt.Errorf("unexpected attempt to resolve: %q", addr)
}
cases := []struct {
urlsA []string
urlsB []string
resolver func(ctx context.Context, addr string) (*net.TCPAddr, error)
}{
{[]string{"http://127.0.0.1:8080"}, []string{"http://127.0.0.1:8080"}, resolveTCPAddrDefault},
{[]string{
"http://host1:8080",
"http://host2:8080",
}, []string{
"http://host1:8080",
"http://host2:8080",
}, errOnResolve},
}
if err != nil {
t.Errorf("unexpected error %v", err)
for idx, c := range cases {
t.Logf("TestURLStringsEqual, case #%d", idx)
resolveTCPAddr = c.resolver
result, err := URLStringsEqual(context.TODO(), zap.NewExample(), c.urlsA, c.urlsB)
if !result {
t.Errorf("unexpected result %v", result)
}
if err != nil {
t.Errorf("unexpected error %v", err)
}
}
}

0 comments on commit 0924bab

Please sign in to comment.