From af0710a0f8842d0775a91ba144887b62d842bdd9 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 13 Apr 2021 10:43:03 -0700 Subject: [PATCH 1/3] libct/cg/sd/v2: fix Set argument For some reason, systemd cgroup v2 driver's Set is not using its container argument when generating systemd unit properties. This bug is not detected by our update tests as we run a new binary every time and thus a new instance of a cgroup manager. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/systemd/v2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go index 7a59181bc62..00532ce8142 100644 --- a/libcontainer/cgroups/systemd/v2.go +++ b/libcontainer/cgroups/systemd/v2.go @@ -421,7 +421,7 @@ func (m *unifiedManager) GetStats() (*cgroups.Stats, error) { } func (m *unifiedManager) Set(container *configs.Config) error { - properties, err := genV2ResourcesProperties(m.cgroups, m.dbus) + properties, err := genV2ResourcesProperties(container.Cgroups, m.dbus) if err != nil { return err } From 3f6594675675d4e88901c782462f56497260b1d2 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 29 Apr 2021 15:24:19 -0700 Subject: [PATCH 2/3] libct/cg: make Set accept configs.Resources A cgroup manager's Set method sets cgroup resources, but historically it was accepting configs.Cgroups. Refactor it to accept resources only. This is an improvement from the API point of view, as the method can not change cgroup configuration (such as path to the cgroup etc), it can only set (modify) its resources/limits. This also lays the foundation for complicated resource updates, as now Set has two sets of resources -- the one that was previously specified during cgroup manager creation (or the previous Set), and the one passed in the argument, so it could deduce the difference between these. This is a long term goal though. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/cgroups.go | 2 +- libcontainer/cgroups/fs/blkio.go | 20 ++++++++-------- libcontainer/cgroups/fs/blkio_test.go | 14 +++++------ libcontainer/cgroups/fs/cpu.go | 28 +++++++++++----------- libcontainer/cgroups/fs/cpu_test.go | 4 ++-- libcontainer/cgroups/fs/cpuacct.go | 2 +- libcontainer/cgroups/fs/cpuset.go | 20 ++++++++-------- libcontainer/cgroups/fs/cpuset_test.go | 4 ++-- libcontainer/cgroups/fs/devices.go | 6 ++--- libcontainer/cgroups/fs/devices_test.go | 2 +- libcontainer/cgroups/fs/freezer.go | 6 ++--- libcontainer/cgroups/fs/freezer_test.go | 4 ++-- libcontainer/cgroups/fs/fs.go | 14 +++++------ libcontainer/cgroups/fs/hugetlb.go | 4 ++-- libcontainer/cgroups/fs/hugetlb_test.go | 2 +- libcontainer/cgroups/fs/memory.go | 18 +++++++------- libcontainer/cgroups/fs/memory_test.go | 12 +++++----- libcontainer/cgroups/fs/name.go | 2 +- libcontainer/cgroups/fs/net_cls.go | 6 ++--- libcontainer/cgroups/fs/net_cls_test.go | 2 +- libcontainer/cgroups/fs/net_prio.go | 4 ++-- libcontainer/cgroups/fs/net_prio_test.go | 2 +- libcontainer/cgroups/fs/perf_event.go | 2 +- libcontainer/cgroups/fs/pids.go | 8 +++---- libcontainer/cgroups/fs/pids_test.go | 4 ++-- libcontainer/cgroups/fs2/cpu.go | 9 ++++--- libcontainer/cgroups/fs2/cpuset.go | 16 ++++++------- libcontainer/cgroups/fs2/create.go | 30 ++++++++++++------------ libcontainer/cgroups/fs2/devices.go | 13 +++++----- libcontainer/cgroups/fs2/fs2.go | 27 ++++++++++----------- libcontainer/cgroups/fs2/hugetlb.go | 10 ++++---- libcontainer/cgroups/fs2/io.go | 30 ++++++++++++------------ libcontainer/cgroups/fs2/memory.go | 17 +++++++------- libcontainer/cgroups/fs2/pids.go | 10 ++++---- libcontainer/cgroups/systemd/v1.go | 21 ++++++++--------- libcontainer/cgroups/systemd/v2.go | 9 ++++--- libcontainer/container_linux.go | 8 +++---- libcontainer/container_linux_test.go | 2 +- libcontainer/process_linux.go | 4 ++-- 39 files changed, 195 insertions(+), 203 deletions(-) diff --git a/libcontainer/cgroups/cgroups.go b/libcontainer/cgroups/cgroups.go index c418fbae9af..1309540c19f 100644 --- a/libcontainer/cgroups/cgroups.go +++ b/libcontainer/cgroups/cgroups.go @@ -30,7 +30,7 @@ type Manager interface { Path(string) string // Sets the cgroup as configured. - Set(container *configs.Config) error + Set(r *configs.Resources) error // GetPaths returns cgroup path(s) to save in a state file in order to restore later. // diff --git a/libcontainer/cgroups/fs/blkio.go b/libcontainer/cgroups/fs/blkio.go index dca2a622078..05fec260307 100644 --- a/libcontainer/cgroups/fs/blkio.go +++ b/libcontainer/cgroups/fs/blkio.go @@ -25,19 +25,19 @@ func (s *BlkioGroup) Apply(path string, d *cgroupData) error { return join(path, d.pid) } -func (s *BlkioGroup) Set(path string, cgroup *configs.Cgroup) error { - if cgroup.Resources.BlkioWeight != 0 { - if err := fscommon.WriteFile(path, "blkio.weight", strconv.FormatUint(uint64(cgroup.Resources.BlkioWeight), 10)); err != nil { +func (s *BlkioGroup) Set(path string, r *configs.Resources) error { + if r.BlkioWeight != 0 { + if err := fscommon.WriteFile(path, "blkio.weight", strconv.FormatUint(uint64(r.BlkioWeight), 10)); err != nil { return err } } - if cgroup.Resources.BlkioLeafWeight != 0 { - if err := fscommon.WriteFile(path, "blkio.leaf_weight", strconv.FormatUint(uint64(cgroup.Resources.BlkioLeafWeight), 10)); err != nil { + if r.BlkioLeafWeight != 0 { + if err := fscommon.WriteFile(path, "blkio.leaf_weight", strconv.FormatUint(uint64(r.BlkioLeafWeight), 10)); err != nil { return err } } - for _, wd := range cgroup.Resources.BlkioWeightDevice { + for _, wd := range r.BlkioWeightDevice { if err := fscommon.WriteFile(path, "blkio.weight_device", wd.WeightString()); err != nil { return err } @@ -45,22 +45,22 @@ func (s *BlkioGroup) Set(path string, cgroup *configs.Cgroup) error { return err } } - for _, td := range cgroup.Resources.BlkioThrottleReadBpsDevice { + for _, td := range r.BlkioThrottleReadBpsDevice { if err := fscommon.WriteFile(path, "blkio.throttle.read_bps_device", td.String()); err != nil { return err } } - for _, td := range cgroup.Resources.BlkioThrottleWriteBpsDevice { + for _, td := range r.BlkioThrottleWriteBpsDevice { if err := fscommon.WriteFile(path, "blkio.throttle.write_bps_device", td.String()); err != nil { return err } } - for _, td := range cgroup.Resources.BlkioThrottleReadIOPSDevice { + for _, td := range r.BlkioThrottleReadIOPSDevice { if err := fscommon.WriteFile(path, "blkio.throttle.read_iops_device", td.String()); err != nil { return err } } - for _, td := range cgroup.Resources.BlkioThrottleWriteIOPSDevice { + for _, td := range r.BlkioThrottleWriteIOPSDevice { if err := fscommon.WriteFile(path, "blkio.throttle.write_iops_device", td.String()); err != nil { return err } diff --git a/libcontainer/cgroups/fs/blkio_test.go b/libcontainer/cgroups/fs/blkio_test.go index 235648052a2..bd96d242bee 100644 --- a/libcontainer/cgroups/fs/blkio_test.go +++ b/libcontainer/cgroups/fs/blkio_test.go @@ -185,7 +185,7 @@ func TestBlkioSetWeight(t *testing.T) { helper.CgroupData.config.Resources.BlkioWeight = weightAfter blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -216,7 +216,7 @@ func TestBlkioSetWeightDevice(t *testing.T) { helper.CgroupData.config.Resources.BlkioWeightDevice = []*configs.WeightDevice{wd} blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -253,7 +253,7 @@ func TestBlkioSetMultipleWeightDevice(t *testing.T) { helper.CgroupData.config.Resources.BlkioWeightDevice = []*configs.WeightDevice{wd1, wd2} blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -746,7 +746,7 @@ func TestBlkioSetThrottleReadBpsDevice(t *testing.T) { helper.CgroupData.config.Resources.BlkioThrottleReadBpsDevice = []*configs.ThrottleDevice{td} blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -776,7 +776,7 @@ func TestBlkioSetThrottleWriteBpsDevice(t *testing.T) { helper.CgroupData.config.Resources.BlkioThrottleWriteBpsDevice = []*configs.ThrottleDevice{td} blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -806,7 +806,7 @@ func TestBlkioSetThrottleReadIOpsDevice(t *testing.T) { helper.CgroupData.config.Resources.BlkioThrottleReadIOPSDevice = []*configs.ThrottleDevice{td} blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -836,7 +836,7 @@ func TestBlkioSetThrottleWriteIOpsDevice(t *testing.T) { helper.CgroupData.config.Resources.BlkioThrottleWriteIOPSDevice = []*configs.ThrottleDevice{td} blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/cpu.go b/libcontainer/cgroups/fs/cpu.go index 194f3a542e3..975df1c4e8a 100644 --- a/libcontainer/cgroups/fs/cpu.go +++ b/libcontainer/cgroups/fs/cpu.go @@ -32,7 +32,7 @@ func (s *CpuGroup) Apply(path string, d *cgroupData) error { // We should set the real-Time group scheduling settings before moving // in the process because if the process is already in SCHED_RR mode // and no RT bandwidth is set, adding it will fail. - if err := s.SetRtSched(path, d.config); err != nil { + if err := s.SetRtSched(path, d.config.Resources); err != nil { return err } // Since we are not using join(), we need to place the pid @@ -40,23 +40,23 @@ func (s *CpuGroup) Apply(path string, d *cgroupData) error { return cgroups.WriteCgroupProc(path, d.pid) } -func (s *CpuGroup) SetRtSched(path string, cgroup *configs.Cgroup) error { - if cgroup.Resources.CpuRtPeriod != 0 { - if err := fscommon.WriteFile(path, "cpu.rt_period_us", strconv.FormatUint(cgroup.Resources.CpuRtPeriod, 10)); err != nil { +func (s *CpuGroup) SetRtSched(path string, r *configs.Resources) error { + if r.CpuRtPeriod != 0 { + if err := fscommon.WriteFile(path, "cpu.rt_period_us", strconv.FormatUint(r.CpuRtPeriod, 10)); err != nil { return err } } - if cgroup.Resources.CpuRtRuntime != 0 { - if err := fscommon.WriteFile(path, "cpu.rt_runtime_us", strconv.FormatInt(cgroup.Resources.CpuRtRuntime, 10)); err != nil { + if r.CpuRtRuntime != 0 { + if err := fscommon.WriteFile(path, "cpu.rt_runtime_us", strconv.FormatInt(r.CpuRtRuntime, 10)); err != nil { return err } } return nil } -func (s *CpuGroup) Set(path string, cgroup *configs.Cgroup) error { - if cgroup.Resources.CpuShares != 0 { - shares := cgroup.Resources.CpuShares +func (s *CpuGroup) Set(path string, r *configs.Resources) error { + if r.CpuShares != 0 { + shares := r.CpuShares if err := fscommon.WriteFile(path, "cpu.shares", strconv.FormatUint(shares, 10)); err != nil { return err } @@ -72,17 +72,17 @@ func (s *CpuGroup) Set(path string, cgroup *configs.Cgroup) error { return fmt.Errorf("the minimum allowed cpu-shares is %d", sharesRead) } } - if cgroup.Resources.CpuPeriod != 0 { - if err := fscommon.WriteFile(path, "cpu.cfs_period_us", strconv.FormatUint(cgroup.Resources.CpuPeriod, 10)); err != nil { + if r.CpuPeriod != 0 { + if err := fscommon.WriteFile(path, "cpu.cfs_period_us", strconv.FormatUint(r.CpuPeriod, 10)); err != nil { return err } } - if cgroup.Resources.CpuQuota != 0 { - if err := fscommon.WriteFile(path, "cpu.cfs_quota_us", strconv.FormatInt(cgroup.Resources.CpuQuota, 10)); err != nil { + if r.CpuQuota != 0 { + if err := fscommon.WriteFile(path, "cpu.cfs_quota_us", strconv.FormatInt(r.CpuQuota, 10)); err != nil { return err } } - return s.SetRtSched(path, cgroup) + return s.SetRtSched(path, r) } func (s *CpuGroup) GetStats(path string, stats *cgroups.Stats) error { diff --git a/libcontainer/cgroups/fs/cpu_test.go b/libcontainer/cgroups/fs/cpu_test.go index 9adc9b31cab..b615594e672 100644 --- a/libcontainer/cgroups/fs/cpu_test.go +++ b/libcontainer/cgroups/fs/cpu_test.go @@ -26,7 +26,7 @@ func TestCpuSetShares(t *testing.T) { helper.CgroupData.config.Resources.CpuShares = sharesAfter cpu := &CpuGroup{} - if err := cpu.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := cpu.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -67,7 +67,7 @@ func TestCpuSetBandWidth(t *testing.T) { helper.CgroupData.config.Resources.CpuRtRuntime = rtRuntimeAfter helper.CgroupData.config.Resources.CpuRtPeriod = rtPeriodAfter cpu := &CpuGroup{} - if err := cpu.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := cpu.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/cpuacct.go b/libcontainer/cgroups/fs/cpuacct.go index 129de6a01cd..0445cd45b4d 100644 --- a/libcontainer/cgroups/fs/cpuacct.go +++ b/libcontainer/cgroups/fs/cpuacct.go @@ -43,7 +43,7 @@ func (s *CpuacctGroup) Apply(path string, d *cgroupData) error { return join(path, d.pid) } -func (s *CpuacctGroup) Set(path string, cgroup *configs.Cgroup) error { +func (s *CpuacctGroup) Set(_ string, _ *configs.Resources) error { return nil } diff --git a/libcontainer/cgroups/fs/cpuset.go b/libcontainer/cgroups/fs/cpuset.go index 5bde025b4b9..0591122bca5 100644 --- a/libcontainer/cgroups/fs/cpuset.go +++ b/libcontainer/cgroups/fs/cpuset.go @@ -24,17 +24,17 @@ func (s *CpusetGroup) Name() string { } func (s *CpusetGroup) Apply(path string, d *cgroupData) error { - return s.ApplyDir(path, d.config, d.pid) + return s.ApplyDir(path, d.config.Resources, d.pid) } -func (s *CpusetGroup) Set(path string, cgroup *configs.Cgroup) error { - if cgroup.Resources.CpusetCpus != "" { - if err := fscommon.WriteFile(path, "cpuset.cpus", cgroup.Resources.CpusetCpus); err != nil { +func (s *CpusetGroup) Set(path string, r *configs.Resources) error { + if r.CpusetCpus != "" { + if err := fscommon.WriteFile(path, "cpuset.cpus", r.CpusetCpus); err != nil { return err } } - if cgroup.Resources.CpusetMems != "" { - if err := fscommon.WriteFile(path, "cpuset.mems", cgroup.Resources.CpusetMems); err != nil { + if r.CpusetMems != "" { + if err := fscommon.WriteFile(path, "cpuset.mems", r.CpusetMems); err != nil { return err } } @@ -144,7 +144,7 @@ func (s *CpusetGroup) GetStats(path string, stats *cgroups.Stats) error { return nil } -func (s *CpusetGroup) ApplyDir(dir string, cgroup *configs.Cgroup, pid int) error { +func (s *CpusetGroup) ApplyDir(dir string, r *configs.Resources, pid int) error { // This might happen if we have no cpuset cgroup mounted. // Just do nothing and don't fail. if dir == "" { @@ -166,7 +166,7 @@ func (s *CpusetGroup) ApplyDir(dir string, cgroup *configs.Cgroup, pid int) erro // specified configs, otherwise, inherit from parent. This makes // cpuset configs work correctly with 'cpuset.cpu_exclusive', and // keep backward compatibility. - if err := s.ensureCpusAndMems(dir, cgroup); err != nil { + if err := s.ensureCpusAndMems(dir, r); err != nil { return err } @@ -241,8 +241,8 @@ func isEmptyCpuset(str string) bool { return str == "" || str == "\n" } -func (s *CpusetGroup) ensureCpusAndMems(path string, cgroup *configs.Cgroup) error { - if err := s.Set(path, cgroup); err != nil { +func (s *CpusetGroup) ensureCpusAndMems(path string, r *configs.Resources) error { + if err := s.Set(path, r); err != nil { return err } return cpusetCopyIfNeeded(path, filepath.Dir(path)) diff --git a/libcontainer/cgroups/fs/cpuset_test.go b/libcontainer/cgroups/fs/cpuset_test.go index 8a49e440fef..5ac9a037244 100644 --- a/libcontainer/cgroups/fs/cpuset_test.go +++ b/libcontainer/cgroups/fs/cpuset_test.go @@ -53,7 +53,7 @@ func TestCPUSetSetCpus(t *testing.T) { helper.CgroupData.config.Resources.CpusetCpus = cpusAfter cpuset := &CpusetGroup{} - if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -82,7 +82,7 @@ func TestCPUSetSetMems(t *testing.T) { helper.CgroupData.config.Resources.CpusetMems = memsAfter cpuset := &CpusetGroup{} - if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/devices.go b/libcontainer/cgroups/fs/devices.go index a098e3b793e..048f11398a9 100644 --- a/libcontainer/cgroups/fs/devices.go +++ b/libcontainer/cgroups/fs/devices.go @@ -54,8 +54,8 @@ func buildEmulator(rules []*devices.Rule) (*cgroupdevices.Emulator, error) { return emu, nil } -func (s *DevicesGroup) Set(path string, cgroup *configs.Cgroup) error { - if userns.RunningInUserNS() || cgroup.SkipDevices { +func (s *DevicesGroup) Set(path string, r *configs.Resources) error { + if userns.RunningInUserNS() || r.SkipDevices { return nil } @@ -65,7 +65,7 @@ func (s *DevicesGroup) Set(path string, cgroup *configs.Cgroup) error { if err != nil { return err } - target, err := buildEmulator(cgroup.Resources.Devices) + target, err := buildEmulator(r.Devices) if err != nil { return err } diff --git a/libcontainer/cgroups/fs/devices_test.go b/libcontainer/cgroups/fs/devices_test.go index 752fadd901f..a0ef145d8a5 100644 --- a/libcontainer/cgroups/fs/devices_test.go +++ b/libcontainer/cgroups/fs/devices_test.go @@ -30,7 +30,7 @@ func TestDevicesSetAllow(t *testing.T) { } d := &DevicesGroup{testingSkipFinalCheck: true} - if err := d.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := d.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/freezer.go b/libcontainer/cgroups/fs/freezer.go index 5aefdf511fe..37bbf38ff40 100644 --- a/libcontainer/cgroups/fs/freezer.go +++ b/libcontainer/cgroups/fs/freezer.go @@ -27,8 +27,8 @@ func (s *FreezerGroup) Apply(path string, d *cgroupData) error { return join(path, d.pid) } -func (s *FreezerGroup) Set(path string, cgroup *configs.Cgroup) (Err error) { - switch cgroup.Resources.Freezer { +func (s *FreezerGroup) Set(path string, r *configs.Resources) (Err error) { + switch r.Freezer { case configs.Frozen: defer func() { if Err != nil { @@ -90,7 +90,7 @@ func (s *FreezerGroup) Set(path string, cgroup *configs.Cgroup) (Err error) { case configs.Undefined: return nil default: - return fmt.Errorf("Invalid argument '%s' to freezer.state", string(cgroup.Resources.Freezer)) + return fmt.Errorf("Invalid argument '%s' to freezer.state", string(r.Freezer)) } } diff --git a/libcontainer/cgroups/fs/freezer_test.go b/libcontainer/cgroups/fs/freezer_test.go index ad80261c805..e41b1ad4a57 100644 --- a/libcontainer/cgroups/fs/freezer_test.go +++ b/libcontainer/cgroups/fs/freezer_test.go @@ -19,7 +19,7 @@ func TestFreezerSetState(t *testing.T) { helper.CgroupData.config.Resources.Freezer = configs.Thawed freezer := &FreezerGroup{} - if err := freezer.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := freezer.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -42,7 +42,7 @@ func TestFreezerSetInvalidState(t *testing.T) { helper.CgroupData.config.Resources.Freezer = invalidArg freezer := &FreezerGroup{} - if err := freezer.Set(helper.CgroupPath, helper.CgroupData.config); err == nil { + if err := freezer.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err == nil { t.Fatal("Failed to return invalid argument error") } } diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go index e8f3890863f..7dc4b9e3784 100644 --- a/libcontainer/cgroups/fs/fs.go +++ b/libcontainer/cgroups/fs/fs.go @@ -44,8 +44,8 @@ type subsystem interface { GetStats(path string, stats *cgroups.Stats) error // Creates and joins the cgroup represented by 'cgroupData'. Apply(path string, c *cgroupData) error - // Set the cgroup represented by cgroup. - Set(path string, cgroup *configs.Cgroup) error + // Set sets the cgroup resources. + Set(path string, r *configs.Resources) error } type manager struct { @@ -274,8 +274,8 @@ func (m *manager) GetStats() (*cgroups.Stats, error) { return stats, nil } -func (m *manager) Set(container *configs.Config) error { - if container.Cgroups == nil { +func (m *manager) Set(r *configs.Resources) error { + if r == nil { return nil } @@ -284,7 +284,7 @@ func (m *manager) Set(container *configs.Config) error { if m.cgroups != nil && m.cgroups.Paths != nil { return nil } - if container.Cgroups.Resources.Unified != nil { + if r.Unified != nil { return cgroups.ErrV1NoUnified } @@ -292,7 +292,7 @@ func (m *manager) Set(container *configs.Config) error { defer m.mu.Unlock() for _, sys := range subsystems { path := m.paths[sys.Name()] - if err := sys.Set(path, container.Cgroups); err != nil { + if err := sys.Set(path, r); err != nil { if m.rootless && sys.Name() == "devices" { continue } @@ -322,7 +322,7 @@ func (m *manager) Freeze(state configs.FreezerState) error { prevState := m.cgroups.Resources.Freezer m.cgroups.Resources.Freezer = state freezer := &FreezerGroup{} - if err := freezer.Set(path, m.cgroups); err != nil { + if err := freezer.Set(path, m.cgroups.Resources); err != nil { m.cgroups.Resources.Freezer = prevState return err } diff --git a/libcontainer/cgroups/fs/hugetlb.go b/libcontainer/cgroups/fs/hugetlb.go index 861793411e6..cf2b93bc536 100644 --- a/libcontainer/cgroups/fs/hugetlb.go +++ b/libcontainer/cgroups/fs/hugetlb.go @@ -22,8 +22,8 @@ func (s *HugetlbGroup) Apply(path string, d *cgroupData) error { return join(path, d.pid) } -func (s *HugetlbGroup) Set(path string, cgroup *configs.Cgroup) error { - for _, hugetlb := range cgroup.Resources.HugetlbLimit { +func (s *HugetlbGroup) Set(path string, r *configs.Resources) error { + for _, hugetlb := range r.HugetlbLimit { if err := fscommon.WriteFile(path, "hugetlb."+hugetlb.Pagesize+".limit_in_bytes", strconv.FormatUint(hugetlb.Limit, 10)); err != nil { return err } diff --git a/libcontainer/cgroups/fs/hugetlb_test.go b/libcontainer/cgroups/fs/hugetlb_test.go index 2d17ca67e22..01b6889d4a0 100644 --- a/libcontainer/cgroups/fs/hugetlb_test.go +++ b/libcontainer/cgroups/fs/hugetlb_test.go @@ -48,7 +48,7 @@ func TestHugetlbSetHugetlb(t *testing.T) { }, } hugetlb := &HugetlbGroup{} - if err := hugetlb.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := hugetlb.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } } diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go index 2007f18a2fb..dc27cb9e9c9 100644 --- a/libcontainer/cgroups/fs/memory.go +++ b/libcontainer/cgroups/fs/memory.go @@ -110,32 +110,32 @@ func setMemoryAndSwap(path string, r *configs.Resources) error { return nil } -func (s *MemoryGroup) Set(path string, cgroup *configs.Cgroup) error { - if err := setMemoryAndSwap(path, cgroup.Resources); err != nil { +func (s *MemoryGroup) Set(path string, r *configs.Resources) error { + if err := setMemoryAndSwap(path, r); err != nil { return err } // ignore KernelMemory and KernelMemoryTCP - if cgroup.Resources.MemoryReservation != 0 { - if err := fscommon.WriteFile(path, "memory.soft_limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemoryReservation, 10)); err != nil { + if r.MemoryReservation != 0 { + if err := fscommon.WriteFile(path, "memory.soft_limit_in_bytes", strconv.FormatInt(r.MemoryReservation, 10)); err != nil { return err } } - if cgroup.Resources.OomKillDisable { + if r.OomKillDisable { if err := fscommon.WriteFile(path, "memory.oom_control", "1"); err != nil { return err } } - if cgroup.Resources.MemorySwappiness == nil || int64(*cgroup.Resources.MemorySwappiness) == -1 { + if r.MemorySwappiness == nil || int64(*r.MemorySwappiness) == -1 { return nil - } else if *cgroup.Resources.MemorySwappiness <= 100 { - if err := fscommon.WriteFile(path, "memory.swappiness", strconv.FormatUint(*cgroup.Resources.MemorySwappiness, 10)); err != nil { + } else if *r.MemorySwappiness <= 100 { + if err := fscommon.WriteFile(path, "memory.swappiness", strconv.FormatUint(*r.MemorySwappiness, 10)); err != nil { return err } } else { - return fmt.Errorf("invalid value:%d. valid memory swappiness range is 0-100", *cgroup.Resources.MemorySwappiness) + return fmt.Errorf("invalid value:%d. valid memory swappiness range is 0-100", *r.MemorySwappiness) } return nil diff --git a/libcontainer/cgroups/fs/memory_test.go b/libcontainer/cgroups/fs/memory_test.go index 701c99da4a8..589e2be325b 100644 --- a/libcontainer/cgroups/fs/memory_test.go +++ b/libcontainer/cgroups/fs/memory_test.go @@ -58,7 +58,7 @@ func TestMemorySetMemory(t *testing.T) { helper.CgroupData.config.Resources.Memory = memoryAfter helper.CgroupData.config.Resources.MemoryReservation = reservationAfter memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -94,7 +94,7 @@ func TestMemorySetMemoryswap(t *testing.T) { helper.CgroupData.config.Resources.MemorySwap = memoryswapAfter memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -131,7 +131,7 @@ func TestMemorySetMemoryLargerThanSwap(t *testing.T) { helper.CgroupData.config.Resources.Memory = memoryAfter helper.CgroupData.config.Resources.MemorySwap = memoryswapAfter memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -170,7 +170,7 @@ func TestMemorySetSwapSmallerThanMemory(t *testing.T) { helper.CgroupData.config.Resources.Memory = memoryAfter helper.CgroupData.config.Resources.MemorySwap = memoryswapAfter memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -203,7 +203,7 @@ func TestMemorySetMemorySwappinessDefault(t *testing.T) { helper.CgroupData.config.Resources.MemorySwappiness = &swappinessAfter memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -414,7 +414,7 @@ func TestMemorySetOomControl(t *testing.T) { }) memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/name.go b/libcontainer/cgroups/fs/name.go index 7cada9149d4..94a94b5e8d0 100644 --- a/libcontainer/cgroups/fs/name.go +++ b/libcontainer/cgroups/fs/name.go @@ -24,7 +24,7 @@ func (s *NameGroup) Apply(path string, d *cgroupData) error { return nil } -func (s *NameGroup) Set(path string, cgroup *configs.Cgroup) error { +func (s *NameGroup) Set(_ string, _ *configs.Resources) error { return nil } diff --git a/libcontainer/cgroups/fs/net_cls.go b/libcontainer/cgroups/fs/net_cls.go index 901e9554431..c824db34dc0 100644 --- a/libcontainer/cgroups/fs/net_cls.go +++ b/libcontainer/cgroups/fs/net_cls.go @@ -21,9 +21,9 @@ func (s *NetClsGroup) Apply(path string, d *cgroupData) error { return join(path, d.pid) } -func (s *NetClsGroup) Set(path string, cgroup *configs.Cgroup) error { - if cgroup.Resources.NetClsClassid != 0 { - if err := fscommon.WriteFile(path, "net_cls.classid", strconv.FormatUint(uint64(cgroup.Resources.NetClsClassid), 10)); err != nil { +func (s *NetClsGroup) Set(path string, r *configs.Resources) error { + if r.NetClsClassid != 0 { + if err := fscommon.WriteFile(path, "net_cls.classid", strconv.FormatUint(uint64(r.NetClsClassid), 10)); err != nil { return err } } diff --git a/libcontainer/cgroups/fs/net_cls_test.go b/libcontainer/cgroups/fs/net_cls_test.go index 602133a265d..1bb99eb22db 100644 --- a/libcontainer/cgroups/fs/net_cls_test.go +++ b/libcontainer/cgroups/fs/net_cls_test.go @@ -24,7 +24,7 @@ func TestNetClsSetClassid(t *testing.T) { helper.CgroupData.config.Resources.NetClsClassid = classidAfter netcls := &NetClsGroup{} - if err := netcls.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := netcls.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/net_prio.go b/libcontainer/cgroups/fs/net_prio.go index a9645de261b..ce4bebc26c7 100644 --- a/libcontainer/cgroups/fs/net_prio.go +++ b/libcontainer/cgroups/fs/net_prio.go @@ -19,8 +19,8 @@ func (s *NetPrioGroup) Apply(path string, d *cgroupData) error { return join(path, d.pid) } -func (s *NetPrioGroup) Set(path string, cgroup *configs.Cgroup) error { - for _, prioMap := range cgroup.Resources.NetPrioIfpriomap { +func (s *NetPrioGroup) Set(path string, r *configs.Resources) error { + for _, prioMap := range r.NetPrioIfpriomap { if err := fscommon.WriteFile(path, "net_prio.ifpriomap", prioMap.CgroupString()); err != nil { return err } diff --git a/libcontainer/cgroups/fs/net_prio_test.go b/libcontainer/cgroups/fs/net_prio_test.go index 2ce8e1922d9..ef7cdb04e71 100644 --- a/libcontainer/cgroups/fs/net_prio_test.go +++ b/libcontainer/cgroups/fs/net_prio_test.go @@ -25,7 +25,7 @@ func TestNetPrioSetIfPrio(t *testing.T) { helper.CgroupData.config.Resources.NetPrioIfpriomap = prioMap netPrio := &NetPrioGroup{} - if err := netPrio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := netPrio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/perf_event.go b/libcontainer/cgroups/fs/perf_event.go index dd1e98d0aed..5da4845fb84 100644 --- a/libcontainer/cgroups/fs/perf_event.go +++ b/libcontainer/cgroups/fs/perf_event.go @@ -18,7 +18,7 @@ func (s *PerfEventGroup) Apply(path string, d *cgroupData) error { return join(path, d.pid) } -func (s *PerfEventGroup) Set(path string, cgroup *configs.Cgroup) error { +func (s *PerfEventGroup) Set(_ string, _ *configs.Resources) error { return nil } diff --git a/libcontainer/cgroups/fs/pids.go b/libcontainer/cgroups/fs/pids.go index 6614df88a78..c12dafd1659 100644 --- a/libcontainer/cgroups/fs/pids.go +++ b/libcontainer/cgroups/fs/pids.go @@ -23,13 +23,13 @@ func (s *PidsGroup) Apply(path string, d *cgroupData) error { return join(path, d.pid) } -func (s *PidsGroup) Set(path string, cgroup *configs.Cgroup) error { - if cgroup.Resources.PidsLimit != 0 { +func (s *PidsGroup) Set(path string, r *configs.Resources) error { + if r.PidsLimit != 0 { // "max" is the fallback value. limit := "max" - if cgroup.Resources.PidsLimit > 0 { - limit = strconv.FormatInt(cgroup.Resources.PidsLimit, 10) + if r.PidsLimit > 0 { + limit = strconv.FormatInt(r.PidsLimit, 10) } if err := fscommon.WriteFile(path, "pids.max", limit); err != nil { diff --git a/libcontainer/cgroups/fs/pids_test.go b/libcontainer/cgroups/fs/pids_test.go index 66f3aa3315e..7f315747563 100644 --- a/libcontainer/cgroups/fs/pids_test.go +++ b/libcontainer/cgroups/fs/pids_test.go @@ -25,7 +25,7 @@ func TestPidsSetMax(t *testing.T) { helper.CgroupData.config.Resources.PidsLimit = maxLimited pids := &PidsGroup{} - if err := pids.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := pids.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } @@ -49,7 +49,7 @@ func TestPidsSetUnlimited(t *testing.T) { helper.CgroupData.config.Resources.PidsLimit = maxUnlimited pids := &PidsGroup{} - if err := pids.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + if err := pids.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs2/cpu.go b/libcontainer/cgroups/fs2/cpu.go index 8ace0ce7b14..404800e99e1 100644 --- a/libcontainer/cgroups/fs2/cpu.go +++ b/libcontainer/cgroups/fs2/cpu.go @@ -12,15 +12,14 @@ import ( "github.com/opencontainers/runc/libcontainer/configs" ) -func isCpuSet(cgroup *configs.Cgroup) bool { - return cgroup.Resources.CpuWeight != 0 || cgroup.Resources.CpuQuota != 0 || cgroup.Resources.CpuPeriod != 0 +func isCpuSet(r *configs.Resources) bool { + return r.CpuWeight != 0 || r.CpuQuota != 0 || r.CpuPeriod != 0 } -func setCpu(dirPath string, cgroup *configs.Cgroup) error { - if !isCpuSet(cgroup) { +func setCpu(dirPath string, r *configs.Resources) error { + if !isCpuSet(r) { return nil } - r := cgroup.Resources // NOTE: .CpuShares is not used here. Conversion is the caller's responsibility. if r.CpuWeight != 0 { diff --git a/libcontainer/cgroups/fs2/cpuset.go b/libcontainer/cgroups/fs2/cpuset.go index fb4456b43c1..713c430dc4d 100644 --- a/libcontainer/cgroups/fs2/cpuset.go +++ b/libcontainer/cgroups/fs2/cpuset.go @@ -7,22 +7,22 @@ import ( "github.com/opencontainers/runc/libcontainer/configs" ) -func isCpusetSet(cgroup *configs.Cgroup) bool { - return cgroup.Resources.CpusetCpus != "" || cgroup.Resources.CpusetMems != "" +func isCpusetSet(r *configs.Resources) bool { + return r.CpusetCpus != "" || r.CpusetMems != "" } -func setCpuset(dirPath string, cgroup *configs.Cgroup) error { - if !isCpusetSet(cgroup) { +func setCpuset(dirPath string, r *configs.Resources) error { + if !isCpusetSet(r) { return nil } - if cgroup.Resources.CpusetCpus != "" { - if err := fscommon.WriteFile(dirPath, "cpuset.cpus", cgroup.Resources.CpusetCpus); err != nil { + if r.CpusetCpus != "" { + if err := fscommon.WriteFile(dirPath, "cpuset.cpus", r.CpusetCpus); err != nil { return err } } - if cgroup.Resources.CpusetMems != "" { - if err := fscommon.WriteFile(dirPath, "cpuset.mems", cgroup.Resources.CpusetMems); err != nil { + if r.CpusetMems != "" { + if err := fscommon.WriteFile(dirPath, "cpuset.mems", r.CpusetMems); err != nil { return err } } diff --git a/libcontainer/cgroups/fs2/create.go b/libcontainer/cgroups/fs2/create.go index 30cf51ee68b..f7a9999b6e0 100644 --- a/libcontainer/cgroups/fs2/create.go +++ b/libcontainer/cgroups/fs2/create.go @@ -10,7 +10,7 @@ import ( "github.com/opencontainers/runc/libcontainer/configs" ) -func supportedControllers(cgroup *configs.Cgroup) (string, error) { +func supportedControllers() (string, error) { return fscommon.ReadFile(UnifiedMountpoint, "/cgroup.controllers") } @@ -18,13 +18,13 @@ func supportedControllers(cgroup *configs.Cgroup) (string, error) { // based on (1) controllers available and (2) resources that are being set. // We don't check "pseudo" controllers such as // "freezer" and "devices". -func needAnyControllers(cgroup *configs.Cgroup) (bool, error) { - if cgroup == nil { +func needAnyControllers(r *configs.Resources) (bool, error) { + if r == nil { return false, nil } // list of all available controllers - content, err := supportedControllers(cgroup) + content, err := supportedControllers() if err != nil { return false, err } @@ -39,22 +39,22 @@ func needAnyControllers(cgroup *configs.Cgroup) (bool, error) { return ok } - if isPidsSet(cgroup) && have("pids") { + if isPidsSet(r) && have("pids") { return true, nil } - if isMemorySet(cgroup) && have("memory") { + if isMemorySet(r) && have("memory") { return true, nil } - if isIoSet(cgroup) && have("io") { + if isIoSet(r) && have("io") { return true, nil } - if isCpuSet(cgroup) && have("cpu") { + if isCpuSet(r) && have("cpu") { return true, nil } - if isCpusetSet(cgroup) && have("cpuset") { + if isCpusetSet(r) && have("cpuset") { return true, nil } - if isHugeTlbSet(cgroup) && have("hugetlb") { + if isHugeTlbSet(r) && have("hugetlb") { return true, nil } @@ -64,8 +64,8 @@ func needAnyControllers(cgroup *configs.Cgroup) (bool, error) { // containsDomainController returns whether the current config contains domain controller or not. // Refer to: http://man7.org/linux/man-pages/man7/cgroups.7.html // As at Linux 4.19, the following controllers are threaded: cpu, perf_event, and pids. -func containsDomainController(cg *configs.Cgroup) bool { - return isMemorySet(cg) || isIoSet(cg) || isCpuSet(cg) || isHugeTlbSet(cg) +func containsDomainController(r *configs.Resources) bool { + return isMemorySet(r) || isIoSet(r) || isCpuSet(r) || isHugeTlbSet(r) } // CreateCgroupPath creates cgroupv2 path, enabling all the supported controllers. @@ -74,7 +74,7 @@ func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) { return fmt.Errorf("invalid cgroup path %s", path) } - content, err := supportedControllers(c) + content, err := supportedControllers() if err != nil { return err } @@ -115,7 +115,7 @@ func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) { // the controllers requested are thread-aware we can simply put the cgroup into // threaded mode. case "domain invalid": - if containsDomainController(c) { + if containsDomainController(c.Resources) { return fmt.Errorf("cannot enter cgroupv2 %q with domain controllers -- it is in an invalid state", current) } else { // Not entirely correct (in theory we'd always want to be a domain -- @@ -129,7 +129,7 @@ func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) { case "domain threaded": fallthrough case "threaded": - if containsDomainController(c) { + if containsDomainController(c.Resources) { return fmt.Errorf("cannot enter cgroupv2 %q with domain controllers -- it is in %s mode", current, cgType) } } diff --git a/libcontainer/cgroups/fs2/devices.go b/libcontainer/cgroups/fs2/devices.go index 61c00a96d6c..fa9194b557a 100644 --- a/libcontainer/cgroups/fs2/devices.go +++ b/libcontainer/cgroups/fs2/devices.go @@ -30,7 +30,7 @@ func isRWM(perms devices.Permissions) bool { // This is similar to the logic applied in crun for handling errors from bpf(2) // . -func canSkipEBPFError(cgroup *configs.Cgroup) bool { +func canSkipEBPFError(r *configs.Resources) bool { // If we're running in a user namespace we can ignore eBPF rules because we // usually cannot use bpf(2), as well as rootless containers usually don't // have the necessary privileges to mknod(2) device inodes or access @@ -46,7 +46,7 @@ func canSkipEBPFError(cgroup *configs.Cgroup) bool { // NOTE: This will sometimes trigger in cases where access modes are split // between different rules but to handle this correctly would require // using ".../libcontainer/cgroup/devices".Emulator. - for _, dev := range cgroup.Resources.Devices { + for _, dev := range r.Devices { if !dev.Allow || !isRWM(dev.Permissions) { return false } @@ -54,15 +54,14 @@ func canSkipEBPFError(cgroup *configs.Cgroup) bool { return true } -func setDevices(dirPath string, cgroup *configs.Cgroup) error { - if cgroup.SkipDevices { +func setDevices(dirPath string, r *configs.Resources) error { + if r.SkipDevices { return nil } // XXX: This is currently a white-list (but all callers pass a blacklist of // devices). This is bad for a whole variety of reasons, but will need // to be fixed with co-ordinated effort with downstreams. - devices := cgroup.Devices - insts, license, err := devicefilter.DeviceFilter(devices) + insts, license, err := devicefilter.DeviceFilter(r.Devices) if err != nil { return err } @@ -83,7 +82,7 @@ func setDevices(dirPath string, cgroup *configs.Cgroup) error { // programs. You could temporarily insert a deny-everything program // but that would result in spurrious failures during updates. if _, err := ebpf.LoadAttachCgroupDeviceFilter(insts, license, dirFD); err != nil { - if !canSkipEBPFError(cgroup) { + if !canSkipEBPFError(r) { return err } } diff --git a/libcontainer/cgroups/fs2/fs2.go b/libcontainer/cgroups/fs2/fs2.go index 2db442a33d8..5da6c51e52e 100644 --- a/libcontainer/cgroups/fs2/fs2.go +++ b/libcontainer/cgroups/fs2/fs2.go @@ -75,7 +75,7 @@ func (m *manager) Apply(pid int) error { // - "runc create (rootless + limits + no cgrouppath + no permission) fails with informative error" if m.rootless { if m.config.Path == "" { - if blNeed, nErr := needAnyControllers(m.config); nErr == nil && !blNeed { + if blNeed, nErr := needAnyControllers(m.config.Resources); nErr == nil && !blNeed { return nil } return errors.Wrap(err, "rootless needs no limits + no cgrouppath when no permission is granted for cgroups") @@ -147,27 +147,24 @@ func (m *manager) Path(_ string) string { return m.dirPath } -func (m *manager) Set(container *configs.Config) error { - if container == nil || container.Cgroups == nil { - return nil - } +func (m *manager) Set(r *configs.Resources) error { if err := m.getControllers(); err != nil { return err } // pids (since kernel 4.5) - if err := setPids(m.dirPath, container.Cgroups); err != nil { + if err := setPids(m.dirPath, r); err != nil { return err } // memory (since kernel 4.5) - if err := setMemory(m.dirPath, container.Cgroups); err != nil { + if err := setMemory(m.dirPath, r); err != nil { return err } // io (since kernel 4.5) - if err := setIo(m.dirPath, container.Cgroups); err != nil { + if err := setIo(m.dirPath, r); err != nil { return err } // cpu (since kernel 4.15) - if err := setCpu(m.dirPath, container.Cgroups); err != nil { + if err := setCpu(m.dirPath, r); err != nil { return err } // devices (since kernel 4.15, pseudo-controller) @@ -175,25 +172,25 @@ func (m *manager) Set(container *configs.Config) error { // When m.rootless is true, errors from the device subsystem are ignored because it is really not expected to work. // However, errors from other subsystems are not ignored. // see @test "runc create (rootless + limits + no cgrouppath + no permission) fails with informative error" - if err := setDevices(m.dirPath, container.Cgroups); err != nil && !m.rootless { + if err := setDevices(m.dirPath, r); err != nil && !m.rootless { return err } // cpuset (since kernel 5.0) - if err := setCpuset(m.dirPath, container.Cgroups); err != nil { + if err := setCpuset(m.dirPath, r); err != nil { return err } // hugetlb (since kernel 5.6) - if err := setHugeTlb(m.dirPath, container.Cgroups); err != nil { + if err := setHugeTlb(m.dirPath, r); err != nil { return err } // freezer (since kernel 5.2, pseudo-controller) - if err := setFreezer(m.dirPath, container.Cgroups.Freezer); err != nil { + if err := setFreezer(m.dirPath, r.Freezer); err != nil { return err } - if err := m.setUnified(container.Cgroups.Unified); err != nil { + if err := m.setUnified(r.Unified); err != nil { return err } - m.config = container.Cgroups + m.config.Resources = r return nil } diff --git a/libcontainer/cgroups/fs2/hugetlb.go b/libcontainer/cgroups/fs2/hugetlb.go index feddc7aa043..76df770109c 100644 --- a/libcontainer/cgroups/fs2/hugetlb.go +++ b/libcontainer/cgroups/fs2/hugetlb.go @@ -12,15 +12,15 @@ import ( "github.com/opencontainers/runc/libcontainer/configs" ) -func isHugeTlbSet(cgroup *configs.Cgroup) bool { - return len(cgroup.Resources.HugetlbLimit) > 0 +func isHugeTlbSet(r *configs.Resources) bool { + return len(r.HugetlbLimit) > 0 } -func setHugeTlb(dirPath string, cgroup *configs.Cgroup) error { - if !isHugeTlbSet(cgroup) { +func setHugeTlb(dirPath string, r *configs.Resources) error { + if !isHugeTlbSet(r) { return nil } - for _, hugetlb := range cgroup.Resources.HugetlbLimit { + for _, hugetlb := range r.HugetlbLimit { if err := fscommon.WriteFile(dirPath, "hugetlb."+hugetlb.Pagesize+".max", strconv.FormatUint(hugetlb.Limit, 10)); err != nil { return err } diff --git a/libcontainer/cgroups/fs2/io.go b/libcontainer/cgroups/fs2/io.go index 34262d3cd58..c07a00867ae 100644 --- a/libcontainer/cgroups/fs2/io.go +++ b/libcontainer/cgroups/fs2/io.go @@ -13,50 +13,50 @@ import ( "github.com/opencontainers/runc/libcontainer/configs" ) -func isIoSet(cgroup *configs.Cgroup) bool { - return cgroup.Resources.BlkioWeight != 0 || - len(cgroup.Resources.BlkioThrottleReadBpsDevice) > 0 || - len(cgroup.Resources.BlkioThrottleWriteBpsDevice) > 0 || - len(cgroup.Resources.BlkioThrottleReadIOPSDevice) > 0 || - len(cgroup.Resources.BlkioThrottleWriteIOPSDevice) > 0 +func isIoSet(r *configs.Resources) bool { + return r.BlkioWeight != 0 || + len(r.BlkioThrottleReadBpsDevice) > 0 || + len(r.BlkioThrottleWriteBpsDevice) > 0 || + len(r.BlkioThrottleReadIOPSDevice) > 0 || + len(r.BlkioThrottleWriteIOPSDevice) > 0 } -func setIo(dirPath string, cgroup *configs.Cgroup) error { - if !isIoSet(cgroup) { +func setIo(dirPath string, r *configs.Resources) error { + if !isIoSet(r) { return nil } - if cgroup.Resources.BlkioWeight != 0 { + if r.BlkioWeight != 0 { filename := "io.bfq.weight" if err := fscommon.WriteFile(dirPath, filename, - strconv.FormatUint(uint64(cgroup.Resources.BlkioWeight), 10)); err != nil { + strconv.FormatUint(uint64(r.BlkioWeight), 10)); err != nil { // if io.bfq.weight does not exist, then bfq module is not loaded. // Fallback to use io.weight with a conversion scheme if !os.IsNotExist(err) { return err } - v := cgroups.ConvertBlkIOToIOWeightValue(cgroup.Resources.BlkioWeight) + v := cgroups.ConvertBlkIOToIOWeightValue(r.BlkioWeight) if err := fscommon.WriteFile(dirPath, "io.weight", strconv.FormatUint(v, 10)); err != nil { return err } } } - for _, td := range cgroup.Resources.BlkioThrottleReadBpsDevice { + for _, td := range r.BlkioThrottleReadBpsDevice { if err := fscommon.WriteFile(dirPath, "io.max", td.StringName("rbps")); err != nil { return err } } - for _, td := range cgroup.Resources.BlkioThrottleWriteBpsDevice { + for _, td := range r.BlkioThrottleWriteBpsDevice { if err := fscommon.WriteFile(dirPath, "io.max", td.StringName("wbps")); err != nil { return err } } - for _, td := range cgroup.Resources.BlkioThrottleReadIOPSDevice { + for _, td := range r.BlkioThrottleReadIOPSDevice { if err := fscommon.WriteFile(dirPath, "io.max", td.StringName("riops")); err != nil { return err } } - for _, td := range cgroup.Resources.BlkioThrottleWriteIOPSDevice { + for _, td := range r.BlkioThrottleWriteIOPSDevice { if err := fscommon.WriteFile(dirPath, "io.max", td.StringName("wiops")); err != nil { return err } diff --git a/libcontainer/cgroups/fs2/memory.go b/libcontainer/cgroups/fs2/memory.go index 3d1284a7ca5..7308f5a205c 100644 --- a/libcontainer/cgroups/fs2/memory.go +++ b/libcontainer/cgroups/fs2/memory.go @@ -33,21 +33,20 @@ func numToStr(value int64) (ret string) { return ret } -func isMemorySet(cgroup *configs.Cgroup) bool { - return cgroup.Resources.MemoryReservation != 0 || - cgroup.Resources.Memory != 0 || cgroup.Resources.MemorySwap != 0 +func isMemorySet(r *configs.Resources) bool { + return r.MemoryReservation != 0 || r.Memory != 0 || r.MemorySwap != 0 } -func setMemory(dirPath string, cgroup *configs.Cgroup) error { - if !isMemorySet(cgroup) { +func setMemory(dirPath string, r *configs.Resources) error { + if !isMemorySet(r) { return nil } - swap, err := cgroups.ConvertMemorySwapToCgroupV2Value(cgroup.Resources.MemorySwap, cgroup.Resources.Memory) + swap, err := cgroups.ConvertMemorySwapToCgroupV2Value(r.MemorySwap, r.Memory) if err != nil { return err } swapStr := numToStr(swap) - if swapStr == "" && swap == 0 && cgroup.Resources.MemorySwap > 0 { + if swapStr == "" && swap == 0 && r.MemorySwap > 0 { // memory and memorySwap set to the same value -- disable swap swapStr = "0" } @@ -58,7 +57,7 @@ func setMemory(dirPath string, cgroup *configs.Cgroup) error { } } - if val := numToStr(cgroup.Resources.Memory); val != "" { + if val := numToStr(r.Memory); val != "" { if err := fscommon.WriteFile(dirPath, "memory.max", val); err != nil { return err } @@ -66,7 +65,7 @@ func setMemory(dirPath string, cgroup *configs.Cgroup) error { // cgroup.Resources.KernelMemory is ignored - if val := numToStr(cgroup.Resources.MemoryReservation); val != "" { + if val := numToStr(r.MemoryReservation); val != "" { if err := fscommon.WriteFile(dirPath, "memory.low", val); err != nil { return err } diff --git a/libcontainer/cgroups/fs2/pids.go b/libcontainer/cgroups/fs2/pids.go index 366212347aa..346fdb67873 100644 --- a/libcontainer/cgroups/fs2/pids.go +++ b/libcontainer/cgroups/fs2/pids.go @@ -14,15 +14,15 @@ import ( "golang.org/x/sys/unix" ) -func isPidsSet(cgroup *configs.Cgroup) bool { - return cgroup.Resources.PidsLimit != 0 +func isPidsSet(r *configs.Resources) bool { + return r.PidsLimit != 0 } -func setPids(dirPath string, cgroup *configs.Cgroup) error { - if !isPidsSet(cgroup) { +func setPids(dirPath string, r *configs.Resources) error { + if !isPidsSet(r) { return nil } - if val := numToStr(cgroup.Resources.PidsLimit); val != "" { + if val := numToStr(r.PidsLimit); val != "" { if err := fscommon.WriteFile(dirPath, "pids.max", val); err != nil { return err } diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index 842fee207db..41de6e8b70f 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -36,8 +36,8 @@ type subsystem interface { Name() string // Returns the stats, as 'stats', corresponding to the cgroup under 'path'. GetStats(path string, stats *cgroups.Stats) error - // Set the cgroup represented by cgroup. - Set(path string, cgroup *configs.Cgroup) error + // Set sets cgroup resource limits. + Set(path string, r *configs.Resources) error } var errSubsystemDoesNotExist = errors.New("cgroup: subsystem does not exist") @@ -58,9 +58,8 @@ var legacySubsystems = []subsystem{ &fs.NameGroup{GroupName: "name=systemd"}, } -func genV1ResourcesProperties(c *configs.Cgroup, cm *dbusConnManager) ([]systemdDbus.Property, error) { +func genV1ResourcesProperties(r *configs.Resources, cm *dbusConnManager) ([]systemdDbus.Property, error) { var properties []systemdDbus.Property - r := c.Resources deviceProperties, err := generateDeviceProperties(r.Devices) if err != nil { @@ -232,7 +231,7 @@ func (m *legacyManager) joinCgroups(pid int) error { case "cpuset": if path, ok := m.paths[name]; ok { s := &fs.CpusetGroup{} - if err := s.ApplyDir(path, m.cgroups, pid); err != nil { + if err := s.ApplyDir(path, m.cgroups.Resources, pid); err != nil { return err } } @@ -285,7 +284,7 @@ func (m *legacyManager) Freeze(state configs.FreezerState) error { prevState := m.cgroups.Resources.Freezer m.cgroups.Resources.Freezer = state freezer := &fs.FreezerGroup{} - if err := freezer.Set(path, m.cgroups); err != nil { + if err := freezer.Set(path, m.cgroups.Resources); err != nil { m.cgroups.Resources.Freezer = prevState return err } @@ -325,16 +324,16 @@ func (m *legacyManager) GetStats() (*cgroups.Stats, error) { return stats, nil } -func (m *legacyManager) Set(container *configs.Config) error { +func (m *legacyManager) Set(r *configs.Resources) error { // If Paths are set, then we are just joining cgroups paths // and there is no need to set any values. if m.cgroups.Paths != nil { return nil } - if container.Cgroups.Resources.Unified != nil { + if r.Unified != nil { return cgroups.ErrV1NoUnified } - properties, err := genV1ResourcesProperties(container.Cgroups, m.dbus) + properties, err := genV1ResourcesProperties(r, m.dbus) if err != nil { return err } @@ -362,7 +361,7 @@ func (m *legacyManager) Set(container *configs.Config) error { } } - if err := setUnitProperties(m.dbus, getUnitName(container.Cgroups), properties...); err != nil { + if err := setUnitProperties(m.dbus, getUnitName(m.cgroups), properties...); err != nil { _ = m.Freeze(targetFreezerState) return err } @@ -377,7 +376,7 @@ func (m *legacyManager) Set(container *configs.Config) error { if !ok { continue } - if err := sys.Set(path, container.Cgroups); err != nil { + if err := sys.Set(path, r); err != nil { return err } } diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go index 00532ce8142..9dcde4233b1 100644 --- a/libcontainer/cgroups/systemd/v2.go +++ b/libcontainer/cgroups/systemd/v2.go @@ -164,9 +164,8 @@ func unifiedResToSystemdProps(cm *dbusConnManager, res map[string]string) (props return props, nil } -func genV2ResourcesProperties(c *configs.Cgroup, cm *dbusConnManager) ([]systemdDbus.Property, error) { +func genV2ResourcesProperties(r *configs.Resources, cm *dbusConnManager) ([]systemdDbus.Property, error) { var properties []systemdDbus.Property - r := c.Resources // NOTE: This is of questionable correctness because we insert our own // devices eBPF program later. Two programs with identical rules @@ -420,8 +419,8 @@ func (m *unifiedManager) GetStats() (*cgroups.Stats, error) { return fsMgr.GetStats() } -func (m *unifiedManager) Set(container *configs.Config) error { - properties, err := genV2ResourcesProperties(container.Cgroups, m.dbus) +func (m *unifiedManager) Set(r *configs.Resources) error { + properties, err := genV2ResourcesProperties(r, m.dbus) if err != nil { return err } @@ -462,7 +461,7 @@ func (m *unifiedManager) Set(container *configs.Config) error { if err != nil { return err } - return fsMgr.Set(container) + return fsMgr.Set(r) } func (m *unifiedManager) GetPaths() map[string]string { diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 5e89285df0a..945a0fa0d57 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -225,9 +225,9 @@ func (c *linuxContainer) Set(config configs.Config) error { if status == Stopped { return newGenericError(errors.New("container not running"), ContainerNotRunning) } - if err := c.cgroupManager.Set(&config); err != nil { + if err := c.cgroupManager.Set(config.Cgroups.Resources); err != nil { // Set configs back - if err2 := c.cgroupManager.Set(c.config); err2 != nil { + if err2 := c.cgroupManager.Set(c.config.Cgroups.Resources); err2 != nil { logrus.Warnf("Setting back cgroup configs failed due to error: %v, your state.json and actual configs might be inconsistent.", err2) } return err @@ -235,7 +235,7 @@ func (c *linuxContainer) Set(config configs.Config) error { if c.intelRdtManager != nil { if err := c.intelRdtManager.Set(&config); err != nil { // Set configs back - if err2 := c.cgroupManager.Set(c.config); err2 != nil { + if err2 := c.cgroupManager.Set(c.config.Cgroups.Resources); err2 != nil { logrus.Warnf("Setting back cgroup configs failed due to error: %v, your state.json and actual configs might be inconsistent.", err2) } if err2 := c.intelRdtManager.Set(c.config); err2 != nil { @@ -1462,7 +1462,7 @@ func (c *linuxContainer) criuApplyCgroups(pid int, req *criurpc.CriuReq) error { return err } - if err := c.cgroupManager.Set(c.config); err != nil { + if err := c.cgroupManager.Set(c.config.Cgroups.Resources); err != nil { return newSystemError(err) } diff --git a/libcontainer/container_linux_test.go b/libcontainer/container_linux_test.go index 78071a92651..880998d9a0e 100644 --- a/libcontainer/container_linux_test.go +++ b/libcontainer/container_linux_test.go @@ -42,7 +42,7 @@ func (m *mockCgroupManager) Apply(pid int) error { return nil } -func (m *mockCgroupManager) Set(container *configs.Config) error { +func (m *mockCgroupManager) Set(_ *configs.Resources) error { return nil } diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 1046594910d..053971b911b 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -448,7 +448,7 @@ func (p *initProcess) start() (retErr error) { // call prestart and CreateRuntime hooks if !p.config.Config.Namespaces.Contains(configs.NEWNS) { // Setup cgroup before the hook, so that the prestart and CreateRuntime hook could apply cgroup permissions. - if err := p.manager.Set(p.config.Config); err != nil { + if err := p.manager.Set(p.config.Config.Cgroups.Resources); err != nil { return newSystemErrorWithCause(err, "setting cgroup config for ready process") } if p.intelRdtManager != nil { @@ -504,7 +504,7 @@ func (p *initProcess) start() (retErr error) { sentRun = true case procHooks: // Setup cgroup before prestart hook, so that the prestart hook could apply cgroup permissions. - if err := p.manager.Set(p.config.Config); err != nil { + if err := p.manager.Set(p.config.Config.Cgroups.Resources); err != nil { return newSystemErrorWithCause(err, "setting cgroup config for procHooks process") } if p.intelRdtManager != nil { From abf12ce0dbf851c93b7bb1eaf7047920ce44c919 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 16 Apr 2021 11:56:05 -0700 Subject: [PATCH 3/3] libc/cg: improve Manager docs Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/cgroups.go | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/libcontainer/cgroups/cgroups.go b/libcontainer/cgroups/cgroups.go index 1309540c19f..68a346ca531 100644 --- a/libcontainer/cgroups/cgroups.go +++ b/libcontainer/cgroups/cgroups.go @@ -7,37 +7,44 @@ import ( ) type Manager interface { - // Applies cgroup configuration to the process with the specified pid + // Apply creates a cgroup, if not yet created, and adds a process + // with the specified pid into that cgroup. A special value of -1 + // can be used to merely create a cgroup. Apply(pid int) error - // Returns the PIDs inside the cgroup set + // GetPids returns the PIDs of all processes inside the cgroup. GetPids() ([]int, error) - // Returns the PIDs inside the cgroup set & all sub-cgroups + // GetAllPids returns the PIDs of all processes inside the cgroup + // any all its sub-cgroups. GetAllPids() ([]int, error) - // Returns statistics for the cgroup set + // GetStats returns cgroups statistics. GetStats() (*Stats, error) - // Toggles the freezer cgroup according with specified state + // Freeze sets the freezer cgroup to the specified state. Freeze(state configs.FreezerState) error - // Destroys the cgroup set + // Destroy removes cgroup. Destroy() error // Path returns a cgroup path to the specified controller/subsystem. // For cgroupv2, the argument is unused and can be empty. Path(string) string - // Sets the cgroup as configured. + // Set sets cgroup resources parameters/limits. If the argument is nil, + // the resources specified during Manager creation (or the previous call + // to Set) are used. Set(r *configs.Resources) error - // GetPaths returns cgroup path(s) to save in a state file in order to restore later. + // GetPaths returns cgroup path(s) to save in a state file in order to + // restore later. // - // For cgroup v1, a key is cgroup subsystem name, and the value is the path - // to the cgroup for this subsystem. + // For cgroup v1, a key is cgroup subsystem name, and the value is the + // path to the cgroup for this subsystem. // - // For cgroup v2 unified hierarchy, a key is "", and the value is the unified path. + // For cgroup v2 unified hierarchy, a key is "", and the value is the + // unified path. GetPaths() map[string]string // GetCgroups returns the cgroup data as configured. @@ -46,7 +53,7 @@ type Manager interface { // GetFreezerState retrieves the current FreezerState of the cgroup. GetFreezerState() (configs.FreezerState, error) - // Whether the cgroup path exists or not + // Exists returns whether the cgroup path exists or not. Exists() bool // OOMKillCount reports OOM kill count for the cgroup.