From 2b10bccceead576912f78abd560606362b394edb Mon Sep 17 00:00:00 2001 From: Gyuho Lee Date: Tue, 23 Jan 2018 15:30:31 -0800 Subject: [PATCH 1/3] etcdserver: add error details on DNS resolution failure on advertise URLs Signed-off-by: Gyuho Lee --- etcdserver/config.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/etcdserver/config.go b/etcdserver/config.go index 249e8d5fc13..056af745dad 100644 --- a/etcdserver/config.go +++ b/etcdserver/config.go @@ -124,7 +124,8 @@ func (c *ServerConfig) advertiseMatchesCluster() error { sort.Strings(apurls) ctx, cancel := context.WithTimeout(context.TODO(), 30*time.Second) defer cancel() - if netutil.URLStringsEqual(ctx, apurls, urls.StringSlice()) { + ok, err := netutil.URLStringsEqual(ctx, apurls, urls.StringSlice()) + if ok { return nil } @@ -148,7 +149,7 @@ func (c *ServerConfig) advertiseMatchesCluster() error { } mstr := strings.Join(missing, ",") apStr := strings.Join(apurls, ",") - return fmt.Errorf("--initial-cluster has %s but missing from --initial-advertise-peer-urls=%s ", mstr, apStr) + return fmt.Errorf("--initial-cluster has %s but missing from --initial-advertise-peer-urls=%s (%v)", mstr, apStr, err) } for url := range apMap { @@ -156,9 +157,16 @@ func (c *ServerConfig) advertiseMatchesCluster() error { missing = append(missing, url) } } - mstr := strings.Join(missing, ",") + if len(missing) > 0 { + mstr := strings.Join(missing, ",") + umap := types.URLsMap(map[string]types.URLs{c.Name: c.PeerURLs}) + return fmt.Errorf("--initial-advertise-peer-urls has %s but missing from --initial-cluster=%s", mstr, umap.String()) + } + + // resolved URLs from "--initial-advertise-peer-urls" and "--initial-cluster" did not match or failed + apStr := strings.Join(apurls, ",") umap := types.URLsMap(map[string]types.URLs{c.Name: c.PeerURLs}) - return fmt.Errorf("--initial-advertise-peer-urls has %s but missing from --initial-cluster=%s", mstr, umap.String()) + return fmt.Errorf("failed to resolve %s to match --initial-cluster=%s (%v)", apStr, umap.String(), err) } func (c *ServerConfig) MemberDir() string { return filepath.Join(c.DataDir, "member") } From e1de74913d4379fbf8e339ab5944e457e9fb4aa5 Mon Sep 17 00:00:00 2001 From: Gyuho Lee Date: Thu, 25 Jan 2018 10:31:59 -0800 Subject: [PATCH 2/3] pkg/netutil: return error from "URLStringsEqual" Signed-off-by: Gyuho Lee --- pkg/netutil/netutil.go | 40 +++++++++++++++++++++++++------------ pkg/netutil/netutil_test.go | 28 ++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/pkg/netutil/netutil.go b/pkg/netutil/netutil.go index 6f7b56abbc4..e3db8c50a0d 100644 --- a/pkg/netutil/netutil.go +++ b/pkg/netutil/netutil.go @@ -17,6 +17,7 @@ package netutil import ( "context" + "fmt" "net" "net/url" "reflect" @@ -73,14 +74,14 @@ func resolveTCPAddrs(ctx context.Context, urls [][]url.URL) ([][]url.URL, error) for i, u := range us { nu, err := url.Parse(u.String()) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to parse %q (%v)", u.String(), err) } nus[i] = *nu } for i, u := range nus { h, err := resolveURL(ctx, u) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to resolve %q (%v)", u.String(), err) } if h != "" { nus[i].Host = h @@ -123,35 +124,41 @@ func resolveURL(ctx context.Context, u url.URL) (string, error) { // urlsEqual checks equality of url.URLS between two arrays. // This check pass even if an URL is in hostname and opposite is in IP address. -func urlsEqual(ctx context.Context, a []url.URL, b []url.URL) bool { +func urlsEqual(ctx context.Context, a []url.URL, b []url.URL) (bool, error) { if len(a) != len(b) { - return false + return false, fmt.Errorf("len(%q) != len(%q)", urlsToStrings(a), urlsToStrings(b)) } urls, err := resolveTCPAddrs(ctx, [][]url.URL{a, b}) if err != nil { - return false + return false, err } + preva, prevb := a, b a, b = urls[0], urls[1] sort.Sort(types.URLs(a)) sort.Sort(types.URLs(b)) for i := range a { if !reflect.DeepEqual(a[i], b[i]) { - return false + return false, fmt.Errorf("%q(resolved from %q) != %q(resolved from %q)", + a[i].String(), preva[i].String(), + b[i].String(), prevb[i].String(), + ) } } - - return true + return true, nil } -func URLStringsEqual(ctx context.Context, a []string, b []string) bool { +// URLStringsEqual returns "true" if given URLs are valid +// and resolved to same IP addresses. Otherwise, return "false" +// and error, if any. +func URLStringsEqual(ctx context.Context, a []string, b []string) (bool, error) { if len(a) != len(b) { - return false + 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 + return false, fmt.Errorf("failed to parse %q", str) } urlsA = append(urlsA, *u) } @@ -159,14 +166,21 @@ func URLStringsEqual(ctx context.Context, a []string, b []string) bool { for _, str := range b { u, err := url.Parse(str) if err != nil { - return false + return false, fmt.Errorf("failed to parse %q", str) } urlsB = append(urlsB, *u) } - return urlsEqual(ctx, urlsA, urlsB) } +func urlsToStrings(us []url.URL) []string { + rs := make([]string, len(us)) + for i := range us { + rs[i] = us[i].String() + } + return rs +} + func IsNetworkTimeoutError(err error) bool { nerr, ok := err.(net.Error) return ok && nerr.Timeout() diff --git a/pkg/netutil/netutil_test.go b/pkg/netutil/netutil_test.go index e748a0a7572..c1f9c5e50c6 100644 --- a/pkg/netutil/netutil_test.go +++ b/pkg/netutil/netutil_test.go @@ -167,6 +167,7 @@ func TestURLsEqual(t *testing.T) { a []url.URL b []url.URL expect bool + err error }{ { a: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}}, @@ -182,11 +183,13 @@ func TestURLsEqual(t *testing.T) { a: []url.URL{{Scheme: "http", Host: "example.com:2379"}}, b: []url.URL{{Scheme: "https", Host: "10.0.10.1:2379"}}, expect: false, + err: errors.New(`"http://10.0.10.1:2379"(resolved from "http://example.com:2379") != "https://10.0.10.1:2379"(resolved from "https://10.0.10.1:2379")`), }, { a: []url.URL{{Scheme: "https", Host: "example.com:2379"}}, b: []url.URL{{Scheme: "http", Host: "10.0.10.1:2379"}}, expect: false, + err: errors.New(`"https://10.0.10.1:2379"(resolved from "https://example.com:2379") != "http://10.0.10.1:2379"(resolved from "http://10.0.10.1:2379")`), }, { a: []url.URL{{Scheme: "unix", Host: "abc:2379"}}, @@ -212,46 +215,55 @@ func TestURLsEqual(t *testing.T) { a: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}}, b: []url.URL{{Scheme: "http", Host: "127.0.0.1:2380"}}, expect: false, + err: errors.New(`"http://127.0.0.1:2379"(resolved from "http://127.0.0.1:2379") != "http://127.0.0.1:2380"(resolved from "http://127.0.0.1:2380")`), }, { a: []url.URL{{Scheme: "http", Host: "example.com:2380"}}, b: []url.URL{{Scheme: "http", Host: "10.0.10.1:2379"}}, expect: false, + err: errors.New(`"http://10.0.10.1:2380"(resolved from "http://example.com:2380") != "http://10.0.10.1:2379"(resolved from "http://10.0.10.1:2379")`), }, { a: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}}, b: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}}, expect: false, + err: errors.New(`"http://127.0.0.1:2379"(resolved from "http://127.0.0.1:2379") != "http://10.0.0.1:2379"(resolved from "http://10.0.0.1:2379")`), }, { a: []url.URL{{Scheme: "http", Host: "example.com:2379"}}, b: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}}, expect: false, + err: errors.New(`"http://10.0.10.1:2379"(resolved from "http://example.com:2379") != "http://10.0.0.1:2379"(resolved from "http://10.0.0.1:2379")`), }, { a: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}}, b: []url.URL{{Scheme: "http", Host: "127.0.0.1:2380"}, {Scheme: "http", Host: "127.0.0.1:2380"}}, expect: false, + err: errors.New(`"http://127.0.0.1:2379"(resolved from "http://127.0.0.1:2379") != "http://127.0.0.1:2380"(resolved from "http://127.0.0.1:2380")`), }, { a: []url.URL{{Scheme: "http", Host: "example.com:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}}, b: []url.URL{{Scheme: "http", Host: "127.0.0.1:2380"}, {Scheme: "http", Host: "127.0.0.1:2380"}}, expect: false, + err: errors.New(`"http://10.0.10.1:2379"(resolved from "http://example.com:2379") != "http://127.0.0.1:2380"(resolved from "http://127.0.0.1:2380")`), }, { a: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}}, b: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}}, expect: false, + err: errors.New(`"http://127.0.0.1:2379"(resolved from "http://127.0.0.1:2379") != "http://10.0.0.1:2379"(resolved from "http://10.0.0.1:2379")`), }, { a: []url.URL{{Scheme: "http", Host: "example.com:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}}, b: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}}, expect: false, + err: errors.New(`"http://10.0.10.1:2379"(resolved from "http://example.com:2379") != "http://10.0.0.1:2379"(resolved from "http://10.0.0.1:2379")`), }, { a: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}}, b: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}}, expect: false, + err: errors.New(`len(["http://10.0.0.1:2379"]) != len(["http://10.0.0.1:2379" "http://127.0.0.1:2380"])`), }, { a: []url.URL{{Scheme: "http", Host: "first.com:2379"}, {Scheme: "http", Host: "second.com:2380"}}, @@ -265,16 +277,24 @@ func TestURLsEqual(t *testing.T) { }, } - for _, test := range tests { - result := urlsEqual(context.TODO(), test.a, test.b) + for i, test := range tests { + result, err := urlsEqual(context.TODO(), test.a, test.b) if result != test.expect { - t.Errorf("a:%v b:%v, expected %v but %v", test.a, test.b, test.expect, result) + t.Errorf("#%d: a:%v b:%v, expected %v but %v", i, test.a, test.b, test.expect, result) + } + if test.err != nil { + if err.Error() != test.err.Error() { + t.Errorf("#%d: err expected %v but %v", i, test.err, err) + } } } } func TestURLStringsEqual(t *testing.T) { - result := URLStringsEqual(context.TODO(), []string{"http://127.0.0.1:8080"}, []string{"http://127.0.0.1:8080"}) + result, err := URLStringsEqual(context.TODO(), []string{"http://127.0.0.1:8080"}, []string{"http://127.0.0.1:8080"}) if !result { t.Errorf("unexpected result %v", result) } + if err != nil { + t.Errorf("unexpected error %v", err) + } } From ce45c83f29a92484534671f4485756b32de274ef Mon Sep 17 00:00:00 2001 From: Gyuho Lee Date: Thu, 25 Jan 2018 10:32:41 -0800 Subject: [PATCH 3/3] etcdserver: add detailed errors in "ValidateClusterAndAssignIDs" Signed-off-by: Gyuho Lee --- etcdserver/membership/cluster.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/etcdserver/membership/cluster.go b/etcdserver/membership/cluster.go index 565354402e2..4f0b1572ef6 100644 --- a/etcdserver/membership/cluster.go +++ b/etcdserver/membership/cluster.go @@ -490,8 +490,8 @@ func ValidateClusterAndAssignIDs(local *RaftCluster, existing *RaftCluster) erro ctx, cancel := context.WithTimeout(context.TODO(), 30*time.Second) defer cancel() for i := range ems { - if !netutil.URLStringsEqual(ctx, ems[i].PeerURLs, lms[i].PeerURLs) { - return fmt.Errorf("unmatched member while checking PeerURLs") + if ok, err := netutil.URLStringsEqual(ctx, ems[i].PeerURLs, lms[i].PeerURLs); !ok { + return fmt.Errorf("unmatched member while checking PeerURLs (%v)", err) } lms[i].ID = ems[i].ID }