From b22bfa32f09ae2b4abc1de0f82ecb76cd4e9ffa3 Mon Sep 17 00:00:00 2001 From: Victor Marmol Date: Wed, 10 Sep 2014 17:56:47 -0700 Subject: [PATCH] Updating libcontainer version. This includes a fix which reduces CPU usage by ~30%. --- Godeps/Godeps.json | 4 +- .../libcontainer/cgroups/fs/apply_raw.go | 28 ++-- .../docker/libcontainer/cgroups/fs/cpuacct.go | 122 +++++------------- .../docker/libcontainer/cgroups/stats.go | 18 +-- .../docker/libcontainer/devices/devices.go | 14 +- .../libcontainer/devices/devices_test.go | 61 +++++++++ .../github.com/docker/libcontainer/factory.go | 14 +- .../docker/libcontainer/mount/mount.go | 5 + .../docker/libcontainer/mount/nodes/nodes.go | 5 + .../docker/libcontainer/syncpipe/sync_pipe.go | 4 + .../libcontainer/syncpipe/sync_pipe_test.go | 13 +- 11 files changed, 173 insertions(+), 115 deletions(-) create mode 100644 Godeps/_workspace/src/github.com/docker/libcontainer/devices/devices_test.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 36cce276..4989db2d 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -9,8 +9,8 @@ }, { "ImportPath": "github.com/docker/libcontainer", - "Comment": "v1.1.0-209-g55430d0", - "Rev": "55430d0db7c6bb1198c0bb573a9700a859d5ec26" + "Comment": "v1.2.0-26-g7f9256c", + "Rev": "7f9256cdc93e36fd7bc3dfec0c3737099e3c053f" }, { "ImportPath": "github.com/fsouza/go-dockerclient", diff --git a/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/fs/apply_raw.go b/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/fs/apply_raw.go index 443dbb69..133241e4 100644 --- a/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/fs/apply_raw.go +++ b/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/fs/apply_raw.go @@ -24,6 +24,23 @@ var ( CgroupProcesses = "cgroup.procs" ) +// The absolute path to the root of the cgroup hierarchies. +var cgroupRoot string + +// TODO(vmarmol): Report error here, we'll probably need to wait for the new API. +func init() { + // we can pick any subsystem to find the root + cpuRoot, err := cgroups.FindCgroupMountpoint("cpu") + if err != nil { + return + } + cgroupRoot = filepath.Dir(cpuRoot) + + if _, err := os.Stat(cgroupRoot); err != nil { + return + } +} + type subsystem interface { // Returns the stats, as 'stats', corresponding to the cgroup under 'path'. GetStats(path string, stats *cgroups.Stats) error @@ -121,15 +138,8 @@ func GetPids(c *cgroups.Cgroup) ([]int, error) { } func getCgroupData(c *cgroups.Cgroup, pid int) (*data, error) { - // we can pick any subsystem to find the root - cgroupRoot, err := cgroups.FindCgroupMountpoint("cpu") - if err != nil { - return nil, err - } - cgroupRoot = filepath.Dir(cgroupRoot) - - if _, err := os.Stat(cgroupRoot); err != nil { - return nil, fmt.Errorf("cgroups fs not found") + if cgroupRoot == "" { + return nil, fmt.Errorf("failed to find the cgroup root") } cgroup := c.Name diff --git a/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/fs/cpuacct.go b/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/fs/cpuacct.go index 853ab6bf..02cdff5a 100644 --- a/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/fs/cpuacct.go +++ b/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/fs/cpuacct.go @@ -1,26 +1,22 @@ package fs import ( - "bufio" "fmt" "io/ioutil" - "os" "path/filepath" - "runtime" "strconv" "strings" - "time" "github.com/docker/libcontainer/cgroups" "github.com/docker/libcontainer/system" ) -var ( - cpuCount = uint64(runtime.NumCPU()) - clockTicks = uint64(system.GetClockTicks()) +const ( + cgroupCpuacctStat = "cpuacct.stat" + nanosecondsInSecond = 1000000000 ) -const nanosecondsInSecond = 1000000000 +var clockTicks = uint64(system.GetClockTicks()) type CpuacctGroup struct { } @@ -39,103 +35,53 @@ func (s *CpuacctGroup) Remove(d *data) error { } func (s *CpuacctGroup) GetStats(path string, stats *cgroups.Stats) error { - var ( - err error - startCpu, lastCpu, startSystem, lastSystem, startUsage, lastUsage, kernelModeUsage, userModeUsage, percentage uint64 - ) - if kernelModeUsage, userModeUsage, err = getCpuUsage(path); err != nil { - return err - } - startCpu = kernelModeUsage + userModeUsage - if startSystem, err = getSystemCpuUsage(); err != nil { - return err - } - startUsageTime := time.Now() - if startUsage, err = getCgroupParamInt(path, "cpuacct.usage"); err != nil { - return err - } - // sample for 100ms - time.Sleep(1000 * time.Millisecond) - if kernelModeUsage, userModeUsage, err = getCpuUsage(path); err != nil { - return err - } - lastCpu = kernelModeUsage + userModeUsage - if lastSystem, err = getSystemCpuUsage(); err != nil { - return err - } - usageSampleDuration := time.Since(startUsageTime) - if lastUsage, err = getCgroupParamInt(path, "cpuacct.usage"); err != nil { + userModeUsage, kernelModeUsage, err := getCpuUsageBreakdown(path) + if err != nil { return err } - var ( - deltaProc = lastCpu - startCpu - deltaSystem = lastSystem - startSystem - deltaUsage = lastUsage - startUsage - ) - if deltaSystem > 0.0 { - percentage = uint64((float64(deltaProc) / float64(deltaSystem)) * float64(clockTicks*cpuCount)) + totalUsage, err := getCgroupParamInt(path, "cpuacct.usage") + if err != nil { + return err } - // NOTE: a percentage over 100% is valid for POSIX because that means the - // processes is using multiple cores - stats.CpuStats.CpuUsage.PercentUsage = percentage - // Delta usage is in nanoseconds of CPU time so get the usage (in cores) over the sample time. - stats.CpuStats.CpuUsage.CurrentUsage = deltaUsage / uint64(usageSampleDuration.Nanoseconds()) + percpuUsage, err := getPercpuUsage(path) if err != nil { return err } - stats.CpuStats.CpuUsage.TotalUsage = lastUsage + + stats.CpuStats.CpuUsage.TotalUsage = totalUsage stats.CpuStats.CpuUsage.PercpuUsage = percpuUsage - stats.CpuStats.CpuUsage.UsageInKernelmode = (kernelModeUsage * nanosecondsInSecond) / clockTicks - stats.CpuStats.CpuUsage.UsageInUsermode = (userModeUsage * nanosecondsInSecond) / clockTicks + stats.CpuStats.CpuUsage.UsageInUsermode = userModeUsage + stats.CpuStats.CpuUsage.UsageInKernelmode = kernelModeUsage return nil } -// TODO(vmarmol): Use cgroups stats. -func getSystemCpuUsage() (uint64, error) { - - f, err := os.Open("/proc/stat") - if err != nil { - return 0, err - } - defer f.Close() - - sc := bufio.NewScanner(f) - for sc.Scan() { - parts := strings.Fields(sc.Text()) - switch parts[0] { - case "cpu": - if len(parts) < 8 { - return 0, fmt.Errorf("invalid number of cpu fields") - } - - var total uint64 - for _, i := range parts[1:8] { - v, err := strconv.ParseUint(i, 10, 64) - if err != nil { - return 0.0, fmt.Errorf("Unable to convert value %s to int: %s", i, err) - } - total += v - } - return total, nil - default: - continue - } - } - return 0, fmt.Errorf("invalid stat format") -} - -func getCpuUsage(path string) (uint64, uint64, error) { - kernelModeUsage := uint64(0) +// Returns user and kernel usage breakdown in nanoseconds. +func getCpuUsageBreakdown(path string) (uint64, uint64, error) { userModeUsage := uint64(0) - data, err := ioutil.ReadFile(filepath.Join(path, "cpuacct.stat")) + kernelModeUsage := uint64(0) + const ( + userField = "user" + systemField = "system" + ) + + // Expected format: + // user + // system + data, err := ioutil.ReadFile(filepath.Join(path, cgroupCpuacctStat)) if err != nil { return 0, 0, err } fields := strings.Fields(string(data)) if len(fields) != 4 { - return 0, 0, fmt.Errorf("Failure - %s is expected to have 4 fields", filepath.Join(path, "cpuacct.stat")) + return 0, 0, fmt.Errorf("failure - %s is expected to have 4 fields", filepath.Join(path, cgroupCpuacctStat)) + } + if fields[0] != userField { + return 0, 0, fmt.Errorf("unexpected field %q in %q, expected %q", fields[0], cgroupCpuacctStat, userField) + } + if fields[2] != systemField { + return 0, 0, fmt.Errorf("unexpected field %q in %q, expected %q", fields[2], cgroupCpuacctStat, systemField) } if userModeUsage, err = strconv.ParseUint(fields[1], 10, 64); err != nil { return 0, 0, err @@ -144,7 +90,7 @@ func getCpuUsage(path string) (uint64, uint64, error) { return 0, 0, err } - return kernelModeUsage, userModeUsage, nil + return (userModeUsage * nanosecondsInSecond) / clockTicks, (kernelModeUsage * nanosecondsInSecond) / clockTicks, nil } func getPercpuUsage(path string) ([]uint64, error) { diff --git a/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/stats.go b/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/stats.go index 49913bce..f5225139 100644 --- a/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/stats.go +++ b/Godeps/_workspace/src/github.com/docker/libcontainer/cgroups/stats.go @@ -9,17 +9,19 @@ type ThrottlingData struct { ThrottledTime uint64 `json:"throttled_time,omitempty"` } +// All CPU stats are aggregate since container inception. type CpuUsage struct { - // percentage of available CPUs currently being used. - PercentUsage uint64 `json:"percent_usage,omitempty"` - // nanoseconds of cpu time consumed over the last 100 ms. - CurrentUsage uint64 `json:"current_usage,omitempty"` - // total nanoseconds of cpu time consumed - TotalUsage uint64 `json:"total_usage,omitempty"` + // Total CPU time consumed. + // Units: nanoseconds. + TotalUsage uint64 `json:"total_usage,omitempty"` + // Total CPU time consumed per core. + // Units: nanoseconds. PercpuUsage []uint64 `json:"percpu_usage,omitempty"` - // Time spent by tasks of the cgroup in kernel mode. Units: nanoseconds. + // Time spent by tasks of the cgroup in kernel mode. + // Units: nanoseconds. UsageInKernelmode uint64 `json:"usage_in_kernelmode"` - // Time spent by tasks of the cgroup in user mode. Units: nanoseconds. + // Time spent by tasks of the cgroup in user mode. + // Units: nanoseconds. UsageInUsermode uint64 `json:"usage_in_usermode"` } diff --git a/Godeps/_workspace/src/github.com/docker/libcontainer/devices/devices.go b/Godeps/_workspace/src/github.com/docker/libcontainer/devices/devices.go index 727a5bdf..558f7f5f 100644 --- a/Godeps/_workspace/src/github.com/docker/libcontainer/devices/devices.go +++ b/Godeps/_workspace/src/github.com/docker/libcontainer/devices/devices.go @@ -17,6 +17,12 @@ var ( ErrNotADeviceNode = errors.New("not a device node") ) +// Testing dependencies +var ( + osLstat = os.Lstat + ioutilReadDir = ioutil.ReadDir +) + type Device struct { Type rune `json:"type,omitempty"` Path string `json:"path,omitempty"` // It is fine if this is an empty string in the case that you are using Wildcards @@ -24,6 +30,8 @@ type Device struct { MinorNumber int64 `json:"minor_number,omitempty"` // Use the wildcard constant for wildcards. CgroupPermissions string `json:"cgroup_permissions,omitempty"` // Typically just "rwm" FileMode os.FileMode `json:"file_mode,omitempty"` // The permission bits of the file's mode + Uid uint32 `json:"uid,omitempty"` + Gid uint32 `json:"gid,omitempty"` } func GetDeviceNumberString(deviceNumber int64) string { @@ -40,7 +48,7 @@ func (device *Device) GetCgroupAllowString() string { // Given the path to a device and it's cgroup_permissions(which cannot be easilly queried) look up the information about a linux device and return that information as a Device struct. func GetDevice(path, cgroupPermissions string) (*Device, error) { - fileInfo, err := os.Lstat(path) + fileInfo, err := osLstat(path) if err != nil { return nil, err } @@ -75,6 +83,8 @@ func GetDevice(path, cgroupPermissions string) (*Device, error) { MinorNumber: Minor(devNumber), CgroupPermissions: cgroupPermissions, FileMode: fileModePermissionBits, + Uid: stat_t.Uid, + Gid: stat_t.Gid, }, nil } @@ -83,7 +93,7 @@ func GetHostDeviceNodes() ([]*Device, error) { } func getDeviceNodes(path string) ([]*Device, error) { - files, err := ioutil.ReadDir(path) + files, err := ioutilReadDir(path) if err != nil { return nil, err } diff --git a/Godeps/_workspace/src/github.com/docker/libcontainer/devices/devices_test.go b/Godeps/_workspace/src/github.com/docker/libcontainer/devices/devices_test.go new file mode 100644 index 00000000..fec40022 --- /dev/null +++ b/Godeps/_workspace/src/github.com/docker/libcontainer/devices/devices_test.go @@ -0,0 +1,61 @@ +package devices + +import ( + "errors" + "os" + "testing" +) + +func TestGetDeviceLstatFailure(t *testing.T) { + testError := errors.New("test error") + + // Override os.Lstat to inject error. + osLstat = func(path string) (os.FileInfo, error) { + return nil, testError + } + + _, err := GetDevice("", "") + if err != testError { + t.Fatalf("Unexpected error %v, expected %v", err, testError) + } +} + +func TestGetHostDeviceNodesIoutilReadDirFailure(t *testing.T) { + testError := errors.New("test error") + + // Override ioutil.ReadDir to inject error. + ioutilReadDir = func(dirname string) ([]os.FileInfo, error) { + return nil, testError + } + + _, err := GetHostDeviceNodes() + if err != testError { + t.Fatalf("Unexpected error %v, expected %v", err, testError) + } +} + +func TestGetHostDeviceNodesIoutilReadDirDeepFailure(t *testing.T) { + testError := errors.New("test error") + called := false + + // Override ioutil.ReadDir to inject error after the first call. + ioutilReadDir = func(dirname string) ([]os.FileInfo, error) { + if called { + return nil, testError + } + called = true + + // Provoke a second call. + fi, err := os.Lstat("/tmp") + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + + return []os.FileInfo{fi}, nil + } + + _, err := GetHostDeviceNodes() + if err != testError { + t.Fatalf("Unexpected error %v, expected %v", err, testError) + } +} diff --git a/Godeps/_workspace/src/github.com/docker/libcontainer/factory.go b/Godeps/_workspace/src/github.com/docker/libcontainer/factory.go index b7449335..f4c31160 100644 --- a/Godeps/_workspace/src/github.com/docker/libcontainer/factory.go +++ b/Godeps/_workspace/src/github.com/docker/libcontainer/factory.go @@ -1,14 +1,20 @@ package libcontainer type Factory interface { - // Creates a new container in the given path. A unique ID is generated for the container and - // starts the initial process inside the container. + + // Creates a new container with the given id and starts the initial process inside it. + // id must be a string containing only letters, digits and underscores and must contain + // between 1 and 1024 characters, inclusive. + // + // The id must not already be in use by an existing container. Containers created using + // a factory with the same path (and file system) must have distinct ids. // // Returns the new container with a running process. // // Errors: - // Path already exists - // Config or initialConfig is invalid + // id is already in use by a container + // id has incorrect format + // config is invalid // System error // // On error, any partially created container parts are cleaned up (the operation is atomic). diff --git a/Godeps/_workspace/src/github.com/docker/libcontainer/mount/mount.go b/Godeps/_workspace/src/github.com/docker/libcontainer/mount/mount.go index c7865b3c..c1b42421 100644 --- a/Godeps/_workspace/src/github.com/docker/libcontainer/mount/mount.go +++ b/Godeps/_workspace/src/github.com/docker/libcontainer/mount/mount.go @@ -17,6 +17,7 @@ type Mount struct { Writable bool `json:"writable,omitempty"` Relabel string `json:"relabel,omitempty"` // Relabel source if set, "z" indicates shared, "Z" indicates unshared Private bool `json:"private,omitempty"` + Slave bool `json:"slave,omitempty"` } func (m *Mount) Mount(rootfs, mountLabel string) error { @@ -40,6 +41,10 @@ func (m *Mount) bindMount(rootfs, mountLabel string) error { flags = flags | syscall.MS_RDONLY } + if m.Slave { + flags = flags | syscall.MS_SLAVE + } + stat, err := os.Stat(m.Source) if err != nil { return err diff --git a/Godeps/_workspace/src/github.com/docker/libcontainer/mount/nodes/nodes.go b/Godeps/_workspace/src/github.com/docker/libcontainer/mount/nodes/nodes.go index 6a984e33..322c0c0e 100644 --- a/Godeps/_workspace/src/github.com/docker/libcontainer/mount/nodes/nodes.go +++ b/Godeps/_workspace/src/github.com/docker/libcontainer/mount/nodes/nodes.go @@ -48,5 +48,10 @@ func CreateDeviceNode(rootfs string, node *devices.Device) error { if err := syscall.Mknod(dest, uint32(fileMode), devices.Mkdev(node.MajorNumber, node.MinorNumber)); err != nil && !os.IsExist(err) { return fmt.Errorf("mknod %s %s", node.Path, err) } + + if err := syscall.Chown(dest, int(node.Uid), int(node.Gid)); err != nil { + return fmt.Errorf("chown %s to %d:%d", node.Path, node.Uid, node.Gid) + } + return nil } diff --git a/Godeps/_workspace/src/github.com/docker/libcontainer/syncpipe/sync_pipe.go b/Godeps/_workspace/src/github.com/docker/libcontainer/syncpipe/sync_pipe.go index d2870f52..f73c354d 100644 --- a/Godeps/_workspace/src/github.com/docker/libcontainer/syncpipe/sync_pipe.go +++ b/Godeps/_workspace/src/github.com/docker/libcontainer/syncpipe/sync_pipe.go @@ -77,6 +77,10 @@ func (s *SyncPipe) ReadFromParent(v interface{}) error { } func (s *SyncPipe) ReportChildError(err error) { + // ensure that any data sent from the parent is consumed so it doesn't + // receive ECONNRESET when the child writes to the pipe. + ioutil.ReadAll(s.child) + s.child.Write([]byte(err.Error())) s.CloseChild() } diff --git a/Godeps/_workspace/src/github.com/docker/libcontainer/syncpipe/sync_pipe_test.go b/Godeps/_workspace/src/github.com/docker/libcontainer/syncpipe/sync_pipe_test.go index 6833277a..906e6ed2 100644 --- a/Godeps/_workspace/src/github.com/docker/libcontainer/syncpipe/sync_pipe_test.go +++ b/Godeps/_workspace/src/github.com/docker/libcontainer/syncpipe/sync_pipe_test.go @@ -2,6 +2,7 @@ package syncpipe import ( "fmt" + "syscall" "testing" ) @@ -20,9 +21,17 @@ func TestSendErrorFromChild(t *testing.T) { } }() - expected := "something bad happened" + childfd, err := syscall.Dup(int(pipe.Child().Fd())) + if err != nil { + t.Fatal(err) + } + childPipe, _ := NewSyncPipeFromFd(0, uintptr(childfd)) - pipe.ReportChildError(fmt.Errorf(expected)) + pipe.CloseChild() + pipe.SendToChild(nil) + + expected := "something bad happened" + childPipe.ReportChildError(fmt.Errorf(expected)) childError := pipe.ReadFromChild() if childError == nil {