From a916f9cde04c17fc1d9a3d26bdf4d949991c2e84 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 23 Jan 2015 11:12:02 -0500 Subject: [PATCH] Fixes panic when ports are not specified Signed-off-by: Brian Goff --- cluster/cluster.go | 2 +- cluster/cluster_test.go | 14 +++++++------- cluster/node.go | 11 ++++++----- cluster/node_test.go | 21 +++++++++++---------- discovery/consul/consul.go | 6 +++++- discovery/discovery.go | 14 ++++++++++---- discovery/etcd/etcd.go | 6 +++++- discovery/file/file.go | 6 +++++- discovery/nodes/nodes.go | 6 +++++- discovery/token/token.go | 6 +++++- discovery/zookeeper/zookeeper.go | 21 ++++++++++++++------- discovery/zookeeper/zookeeper_test.go | 12 +++++++----- scheduler/filter/affinity_test.go | 6 +++--- scheduler/filter/constraint_test.go | 10 +++++----- scheduler/filter/port_test.go | 18 +++++++++--------- scheduler/strategy/binpacking_test.go | 2 +- 16 files changed, 99 insertions(+), 62 deletions(-) diff --git a/cluster/cluster.go b/cluster/cluster.go index daa06ed838..79388819dd 100644 --- a/cluster/cluster.go +++ b/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 diff --git a/cluster/cluster_test.go b/cluster/cluster_test.go index 0bbbb59ced..19513da448 100644 --- a/cluster/cluster_test.go +++ b/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 { - node := NewNode(ID, 0) +func createNode(t *testing.T, ID string, Port string, containers ...dockerclient.Container) *Node { + 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) diff --git a/cluster/node.go b/cluster/node.go index ab77593f6b..0703cf95c5 100644 --- a/cluster/node.go +++ b/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", parts[0]) + addr, err := net.ResolveIPAddr("ip4", n.Addr) 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 } diff --git a/cluster/node_test.go b/cluster/node_test.go index 5a38bd1f3b..4ae358a196 100644 --- a/cluster/node_test.go +++ b/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) } diff --git a/discovery/consul/consul.go b/discovery/consul/consul.go index 7d45147123..5eb595b871 100644 --- a/discovery/consul/consul.go +++ b/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 } diff --git a/discovery/discovery.go b/discovery/discovery.go index 8c02cf2f6e..1642f8412f 100644 --- a/discovery/discovery.go +++ b/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 { - return &Node{url: url} +func NewNode(url string) (*Node, error) { + 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) diff --git a/discovery/etcd/etcd.go b/discovery/etcd/etcd.go index 91641498d1..744162473a 100644 --- a/discovery/etcd/etcd.go +++ b/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 } diff --git a/discovery/file/file.go b/discovery/file/file.go index 47ed538613..2594ca7b3f 100644 --- a/discovery/file/file.go +++ b/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 diff --git a/discovery/nodes/nodes.go b/discovery/nodes/nodes.go index 9428a1aada..8c32ff8330 100644 --- a/discovery/nodes/nodes.go +++ b/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 diff --git a/discovery/token/token.go b/discovery/token/token.go index 633e8d1d52..4ccea845cb 100644 --- a/discovery/token/token.go +++ b/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 diff --git a/discovery/zookeeper/zookeeper.go b/discovery/zookeeper/zookeeper.go index 8c990520af..f860d4fa5b 100644 --- a/discovery/zookeeper/zookeeper.go +++ b/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) { - nodes = make([]*discovery.Node, 0) +func (s *ZkDiscoveryService) createNodes(addrs []string) ([]*discovery.Node, error) { + 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 { diff --git a/discovery/zookeeper/zookeeper_test.go b/discovery/zookeeper/zookeeper_test.go index c07a879aaa..55fee546f6 100644 --- a/discovery/zookeeper/zookeeper_test.go +++ b/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{}) - nodes := service.createNodes([]string{"127.0.0.1", "127.0.0.2"}) - assert.Equal(t, nodes[0].String(), "127.0.0.1") - assert.Equal(t, nodes[1].String(), "127.0.0.2") + _, err := service.createNodes(nil) + assert.Error(t, err) + + 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") } diff --git a/scheduler/filter/affinity_test.go b/scheduler/filter/affinity_test.go index 64f54ce7b8..41597c8d7b 100644 --- a/scheduler/filter/affinity_test.go +++ b/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-1", 0), - cluster.NewNode("node-2", 0), + cluster.NewNode("node-0", "2375", 0), + cluster.NewNode("node-1", "2375", 0), + cluster.NewNode("node-2", "2375", 0), } result []*cluster.Node err error diff --git a/scheduler/filter/constraint_test.go b/scheduler/filter/constraint_test.go index 016aeeaada..76c82bc996 100644 --- a/scheduler/filter/constraint_test.go +++ b/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-1", 0), - cluster.NewNode("node-2", 0), + cluster.NewNode("node-0", "2375", 0), + cluster.NewNode("node-1", "2375", 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{ diff --git a/scheduler/filter/port_test.go b/scheduler/filter/port_test.go index 48a5ec5e2b..1221ce6f94 100644 --- a/scheduler/filter/port_test.go +++ b/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-2", 0), - cluster.NewNode("node-3", 0), + cluster.NewNode("node-1", "2375", 0), + cluster.NewNode("node-2", "2375", 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-2", 0), - cluster.NewNode("node-3", 0), + cluster.NewNode("node-1", "2375", 0), + cluster.NewNode("node-2", "2375", 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-2", 0), - cluster.NewNode("node-3", 0), + cluster.NewNode("node-1", "2375", 0), + cluster.NewNode("node-2", "2375", 0), + cluster.NewNode("node-3", "2375", 0), } result []*cluster.Node err error diff --git a/scheduler/strategy/binpacking_test.go b/scheduler/strategy/binpacking_test.go index 6a46e443f4..4b5034b8bd 100644 --- a/scheduler/strategy/binpacking_test.go +++ b/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