From 1edb798de5ee35745a97592e97ccd2d3a7b89f17 Mon Sep 17 00:00:00 2001 From: Vishnu Kannan Date: Sat, 19 Jul 2014 00:34:55 +0000 Subject: [PATCH 1/8] Features: Added Network Stats to Container Info. It still not exposed via the HTTP UI. Bug fixes: 1. Modified docker handler to use libcontainer.GetStats instead of quering the fs package. 2. cAdvisor will not stall if any of its core operations fail. 3. cAdvisor will safely ignore any inactive or leaked docker containers. When containers are leaked cgroup state exists but docker is not aware of them. --- cadvisor.go | 8 +++- container/container.go | 8 +++- container/docker/handler.go | 72 +++++++++++++++++++++++-------- container/filter_test.go | 12 +++--- container/libcontainer/helpers.go | 23 +++------- container/raw/handler.go | 2 +- info/container.go | 26 +++++++++-- manager/container.go | 2 +- manager/manager.go | 28 +++++++----- 9 files changed, 123 insertions(+), 58 deletions(-) diff --git a/cadvisor.go b/cadvisor.go index 31b18f71..4d60300d 100644 --- a/cadvisor.go +++ b/cadvisor.go @@ -83,11 +83,15 @@ func main() { } }) - go containerManager.Start() + errChan := make(chan error) + go containerManager.Start(errChan) log.Printf("Starting cAdvisor version: %q", info.VERSION) log.Print("About to serve on port ", *argPort) addr := fmt.Sprintf(":%v", *argPort) - log.Fatal(http.ListenAndServe(addr, nil)) + go func() { + errChan <- http.ListenAndServe(addr, nil) + }() + log.Fatal(<-errChan) } diff --git a/container/container.go b/container/container.go index de478fd0..efe92397 100644 --- a/container/container.go +++ b/container/container.go @@ -14,7 +14,11 @@ package container -import "github.com/google/cadvisor/info" +import ( + "errors" + + "github.com/google/cadvisor/info" +) // Listing types. const ( @@ -22,6 +26,8 @@ const ( LIST_RECURSIVE ) +var NotActive = errors.New("Container is not active") + type ListType int // Interface for container operation handlers. diff --git a/container/docker/handler.go b/container/docker/handler.go index b689e3f8..77eb035f 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -26,13 +26,16 @@ import ( "time" "github.com/docker/libcontainer" - "github.com/docker/libcontainer/cgroups" "github.com/fsouza/go-dockerclient" "github.com/google/cadvisor/container" containerLibcontainer "github.com/google/cadvisor/container/libcontainer" "github.com/google/cadvisor/info" + "github.com/google/cadvisor/utils" ) +// Basepath to all container specific information that libcontainer stores. +const dockerRootDir = "/var/lib/docker/execdriver/native" + type dockerContainerHandler struct { client *docker.Client name string @@ -61,8 +64,9 @@ func newDockerContainerHandler( return nil, fmt.Errorf("invalid docker container %v: %v", name, err) } ctnr, err := client.InspectContainer(id) - if err != nil { - return nil, fmt.Errorf("unable to inspect container %v: %v", name, err) + // We assume that if Inspect fails then the container is not known to docker. + if err != nil || !ctnr.State.Running { + return nil, container.NotActive } handler.aliases = append(handler.aliases, path.Join("/docker", ctnr.Name)) return handler, nil @@ -125,21 +129,50 @@ func (self *dockerContainerHandler) isDockerContainer() bool { } // TODO(vmarmol): Switch to getting this from libcontainer once we have a solid API. -func readLibcontainerSpec(id string) (spec *libcontainer.Config, err error) { - dir := "/var/lib/docker/execdriver/native" - configPath := path.Join(dir, id, "container.json") +func readLibcontainerConfig(id string) (config *libcontainer.Config, err error) { + configPath := path.Join(dockerRootDir, id, "container.json") + if !utils.FileExists(configPath) { + err = container.NotActive + return + } f, err := os.Open(configPath) if err != nil { return } defer f.Close() d := json.NewDecoder(f) - ret := new(libcontainer.Config) - err = d.Decode(ret) + retConfig := new(libcontainer.Config) + err = d.Decode(retConfig) if err != nil { return } - spec = ret + config = retConfig + + return +} + +func readLibcontainerState(id string) (state *libcontainer.State, err error) { + statePath := path.Join(dockerRootDir, id, "state.json") + if !utils.FileExists(statePath) { + err = container.NotActive + return + } + f, err := os.Open(statePath) + if err != nil { + if os.IsNotExist(err) { + err = container.NotActive + } + return + } + defer f.Close() + d := json.NewDecoder(f) + retState := new(libcontainer.State) + err = d.Decode(retState) + if err != nil { + return + } + state = retState + return } @@ -183,12 +216,12 @@ func (self *dockerContainerHandler) GetSpec() (spec *info.ContainerSpec, err err if err != nil { return } - libcontainerSpec, err := readLibcontainerSpec(id) + libcontainerConfig, err := readLibcontainerConfig(id) if err != nil { return } - spec = libcontainerConfigToContainerSpec(libcontainerSpec, mi) + spec = libcontainerConfigToContainerSpec(libcontainerConfig, mi) return } @@ -199,15 +232,20 @@ func (self *dockerContainerHandler) GetStats() (stats *info.ContainerStats, err stats.Timestamp = time.Now() return } - parent, id, err := self.splitName() + _, id, err := self.splitName() if err != nil { return } - cg := &cgroups.Cgroup{ - Parent: parent, - Name: id, + config, err := readLibcontainerConfig(id) + if err != nil { + return } - return containerLibcontainer.GetStats(cg, self.useSystemd) + state, err := readLibcontainerState(id) + if err != nil { + return + } + + return containerLibcontainer.GetStats(config, state) } func (self *dockerContainerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { @@ -215,7 +253,7 @@ func (self *dockerContainerHandler) ListContainers(listType container.ListType) return nil, nil } if self.isRootContainer() && listType == container.LIST_SELF { - return []info.ContainerReference{info.ContainerReference{Name: "/docker"}}, nil + return []info.ContainerReference{{Name: "/docker"}}, nil } opt := docker.ListContainersOptions{ All: true, diff --git a/container/filter_test.go b/container/filter_test.go index 5e8ef538..0c6e9efa 100644 --- a/container/filter_test.go +++ b/container/filter_test.go @@ -60,9 +60,9 @@ func TestWhiteListContainerFilter(t *testing.T) { mockc := &mockContainerHandler{} mockc.On("ListContainers", LIST_RECURSIVE).Return( []info.ContainerReference{ - info.ContainerReference{Name: "/docker/ee0103"}, - info.ContainerReference{Name: "/container/created/by/lmctfy"}, - info.ContainerReference{Name: "/user/something"}, + {Name: "/docker/ee0103"}, + {Name: "/container/created/by/lmctfy"}, + {Name: "/user/something"}, }, nil, ) @@ -95,9 +95,9 @@ func TestBlackListContainerFilter(t *testing.T) { mockc := &mockContainerHandler{} mockc.On("ListContainers", LIST_RECURSIVE).Return( []info.ContainerReference{ - info.ContainerReference{Name: "/docker/ee0103"}, - info.ContainerReference{Name: "/container/created/by/lmctfy"}, - info.ContainerReference{Name: "/user/something"}, + {Name: "/docker/ee0103"}, + {Name: "/container/created/by/lmctfy"}, + {Name: "/user/something"}, }, nil, ) diff --git a/container/libcontainer/helpers.go b/container/libcontainer/helpers.go index 957be091..99b43994 100644 --- a/container/libcontainer/helpers.go +++ b/container/libcontainer/helpers.go @@ -3,33 +3,23 @@ package libcontainer import ( "time" - "github.com/docker/libcontainer/cgroups" - "github.com/docker/libcontainer/cgroups/fs" - "github.com/docker/libcontainer/cgroups/systemd" + "github.com/docker/libcontainer" "github.com/google/cadvisor/info" ) // Get stats of the specified cgroup -func GetStats(cgroup *cgroups.Cgroup, useSystemd bool) (*info.ContainerStats, error) { +func GetStats(config *libcontainer.Config, state *libcontainer.State) (*info.ContainerStats, error) { // TODO(vmarmol): Use libcontainer's Stats() in the new API when that is ready. - // Use systemd paths if systemd is being used. - var ( - s *cgroups.Stats - err error - ) - if useSystemd { - s, err = systemd.GetStats(cgroup) - } else { - s, err = fs.GetStats(cgroup) - } + libcontainerStats, err := libcontainer.GetStats(config, state) if err != nil { return nil, err } - return toContainerStats(s), nil + return toContainerStats(libcontainerStats), nil } // Convert libcontainer stats to info.ContainerStats. -func toContainerStats(s *cgroups.Stats) *info.ContainerStats { +func toContainerStats(libcontainerStats *libcontainer.ContainerStats) *info.ContainerStats { + s := libcontainerStats.CgroupStats ret := new(info.ContainerStats) ret.Timestamp = time.Now() ret.Cpu = new(info.CpuStats) @@ -59,5 +49,6 @@ func toContainerStats(s *cgroups.Stats) *info.ContainerStats { ret.Memory.WorkingSet -= v } } + ret.Network = (*info.NetworkStats)(&libcontainerStats.NetworkStats) return ret } diff --git a/container/raw/handler.go b/container/raw/handler.go index 47251276..f7948946 100644 --- a/container/raw/handler.go +++ b/container/raw/handler.go @@ -96,7 +96,7 @@ func (self *rawContainerHandler) ListContainers(listType container.ListType) ([] // Make into container references. ret := make([]info.ContainerReference, 0, len(containers)) - for cont, _ := range containers { + for cont := range containers { ret = append(ret, info.ContainerReference{ Name: cont, }) diff --git a/info/container.go b/info/container.go index 57ba8141..194f77a3 100644 --- a/info/container.go +++ b/info/container.go @@ -239,11 +239,31 @@ type MemoryStatsMemoryData struct { Pgmajfault uint64 `json:"pgmajfault,omitempty"` } +type NetworkStats struct { + // Cumulative count of bytes received. + RxBytes uint64 `json:"rx_bytes,omitempty"` + // Cumulative count of packets received. + RxPackets uint64 `json:"rx_packets,omitempty"` + // Cumulative count of receive errors encountered. + RxErrors uint64 `json:"rx_errors,omitempty"` + // Cumulative count of packets dropped while receiving. + RxDropped uint64 `json:"rx_dropped,omitempty"` + // Cumulative count of bytes transmitted. + TxBytes uint64 `json:"tx_bytes,omitempty"` + // Cumulative count of packets transmitted. + TxPackets uint64 `json:"tx_packets,omitempty"` + // Cumulative count of transmit errors encountered. + TxErrors uint64 `json:"tx_errors,omitempty"` + // Cumulative count of packets dropped while transmitting. + TxDropped uint64 `json:"tx_dropped,omitempty"` +} + type ContainerStats struct { // The time of this stat point. - Timestamp time.Time `json:"timestamp"` - Cpu *CpuStats `json:"cpu,omitempty"` - Memory *MemoryStats `json:"memory,omitempty"` + Timestamp time.Time `json:"timestamp"` + Cpu *CpuStats `json:"cpu,omitempty"` + Memory *MemoryStats `json:"memory,omitempty"` + Network *NetworkStats `json:"network,omitempty"` } // Makes a deep copy of the ContainerStats and returns a pointer to the new diff --git a/manager/container.go b/manager/container.go index b366c9d6..60f641fc 100644 --- a/manager/container.go +++ b/manager/container.go @@ -127,7 +127,7 @@ func (c *containerData) housekeeping() { func (c *containerData) housekeepingTick() { err := c.updateStats() - if err != nil { + if err != nil && err != container.NotActive { log.Printf("Failed to update stats for container \"%s\": %s", c.info.Name, err) } } diff --git a/manager/manager.go b/manager/manager.go index 696e0bad..041599a4 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -27,7 +27,7 @@ import ( type Manager interface { // Start the manager, blocks forever. - Start() error + Start(chanErr chan error) // Get information about a container. GetContainerInfo(containerName string, query *info.ContainerInfoRequest) (*info.ContainerInfo, error) @@ -73,16 +73,18 @@ type manager struct { } // Start the container manager. -func (m *manager) Start() error { +func (m *manager) Start(errChan chan error) { // Create root and then recover all containers. _, err := m.createContainer("/") if err != nil { - return err + errChan <- err + return } log.Printf("Starting recovery of all containers") err = m.detectContainers() if err != nil { - return err + errChan <- err + return } log.Printf("Recovery completed") @@ -102,7 +104,7 @@ func (m *manager) Start() error { log.Printf("Global Housekeeping(%d) took %s", t.Unix(), duration) } } - return nil + errChan <- nil } // Get a container by name. @@ -281,18 +283,22 @@ func (m *manager) detectContainers() error { } // Add the new containers. - for _, container := range added { - _, err = m.createContainer(container.Name) + for _, cont := range added { + _, err = m.createContainer(cont.Name) if err != nil { - return fmt.Errorf("Failed to create existing container: %s: %s", container.Name, err) + if err != container.NotActive { + log.Printf("failed to create existing container: %s: %s", cont.Name, err) + } } } // Remove the old containers. - for _, container := range removed { - err = m.destroyContainer(container.Name) + for _, cont := range removed { + err = m.destroyContainer(cont.Name) if err != nil { - return fmt.Errorf("Failed to destroy existing container: %s: %s", container.Name, err) + if err != container.NotActive { + log.Printf("failed to destroy existing container: %s: %s", cont.Name, err) + } } } From abfcd4923a0ad06950fd7898ba2f9f8efdef9996 Mon Sep 17 00:00:00 2001 From: Vishnu Kannan Date: Mon, 21 Jul 2014 05:04:18 +0000 Subject: [PATCH 2/8] Adding utils package --- utils/path.go | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 utils/path.go diff --git a/utils/path.go b/utils/path.go new file mode 100644 index 00000000..e842a519 --- /dev/null +++ b/utils/path.go @@ -0,0 +1,10 @@ +package utils + +import "os" + +func FileExists(file string) bool { + if _, err := os.Stat(file); err != nil { + return true + } + return false +} From 7f96c90c720879c60bb58bff40c502dc02f67b13 Mon Sep 17 00:00:00 2001 From: Vishnu Kannan Date: Mon, 21 Jul 2014 05:25:49 +0000 Subject: [PATCH 3/8] Fix raw cgroups handler. --- container/libcontainer/helpers.go | 22 +++++++++++++++++++++- container/raw/handler.go | 2 +- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/container/libcontainer/helpers.go b/container/libcontainer/helpers.go index 99b43994..b021ba08 100644 --- a/container/libcontainer/helpers.go +++ b/container/libcontainer/helpers.go @@ -4,10 +4,13 @@ import ( "time" "github.com/docker/libcontainer" + "github.com/docker/libcontainer/cgroups" + "github.com/docker/libcontainer/cgroups/fs" + "github.com/docker/libcontainer/cgroups/systemd" "github.com/google/cadvisor/info" ) -// Get stats of the specified cgroup +// Get stats of the specified container func GetStats(config *libcontainer.Config, state *libcontainer.State) (*info.ContainerStats, error) { // TODO(vmarmol): Use libcontainer's Stats() in the new API when that is ready. libcontainerStats, err := libcontainer.GetStats(config, state) @@ -17,6 +20,23 @@ func GetStats(config *libcontainer.Config, state *libcontainer.State) (*info.Con return toContainerStats(libcontainerStats), nil } +func GetStatsCgroupOnly(cgroup *cgroups.Cgroup, useSystemd bool) (*info.ContainerStats, error) { + var ( + s *cgroups.Stats + err error + ) + // Use systemd paths if systemd is being used. + if useSystemd { + s, err = systemd.GetStats(cgroup) + } else { + s, err = fs.GetStats(cgroup) + } + if err != nil { + return nil, err + } + return toContainerStats(&libcontainer.ContainerStats{CgroupStats: s}), nil +} + // Convert libcontainer stats to info.ContainerStats. func toContainerStats(libcontainerStats *libcontainer.ContainerStats) *info.ContainerStats { s := libcontainerStats.CgroupStats diff --git a/container/raw/handler.go b/container/raw/handler.go index f7948946..2f684c20 100644 --- a/container/raw/handler.go +++ b/container/raw/handler.go @@ -58,7 +58,7 @@ func (self *rawContainerHandler) GetStats() (stats *info.ContainerStats, err err Name: self.name, } - return libcontainer.GetStats(cgroup, false) + return libcontainer.GetStatsCgroupOnly(cgroup, false) } // Lists all directories under "path" and outputs the results as children of "parent". From 5aae36726f599ba8dce2d261964e62d57c9d9e39 Mon Sep 17 00:00:00 2001 From: Vishnu Kannan Date: Mon, 21 Jul 2014 05:49:51 +0000 Subject: [PATCH 4/8] Fixed bugs introduced in previous patches. --- container/factory.go | 2 -- utils/path.go | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/container/factory.go b/container/factory.go index 54cbbc6c..f96a23e7 100644 --- a/container/factory.go +++ b/container/factory.go @@ -16,7 +16,6 @@ package container import ( "fmt" - "log" "sync" ) @@ -53,7 +52,6 @@ func NewContainerHandler(name string) (ContainerHandler, error) { // Create the ContainerHandler with the first factory that supports it. for _, factory := range factories { if factory.CanHandle(name) { - log.Printf("Using factory %q for container %q", factory.String(), name) return factory.NewContainerHandler(name) } } diff --git a/utils/path.go b/utils/path.go index e842a519..115ddf2d 100644 --- a/utils/path.go +++ b/utils/path.go @@ -4,7 +4,7 @@ import "os" func FileExists(file string) bool { if _, err := os.Stat(file); err != nil { - return true + return false } - return false + return true } From ef1344003409f4e3c6dfa8b8ffc02078463829ef Mon Sep 17 00:00:00 2001 From: Vishnu Kannan Date: Tue, 22 Jul 2014 02:50:32 +0000 Subject: [PATCH 5/8] Addressed comments. Another series of bug fixes. Modified the docker driver and lmctfy driver to skip containers they cannot handle. --- cadvisor.go | 11 ++-- container/docker/factory.go | 38 ++++++++++-- container/docker/handler.go | 98 ++++++++++++++----------------- container/factory.go | 10 +++- container/libcontainer/helpers.go | 16 ++--- container/lmctfy/factory.go | 6 ++ container/raw/handler.go | 2 +- manager/manager.go | 12 ++-- utils/path.go | 14 +++++ 9 files changed, 117 insertions(+), 90 deletions(-) diff --git a/cadvisor.go b/cadvisor.go index 4d60300d..75c12931 100644 --- a/cadvisor.go +++ b/cadvisor.go @@ -83,15 +83,14 @@ func main() { } }) - errChan := make(chan error) - go containerManager.Start(errChan) + go func() { + log.Fatal(containerManager.Start()) + }() log.Printf("Starting cAdvisor version: %q", info.VERSION) log.Print("About to serve on port ", *argPort) addr := fmt.Sprintf(":%v", *argPort) - go func() { - errChan <- http.ListenAndServe(addr, nil) - }() - log.Fatal(<-errChan) + + log.Fatal(http.ListenAndServe(addr, nil)) } diff --git a/container/docker/factory.go b/container/docker/factory.go index 1aff45db..a8b3c186 100644 --- a/container/docker/factory.go +++ b/container/docker/factory.go @@ -28,7 +28,10 @@ import ( "github.com/google/cadvisor/info" ) -var ArgDockerEndpoint = flag.String("docker", "unix:///var/run/docker.sock", "docker endpoint") +var ( + ArgDockerEndpoint = flag.String("docker", "unix:///var/run/docker.sock", "docker endpoint") + defaultClient *docker.Client +) type dockerFactory struct { machineInfoFactory info.MachineInfoFactory @@ -56,12 +59,35 @@ func (self *dockerFactory) NewContainerHandler(name string) (handler container.C } // Docker handles all containers under /docker +// TODO(vishh): Change the CanHandle interface to be able to return errors. func (self *dockerFactory) CanHandle(name string) bool { // In systemd systems the containers are: /docker-{ID} if self.useSystemd { - return strings.HasPrefix(name, "/docker-") + if !strings.HasPrefix(name, "/docker-") { + return false + } + } else if name == "/" { + return false + } else if name == "/docker" { + // We need the docker driver to handle /docker. Otherwise the aggregation at the API level will break. + return true } - return strings.HasPrefix(name, "/docker/") + // Check if the container is known to docker and it is active. + _, id, err := splitName(name) + if err != nil { + return false + } + if defaultClient == nil { + log.Fatal("Default docker client is nil") + } + ctnr, err := defaultClient.InspectContainer(id) + // We assume that if Inspect fails then the container is not known to docker. + // TODO(vishh): Detect lxc containers and avoid handling them. + if err != nil || !ctnr.State.Running { + return false + } + + return true } func parseDockerVersion(full_version_string string) ([]int, error) { @@ -84,12 +110,12 @@ func parseDockerVersion(full_version_string string) ([]int, error) { } // Register root container before running this function! -func Register(factory info.MachineInfoFactory) error { - client, err := docker.NewClient(*ArgDockerEndpoint) +func Register(factory info.MachineInfoFactory) (err error) { + defaultClient, err = docker.NewClient(*ArgDockerEndpoint) if err != nil { return fmt.Errorf("unable to communicate with docker daemon: %v", err) } - if version, err := client.Version(); err != nil { + if version, err := defaultClient.Version(); err != nil { return fmt.Errorf("unable to communicate with docker daemon: %v", err) } else { expected_version := []int{0, 11, 1} diff --git a/container/docker/handler.go b/container/docker/handler.go index 77eb035f..c6800c1a 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -17,13 +17,13 @@ package docker import ( "bufio" "encoding/json" + "errors" "fmt" "math" "os" "path" "path/filepath" "strings" - "time" "github.com/docker/libcontainer" "github.com/fsouza/go-dockerclient" @@ -36,9 +36,13 @@ import ( // Basepath to all container specific information that libcontainer stores. const dockerRootDir = "/var/lib/docker/execdriver/native" +var fileNotFound = errors.New("file not found") + type dockerContainerHandler struct { client *docker.Client name string + parent string + ID string aliases []string machineInfoFactory info.MachineInfoFactory useSystemd bool @@ -56,13 +60,15 @@ func newDockerContainerHandler( machineInfoFactory: machineInfoFactory, useSystemd: useSystemd, } - if !handler.isDockerContainer() { + if handler.isDockerRoot() { return handler, nil } - _, id, err := handler.splitName() + parent, id, err := splitName(name) if err != nil { return nil, fmt.Errorf("invalid docker container %v: %v", name, err) } + handler.parent = parent + handler.ID = id ctnr, err := client.InspectContainer(id) // We assume that if Inspect fails then the container is not known to docker. if err != nil || !ctnr.State.Running { @@ -79,8 +85,13 @@ func (self *dockerContainerHandler) ContainerReference() (info.ContainerReferenc }, nil } -func (self *dockerContainerHandler) splitName() (string, string, error) { - parent, id := path.Split(self.name) +func (self *dockerContainerHandler) isDockerRoot() bool { + // TODO(dengnan): Should we consider other cases? + return self.name == "/docker" +} + +func splitName(containerName string) (string, string, error) { + parent, id := path.Split(containerName) cgroupSelf, err := os.Open("/proc/self/cgroup") if err != nil { return "", "", err @@ -115,24 +126,11 @@ func (self *dockerContainerHandler) splitName() (string, string, error) { return parent, id, nil } -func (self *dockerContainerHandler) isDockerRoot() bool { - // TODO(dengnan): Should we consider other cases? - return self.name == "/docker" -} - -func (self *dockerContainerHandler) isRootContainer() bool { - return self.name == "/" -} - -func (self *dockerContainerHandler) isDockerContainer() bool { - return (!self.isDockerRoot()) && (!self.isRootContainer()) -} - -// TODO(vmarmol): Switch to getting this from libcontainer once we have a solid API. -func readLibcontainerConfig(id string) (config *libcontainer.Config, err error) { - configPath := path.Join(dockerRootDir, id, "container.json") +// TODO(vmarmol): Switch to getting this from libcontainer once we have a solID API. +func (self *dockerContainerHandler) readLibcontainerConfig() (config *libcontainer.Config, err error) { + configPath := path.Join(dockerRootDir, self.ID, "container.json") if !utils.FileExists(configPath) { - err = container.NotActive + err = fileNotFound return } f, err := os.Open(configPath) @@ -151,10 +149,10 @@ func readLibcontainerConfig(id string) (config *libcontainer.Config, err error) return } -func readLibcontainerState(id string) (state *libcontainer.State, err error) { - statePath := path.Join(dockerRootDir, id, "state.json") +func (self *dockerContainerHandler) readLibcontainerState() (state *libcontainer.State, err error) { + statePath := path.Join(dockerRootDir, self.ID, "state.json") if !utils.FileExists(statePath) { - err = container.NotActive + err = fileNotFound return } f, err := os.Open(statePath) @@ -204,19 +202,14 @@ func libcontainerConfigToContainerSpec(config *libcontainer.Config, mi *info.Mac } func (self *dockerContainerHandler) GetSpec() (spec *info.ContainerSpec, err error) { - if !self.isDockerContainer() { - spec = new(info.ContainerSpec) - return + if self.isDockerRoot() { + return &info.ContainerSpec{}, nil } mi, err := self.machineInfoFactory.GetMachineInfo() if err != nil { return } - _, id, err := self.splitName() - if err != nil { - return - } - libcontainerConfig, err := readLibcontainerConfig(id) + libcontainerConfig, err := self.readLibcontainerConfig() if err != nil { return } @@ -226,22 +219,21 @@ func (self *dockerContainerHandler) GetSpec() (spec *info.ContainerSpec, err err } func (self *dockerContainerHandler) GetStats() (stats *info.ContainerStats, err error) { - if !self.isDockerContainer() { - // Return empty stats for root containers. - stats = new(info.ContainerStats) - stats.Timestamp = time.Now() + if self.isDockerRoot() { + return &info.ContainerStats{}, nil + } + config, err := self.readLibcontainerConfig() + if err != nil { + if err == fileNotFound { + return &info.ContainerStats{}, nil + } return } - _, id, err := self.splitName() - if err != nil { - return - } - config, err := readLibcontainerConfig(id) - if err != nil { - return - } - state, err := readLibcontainerState(id) + state, err := self.readLibcontainerState() if err != nil { + if err == fileNotFound { + return &info.ContainerStats{}, nil + } return } @@ -249,12 +241,6 @@ func (self *dockerContainerHandler) GetStats() (stats *info.ContainerStats, err } func (self *dockerContainerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { - if self.isDockerContainer() { - return nil, nil - } - if self.isRootContainer() && listType == container.LIST_SELF { - return []info.ContainerReference{{Name: "/docker"}}, nil - } opt := docker.ListContainersOptions{ All: true, } @@ -264,6 +250,10 @@ func (self *dockerContainerHandler) ListContainers(listType container.ListType) } ret := make([]info.ContainerReference, 0, len(containers)+1) for _, c := range containers { + if c.ID == self.ID { + // Skip self. + continue + } if !strings.HasPrefix(c.Status, "Up ") { continue } @@ -275,9 +265,7 @@ func (self *dockerContainerHandler) ListContainers(listType container.ListType) } ret = append(ret, ref) } - if self.isRootContainer() { - ret = append(ret, info.ContainerReference{Name: "/docker"}) - } + return ret, nil } diff --git a/container/factory.go b/container/factory.go index f96a23e7..369925d6 100644 --- a/container/factory.go +++ b/container/factory.go @@ -16,6 +16,7 @@ package container import ( "fmt" + "log" "sync" ) @@ -32,8 +33,10 @@ type ContainerHandlerFactory interface { // TODO(vmarmol): Consider not making this global. // Global list of factories. -var factories []ContainerHandlerFactory -var factoriesLock sync.RWMutex +var ( + factories []ContainerHandlerFactory + factoriesLock sync.RWMutex +) // Register a ContainerHandlerFactory. These should be registered from least general to most general // as they will be asked in order whether they can handle a particular container. @@ -52,11 +55,12 @@ func NewContainerHandler(name string) (ContainerHandler, error) { // Create the ContainerHandler with the first factory that supports it. for _, factory := range factories { if factory.CanHandle(name) { + log.Printf("Using factory %q for container %q", factory.String(), name) return factory.NewContainerHandler(name) } } - return nil, fmt.Errorf("no known factory can handle creation of container %q", name) + return nil, fmt.Errorf("no known factory can handle creation of container") } // Clear the known factories. diff --git a/container/libcontainer/helpers.go b/container/libcontainer/helpers.go index b021ba08..e5ace212 100644 --- a/container/libcontainer/helpers.go +++ b/container/libcontainer/helpers.go @@ -6,7 +6,6 @@ import ( "github.com/docker/libcontainer" "github.com/docker/libcontainer/cgroups" "github.com/docker/libcontainer/cgroups/fs" - "github.com/docker/libcontainer/cgroups/systemd" "github.com/google/cadvisor/info" ) @@ -20,17 +19,8 @@ func GetStats(config *libcontainer.Config, state *libcontainer.State) (*info.Con return toContainerStats(libcontainerStats), nil } -func GetStatsCgroupOnly(cgroup *cgroups.Cgroup, useSystemd bool) (*info.ContainerStats, error) { - var ( - s *cgroups.Stats - err error - ) - // Use systemd paths if systemd is being used. - if useSystemd { - s, err = systemd.GetStats(cgroup) - } else { - s, err = fs.GetStats(cgroup) - } +func GetStatsCgroupOnly(cgroup *cgroups.Cgroup) (*info.ContainerStats, error) { + s, err := fs.GetStats(cgroup) if err != nil { return nil, err } @@ -69,6 +59,8 @@ func toContainerStats(libcontainerStats *libcontainer.ContainerStats) *info.Cont ret.Memory.WorkingSet -= v } } + // TODO(vishh): Perform a deep copy or alias libcontainer network stats. ret.Network = (*info.NetworkStats)(&libcontainerStats.NetworkStats) + return ret } diff --git a/container/lmctfy/factory.go b/container/lmctfy/factory.go index bd9e17c8..b8782675 100644 --- a/container/lmctfy/factory.go +++ b/container/lmctfy/factory.go @@ -50,5 +50,11 @@ func (self *lmctfyFactory) NewContainerHandler(name string) (container.Container func (self *lmctfyFactory) CanHandle(name string) bool { // TODO(vmarmol): Try to attach to the container before blindly saying true. + cmd := exec.Command(lmctfyBinary, "stats", "summary", name) + _, err := cmd.Output() + if err != nil { + return false + } + return true } diff --git a/container/raw/handler.go b/container/raw/handler.go index 2f684c20..07a1a82e 100644 --- a/container/raw/handler.go +++ b/container/raw/handler.go @@ -58,7 +58,7 @@ func (self *rawContainerHandler) GetStats() (stats *info.ContainerStats, err err Name: self.name, } - return libcontainer.GetStatsCgroupOnly(cgroup, false) + return libcontainer.GetStatsCgroupOnly(cgroup) } // Lists all directories under "path" and outputs the results as children of "parent". diff --git a/manager/manager.go b/manager/manager.go index 041599a4..4ec6fc49 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -27,7 +27,7 @@ import ( type Manager interface { // Start the manager, blocks forever. - Start(chanErr chan error) + Start() error // Get information about a container. GetContainerInfo(containerName string, query *info.ContainerInfoRequest) (*info.ContainerInfo, error) @@ -73,18 +73,16 @@ type manager struct { } // Start the container manager. -func (m *manager) Start(errChan chan error) { +func (m *manager) Start() error { // Create root and then recover all containers. _, err := m.createContainer("/") if err != nil { - errChan <- err - return + return err } log.Printf("Starting recovery of all containers") err = m.detectContainers() if err != nil { - errChan <- err - return + return err } log.Printf("Recovery completed") @@ -104,7 +102,7 @@ func (m *manager) Start(errChan chan error) { log.Printf("Global Housekeeping(%d) took %s", t.Unix(), duration) } } - errChan <- nil + return nil } // Get a container by name. diff --git a/utils/path.go b/utils/path.go index 115ddf2d..a7aceee6 100644 --- a/utils/path.go +++ b/utils/path.go @@ -1,3 +1,17 @@ +// Copyright 2014 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package utils import "os" From 5dfa7b64bada87c950dc30c3769b0a4edc9ca8e9 Mon Sep 17 00:00:00 2001 From: Vishnu Kannan Date: Tue, 22 Jul 2014 17:30:38 +0000 Subject: [PATCH 6/8] Removed NotActive error message from container package. Imporved error messages. --- container/container.go | 8 +------- container/docker/factory.go | 19 ++++++++----------- container/docker/handler.go | 15 +++++++-------- container/lmctfy/factory.go | 1 - manager/container.go | 2 +- manager/manager.go | 8 ++------ 6 files changed, 19 insertions(+), 34 deletions(-) diff --git a/container/container.go b/container/container.go index efe92397..de478fd0 100644 --- a/container/container.go +++ b/container/container.go @@ -14,11 +14,7 @@ package container -import ( - "errors" - - "github.com/google/cadvisor/info" -) +import "github.com/google/cadvisor/info" // Listing types. const ( @@ -26,8 +22,6 @@ const ( LIST_RECURSIVE ) -var NotActive = errors.New("Container is not active") - type ListType int // Interface for container operation handlers. diff --git a/container/docker/factory.go b/container/docker/factory.go index a8b3c186..0caf8c93 100644 --- a/container/docker/factory.go +++ b/container/docker/factory.go @@ -28,16 +28,15 @@ import ( "github.com/google/cadvisor/info" ) -var ( - ArgDockerEndpoint = flag.String("docker", "unix:///var/run/docker.sock", "docker endpoint") - defaultClient *docker.Client -) +var ArgDockerEndpoint = flag.String("docker", "unix:///var/run/docker.sock", "docker endpoint") type dockerFactory struct { machineInfoFactory info.MachineInfoFactory // Whether this system is using systemd. useSystemd bool + + client *docker.Client } func (self *dockerFactory) String() string { @@ -77,10 +76,7 @@ func (self *dockerFactory) CanHandle(name string) bool { if err != nil { return false } - if defaultClient == nil { - log.Fatal("Default docker client is nil") - } - ctnr, err := defaultClient.InspectContainer(id) + ctnr, err := self.client.InspectContainer(id) // We assume that if Inspect fails then the container is not known to docker. // TODO(vishh): Detect lxc containers and avoid handling them. if err != nil || !ctnr.State.Running { @@ -110,12 +106,12 @@ func parseDockerVersion(full_version_string string) ([]int, error) { } // Register root container before running this function! -func Register(factory info.MachineInfoFactory) (err error) { - defaultClient, err = docker.NewClient(*ArgDockerEndpoint) +func Register(factory info.MachineInfoFactory) error { + client, err := docker.NewClient(*ArgDockerEndpoint) if err != nil { return fmt.Errorf("unable to communicate with docker daemon: %v", err) } - if version, err := defaultClient.Version(); err != nil { + if version, err := client.Version(); err != nil { return fmt.Errorf("unable to communicate with docker daemon: %v", err) } else { expected_version := []int{0, 11, 1} @@ -135,6 +131,7 @@ func Register(factory info.MachineInfoFactory) (err error) { f := &dockerFactory{ machineInfoFactory: factory, useSystemd: systemd.UseSystemd(), + client: client, } log.Printf("Registering Docker factory") container.RegisterContainerHandlerFactory(f) diff --git a/container/docker/handler.go b/container/docker/handler.go index c6800c1a..0296090f 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -71,8 +71,8 @@ func newDockerContainerHandler( handler.ID = id ctnr, err := client.InspectContainer(id) // We assume that if Inspect fails then the container is not known to docker. - if err != nil || !ctnr.State.Running { - return nil, container.NotActive + 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)) return handler, nil @@ -126,16 +126,17 @@ func splitName(containerName string) (string, string, error) { return parent, id, nil } -// TODO(vmarmol): Switch to getting this from libcontainer once we have a solID API. +// TODO(vmarmol): Switch to getting this from libcontainer once we have a solid API. func (self *dockerContainerHandler) readLibcontainerConfig() (config *libcontainer.Config, err error) { configPath := path.Join(dockerRootDir, self.ID, "container.json") if !utils.FileExists(configPath) { + // TODO(vishh): Return file name as well once we have a better error interface. err = fileNotFound return } f, err := os.Open(configPath) if err != nil { - return + return nil, fmt.Errorf("failed to open %s - %s\n", configPath, err) } defer f.Close() d := json.NewDecoder(f) @@ -152,15 +153,13 @@ func (self *dockerContainerHandler) readLibcontainerConfig() (config *libcontain func (self *dockerContainerHandler) readLibcontainerState() (state *libcontainer.State, err error) { statePath := path.Join(dockerRootDir, self.ID, "state.json") if !utils.FileExists(statePath) { + // TODO(vishh): Return file name as well once we have a better error interface. err = fileNotFound return } f, err := os.Open(statePath) if err != nil { - if os.IsNotExist(err) { - err = container.NotActive - } - return + return nil, fmt.Errorf("failed to open %s - %s\n", statePath, err) } defer f.Close() d := json.NewDecoder(f) diff --git a/container/lmctfy/factory.go b/container/lmctfy/factory.go index b8782675..5de660c7 100644 --- a/container/lmctfy/factory.go +++ b/container/lmctfy/factory.go @@ -49,7 +49,6 @@ func (self *lmctfyFactory) NewContainerHandler(name string) (container.Container } func (self *lmctfyFactory) CanHandle(name string) bool { - // TODO(vmarmol): Try to attach to the container before blindly saying true. cmd := exec.Command(lmctfyBinary, "stats", "summary", name) _, err := cmd.Output() if err != nil { diff --git a/manager/container.go b/manager/container.go index 60f641fc..b366c9d6 100644 --- a/manager/container.go +++ b/manager/container.go @@ -127,7 +127,7 @@ func (c *containerData) housekeeping() { func (c *containerData) housekeepingTick() { err := c.updateStats() - if err != nil && err != container.NotActive { + if err != nil { log.Printf("Failed to update stats for container \"%s\": %s", c.info.Name, err) } } diff --git a/manager/manager.go b/manager/manager.go index 4ec6fc49..1f684f50 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -284,9 +284,7 @@ func (m *manager) detectContainers() error { for _, cont := range added { _, err = m.createContainer(cont.Name) if err != nil { - if err != container.NotActive { - log.Printf("failed to create existing container: %s: %s", cont.Name, err) - } + log.Printf("failed to create existing container: %s: %s", cont.Name, err) } } @@ -294,9 +292,7 @@ func (m *manager) detectContainers() error { for _, cont := range removed { err = m.destroyContainer(cont.Name) if err != nil { - if err != container.NotActive { - log.Printf("failed to destroy existing container: %s: %s", cont.Name, err) - } + log.Printf("failed to destroy existing container: %s: %s", cont.Name, err) } } From f147996e9dcbf00376c224b840164af5cb0477ef Mon Sep 17 00:00:00 2001 From: Vishnu Kannan Date: Tue, 22 Jul 2014 18:03:29 +0000 Subject: [PATCH 7/8] Ignore non '/docker' containers in the docker driver. --- container/docker/factory.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/container/docker/factory.go b/container/docker/factory.go index 0caf8c93..a379524a 100644 --- a/container/docker/factory.go +++ b/container/docker/factory.go @@ -67,6 +67,8 @@ func (self *dockerFactory) CanHandle(name string) bool { } } else if name == "/" { return false + } else if !strings.HasPrefix(name, "/docker") { + return false } else if name == "/docker" { // We need the docker driver to handle /docker. Otherwise the aggregation at the API level will break. return true From a748b5374396409cff34391dba3d0f390065d9d4 Mon Sep 17 00:00:00 2001 From: Vishnu Kannan Date: Tue, 22 Jul 2014 18:55:19 +0000 Subject: [PATCH 8/8] Docker ListContainers will work only for the '/docker' container. --- container/docker/factory.go | 4 ++-- container/docker/handler.go | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/container/docker/factory.go b/container/docker/factory.go index a379524a..05ae89d9 100644 --- a/container/docker/factory.go +++ b/container/docker/factory.go @@ -67,11 +67,11 @@ func (self *dockerFactory) CanHandle(name string) bool { } } else if name == "/" { return false - } else if !strings.HasPrefix(name, "/docker") { - return false } else if name == "/docker" { // We need the docker driver to handle /docker. Otherwise the aggregation at the API level will break. return true + } else if !strings.HasPrefix(name, "/docker/") { + return false } // Check if the container is known to docker and it is active. _, id, err := splitName(name) diff --git a/container/docker/handler.go b/container/docker/handler.go index 0296090f..76dbfc04 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -86,7 +86,6 @@ func (self *dockerContainerHandler) ContainerReference() (info.ContainerReferenc } func (self *dockerContainerHandler) isDockerRoot() bool { - // TODO(dengnan): Should we consider other cases? return self.name == "/docker" } @@ -240,6 +239,9 @@ func (self *dockerContainerHandler) GetStats() (stats *info.ContainerStats, err } func (self *dockerContainerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { + if self.name != "/docker" { + return []info.ContainerReference{}, nil + } opt := docker.ListContainersOptions{ All: true, } @@ -249,10 +251,6 @@ func (self *dockerContainerHandler) ListContainers(listType container.ListType) } ret := make([]info.ContainerReference, 0, len(containers)+1) for _, c := range containers { - if c.ID == self.ID { - // Skip self. - continue - } if !strings.HasPrefix(c.Status, "Up ") { continue }