Merge pull request #9210 from gyuho/dns-error

*: add detailed error message on URLs equal check
etcd-io · Jan 25, 2018 · 0c692fb · 0c692fb
2 parents 7dd9c30 + ce45c83
commit 0c692fb
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 23 deletions.
diff --git 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,17 +149,24 @@ 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 {
 		if _, ok := initMap[url]; !ok {
 			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") }

diff --git 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
 	}

diff --git 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,50 +124,63 @@ 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)
 	}
 	urlsB := make([]url.URL, 0)
 	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
@@ -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)
+	}
 }