From c4a4f0dcc44710e27aeff73f0c4ddfc99e3cc089 Mon Sep 17 00:00:00 2001 From: Travis Date: Mon, 31 Jul 2017 10:22:28 -0500 Subject: [PATCH] don't normalize bind address after all, just have it use defaults when necessary --- config.go | 13 +++++----- config_test.go | 13 ++++------ gossip/gossip.go | 2 +- pilosa.go | 60 ++++++++++++++++++++++++++++--------------- server/server.go | 23 +++++------------ server/server_test.go | 10 +++----- 6 files changed, 62 insertions(+), 59 deletions(-) diff --git a/config.go b/config.go index 63b8b807f..9743d4c1b 100644 --- a/config.go +++ b/config.go @@ -105,12 +105,13 @@ func (c *Config) Validate() error { if c.Cluster.Type == ClusterGossip { if len(c.Cluster.Hosts) > 0 { - // TODO travis: revisit this logic as it doesn't work well with defaults. - /* - if !foundItem(c.Cluster.Hosts, c.Bind) { - return ErrConfigHostsMissing - } - */ + bindWithDefaults, err := AddressWithDefaults(c.Bind) + if err != nil { + return err + } + if !foundItem(c.Cluster.Hosts, bindWithDefaults) { + return ErrConfigHostsMissing + } } } diff --git a/config_test.go b/config_test.go index db9e0bfe1..c409fd80f 100644 --- a/config_test.go +++ b/config_test.go @@ -22,14 +22,11 @@ func Test_NewConfig(t *testing.T) { // Change cluster type back to gossip. c.Cluster.Type = pilosa.ClusterGossip - // TODO travis: revisit this - /* - // Check for bind address in cluster hosts. - c.Bind = "localhost:1" - if err := c.Validate(); err != pilosa.ErrConfigHostsMissing { - t.Fatal(err) - } - */ + // Check for bind address in cluster hosts. + c.Bind = "localhost:1" + if err := c.Validate(); err != pilosa.ErrConfigHostsMissing { + t.Fatal(err) + } c.Bind = "localhost:10101" c.Cluster.ReplicaN = 2 diff --git a/gossip/gossip.go b/gossip/gossip.go index 95282a658..a967079cb 100644 --- a/gossip/gossip.go +++ b/gossip/gossip.go @@ -136,7 +136,7 @@ func NewGossipNodeSet(name string, gossipHost string, gossipPort int, gossipSeed g.config.memberlistConfig.Name = name g.config.memberlistConfig.BindAddr = gossipHost g.config.memberlistConfig.BindPort = gossipPort - g.config.memberlistConfig.AdvertiseAddr = gossipHost + g.config.memberlistConfig.AdvertiseAddr = pilosa.HostToIP(gossipHost) g.config.memberlistConfig.AdvertisePort = gossipPort g.config.memberlistConfig.Delegate = g diff --git a/pilosa.go b/pilosa.go index 26b135532..6dc06af75 100644 --- a/pilosa.go +++ b/pilosa.go @@ -172,9 +172,45 @@ func ContainsSubstring(a string, list []string) bool { // NormalizeAddress converts addr into a valid "IP4:port" string. func NormalizeAddress(addr string) (string, error) { - var host, port string - var err error + host, port, err := hostPortWithDefaults(addr) + if err != nil { + return "", err + } + return net.JoinHostPort(HostToIP(host), port), nil +} +// HostToIP converts host to an IP4 address based on net.LookupIP(). +func HostToIP(host string) string { + // if host is not an IP addr, check net.LookupIP() + if net.ParseIP(host) == nil { + hosts, err := net.LookupIP(host) + if err != nil { + return host + } + for _, h := range hosts { + // this restricts pilosa to IP4 + if h.To4() != nil { + return h.String() + } + } + } + return host +} + +// AddressWithDefaults converts addr into a valid address, +// using defaults when necessary. +func AddressWithDefaults(addr string) (string, error) { + host, port, err := hostPortWithDefaults(addr) + if err != nil { + return "", err + } + + return net.JoinHostPort(host, port), nil +} + +// hostPortWithDefaults returns the host and port portions of addr +// using defaults when necessary. +func hostPortWithDefaults(addr string) (host, port string, err error) { // check for a colon between host and port if !hasPort(addr) { addr += ":" @@ -183,7 +219,7 @@ func NormalizeAddress(addr string) (string, error) { // break into host, port host, port, err = net.SplitHostPort(addr) if err != nil { - return "", err + return host, port, err } // use defaults when not provided @@ -194,23 +230,7 @@ func NormalizeAddress(addr string) (string, error) { port = DefaultPort } - // if host is not an IP addr, check net.LookupIP() - ip := net.ParseIP(host) - if ip == nil { - hosts, err := net.LookupIP(host) - if err != nil { - return "", err - } - for _, h := range hosts { - // this restricts pilosa to IP4 - if h.To4() != nil { - host = h.String() - break - } - } - } - - return net.JoinHostPort(host, port), nil + return host, port, nil } func hasPort(s string) bool { diff --git a/server/server.go b/server/server.go index 2311365ee..157fc69f3 100644 --- a/server/server.go +++ b/server/server.go @@ -115,11 +115,7 @@ func (m *Command) SetupServer() error { cluster.ReplicaN = m.Config.Cluster.ReplicaN for _, hostport := range m.Config.Cluster.Hosts { - addr, err := pilosa.NormalizeAddress(hostport) - if err != nil { - return err - } - cluster.Nodes = append(cluster.Nodes, &pilosa.Node{Host: addr}) + cluster.Nodes = append(cluster.Nodes, &pilosa.Node{Host: hostport}) } m.Server.Cluster = cluster @@ -143,10 +139,11 @@ func (m *Command) SetupServer() error { // Copy configuration flags. m.Server.MaxWritesPerRequest = m.Config.MaxWritesPerRequest - m.Server.Host, err = pilosa.NormalizeAddress(m.Config.Bind) + bindWithDefaults, err := pilosa.AddressWithDefaults(m.Config.Bind) if err != nil { return err } + m.Server.Host = bindWithDefaults // Set internal port (string). gossipPortStr := pilosa.DefaultGossipPort @@ -160,25 +157,17 @@ func (m *Command) SetupServer() error { if err != nil { return err } - gossipSeed := ":" + pilosa.DefaultGossipPort + gossipSeed := pilosa.DefaultHost + ":" + pilosa.DefaultGossipPort if m.Config.GossipSeed != "" { gossipSeed = m.Config.GossipSeed } - gossipSeed, err = pilosa.NormalizeAddress(gossipSeed) - if err != nil { - return err - } // get the host portion of addr to use for binding - bind, err := pilosa.NormalizeAddress(m.Config.Bind) - if err != nil { - return err - } - gossipHost, _, err := net.SplitHostPort(bind) + gossipHost, _, err := net.SplitHostPort(bindWithDefaults) if err != nil { gossipHost = m.Config.Bind } - gossipNodeSet := gossip.NewGossipNodeSet(m.Config.Bind, gossipHost, gossipPort, gossipSeed, m.Server) + gossipNodeSet := gossip.NewGossipNodeSet(bindWithDefaults, gossipHost, gossipPort, gossipSeed, m.Server) m.Server.Cluster.NodeSet = gossipNodeSet m.Server.Broadcaster = gossipNodeSet m.Server.BroadcastReceiver = gossipNodeSet diff --git a/server/server_test.go b/server/server_test.go index f5f6d977d..c1a90121d 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -439,10 +439,6 @@ func TestMain_SendReceiveMessage(t *testing.T) { if err != nil { gossipHost = m0.Server.Host } - // TODO travis: remove these checks - if gossipHost == "localhost" { - gossipHost = "127.0.0.1" - } gossipPort, err := strconv.Atoi(freePorts[0]) if err != nil { t.Fatal(err) @@ -471,9 +467,6 @@ func TestMain_SendReceiveMessage(t *testing.T) { if err != nil { gossipHost = m1.Server.Host } - if gossipHost == "localhost" { - gossipHost = "127.0.0.1" - } gossipPort, err = strconv.Atoi(freePorts[1]) if err != nil { t.Fatal(err) @@ -587,6 +580,9 @@ func TestMain_SendReceiveMessage(t *testing.T) { t.Fatal(err) } + // We have to wait for the broadcast message to be sent before checking state. + time.Sleep(1 * time.Second) + frame0 := m0.Server.Holder.Frame("i", "event-time") if frame0 == nil { t.Fatal("frame not found")