From b7ee8f4967dc457f369fcda74be4cc582171a649 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Fri, 3 Mar 2017 13:32:46 -0800 Subject: [PATCH 1/2] embed: use machine default host only for default value, 0.0.0.0 Signed-off-by: Gyu-Ho Lee --- embed/config.go | 74 +++++++++++++++++++++++++------------------- embed/config_test.go | 68 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 32 deletions(-) diff --git a/embed/config.go b/embed/config.go index db35e84cbb7..6ae19479b8f 100644 --- a/embed/config.go +++ b/embed/config.go @@ -57,20 +57,12 @@ var ( DefaultInitialAdvertisePeerURLs = "http://localhost:2380" DefaultAdvertiseClientURLs = "http://localhost:2379" - defaultHostname string = "localhost" + defaultHostname string defaultHostStatus error ) func init() { - ip, err := netutil.GetDefaultHost() - if err != nil { - defaultHostStatus = err - return - } - // found default host, advertise on it - DefaultInitialAdvertisePeerURLs = "http://" + net.JoinHostPort(ip, "2380") - DefaultAdvertiseClientURLs = "http://" + net.JoinHostPort(ip, "2379") - defaultHostname = ip + defaultHostname, defaultHostStatus = netutil.GetDefaultHost() } // Config holds the arguments for configuring an etcd server. @@ -358,34 +350,52 @@ func (cfg Config) InitialClusterFromName(name string) (ret string) { func (cfg Config) IsNewCluster() bool { return cfg.ClusterState == ClusterStateFlagNew } func (cfg Config) ElectionTicks() int { return int(cfg.ElectionMs / cfg.TickMs) } -// IsDefaultHost returns the default hostname, if used, and the error, if any, -// from getting the machine's default host. -func (cfg Config) IsDefaultHost() (string, error) { - if len(cfg.APUrls) == 1 && cfg.APUrls[0].String() == DefaultInitialAdvertisePeerURLs { - return defaultHostname, defaultHostStatus - } - if len(cfg.ACUrls) == 1 && cfg.ACUrls[0].String() == DefaultAdvertiseClientURLs { - return defaultHostname, defaultHostStatus - } - return "", defaultHostStatus +func (cfg Config) defaultPeerHost() bool { + return len(cfg.APUrls) == 1 && cfg.APUrls[0].String() == DefaultInitialAdvertisePeerURLs } -// UpdateDefaultClusterFromName updates cluster advertise URLs with default host. +func (cfg Config) defaultClientHost() bool { + return len(cfg.ACUrls) == 1 && cfg.ACUrls[0].String() == DefaultAdvertiseClientURLs +} + +// UpdateDefaultClusterFromName updates cluster advertise URLs with, if available, default host, +// if advertise URLs are default values(localhost:2379,2380) AND if listen URL is 0.0.0.0. +// e.g. advertise peer URL localhost:2380 or listen peer URL 0.0.0.0:2380 +// then the advertise peer host would be updated with machine's default host, +// while keeping the listen URL's port. +// User can work around this by explicitly setting URL with 127.0.0.1. +// It returns the default hostname, if used, and the error, if any, from getting the machine's default host. // TODO: check whether fields are set instead of whether fields have default value -func (cfg *Config) UpdateDefaultClusterFromName(defaultInitialCluster string) { - defaultHost, defaultHostErr := cfg.IsDefaultHost() - defaultHostOverride := defaultHost == "" || defaultHostErr == nil - if (defaultHostOverride || cfg.Name != DefaultName) && cfg.InitialCluster == defaultInitialCluster { - cfg.InitialCluster = cfg.InitialClusterFromName(cfg.Name) - ip, _, _ := net.SplitHostPort(cfg.LCUrls[0].Host) - // if client-listen-url is 0.0.0.0, just use detected default host - // otherwise, rewrite advertise-client-url with localhost - if ip != "0.0.0.0" { - _, acPort, _ := net.SplitHostPort(cfg.ACUrls[0].Host) - cfg.ACUrls[0] = url.URL{Scheme: cfg.ACUrls[0].Scheme, Host: fmt.Sprintf("localhost:%s", acPort)} +func (cfg *Config) UpdateDefaultClusterFromName(defaultInitialCluster string) (string, error) { + if defaultHostname == "" || defaultHostStatus != nil { + // update 'initial-cluster' when only the name is specified (e.g. 'etcd --name=abc') + if cfg.Name != DefaultName && cfg.InitialCluster == defaultInitialCluster { cfg.InitialCluster = cfg.InitialClusterFromName(cfg.Name) } + return "", defaultHostStatus + } + + used := false + pip, pport, _ := net.SplitHostPort(cfg.LPUrls[0].Host) + if cfg.defaultPeerHost() && pip == "0.0.0.0" { + cfg.APUrls[0] = url.URL{Scheme: cfg.APUrls[0].Scheme, Host: fmt.Sprintf("%s:%s", defaultHostname, pport)} + used = true + } + // update 'initial-cluster' when only the name is specified (e.g. 'etcd --name=abc') + if cfg.Name != DefaultName && cfg.InitialCluster == defaultInitialCluster { + cfg.InitialCluster = cfg.InitialClusterFromName(cfg.Name) + } + + cip, cport, _ := net.SplitHostPort(cfg.LCUrls[0].Host) + if cfg.defaultClientHost() && cip == "0.0.0.0" { + cfg.ACUrls[0] = url.URL{Scheme: cfg.ACUrls[0].Scheme, Host: fmt.Sprintf("%s:%s", defaultHostname, cport)} + used = true + } + dhost := defaultHostname + if !used { + dhost = "" } + return dhost, defaultHostStatus } // checkBindURLs returns an error if any URL uses a domain name. diff --git a/embed/config_test.go b/embed/config_test.go index 5430612060a..1be6bcd9c14 100644 --- a/embed/config_test.go +++ b/embed/config_test.go @@ -15,11 +15,15 @@ package embed import ( + "fmt" "io/ioutil" + "net" + "net/url" "os" "testing" "github.com/coreos/etcd/pkg/transport" + "github.com/ghodss/yaml" ) @@ -61,6 +65,70 @@ func TestConfigFileOtherFields(t *testing.T) { } } +// TestUpdateDefaultClusterFromName ensures that etcd can start with 'etcd --name=abc'. +func TestUpdateDefaultClusterFromName(t *testing.T) { + cfg := NewConfig() + defaultInitialCluster := cfg.InitialCluster + oldscheme := cfg.APUrls[0].Scheme + origpeer := cfg.APUrls[0].String() + origadvc := cfg.ACUrls[0].String() + + cfg.Name = "abc" + _, lpport, _ := net.SplitHostPort(cfg.LPUrls[0].Host) + + // in case of 'etcd --name=abc' + exp := fmt.Sprintf("%s=%s://localhost:%s", cfg.Name, oldscheme, lpport) + cfg.UpdateDefaultClusterFromName(defaultInitialCluster) + if exp != cfg.InitialCluster { + t.Fatalf("initial-cluster expected %q, got %q", exp, cfg.InitialCluster) + } + // advertise peer URL should not be affected + if origpeer != cfg.APUrls[0].String() { + t.Fatalf("advertise peer url expected %q, got %q", origadvc, cfg.APUrls[0].String()) + } + // advertise client URL should not be affected + if origadvc != cfg.ACUrls[0].String() { + t.Fatalf("advertise client url expected %q, got %q", origadvc, cfg.ACUrls[0].String()) + } +} + +// TestUpdateDefaultClusterFromNameOverwrite ensures that machine's default host is only used +// if advertise URLs are default values(localhost:2379,2380) AND if listen URL is 0.0.0.0. +func TestUpdateDefaultClusterFromNameOverwrite(t *testing.T) { + if defaultHostname == "" { + t.Skip("machine's default host not found") + } + + cfg := NewConfig() + defaultInitialCluster := cfg.InitialCluster + oldscheme := cfg.APUrls[0].Scheme + origadvc := cfg.ACUrls[0].String() + + cfg.Name = "abc" + _, lpport, _ := net.SplitHostPort(cfg.LPUrls[0].Host) + cfg.LPUrls[0] = url.URL{Scheme: cfg.LPUrls[0].Scheme, Host: fmt.Sprintf("0.0.0.0:%s", lpport)} + dhost, _ := cfg.UpdateDefaultClusterFromName(defaultInitialCluster) + if dhost != defaultHostname { + t.Fatalf("expected default host %q, got %q", defaultHostname, dhost) + } + aphost, apport, _ := net.SplitHostPort(cfg.APUrls[0].Host) + if apport != lpport { + t.Fatalf("advertise peer url got different port %s, expected %s", apport, lpport) + } + if aphost != defaultHostname { + t.Fatalf("advertise peer url expected machine default host %q, got %q", defaultHostname, aphost) + } + expected := fmt.Sprintf("%s=%s://%s:%s", cfg.Name, oldscheme, defaultHostname, lpport) + if expected != cfg.InitialCluster { + t.Fatalf("initial-cluster expected %q, got %q", expected, cfg.InitialCluster) + } + + // advertise client URL should not be affected + if origadvc != cfg.ACUrls[0].String() { + t.Fatalf("advertise-client-url expected %q, got %q", origadvc, cfg.ACUrls[0].String()) + } +} + func (s *securityConfig) equals(t *transport.TLSInfo) bool { return s.CAFile == t.CAFile && s.CertFile == t.CertFile && From 4aa68e0231bcb46b87a6dd42723fcf987b566cac Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Fri, 3 Mar 2017 13:34:03 -0800 Subject: [PATCH 2/2] etcdmain: log machine default host after update check Signed-off-by: Gyu-Ho Lee --- etcdmain/etcd.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/etcdmain/etcd.go b/etcdmain/etcd.go index 321e1ce4703..b4220d479fc 100644 --- a/etcdmain/etcd.go +++ b/etcdmain/etcd.go @@ -85,7 +85,13 @@ func startEtcdOrProxyV2() { GoMaxProcs := runtime.GOMAXPROCS(0) plog.Infof("setting maximum number of CPUs to %d, total number of available CPUs is %d", GoMaxProcs, runtime.NumCPU()) - (&cfg.Config).UpdateDefaultClusterFromName(defaultInitialCluster) + defaultHost, dhErr := (&cfg.Config).UpdateDefaultClusterFromName(defaultInitialCluster) + if defaultHost != "" { + plog.Infof("advertising using detected default host %q", defaultHost) + } + if dhErr != nil { + plog.Noticef("failed to detect default host (%v)", dhErr) + } if cfg.Dir == "" { cfg.Dir = fmt.Sprintf("%v.etcd", cfg.Name) @@ -184,15 +190,6 @@ func startEtcdOrProxyV2() { // startEtcd runs StartEtcd in addition to hooks needed for standalone etcd. func startEtcd(cfg *embed.Config) (<-chan struct{}, <-chan error, error) { - defaultHost, dhErr := cfg.IsDefaultHost() - if defaultHost != "" { - if dhErr == nil { - plog.Infof("advertising using detected default host %q", defaultHost) - } else { - plog.Noticef("failed to detect default host, advertise falling back to %q (%v)", defaultHost, dhErr) - } - } - if cfg.Metrics == "extensive" { grpc_prometheus.EnableHandlingTimeHistogram() }