From f97e57df889ef35bc08bc2796625171c41d6ad3f Mon Sep 17 00:00:00 2001 From: Victor Marmol Date: Fri, 7 Nov 2014 10:00:29 -0800 Subject: [PATCH] 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: /// 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 | 18 +++---- container/docker/handler.go | 22 ++++----- info/container.go | 24 +++++---- manager/container.go | 3 +- manager/manager.go | 98 +++++++++++++++++++++++-------------- manager/manager_test.go | 16 +++++- pages/docker.go | 2 +- 7 files changed, 111 insertions(+), 72 deletions(-) diff --git a/container/docker/factory.go b/container/docker/factory.go index e46ad850..0811065a 100644 --- a/container/docker/factory.go +++ b/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. -func FullDockerContainerNames(dockerName string) []string { - names := make([]string, 0, 2) - +// Returns a full container name for the specified Docker ID. +func FullContainerName(dockerId string) string { // 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 diff --git a/container/docker/handler.go b/container/docker/handler.go index e3d8ee4e..dc864ffe 100644 --- a/container/docker/handler.go +++ b/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, - Aliases: self.aliases, + Name: self.name, + 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), - Aliases: c.Names, + Name: FullContainerName(c.ID), + Aliases: append(c.Names, c.ID), + Namespace: DockerNamespace, } ret = append(ret, ref) } diff --git a/info/container.go b/info/container.go index 115c74e9..7e0f599e 100644 --- a/info/container.go +++ b/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"` - IoServiced []PerDiskStats `json:"io_serviced,omitempty"` - IoQueued []PerDiskStats `json:"io_queued,omitempty"` - Sectors []PerDiskStats `json:"sectors,omitempty"` - IoServiceTime []PerDiskStats `json:"io_service_time,omitempty"` - IoWaitTime []PerDiskStats `json:"io_wait_time,omitempty"` - IoMerged []PerDiskStats `json:"io_merged,omitempty"` - IoTime []PerDiskStats `json:"io_time,omitempty"` + IoServiceBytes []PerDiskStats `json:"io_service_bytes,omitempty"` + IoServiced []PerDiskStats `json:"io_serviced,omitempty"` + IoQueued []PerDiskStats `json:"io_queued,omitempty"` + Sectors []PerDiskStats `json:"sectors,omitempty"` + IoServiceTime []PerDiskStats `json:"io_service_time,omitempty"` + IoWaitTime []PerDiskStats `json:"io_wait_time,omitempty"` + IoMerged []PerDiskStats `json:"io_merged,omitempty"` + IoTime []PerDiskStats `json:"io_time,omitempty"` } type MemoryStats struct { diff --git a/manager/container.go b/manager/container.go index 9e911b79..e1616fee 100644 --- a/manager/container.go +++ b/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.Aliases = ref.Aliases + cont.info.ContainerReference = ref return cont, nil } diff --git a/manager/manager.go b/manager/manager.go index f6ceaed5..3b02d3bc 100644 --- a/manager/manager.go +++ b/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{ - Name: cinfo.Name, - Aliases: cinfo.Aliases, - }, - Subcontainers: cinfo.Subcontainers, - Spec: cinfo.Spec, - Stats: stats, + ContainerReference: cinfo.ContainerReference, + 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 "/" - containerName = strings.Trim(containerName, "/") - - // Get the specified container (check all possible Docker names). - possibleNames := docker.FullDockerContainerNames(containerName) - for _, fullName := range possibleNames { - cont, ok := self.containers[fullName] - if ok { - containers = append(containers, cont) - break - } - } - if len(containers) == 0 { - return fmt.Errorf("unable to find Docker container %q with full names %v", containerName, possibleNames) + // Check for the container in the Docker container namespace. + cont, ok := self.containers[namespacedContainerName{ + Namespace: docker.DockerNamespace, + Name: containerName, + }] + if ok { + containers = append(containers, cont) + } else { + return fmt.Errorf("unable to find Docker container %q", containerName) } } 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. - m.containers[containerName] = cont + // Add the container name and all its aliases. The aliases must be within the namespace of the factory. + 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 { - allContainersSet[name] = d + if d.info.Name == name.Name { + 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") diff --git a/manager/manager_test.go b/manager/manager_test.go index cff0a4c6..8560eec6 100644 --- a/manager/manager_test.go +++ b/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 diff --git a/pages/docker.go b/pages/docker.go index 116adc3b..6810d18f 100644 --- a/pages/docker.go +++ b/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 == "/" {