Fixes panic when ports are not specified

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

cluster/cluster.go

@@ -102,7 +102,7 @@ func (c *Cluster) UpdateNodes(nodes []*discovery.Node) {
 	for _, addr := range nodes {
 		go func(node *discovery.Node) {
 			if c.Node(node.String()) == nil {
-				n := NewNode(node.String(), c.overcommitRatio)
+				n := NewNode(node.Host, node.Port, c.overcommitRatio)
 				if err := n.Connect(c.tlsConfig); err != nil {
 					log.Error(err)
 					return

cluster/cluster_test.go

@@ -11,8 +11,8 @@ import (
 	"github.com/stretchr/testify/mock"
 )
 
-func createNode(t *testing.T, ID string, containers ...dockerclient.Container) *Node {
+func createNode(t *testing.T, ID string, Port string, containers ...dockerclient.Container) *Node {
-	node := NewNode(ID, 0)
+	node := NewNode(ID, Port, 0)
 	node.Name = ID
 
 	assert.False(t, node.IsConnected())
@@ -46,15 +46,15 @@ func TestAddNode(t *testing.T) {
 	assert.Nil(t, c.Node("test"))
 	assert.Nil(t, c.Node("test2"))
 
-	assert.NoError(t, c.AddNode(createNode(t, "test")))
+	assert.NoError(t, c.AddNode(createNode(t, "test", "2375")))
 	assert.Equal(t, len(c.Nodes()), 1)
 	assert.NotNil(t, c.Node("test"))
 
-	assert.Error(t, c.AddNode(createNode(t, "test")))
+	assert.Error(t, c.AddNode(createNode(t, "test", "2375")))
 	assert.Equal(t, len(c.Nodes()), 1)
 	assert.NotNil(t, c.Node("test"))
 
-	assert.NoError(t, c.AddNode(createNode(t, "test2")))
+	assert.NoError(t, c.AddNode(createNode(t, "test2", "2375")))
 	assert.Equal(t, len(c.Nodes()), 2)
 	assert.NotNil(t, c.Node("test2"))
 }
@@ -65,7 +65,7 @@ func TestContainerLookup(t *testing.T) {
 		Id:    "container-id",
 		Names: []string{"/container-name1", "/container-name2"},
 	}
-	node := createNode(t, "test-node", container)
+	node := createNode(t, "test-node", "2375", container)
 	assert.NoError(t, c.AddNode(node))
 
 	// Invalid lookup
@@ -85,7 +85,7 @@ func TestContainerLookup(t *testing.T) {
 
 func TestDeployContainer(t *testing.T) {
 	// Create a test node.
-	node := createNode(t, "test")
+	node := createNode(t, "test", "2375")
 
 	// Create a test cluster.
 	c := newCluster(t)

cluster/node.go

@@ -21,9 +21,10 @@ const (
 	requestTimeout = 10 * time.Second
 )
 
-func NewNode(addr string, overcommitRatio float64) *Node {
+func NewNode(host, port string, overcommitRatio float64) *Node {
 	e := &Node{
-		Addr:            addr,
+		Addr:            host,
+		Port:            port,
 		Labels:          make(map[string]string),
 		ch:              make(chan bool),
 		containers:      make(map[string]*Container),
@@ -39,6 +40,7 @@ type Node struct {
 	ID     string
 	IP     string
 	Addr   string
+	Port   string
 	Name   string
 	Cpus   int64
 	Memory int64
@@ -56,14 +58,13 @@ type Node struct {
 // Connect will initialize a connection to the Docker daemon running on the
 // host, gather machine specs (memory, cpu, ...) and monitor state changes.
 func (n *Node) Connect(config *tls.Config) error {
-	parts := strings.Split(n.Addr, ":")
+	addr, err := net.ResolveIPAddr("ip4", n.Addr)
-	addr, err := net.ResolveIPAddr("ip4", parts[0])
 	if err != nil {
 		return err
 	}
 	n.IP = addr.IP.String()
 
-	c, err := dockerclient.NewDockerClientTimeout(n.IP+":"+parts[1], config, time.Duration(requestTimeout))
+	c, err := dockerclient.NewDockerClientTimeout(n.IP+":"+n.Port, config, time.Duration(requestTimeout))
 	if err != nil {
 		return err
 	}

cluster/node_test.go

@@ -26,7 +26,7 @@ var (
 )
 
 func TestNodeConnectionFailure(t *testing.T) {
-	node := NewNode("test", 0)
+	node := NewNode("test", "2375", 0)
 	assert.False(t, node.IsConnected())
 
 	// Always fail.
@@ -41,7 +41,7 @@ func TestNodeConnectionFailure(t *testing.T) {
 }
 
 func TestOutdatedNode(t *testing.T) {
-	node := NewNode("test", 0)
+	node := NewNode("test", "2375", 0)
 	client := mockclient.NewMockClient()
 	client.On("Info").Return(&dockerclient.Info{}, nil)
 
@@ -52,7 +52,7 @@ func TestOutdatedNode(t *testing.T) {
 }
 
 func TestNodeCpusMemory(t *testing.T) {
-	node := NewNode("test", 0)
+	node := NewNode("test", "2375", 0)
 	assert.False(t, node.IsConnected())
 
 	client := mockclient.NewMockClient()
@@ -72,7 +72,7 @@ func TestNodeCpusMemory(t *testing.T) {
 }
 
 func TestNodeSpecs(t *testing.T) {
-	node := NewNode("test", 0)
+	node := NewNode("test", "2375", 0)
 	assert.False(t, node.IsConnected())
 
 	client := mockclient.NewMockClient()
@@ -97,7 +97,7 @@ func TestNodeSpecs(t *testing.T) {
 }
 
 func TestNodeState(t *testing.T) {
-	node := NewNode("test", 0)
+	node := NewNode("test", "2375", 0)
 	assert.False(t, node.IsConnected())
 
 	client := mockclient.NewMockClient()
@@ -143,7 +143,7 @@ func TestCreateContainer(t *testing.T) {
 			Cmd:       []string{"date"},
 			Tty:       false,
 		}
-		node   = NewNode("test", 0)
+		node   = NewNode("test", "2375", 0)
 		client = mockclient.NewMockClient()
 	)
 
@@ -194,21 +194,22 @@ func TestCreateContainer(t *testing.T) {
 }
 
 func TestUsableMemory(t *testing.T) {
-	node := NewNode("test", 0.05)
+	node := NewNode("test", "2375", 0.05)
 	node.Memory = 1024
 	assert.Equal(t, node.UsableMemory(), 1024+1024*5/100)
 
-	node = NewNode("test", 0)
+	node = NewNode("test", "2375", 0)
 	node.Memory = 1024
 	assert.Equal(t, node.UsableMemory(), 1024)
 }
 
 func TestUsableCpus(t *testing.T) {
-	node := NewNode("test", 0.05)
+	node := NewNode("test", "2375", 0.05)
+
 	node.Cpus = 2
 	assert.Equal(t, node.UsableCpus(), 2+2*5/100)
 
-	node = NewNode("test", 0)
+	node = NewNode("test", "2375", 0)
 	node.Cpus = 2
 	assert.Equal(t, node.UsableCpus(), 2)
 }

discovery/consul/consul.go

@@ -65,7 +65,11 @@ func (s *ConsulDiscoveryService) Fetch() ([]*discovery.Node, error) {
 		if pair.Key == s.prefix {
 			continue
 		}
-		nodes = append(nodes, discovery.NewNode(string(pair.Value)))
+		node, err := discovery.NewNode(string(pair.Value))
+		if err != nil {
+			return nil, err
+		}
+		nodes = append(nodes, node)
 	}
 	return nodes, nil
 }

discovery/discovery.go

@@ -3,21 +3,27 @@ package discovery
 import (
 	"errors"
 	"fmt"
+	"net"
 	"strings"
 
 	log "github.com/Sirupsen/logrus"
 )
 
 type Node struct {
-	url string
+	Host string
+	Port string
 }
 
-func NewNode(url string) *Node {
+func NewNode(url string) (*Node, error) {
-	return &Node{url: url}
+	host, port, err := net.SplitHostPort(url)
+	if err != nil {
+		return nil, err
+	}
+	return &Node{host, port}, nil
 }
 
 func (n Node) String() string {
-	return n.url
+	return fmt.Sprintf("%s:%s", n.Host, n.Port)
 }
 
 type WatchCallback func(nodes []*Node)

discovery/etcd/etcd.go

@@ -60,7 +60,11 @@ func (s *EtcdDiscoveryService) Fetch() ([]*discovery.Node, error) {
 	var nodes []*discovery.Node
 
 	for _, n := range resp.Node.Nodes {
-		nodes = append(nodes, discovery.NewNode(n.Value))
+		node, err := discovery.NewNode(n.Value)
+		if err != nil {
+			return nil, err
+		}
+		nodes = append(nodes, node)
 	}
 	return nodes, nil
 }

discovery/file/file.go

@@ -33,7 +33,11 @@ func (s *FileDiscoveryService) Fetch() ([]*discovery.Node, error) {
 
 	for _, line := range strings.Split(string(data), "\n") {
 		if line != "" {
-			nodes = append(nodes, discovery.NewNode(line))
+			node, err := discovery.NewNode(line)
+			if err != nil {
+				return nil, err
+			}
+			nodes = append(nodes, node)
 		}
 	}
 	return nodes, nil

discovery/nodes/nodes.go

@@ -16,7 +16,11 @@ func init() {
 
 func (s *NodesDiscoveryService) Initialize(uris string, _ int) error {
 	for _, ip := range strings.Split(uris, ",") {
-		s.nodes = append(s.nodes, discovery.NewNode(ip))
+		node, err := discovery.NewNode(ip)
+		if err != nil {
+			return err
+		}
+		s.nodes = append(s.nodes, node)
 	}
 
 	return nil

discovery/token/token.go

@@ -56,7 +56,11 @@ func (s *TokenDiscoveryService) Fetch() ([]*discovery.Node, error) {
 
 	var nodes []*discovery.Node
 	for _, addr := range addrs {
-		nodes = append(nodes, discovery.NewNode(addr))
+		node, err := discovery.NewNode(addr)
+		if err != nil {
+			return nil, err
+		}
+		nodes = append(nodes, node)
 	}
 
 	return nodes, nil

discovery/zookeeper/zookeeper.go

@@ -61,19 +61,23 @@ func (s *ZkDiscoveryService) Fetch() ([]*discovery.Node, error) {
 		return nil, err
 	}
 
-	return s.createNodes(addrs), nil
+	return s.createNodes(addrs)
 }
 
-func (s *ZkDiscoveryService) createNodes(addrs []string) (nodes []*discovery.Node) {
+func (s *ZkDiscoveryService) createNodes(addrs []string) ([]*discovery.Node, error) {
-	nodes = make([]*discovery.Node, 0)
+	nodes := make([]*discovery.Node, 0)
 	if addrs == nil {
-		return
+		return nil, fmt.Errorf("no nodes to discover")
 	}
 
 	for _, addr := range addrs {
-		nodes = append(nodes, discovery.NewNode(addr))
+		node, err := discovery.NewNode(addr)
+		if err != nil {
+			return nil, err
+		}
+		nodes = append(nodes, node)
 	}
-	return
+	return nodes, nil
 }
 
 func (s *ZkDiscoveryService) Watch(callback discovery.WatchCallback) {
@@ -83,7 +87,10 @@ func (s *ZkDiscoveryService) Watch(callback discovery.WatchCallback) {
 		log.Debugf("[ZK] Watch aborted")
 		return
 	}
-	nodes := s.createNodes(addrs)
+	nodes, err := s.createNodes(addrs)
+	if err != nil {
+		return
+	}
 	callback(nodes)
 
 	for e := range eventChan {

discovery/zookeeper/zookeeper_test.go

@@ -3,7 +3,6 @@ package zookeeper
 import (
 	"testing"
 
-	"github.com/docker/swarm/discovery"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -21,8 +20,11 @@ func TestInitialize(t *testing.T) {
 
 func TestCreateNodes(t *testing.T) {
 	service := &ZkDiscoveryService{}
-	assert.Equal(t, service.createNodes(nil), []*discovery.Node{})
+	_, err := service.createNodes(nil)
-	nodes := service.createNodes([]string{"127.0.0.1", "127.0.0.2"})
+	assert.Error(t, err)
-	assert.Equal(t, nodes[0].String(), "127.0.0.1")
+
-	assert.Equal(t, nodes[1].String(), "127.0.0.2")
+	nodes, err := service.createNodes([]string{"127.0.0.1:2375", "127.0.0.2:2375"})
+	assert.NoError(t, err)
+	assert.Equal(t, nodes[0].String(), "127.0.0.1:2375")
+	assert.Equal(t, nodes[1].String(), "127.0.0.2:2375")
 }

scheduler/filter/affinity_test.go

@@ -12,9 +12,9 @@ func TestAffinityFilter(t *testing.T) {
 	var (
 		f     = AffinityFilter{}
 		nodes = []*cluster.Node{
-			cluster.NewNode("node-0", 0),
+			cluster.NewNode("node-0", "2375", 0),
-			cluster.NewNode("node-1", 0),
+			cluster.NewNode("node-1", "2375", 0),
-			cluster.NewNode("node-2", 0),
+			cluster.NewNode("node-2", "2375", 0),
 		}
 		result []*cluster.Node
 		err    error

scheduler/filter/constraint_test.go

@@ -10,9 +10,9 @@ import (
 
 func testFixtures() (nodes []*cluster.Node) {
 	nodes = []*cluster.Node{
-		cluster.NewNode("node-0", 0),
+		cluster.NewNode("node-0", "2375", 0),
-		cluster.NewNode("node-1", 0),
+		cluster.NewNode("node-1", "2375", 0),
-		cluster.NewNode("node-2", 0),
+		cluster.NewNode("node-2", "2375", 0),
 	}
 	nodes[0].ID = "node-0-id"
 	nodes[0].Name = "node-0-name"
@@ -205,7 +205,7 @@ func TestFilterRegExpCaseInsensitive(t *testing.T) {
 	)
 
 	// Prepare node with a strange name
-	node3 := cluster.NewNode("node-3", 0)
+	node3 := cluster.NewNode("node-3", "2375", 0)
 	node3.ID = "node-3-id"
 	node3.Name = "node-3-name"
 	node3.Labels = map[string]string{
@@ -249,7 +249,7 @@ func TestFilterWithRelativeComparisons(t *testing.T) {
 	)
 
 	// Prepare node with a strange name
-	node3 := cluster.NewNode("node-3", 0)
+	node3 := cluster.NewNode("node-3", "2375", 0)
 	node3.ID = "node-3-id"
 	node3.Name = "node-3-name"
 	node3.Labels = map[string]string{

scheduler/filter/port_test.go

@@ -24,9 +24,9 @@ func TestPortFilterNoConflicts(t *testing.T) {
 	var (
 		p     = PortFilter{}
 		nodes = []*cluster.Node{
-			cluster.NewNode("node-1", 0),
+			cluster.NewNode("node-1", "2375", 0),
-			cluster.NewNode("node-2", 0),
+			cluster.NewNode("node-2", "2375", 0),
-			cluster.NewNode("node-3", 0),
+			cluster.NewNode("node-3", "2375", 0),
 		}
 		result []*cluster.Node
 		err    error
@@ -70,9 +70,9 @@ func TestPortFilterSimple(t *testing.T) {
 	var (
 		p     = PortFilter{}
 		nodes = []*cluster.Node{
-			cluster.NewNode("node-1", 0),
+			cluster.NewNode("node-1", "2375", 0),
-			cluster.NewNode("node-2", 0),
+			cluster.NewNode("node-2", "2375", 0),
-			cluster.NewNode("node-3", 0),
+			cluster.NewNode("node-3", "2375", 0),
 		}
 		result []*cluster.Node
 		err    error
@@ -99,9 +99,9 @@ func TestPortFilterDifferentInterfaces(t *testing.T) {
 	var (
 		p     = PortFilter{}
 		nodes = []*cluster.Node{
-			cluster.NewNode("node-1", 0),
+			cluster.NewNode("node-1", "2375", 0),
-			cluster.NewNode("node-2", 0),
+			cluster.NewNode("node-2", "2375", 0),
-			cluster.NewNode("node-3", 0),
+			cluster.NewNode("node-3", "2375", 0),
 		}
 		result []*cluster.Node
 		err    error

scheduler/strategy/binpacking_test.go

@@ -10,7 +10,7 @@ import (
 )
 
 func createNode(ID string, memory int64, cpus int64) *cluster.Node {
-	node := cluster.NewNode(ID, 0.05)
+	node := cluster.NewNode(ID, "2375", 0.05)
 	node.ID = ID
 	node.Memory = memory * 1024 * 1024 * 1024
 	node.Cpus = cpus