From 67e347fa1ace871927a19298b22665882e470364 Mon Sep 17 00:00:00 2001
From: Andrea Luzzardi <aluzzardi@gmail.com>
Date: Tue, 10 Feb 2015 10:36:31 -0800
Subject: [PATCH] Move container name matching logic into Node.

- Add `Node.Container()`
- Make `Cluster.Container()` use `Node.Container()`
- Added missing locks and converted existing ones to RLock.

Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
---
 cluster/cluster.go   | 21 ++++++++-------------
 cluster/node.go      | 31 +++++++++++++++++++++++++++++++
 cluster/node_test.go | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 13 deletions(-)

diff --git a/cluster/cluster.go b/cluster/cluster.go
index 39a38a86b3..f9c51c6aa6 100644
--- a/cluster/cluster.go
+++ b/cluster/cluster.go
@@ -3,7 +3,6 @@ package cluster
 import (
 	"crypto/tls"
 	"errors"
-	"strings"
 	"sync"
 
 	log "github.com/Sirupsen/logrus"
@@ -119,8 +118,8 @@ func (c *Cluster) UpdateNodes(entries []*discovery.Entry) {
 
 // Containers returns all the containers in the cluster.
 func (c *Cluster) Containers() []*Container {
-	c.Lock()
-	defer c.Unlock()
+	c.RLock()
+	defer c.RUnlock()
 
 	out := []*Container{}
 	for _, n := range c.nodes {
@@ -139,17 +138,13 @@ func (c *Cluster) Container(IdOrName string) *Container {
 	if len(IdOrName) == 0 {
 		return nil
 	}
-	for _, container := range c.Containers() {
-		// Match ID prefix.
-		if strings.HasPrefix(container.Id, IdOrName) {
-			return container
-		}
 
-		// Match name, /name or engine/name.
-		for _, name := range container.Names {
-			if name == IdOrName || name == "/"+IdOrName || container.Node.ID+name == IdOrName || container.Node.Name+name == IdOrName {
-				return container
-			}
+	c.RLock()
+	defer c.RUnlock()
+
+	for _, n := range c.nodes {
+		if container := n.Container(IdOrName); container != nil {
+			return container
 		}
 	}
 
diff --git a/cluster/node.go b/cluster/node.go
index eece6bffd1..590c45fee8 100644
--- a/cluster/node.go
+++ b/cluster/node.go
@@ -394,6 +394,7 @@ func (n *Node) Events(h EventHandler) error {
 	return nil
 }
 
+// Containers returns all the containers in the node.
 func (n *Node) Containers() []*Container {
 	containers := []*Container{}
 	n.RLock()
@@ -404,6 +405,33 @@ func (n *Node) Containers() []*Container {
 	return containers
 }
 
+// Container returns the container with IdOrName in the node.
+func (n *Node) Container(IdOrName string) *Container {
+	// Abort immediately if the name is empty.
+	if len(IdOrName) == 0 {
+		return nil
+	}
+
+	n.RLock()
+	defer n.RUnlock()
+
+	for _, container := range n.Containers() {
+		// Match ID prefix.
+		if strings.HasPrefix(container.Id, IdOrName) {
+			return container
+		}
+
+		// Match name, /name or engine/name.
+		for _, name := range container.Names {
+			if name == IdOrName || name == "/"+IdOrName || container.Node.ID+name == IdOrName || container.Node.Name+name == IdOrName {
+				return container
+			}
+		}
+	}
+
+	return nil
+}
+
 func (n *Node) Images() []*dockerclient.Image {
 	images := []*dockerclient.Image{}
 	n.RLock()
@@ -416,6 +444,9 @@ func (n *Node) Images() []*dockerclient.Image {
 
 // Image returns the image with IdOrName in the node
 func (n *Node) Image(IdOrName string) *dockerclient.Image {
+	n.RLock()
+	defer n.RUnlock()
+
 	size := len(IdOrName)
 	for _, image := range n.Images() {
 		if image.Id == IdOrName || (size > 2 && strings.HasPrefix(image.Id, IdOrName)) {
diff --git a/cluster/node_test.go b/cluster/node_test.go
index 60d953f1f1..7a53399f8e 100644
--- a/cluster/node_test.go
+++ b/cluster/node_test.go
@@ -135,6 +135,38 @@ func TestNodeState(t *testing.T) {
 	client.Mock.AssertExpectations(t)
 }
 
+func TestNodeContainerLookup(t *testing.T) {
+	node := NewNode("test-node", 0)
+	assert.False(t, node.IsConnected())
+
+	client := mockclient.NewMockClient()
+	client.On("Info").Return(mockInfo, nil)
+	client.On("StartMonitorEvents", mock.Anything, mock.Anything, mock.Anything).Return()
+
+	client.On("ListContainers", true, false, "").Return([]dockerclient.Container{{Id: "container-id", Names: []string{"/container-name1", "/container-name2"}}}, nil).Once()
+	client.On("ListImages").Return([]*dockerclient.Image{}, nil).Once()
+	client.On("InspectContainer", "container-id").Return(&dockerclient.ContainerInfo{Config: &dockerclient.ContainerConfig{CpuShares: 100}}, nil).Once()
+
+	assert.NoError(t, node.connectClient(client))
+	assert.True(t, node.IsConnected())
+
+	// Invalid lookup
+	assert.Nil(t, node.Container("invalid-id"))
+	assert.Nil(t, node.Container(""))
+	// Container ID lookup.
+	assert.NotNil(t, node.Container("container-id"))
+	// Container ID prefix lookup.
+	assert.NotNil(t, node.Container("container-"))
+	// Container name lookup.
+	assert.NotNil(t, node.Container("container-name1"))
+	assert.NotNil(t, node.Container("container-name2"))
+	// Container node/name matching.
+	assert.NotNil(t, node.Container("id/container-name1"))
+	assert.NotNil(t, node.Container("id/container-name2"))
+
+	client.Mock.AssertExpectations(t)
+}
+
 func TestCreateContainer(t *testing.T) {
 	var (
 		config = &dockerclient.ContainerConfig{