Simplify how the Docker containers are handled.

This is done by introducting the concept of "namespaces" of container
names. The aliases of a container are under this namespace. Namespace
names are of the form:

//<namespace>/<alias>

This allows us to (within cAdvisor) query all docker containers as
//docker regardless of whether this is a systemd or a non-systemd system.

This does break our ability to handle Docker aliases with the /container
endpoint. I think this is acceptable as our support there was not
consistent between system types.

container/docker/factory.go

@@ -31,6 +31,9 @@ import (
 
 var ArgDockerEndpoint = flag.String("docker", "unix:///var/run/docker.sock", "docker endpoint")
 
+// The namespace under which Docker aliases are unique.
+var DockerNamespace = "docker"
+
 // Basepath to all container specific information that libcontainer stores.
 var dockerRootDir = flag.String("docker_root", "/var/lib/docker", "Absolute path to the Docker state root directory (default: /var/lib/docker)")
 
@@ -54,7 +57,7 @@ type dockerFactory struct {
 }
 
 func (self *dockerFactory) String() string {
-	return "docker"
+	return DockerNamespace
 }
 
 func (self *dockerFactory) NewContainerHandler(name string) (handler container.ContainerHandler, err error) {
@@ -102,19 +105,14 @@ func ContainerNameToDockerId(name string) string {
 	return id
 }
 
-// Returns a list of possible full container names for the specified Docker container name.
+// Returns a full container name for the specified Docker ID.
-func FullDockerContainerNames(dockerName string) []string {
+func FullContainerName(dockerId string) string {
-	names := make([]string, 0, 2)
-
 	// Add the full container name.
 	if useSystemd {
-		names = append(names, path.Join("/system.slice", fmt.Sprintf("docker-%s.scope", dockerName)))
+		return path.Join("/system.slice", fmt.Sprintf("docker-%s.scope", dockerId))
 	} else {
-		names = append(names, path.Join("/docker", dockerName))
+		return path.Join("/docker", dockerId)
 	}
-
-	// Add the Docker alias name.
-	return append(names, path.Join("/docker", dockerName))
 }
 
 // Docker handles all containers under /docker

container/docker/handler.go

@@ -92,14 +92,19 @@ func newDockerContainerHandler(
 	if err != nil {
 		return nil, fmt.Errorf("failed to inspect container %s - %s\n", id, err)
 	}
-	handler.aliases = append(handler.aliases, path.Join("/docker", ctnr.Name))
+
+	// Add the name and bare ID as aliases of the container.
+	handler.aliases = append(handler.aliases, strings.TrimPrefix(ctnr.Name, "/"))
+	handler.aliases = append(handler.aliases, id)
+
 	return handler, nil
 }
 
 func (self *dockerContainerHandler) ContainerReference() (info.ContainerReference, error) {
 	return info.ContainerReference{
-		Name:    self.name,
+		Name:      self.name,
-		Aliases: self.aliases,
+		Aliases:   self.aliases,
+		Namespace: DockerNamespace,
 	}, nil
 }
 
@@ -297,12 +302,6 @@ func (self *dockerContainerHandler) ListContainers(listType container.ListType)
 		return nil, err
 	}
 
-	// On non-systemd systems Docker containers are under /docker.
-	containerPrefix := "/docker"
-	if useSystemd {
-		containerPrefix = "/system.slice"
-	}
-
 	ret := make([]info.ContainerReference, 0, len(containers)+1)
 	for _, c := range containers {
 		if !strings.HasPrefix(c.Status, "Up ") {
@@ -310,8 +309,9 @@ func (self *dockerContainerHandler) ListContainers(listType container.ListType)
 		}
 
 		ref := info.ContainerReference{
-			Name:    path.Join(containerPrefix, c.ID),
+			Name:      FullContainerName(c.ID),
-			Aliases: c.Names,
+			Aliases:   append(c.Names, c.ID),
+			Namespace: DockerNamespace,
 		}
 		ret = append(ret, ref)
 	}

info/container.go

@@ -53,10 +53,16 @@ type ContainerSpec struct {
 
 // Container reference contains enough information to uniquely identify a container
 type ContainerReference struct {
-	// The absolute name of the container.
+	// The absolute name of the container. This is unique on the machine.
 	Name string `json:"name"`
 
+	// Other names by which the container is known within a certain namespace.
+	// This is unique within that namespace.
 	Aliases []string `json:"aliases,omitempty"`
+
+	// Namespace under which the aliases of a container are unique.
+	// An example of a namespace is "docker" for Docker containers.
+	Namespace string `json:"namespace,omitempty"`
 }
 
 // ContainerInfoQuery is used when users check a container info from the REST api.
@@ -180,14 +186,14 @@ type PerDiskStats struct {
 }
 
 type DiskIoStats struct {
-	IoServiceBytes 	[]PerDiskStats `json:"io_service_bytes,omitempty"`
+	IoServiceBytes []PerDiskStats `json:"io_service_bytes,omitempty"`
-	IoServiced     	[]PerDiskStats `json:"io_serviced,omitempty"`
+	IoServiced     []PerDiskStats `json:"io_serviced,omitempty"`
-	IoQueued       	[]PerDiskStats `json:"io_queued,omitempty"`
+	IoQueued       []PerDiskStats `json:"io_queued,omitempty"`
-	Sectors        	[]PerDiskStats `json:"sectors,omitempty"`
+	Sectors        []PerDiskStats `json:"sectors,omitempty"`
-	IoServiceTime	[]PerDiskStats `json:"io_service_time,omitempty"`
+	IoServiceTime  []PerDiskStats `json:"io_service_time,omitempty"`
-	IoWaitTime 		[]PerDiskStats `json:"io_wait_time,omitempty"`
+	IoWaitTime     []PerDiskStats `json:"io_wait_time,omitempty"`
-	IoMerged []PerDiskStats `json:"io_merged,omitempty"`
+	IoMerged       []PerDiskStats `json:"io_merged,omitempty"`
-	IoTime []PerDiskStats `json:"io_time,omitempty"`
+	IoTime         []PerDiskStats `json:"io_time,omitempty"`
 }
 
 type MemoryStats struct {

manager/container.go

@@ -104,8 +104,7 @@ func newContainerData(containerName string, driver storage.StorageDriver, handle
 		logUsage:             logUsage,
 		stop:                 make(chan bool, 1),
 	}
-	cont.info.Name = ref.Name
+	cont.info.ContainerReference = ref
-	cont.info.Aliases = ref.Aliases
 
 	return cont, nil
 }

manager/manager.go

@@ -75,7 +75,7 @@ func New(driver storage.StorageDriver) (Manager, error) {
 	glog.Infof("cAdvisor running in container: %q", selfContainer)
 
 	newManager := &manager{
-		containers:        make(map[string]*containerData),
+		containers:        make(map[namespacedContainerName]*containerData),
 		quitChannels:      make([]chan error, 0, 2),
 		storageDriver:     driver,
 		cadvisorContainer: selfContainer,
@@ -99,8 +99,17 @@ func New(driver storage.StorageDriver) (Manager, error) {
 	return newManager, nil
 }
 
+// A namespaced container name.
+type namespacedContainerName struct {
+	// The namespace of the container. Can be empty for the root namespace.
+	Namespace string
+
+	// The name of the container in this namespace.
+	Name string
+}
+
 type manager struct {
-	containers             map[string]*containerData
+	containers             map[namespacedContainerName]*containerData
 	containersLock         sync.RWMutex
 	storageDriver          storage.StorageDriver
 	machineInfo            info.MachineInfo
@@ -198,7 +207,9 @@ func (self *manager) GetContainerInfo(containerName string, query *info.Containe
 		defer self.containersLock.RUnlock()
 
 		// Ensure we have the container.
-		cont, ok = self.containers[containerName]
+		cont, ok = self.containers[namespacedContainerName{
+			Name: containerName,
+		}]
 	}()
 	if !ok {
 		return nil, fmt.Errorf("unknown container %q", containerName)
@@ -221,13 +232,10 @@ func (self *manager) containerDataToContainerInfo(cont *containerData, query *in
 
 	// Make a copy of the info for the user.
 	ret := &info.ContainerInfo{
-		ContainerReference: info.ContainerReference{
+		ContainerReference: cinfo.ContainerReference,
-			Name:    cinfo.Name,
+		Subcontainers:      cinfo.Subcontainers,
-			Aliases: cinfo.Aliases,
+		Spec:               cinfo.Spec,
-		},
+		Stats:              stats,
-		Subcontainers: cinfo.Subcontainers,
-		Spec:          cinfo.Spec,
-		Stats:         stats,
 	}
 
 	// Set default value to an actual value
@@ -275,20 +283,15 @@ func (self *manager) DockerContainersInfo(containerName string, query *info.Cont
 				}
 			}
 		} else {
-			// Strip first "/"
+			// Check for the container in the Docker container namespace.
-			containerName = strings.Trim(containerName, "/")
+			cont, ok := self.containers[namespacedContainerName{
-
+				Namespace: docker.DockerNamespace,
-			// Get the specified container (check all possible Docker names).
+				Name:      containerName,
-			possibleNames := docker.FullDockerContainerNames(containerName)
+			}]
-			for _, fullName := range possibleNames {
+			if ok {
-				cont, ok := self.containers[fullName]
+				containers = append(containers, cont)
-				if ok {
+			} else {
-					containers = append(containers, cont)
+				return fmt.Errorf("unable to find Docker container %q", containerName)
-					break
-				}
-			}
-			if len(containers) == 0 {
-				return fmt.Errorf("unable to find Docker container %q with full names %v", containerName, possibleNames)
 			}
 		}
 		return nil
@@ -345,16 +348,23 @@ func (m *manager) createContainer(containerName string) error {
 		m.containersLock.Lock()
 		defer m.containersLock.Unlock()
 
+		namespacedName := namespacedContainerName{
+			Name: containerName,
+		}
+
 		// Check that the container didn't already exist.
-		_, ok := m.containers[containerName]
+		_, ok := m.containers[namespacedName]
 		if ok {
 			return true
 		}
 
-		// Add the container name and all its aliases.
+		// Add the container name and all its aliases. The aliases must be within the namespace of the factory.
-		m.containers[containerName] = cont
+		m.containers[namespacedName] = cont
 		for _, alias := range cont.info.Aliases {
-			m.containers[alias] = cont
+			m.containers[namespacedContainerName{
+				Namespace: cont.info.Namespace,
+				Name:      alias,
+			}] = cont
 		}
 
 		return false
@@ -362,7 +372,7 @@ func (m *manager) createContainer(containerName string) error {
 	if alreadyExists {
 		return nil
 	}
-	glog.Infof("Added container: %q (aliases: %s)", containerName, cont.info.Aliases)
+	glog.Infof("Added container: %q (aliases: %v, namespace: %q)", containerName, cont.info.Aliases, cont.info.Namespace)
 
 	// Start the container's housekeeping.
 	cont.Start()
@@ -373,7 +383,10 @@ func (m *manager) destroyContainer(containerName string) error {
 	m.containersLock.Lock()
 	defer m.containersLock.Unlock()
 
-	cont, ok := m.containers[containerName]
+	namespacedName := namespacedContainerName{
+		Name: containerName,
+	}
+	cont, ok := m.containers[namespacedName]
 	if !ok {
 		// Already destroyed, done.
 		return nil
@@ -386,11 +399,14 @@ func (m *manager) destroyContainer(containerName string) error {
 	}
 
 	// Remove the container from our records (and all its aliases).
-	delete(m.containers, containerName)
+	delete(m.containers, namespacedName)
 	for _, alias := range cont.info.Aliases {
-		delete(m.containers, alias)
+		delete(m.containers, namespacedContainerName{
+			Namespace: cont.info.Namespace,
+			Name:      alias,
+		})
 	}
-	glog.Infof("Destroyed container: %s (aliases: %s)", containerName, cont.info.Aliases)
+	glog.Infof("Destroyed container: %q (aliases: %v, namespace: %q)", containerName, cont.info.Aliases, cont.info.Namespace)
 	return nil
 }
 
@@ -400,7 +416,9 @@ func (m *manager) getContainersDiff(containerName string) (added []info.Containe
 	defer m.containersLock.RUnlock()
 
 	// Get all subcontainers recursively.
-	cont, ok := m.containers[containerName]
+	cont, ok := m.containers[namespacedContainerName{
+		Name: containerName,
+	}]
 	if !ok {
 		return nil, nil, fmt.Errorf("failed to find container %q while checking for new containers", containerName)
 	}
@@ -414,15 +432,17 @@ func (m *manager) getContainersDiff(containerName string) (added []info.Containe
 	allContainersSet := make(map[string]*containerData)
 	for name, d := range m.containers {
 		// Only add the canonical name.
-		if d.info.Name == name {
+		if d.info.Name == name.Name {
-			allContainersSet[name] = d
+			allContainersSet[name.Name] = d
 		}
 	}
 
 	// Added containers
 	for _, c := range allContainers {
 		delete(allContainersSet, c.Name)
-		_, ok := m.containers[c.Name]
+		_, ok := m.containers[namespacedContainerName{
+			Name: c.Name,
+		}]
 		if !ok {
 			added = append(added, c)
 		}
@@ -474,7 +494,9 @@ func (self *manager) watchForNewContainers(quit chan error) error {
 	func() {
 		self.containersLock.RLock()
 		defer self.containersLock.RUnlock()
-		root, ok = self.containers["/"]
+		root, ok = self.containers[namespacedContainerName{
+			Name: "/",
+		}]
 	}()
 	if !ok {
 		return fmt.Errorf("Root container does not exist when watching for new containers")

manager/manager_test.go

@@ -18,15 +18,19 @@ package manager
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/google/cadvisor/container"
+	"github.com/google/cadvisor/container/docker"
 	"github.com/google/cadvisor/info"
 	itest "github.com/google/cadvisor/info/test"
 	stest "github.com/google/cadvisor/storage/test"
 )
 
+// TODO(vmarmol): Refactor these tests.
+
 func createManagerAndAddContainers(
 	driver *stest.MockStorageDriver,
 	containers []string,
@@ -44,10 +48,20 @@ func createManagerAndAddContainers(
 	if ret, ok := mif.(*manager); ok {
 		for _, name := range containers {
 			mockHandler := container.NewMockContainerHandler(name)
-			ret.containers[name], err = newContainerData(name, driver, mockHandler, false)
+			cont, err := newContainerData(name, driver, mockHandler, false)
 			if err != nil {
 				t.Fatal(err)
 			}
+			ret.containers[namespacedContainerName{
+				Name: name,
+			}] = cont
+			// Add Docker containers under their namespace.
+			if strings.HasPrefix(name, "/docker") {
+				ret.containers[namespacedContainerName{
+					Namespace: docker.DockerNamespace,
+					Name:      strings.TrimPrefix(name, "/docker/"),
+				}] = cont
+			}
 			f(mockHandler)
 		}
 		return ret

pages/docker.go

@@ -19,7 +19,7 @@ func serveDockerPage(m manager.Manager, w http.ResponseWriter, u *url.URL) error
 	start := time.Now()
 
 	// The container name is the path after the handler
-	containerName := u.Path[len(DockerPage)-1:]
+	containerName := u.Path[len(DockerPage):]
 
 	var data *pageData
 	if containerName == "/" {