diff --git a/libcontainer/cgroups/fs/apply_raw.go b/libcontainer/cgroups/fs/apply_raw.go index 60646cd40..c67c55acd 100644 --- a/libcontainer/cgroups/fs/apply_raw.go +++ b/libcontainer/cgroups/fs/apply_raw.go @@ -106,8 +106,6 @@ func (m *Manager) Apply(pid int) (err error) { return nil } - var c = m.Cgroups - d, err := getCgroupData(m.Cgroups, pid) if err != nil { return err @@ -136,13 +134,6 @@ func (m *Manager) Apply(pid int) (err error) { paths[sys.Name()] = p } m.Paths = paths - - if paths["cpu"] != "" { - if err := CheckCpushares(paths["cpu"], c.Resources.CpuShares); err != nil { - return err - } - } - return nil } @@ -181,6 +172,11 @@ func (m *Manager) GetStats() (*cgroups.Stats, error) { func (m *Manager) Set(container *configs.Config) error { for name, path := range m.Paths { + // We can't set this here, because after being applied, memcg doesn't + // allow a non-empty cgroup from having its limits changed. + if name == "memory" { + continue + } sys, err := subsystems.Get(name) if err == errSubsystemDoesNotExist || !cgroups.PathExists(path) { continue @@ -189,6 +185,12 @@ func (m *Manager) Set(container *configs.Config) error { return err } } + + if m.Paths["cpu"] != "" { + if err := CheckCpushares(m.Paths["cpu"], container.Cgroups.Resources.CpuShares); err != nil { + return err + } + } return nil } diff --git a/libcontainer/cgroups/fs/blkio.go b/libcontainer/cgroups/fs/blkio.go index 518cb63f6..a142cb991 100644 --- a/libcontainer/cgroups/fs/blkio.go +++ b/libcontainer/cgroups/fs/blkio.go @@ -22,15 +22,10 @@ func (s *BlkioGroup) Name() string { } func (s *BlkioGroup) Apply(d *cgroupData) error { - dir, err := d.join("blkio") + _, err := d.join("blkio") if err != nil && !cgroups.IsNotFound(err) { return err } - - if err := s.Set(dir, d.config); err != nil { - return err - } - return nil } diff --git a/libcontainer/cgroups/fs/cpu.go b/libcontainer/cgroups/fs/cpu.go index ad5f427ec..a4ef28a60 100644 --- a/libcontainer/cgroups/fs/cpu.go +++ b/libcontainer/cgroups/fs/cpu.go @@ -22,15 +22,10 @@ func (s *CpuGroup) Name() string { func (s *CpuGroup) Apply(d *cgroupData) error { // We always want to join the cpu group, to allow fair cpu scheduling // on a container basis - dir, err := d.join("cpu") + _, err := d.join("cpu") if err != nil && !cgroups.IsNotFound(err) { return err } - - if err := s.Set(dir, d.config); err != nil { - return err - } - return nil } diff --git a/libcontainer/cgroups/fs/cpuset.go b/libcontainer/cgroups/fs/cpuset.go index b7632108f..a9e3ab72a 100644 --- a/libcontainer/cgroups/fs/cpuset.go +++ b/libcontainer/cgroups/fs/cpuset.go @@ -63,11 +63,6 @@ func (s *CpusetGroup) ApplyDir(dir string, cgroup *configs.Cgroup, pid int) erro if err := s.ensureParent(dir, root); err != nil { return err } - // the default values inherit from parent cgroup are already set in - // s.ensureParent, cover these if we have our own - if err := s.Set(dir, cgroup); err != nil { - return err - } // because we are not using d.join we need to place the pid into the procs file // unlike the other subsystems if err := writeFile(dir, "cgroup.procs", strconv.Itoa(pid)); err != nil { diff --git a/libcontainer/cgroups/fs/devices.go b/libcontainer/cgroups/fs/devices.go index a9883eb4b..a41ce801f 100644 --- a/libcontainer/cgroups/fs/devices.go +++ b/libcontainer/cgroups/fs/devices.go @@ -15,17 +15,12 @@ func (s *DevicesGroup) Name() string { } func (s *DevicesGroup) Apply(d *cgroupData) error { - dir, err := d.join("devices") + _, err := d.join("devices") if err != nil { // We will return error even it's `not found` error, devices // cgroup is hard requirement for container's security. return err } - - if err := s.Set(dir, d.config); err != nil { - return err - } - return nil } diff --git a/libcontainer/cgroups/fs/freezer.go b/libcontainer/cgroups/fs/freezer.go index 6aaad4e00..e70dfe3b9 100644 --- a/libcontainer/cgroups/fs/freezer.go +++ b/libcontainer/cgroups/fs/freezer.go @@ -19,15 +19,10 @@ func (s *FreezerGroup) Name() string { } func (s *FreezerGroup) Apply(d *cgroupData) error { - dir, err := d.join("freezer") + _, err := d.join("freezer") if err != nil && !cgroups.IsNotFound(err) { return err } - - if err := s.Set(dir, d.config); err != nil { - return err - } - return nil } diff --git a/libcontainer/cgroups/fs/hugetlb.go b/libcontainer/cgroups/fs/hugetlb.go index ca106da44..2f9727719 100644 --- a/libcontainer/cgroups/fs/hugetlb.go +++ b/libcontainer/cgroups/fs/hugetlb.go @@ -19,15 +19,10 @@ func (s *HugetlbGroup) Name() string { } func (s *HugetlbGroup) Apply(d *cgroupData) error { - dir, err := d.join("hugetlb") + _, err := d.join("hugetlb") if err != nil && !cgroups.IsNotFound(err) { return err } - - if err := s.Set(dir, d.config); err != nil { - return err - } - return nil } diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go index 6b9687ca2..0329692ae 100644 --- a/libcontainer/cgroups/fs/memory.go +++ b/libcontainer/cgroups/fs/memory.go @@ -32,7 +32,6 @@ func (s *MemoryGroup) Apply(d *cgroupData) (err error) { return err } } - if err := s.Set(path, d.config); err != nil { return err } diff --git a/libcontainer/cgroups/fs/net_cls.go b/libcontainer/cgroups/fs/net_cls.go index 638237312..8a4054ba8 100644 --- a/libcontainer/cgroups/fs/net_cls.go +++ b/libcontainer/cgroups/fs/net_cls.go @@ -15,15 +15,10 @@ func (s *NetClsGroup) Name() string { } func (s *NetClsGroup) Apply(d *cgroupData) error { - dir, err := d.join("net_cls") + _, err := d.join("net_cls") if err != nil && !cgroups.IsNotFound(err) { return err } - - if err := s.Set(dir, d.config); err != nil { - return err - } - return nil } diff --git a/libcontainer/cgroups/fs/net_prio.go b/libcontainer/cgroups/fs/net_prio.go index 0dabaae72..d0ab2af89 100644 --- a/libcontainer/cgroups/fs/net_prio.go +++ b/libcontainer/cgroups/fs/net_prio.go @@ -15,15 +15,10 @@ func (s *NetPrioGroup) Name() string { } func (s *NetPrioGroup) Apply(d *cgroupData) error { - dir, err := d.join("net_prio") + _, err := d.join("net_prio") if err != nil && !cgroups.IsNotFound(err) { return err } - - if err := s.Set(dir, d.config); err != nil { - return err - } - return nil } diff --git a/libcontainer/cgroups/fs/pids.go b/libcontainer/cgroups/fs/pids.go index 23893db5d..96cbb896c 100644 --- a/libcontainer/cgroups/fs/pids.go +++ b/libcontainer/cgroups/fs/pids.go @@ -18,15 +18,10 @@ func (s *PidsGroup) Name() string { } func (s *PidsGroup) Apply(d *cgroupData) error { - dir, err := d.join("pids") + _, err := d.join("pids") if err != nil && !cgroups.IsNotFound(err) { return err } - - if err := s.Set(dir, d.config); err != nil { - return err - } - return nil } diff --git a/libcontainer/cgroups/systemd/apply_systemd.go b/libcontainer/cgroups/systemd/apply_systemd.go index b8f92dbc7..952820e62 100644 --- a/libcontainer/cgroups/systemd/apply_systemd.go +++ b/libcontainer/cgroups/systemd/apply_systemd.go @@ -282,13 +282,6 @@ func (m *Manager) Apply(pid int) error { paths[s.Name()] = subsystemPath } m.Paths = paths - - if paths["cpu"] != "" { - if err := fs.CheckCpushares(paths["cpu"], c.Resources.CpuShares); err != nil { - return err - } - } - return nil } @@ -335,80 +328,43 @@ func join(c *configs.Cgroup, subsystem string, pid int) (string, error) { } func joinCpu(c *configs.Cgroup, pid int) error { - path, err := getSubsystemPath(c, "cpu") + _, err := join(c, "cpu", pid) if err != nil && !cgroups.IsNotFound(err) { return err } - if c.Resources.CpuQuota != 0 { - if err = writeFile(path, "cpu.cfs_quota_us", strconv.FormatInt(c.Resources.CpuQuota, 10)); err != nil { - return err - } - } - if c.Resources.CpuPeriod != 0 { - if err = writeFile(path, "cpu.cfs_period_us", strconv.FormatInt(c.Resources.CpuPeriod, 10)); err != nil { - return err - } - } - if c.Resources.CpuRtPeriod != 0 { - if err = writeFile(path, "cpu.rt_period_us", strconv.FormatInt(c.Resources.CpuRtPeriod, 10)); err != nil { - return err - } - } - if c.Resources.CpuRtRuntime != 0 { - if err = writeFile(path, "cpu.rt_runtime_us", strconv.FormatInt(c.Resources.CpuRtRuntime, 10)); err != nil { - return err - } - } - return nil } func joinFreezer(c *configs.Cgroup, pid int) error { - path, err := join(c, "freezer", pid) + _, err := join(c, "freezer", pid) if err != nil && !cgroups.IsNotFound(err) { return err } - freezer, err := subsystems.Get("freezer") - if err != nil { - return err - } - return freezer.Set(path, c) + return nil } func joinNetPrio(c *configs.Cgroup, pid int) error { - path, err := join(c, "net_prio", pid) + _, err := join(c, "net_prio", pid) if err != nil && !cgroups.IsNotFound(err) { return err } - netPrio, err := subsystems.Get("net_prio") - if err != nil { - return err - } - return netPrio.Set(path, c) + return nil } func joinNetCls(c *configs.Cgroup, pid int) error { - path, err := join(c, "net_cls", pid) + _, err := join(c, "net_cls", pid) if err != nil && !cgroups.IsNotFound(err) { return err } - netcls, err := subsystems.Get("net_cls") - if err != nil { - return err - } - return netcls.Set(path, c) + return nil } func joinPids(c *configs.Cgroup, pid int) error { - path, err := join(c, "pids", pid) + _, err := join(c, "pids", pid) if err != nil && !cgroups.IsNotFound(err) { return err } - pids, err := subsystems.Get("pids") - if err != nil { - return err - } - return pids.Set(path, c) + return nil } func getSubsystemPath(c *configs.Cgroup, subsystem string) (string, error) { @@ -476,6 +432,11 @@ func (m *Manager) GetStats() (*cgroups.Stats, error) { func (m *Manager) Set(container *configs.Config) error { for name, path := range m.Paths { + // We can't set this here, because after being applied, memcg doesn't + // allow a non-empty cgroup from having its limits changed. + if name == "memory" { + continue + } sys, err := subsystems.Get(name) if err == errSubsystemDoesNotExist || !cgroups.PathExists(path) { continue @@ -485,6 +446,11 @@ func (m *Manager) Set(container *configs.Config) error { } } + if m.Paths["cpu"] != "" { + if err := fs.CheckCpushares(m.Paths["cpu"], container.Cgroups.Resources.CpuShares); err != nil { + return err + } + } return nil } @@ -504,17 +470,13 @@ func getUnitName(c *configs.Cgroup) string { // because systemd will re-write the device settings if it needs to re-apply the cgroup context. // This happens at least for v208 when any sibling unit is started. func joinDevices(c *configs.Cgroup, pid int) error { - path, err := join(c, "devices", pid) + _, err := join(c, "devices", pid) // Even if it's `not found` error, we'll return err because devices cgroup // is hard requirement for container security. if err != nil { return err } - devices, err := subsystems.Get("devices") - if err != nil { - return err - } - return devices.Set(path, c) + return nil } func setKernelMemory(c *configs.Cgroup) error { @@ -527,9 +489,10 @@ func setKernelMemory(c *configs.Cgroup) error { return err } + // Shamelessly copied from fs.(*MemoryGroup).Set(). This doesn't get called + // by manager.Set, so we need to do it here. if c.Resources.KernelMemory > 0 { - err = writeFile(path, "memory.kmem.limit_in_bytes", strconv.FormatInt(c.Resources.KernelMemory, 10)) - if err != nil { + if err := writeFile(path, "memory.kmem.limit_in_bytes", strconv.FormatInt(c.Resources.KernelMemory, 10)); err != nil { return err } } @@ -538,21 +501,27 @@ func setKernelMemory(c *configs.Cgroup) error { } func joinMemory(c *configs.Cgroup, pid int) error { - path, err := getSubsystemPath(c, "memory") + path, err := join(c, "memory", pid) if err != nil && !cgroups.IsNotFound(err) { return err } - // -1 disables memoryswap - if c.Resources.MemorySwap > 0 { - err = writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(c.Resources.MemorySwap, 10)) - if err != nil { + // Shamelessly copied from fs.(*MemoryGroup).Set(). This doesn't include the + // kernel memory limit (which is done in setKernelMemory). All of these are + // not set by manager.Set, we have do do it here. + + if c.Resources.Memory != 0 { + if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(c.Resources.Memory, 10)); err != nil { return err } } - if c.Resources.MemoryReservation > 0 { - err = writeFile(path, "memory.soft_limit_in_bytes", strconv.FormatInt(c.Resources.MemoryReservation, 10)) - if err != nil { + if c.Resources.MemoryReservation != 0 { + if err := writeFile(path, "memory.soft_limit_in_bytes", strconv.FormatInt(c.Resources.MemoryReservation, 10)); err != nil { + return err + } + } + if c.Resources.MemorySwap > 0 { + if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(c.Resources.MemorySwap, 10)); err != nil { return err } } @@ -561,10 +530,8 @@ func joinMemory(c *configs.Cgroup, pid int) error { return err } } - if c.Resources.MemorySwappiness >= 0 && c.Resources.MemorySwappiness <= 100 { - err = writeFile(path, "memory.swappiness", strconv.FormatInt(c.Resources.MemorySwappiness, 10)) - if err != nil { + if err := writeFile(path, "memory.swappiness", strconv.FormatInt(c.Resources.MemorySwappiness, 10)); err != nil { return err } } else if c.Resources.MemorySwappiness == -1 { @@ -594,68 +561,25 @@ func joinCpuset(c *configs.Cgroup, pid int) error { // expects device path instead of major minor numbers, which is also confusing // for users. So we use fs work around for now. func joinBlkio(c *configs.Cgroup, pid int) error { - path, err := getSubsystemPath(c, "blkio") + _, err := join(c, "blkio", pid) if err != nil { return err } - // systemd doesn't directly support this in the dbus properties - if c.Resources.BlkioLeafWeight != 0 { - if err := writeFile(path, "blkio.leaf_weight", strconv.FormatUint(uint64(c.Resources.BlkioLeafWeight), 10)); err != nil { - return err - } - } - for _, wd := range c.Resources.BlkioWeightDevice { - if err := writeFile(path, "blkio.weight_device", wd.WeightString()); err != nil { - return err - } - if err := writeFile(path, "blkio.leaf_weight_device", wd.LeafWeightString()); err != nil { - return err - } - } - for _, td := range c.Resources.BlkioThrottleReadBpsDevice { - if err := writeFile(path, "blkio.throttle.read_bps_device", td.String()); err != nil { - return err - } - } - for _, td := range c.Resources.BlkioThrottleWriteBpsDevice { - if err := writeFile(path, "blkio.throttle.write_bps_device", td.String()); err != nil { - return err - } - } - for _, td := range c.Resources.BlkioThrottleReadIOPSDevice { - if err := writeFile(path, "blkio.throttle.read_iops_device", td.String()); err != nil { - return err - } - } - for _, td := range c.Resources.BlkioThrottleWriteIOPSDevice { - if err := writeFile(path, "blkio.throttle.write_iops_device", td.String()); err != nil { - return err - } - } - return nil } func joinHugetlb(c *configs.Cgroup, pid int) error { - path, err := join(c, "hugetlb", pid) + _, err := join(c, "hugetlb", pid) if err != nil && !cgroups.IsNotFound(err) { return err } - hugetlb, err := subsystems.Get("hugetlb") - if err != nil { - return err - } - return hugetlb.Set(path, c) + return nil } func joinPerfEvent(c *configs.Cgroup, pid int) error { - path, err := join(c, "perf_event", pid) + _, err := join(c, "perf_event", pid) if err != nil && !cgroups.IsNotFound(err) { return err } - perfEvent, err := subsystems.Get("perf_event") - if err != nil { - return err - } - return perfEvent.Set(path, c) + return nil } diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 114c71b3b..5b32eb108 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -204,6 +204,9 @@ func (p *initProcess) start() (err error) { if err := p.manager.Apply(p.pid()); err != nil { return newSystemError(err) } + if err := p.manager.Set(p.config.Config); err != nil { + return newSystemError(err) + } defer func() { if err != nil { // TODO: should not be the responsibility to call here