From e6928865631319907e61bf9bfee02ebae210adc9 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 10 Aug 2021 13:42:09 -0700 Subject: [PATCH] libct/cg/fs: refactor 1. Dismantle and remove struct cgroupData. It contained three unrelated entities (cgroup paths, pid, and resources), and made the code harder to read. Most importantly, though, it is not needed. Now, subsystems' Apply methods take path, resources, and pid. To a reviewer -- the core of the changes is in fs.go and paths.go, the rest of it is adapting to the new signatures and related test changes. 2. Dismantle and remove struct cgroupTestUtil. This is a followup to the previous item -- since cgroupData is gone, there is nothing to hold in cgroupTestUtil. The change itself is very small (see util_test.go), but this patch is big because of it -- mostly because we had to replace helper.cgroup.Resources with &config.Resources{}. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs/blkio.go | 4 +- libcontainer/cgroups/fs/blkio_test.go | 148 ++++++++++++---------- libcontainer/cgroups/fs/cpu.go | 15 +-- libcontainer/cgroups/fs/cpu_test.go | 72 ++++++----- libcontainer/cgroups/fs/cpuacct.go | 4 +- libcontainer/cgroups/fs/cpuacct_test.go | 12 +- libcontainer/cgroups/fs/cpuset.go | 9 +- libcontainer/cgroups/fs/cpuset_test.go | 41 +++--- libcontainer/cgroups/fs/devices.go | 7 +- libcontainer/cgroups/fs/devices_test.go | 27 ++-- libcontainer/cgroups/fs/freezer.go | 4 +- libcontainer/cgroups/fs/freezer_test.go | 24 ++-- libcontainer/cgroups/fs/fs.go | 16 ++- libcontainer/cgroups/fs/hugetlb.go | 4 +- libcontainer/cgroups/fs/hugetlb_test.go | 41 +++--- libcontainer/cgroups/fs/memory.go | 4 +- libcontainer/cgroups/fs/memory_test.go | 152 ++++++++++++----------- libcontainer/cgroups/fs/name.go | 6 +- libcontainer/cgroups/fs/net_cls.go | 4 +- libcontainer/cgroups/fs/net_cls_test.go | 13 +- libcontainer/cgroups/fs/net_prio.go | 4 +- libcontainer/cgroups/fs/net_prio_test.go | 10 +- libcontainer/cgroups/fs/paths.go | 37 ++---- libcontainer/cgroups/fs/paths_test.go | 10 +- libcontainer/cgroups/fs/perf_event.go | 4 +- libcontainer/cgroups/fs/pids.go | 4 +- libcontainer/cgroups/fs/pids_test.go | 37 +++--- libcontainer/cgroups/fs/rdma.go | 4 +- libcontainer/cgroups/fs/util_test.go | 36 ++---- 29 files changed, 386 insertions(+), 367 deletions(-) diff --git a/libcontainer/cgroups/fs/blkio.go b/libcontainer/cgroups/fs/blkio.go index 14b572cfe..c81b6562a 100644 --- a/libcontainer/cgroups/fs/blkio.go +++ b/libcontainer/cgroups/fs/blkio.go @@ -20,8 +20,8 @@ func (s *BlkioGroup) Name() string { return "blkio" } -func (s *BlkioGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *BlkioGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *BlkioGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/blkio_test.go b/libcontainer/cgroups/fs/blkio_test.go index eb6257c0d..7810a67fc 100644 --- a/libcontainer/cgroups/fs/blkio_test.go +++ b/libcontainer/cgroups/fs/blkio_test.go @@ -176,25 +176,27 @@ func TestBlkioSetWeight(t *testing.T) { for _, legacyIOScheduler := range []bool{false, true} { // Populate cgroup - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") weightFilename := "blkio.bfq.weight" if legacyIOScheduler { weightFilename = "blkio.weight" } - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ weightFilename: strconv.Itoa(weightBefore), }) // Apply new configuration - helper.CgroupData.config.Resources.BlkioWeight = weightAfter + r := &configs.Resources{ + BlkioWeight: weightAfter, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } // Verify results if weightFilename != blkio.weightFilename { t.Fatalf("weight filename detection failed: expected %q, detected %q", weightFilename, blkio.weightFilename) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, weightFilename) + value, err := fscommon.GetCgroupParamUint(path, weightFilename) if err != nil { t.Fatal(err) } @@ -211,30 +213,32 @@ func TestBlkioSetWeightDevice(t *testing.T) { for _, legacyIOScheduler := range []bool{false, true} { // Populate cgroup - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") weightFilename := "blkio.bfq.weight" weightDeviceFilename := "blkio.bfq.weight_device" if legacyIOScheduler { weightFilename = "blkio.weight" weightDeviceFilename = "blkio.weight_device" } - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ weightFilename: "", weightDeviceFilename: weightDeviceBefore, }) // Apply new configuration wd := configs.NewWeightDevice(8, 0, 500, 0) weightDeviceAfter := wd.WeightString() - helper.CgroupData.config.Resources.BlkioWeightDevice = []*configs.WeightDevice{wd} + r := &configs.Resources{ + BlkioWeightDevice: []*configs.WeightDevice{wd}, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } // Verify results if weightDeviceFilename != blkio.weightDeviceFilename { t.Fatalf("weight_device filename detection failed: expected %q, detected %q", weightDeviceFilename, blkio.weightDeviceFilename) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, weightDeviceFilename) + value, err := fscommon.GetCgroupParamString(path, weightDeviceFilename) if err != nil { t.Fatal(err) } @@ -246,7 +250,7 @@ func TestBlkioSetWeightDevice(t *testing.T) { // regression #274 func TestBlkioSetMultipleWeightDevice(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") const ( weightDeviceBefore = "8:0 400" @@ -261,20 +265,22 @@ func TestBlkioSetMultipleWeightDevice(t *testing.T) { weightDeviceAfter := wd2.WeightString() blkio := &BlkioGroup{} - blkio.detectWeightFilenames(helper.CgroupPath) + blkio.detectWeightFilenames(path) if blkio.weightDeviceFilename != "blkio.bfq.weight_device" { t.Fatalf("when blkio controller is unavailable, expected to use \"blkio.bfq.weight_device\", tried to use %q", blkio.weightDeviceFilename) } - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ blkio.weightDeviceFilename: weightDeviceBefore, }) - helper.CgroupData.config.Resources.BlkioWeightDevice = []*configs.WeightDevice{wd1, wd2} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + r := &configs.Resources{ + BlkioWeightDevice: []*configs.WeightDevice{wd1, wd2}, + } + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, blkio.weightDeviceFilename) + value, err := fscommon.GetCgroupParamString(path, blkio.weightDeviceFilename) if err != nil { t.Fatal(err) } @@ -284,11 +290,11 @@ func TestBlkioSetMultipleWeightDevice(t *testing.T) { } func TestBlkioBFQDebugStats(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(blkioBFQDebugStatsTestFiles) + path := tempDir(t, "blkio") + writeFileContents(t, path, blkioBFQDebugStatsTestFiles) blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -338,12 +344,12 @@ func TestBlkioBFQDebugStats(t *testing.T) { } func TestBlkioMultipleStatsFiles(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(blkioBFQDebugStatsTestFiles) - helper.writeFileContents(blkioCFQStatsTestFiles) + path := tempDir(t, "blkio") + writeFileContents(t, path, blkioBFQDebugStatsTestFiles) + writeFileContents(t, path, blkioCFQStatsTestFiles) blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -393,11 +399,11 @@ func TestBlkioMultipleStatsFiles(t *testing.T) { } func TestBlkioBFQStats(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(blkioBFQStatsTestFiles) + path := tempDir(t, "blkio") + writeFileContents(t, path, blkioBFQStatsTestFiles) blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -459,7 +465,7 @@ func TestBlkioStatsNoFilesBFQDebug(t *testing.T) { } for _, testCase := range testCases { - helper := NewCgroupTestUtil("cpuset", t) + path := tempDir(t, "cpuset") tempBlkioTestFiles := map[string]string{} for i, v := range blkioBFQDebugStatsTestFiles { @@ -467,10 +473,10 @@ func TestBlkioStatsNoFilesBFQDebug(t *testing.T) { } delete(tempBlkioTestFiles, testCase.filename) - helper.writeFileContents(tempBlkioTestFiles) + writeFileContents(t, path, tempBlkioTestFiles) cpuset := &CpusetGroup{} actualStats := *cgroups.NewStats() - err := cpuset.GetStats(helper.CgroupPath, &actualStats) + err := cpuset.GetStats(path, &actualStats) if err != nil { t.Errorf(fmt.Sprintf("test case '%s' failed unexpectedly: %s", testCase.desc, err)) } @@ -478,12 +484,12 @@ func TestBlkioStatsNoFilesBFQDebug(t *testing.T) { } func TestBlkioCFQStats(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(blkioCFQStatsTestFiles) + path := tempDir(t, "blkio") + writeFileContents(t, path, blkioCFQStatsTestFiles) blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -573,7 +579,7 @@ func TestBlkioStatsNoFilesCFQ(t *testing.T) { } for _, testCase := range testCases { - helper := NewCgroupTestUtil("cpuset", t) + path := tempDir(t, "cpuset") tempBlkioTestFiles := map[string]string{} for i, v := range blkioCFQStatsTestFiles { @@ -581,10 +587,10 @@ func TestBlkioStatsNoFilesCFQ(t *testing.T) { } delete(tempBlkioTestFiles, testCase.filename) - helper.writeFileContents(tempBlkioTestFiles) + writeFileContents(t, path, tempBlkioTestFiles) cpuset := &CpusetGroup{} actualStats := *cgroups.NewStats() - err := cpuset.GetStats(helper.CgroupPath, &actualStats) + err := cpuset.GetStats(path, &actualStats) if err != nil { t.Errorf(fmt.Sprintf("test case '%s' failed unexpectedly: %s", testCase.desc, err)) } @@ -592,8 +598,8 @@ func TestBlkioStatsNoFilesCFQ(t *testing.T) { } func TestBlkioStatsUnexpectedNumberOfFields(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "blkio") + writeFileContents(t, path, map[string]string{ "blkio.io_service_bytes_recursive": "8:0 Read 100 100", "blkio.io_serviced_recursive": servicedRecursiveContents, "blkio.io_queued_recursive": queuedRecursiveContents, @@ -606,15 +612,15 @@ func TestBlkioStatsUnexpectedNumberOfFields(t *testing.T) { blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected to fail, but did not") } } func TestBlkioStatsUnexpectedFieldType(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "blkio") + writeFileContents(t, path, map[string]string{ "blkio.io_service_bytes_recursive": "8:0 Read Write", "blkio.io_serviced_recursive": servicedRecursiveContents, "blkio.io_queued_recursive": queuedRecursiveContents, @@ -627,15 +633,15 @@ func TestBlkioStatsUnexpectedFieldType(t *testing.T) { blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected to fail, but did not") } } func TestThrottleRecursiveBlkioStats(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "blkio") + writeFileContents(t, path, map[string]string{ "blkio.io_service_bytes_recursive": "", "blkio.io_serviced_recursive": "", "blkio.io_queued_recursive": "", @@ -650,7 +656,7 @@ func TestThrottleRecursiveBlkioStats(t *testing.T) { blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -684,8 +690,8 @@ func TestThrottleRecursiveBlkioStats(t *testing.T) { } func TestThrottleBlkioStats(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "blkio") + writeFileContents(t, path, map[string]string{ "blkio.io_service_bytes_recursive": "", "blkio.io_serviced_recursive": "", "blkio.io_queued_recursive": "", @@ -700,7 +706,7 @@ func TestThrottleBlkioStats(t *testing.T) { blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -734,7 +740,7 @@ func TestThrottleBlkioStats(t *testing.T) { } func TestBlkioSetThrottleReadBpsDevice(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") const ( throttleBefore = `8:0 1024` @@ -743,17 +749,19 @@ func TestBlkioSetThrottleReadBpsDevice(t *testing.T) { td := configs.NewThrottleDevice(8, 0, 2048) throttleAfter := td.String() - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "blkio.throttle.read_bps_device": throttleBefore, }) - helper.CgroupData.config.Resources.BlkioThrottleReadBpsDevice = []*configs.ThrottleDevice{td} + r := &configs.Resources{ + BlkioThrottleReadBpsDevice: []*configs.ThrottleDevice{td}, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "blkio.throttle.read_bps_device") + value, err := fscommon.GetCgroupParamString(path, "blkio.throttle.read_bps_device") if err != nil { t.Fatal(err) } @@ -763,7 +771,7 @@ func TestBlkioSetThrottleReadBpsDevice(t *testing.T) { } func TestBlkioSetThrottleWriteBpsDevice(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") const ( throttleBefore = `8:0 1024` @@ -772,17 +780,19 @@ func TestBlkioSetThrottleWriteBpsDevice(t *testing.T) { td := configs.NewThrottleDevice(8, 0, 2048) throttleAfter := td.String() - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "blkio.throttle.write_bps_device": throttleBefore, }) - helper.CgroupData.config.Resources.BlkioThrottleWriteBpsDevice = []*configs.ThrottleDevice{td} + r := &configs.Resources{ + BlkioThrottleWriteBpsDevice: []*configs.ThrottleDevice{td}, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "blkio.throttle.write_bps_device") + value, err := fscommon.GetCgroupParamString(path, "blkio.throttle.write_bps_device") if err != nil { t.Fatal(err) } @@ -792,7 +802,7 @@ func TestBlkioSetThrottleWriteBpsDevice(t *testing.T) { } func TestBlkioSetThrottleReadIOpsDevice(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") const ( throttleBefore = `8:0 1024` @@ -801,17 +811,19 @@ func TestBlkioSetThrottleReadIOpsDevice(t *testing.T) { td := configs.NewThrottleDevice(8, 0, 2048) throttleAfter := td.String() - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "blkio.throttle.read_iops_device": throttleBefore, }) - helper.CgroupData.config.Resources.BlkioThrottleReadIOPSDevice = []*configs.ThrottleDevice{td} + r := &configs.Resources{ + BlkioThrottleReadIOPSDevice: []*configs.ThrottleDevice{td}, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "blkio.throttle.read_iops_device") + value, err := fscommon.GetCgroupParamString(path, "blkio.throttle.read_iops_device") if err != nil { t.Fatal(err) } @@ -821,7 +833,7 @@ func TestBlkioSetThrottleReadIOpsDevice(t *testing.T) { } func TestBlkioSetThrottleWriteIOpsDevice(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") const ( throttleBefore = `8:0 1024` @@ -830,17 +842,19 @@ func TestBlkioSetThrottleWriteIOpsDevice(t *testing.T) { td := configs.NewThrottleDevice(8, 0, 2048) throttleAfter := td.String() - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "blkio.throttle.write_iops_device": throttleBefore, }) - helper.CgroupData.config.Resources.BlkioThrottleWriteIOPSDevice = []*configs.ThrottleDevice{td} + r := &configs.Resources{ + BlkioThrottleWriteIOPSDevice: []*configs.ThrottleDevice{td}, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "blkio.throttle.write_iops_device") + value, err := fscommon.GetCgroupParamString(path, "blkio.throttle.write_iops_device") if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/cpu.go b/libcontainer/cgroups/fs/cpu.go index cd81cabc1..6c79f899b 100644 --- a/libcontainer/cgroups/fs/cpu.go +++ b/libcontainer/cgroups/fs/cpu.go @@ -19,24 +19,19 @@ func (s *CpuGroup) Name() string { return "cpu" } -func (s *CpuGroup) Apply(path string, d *cgroupData) error { - // This might happen if we have no cpu cgroup mounted. - // Just do nothing and don't fail. - if path == "" { - return nil - } +func (s *CpuGroup) Apply(path string, r *configs.Resources, pid int) error { if err := os.MkdirAll(path, 0o755); err != nil { return err } // 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.Resources); err != nil { + if err := s.SetRtSched(path, r); err != nil { return err } - // Since we are not using join(), we need to place the pid - // into the procs file unlike other subsystems. - return cgroups.WriteCgroupProc(path, d.pid) + // Since we are not using apply(), we need to place the pid + // into the procs file. + return cgroups.WriteCgroupProc(path, pid) } func (s *CpuGroup) SetRtSched(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/cpu_test.go b/libcontainer/cgroups/fs/cpu_test.go index 958e6040e..bbdd45a71 100644 --- a/libcontainer/cgroups/fs/cpu_test.go +++ b/libcontainer/cgroups/fs/cpu_test.go @@ -7,27 +7,30 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" ) func TestCpuSetShares(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") const ( sharesBefore = 1024 sharesAfter = 512 ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpu.shares": strconv.Itoa(sharesBefore), }) - helper.CgroupData.config.Resources.CpuShares = sharesAfter + r := &configs.Resources{ + CpuShares: sharesAfter, + } cpu := &CpuGroup{} - if err := cpu.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := cpu.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.shares") + value, err := fscommon.GetCgroupParamUint(path, "cpu.shares") if err != nil { t.Fatal(err) } @@ -37,7 +40,7 @@ func TestCpuSetShares(t *testing.T) { } func TestCpuSetBandWidth(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") const ( quotaBefore = 8000 @@ -50,23 +53,25 @@ func TestCpuSetBandWidth(t *testing.T) { rtPeriodAfter = 7000 ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpu.cfs_quota_us": strconv.Itoa(quotaBefore), "cpu.cfs_period_us": strconv.Itoa(periodBefore), "cpu.rt_runtime_us": strconv.Itoa(rtRuntimeBefore), "cpu.rt_period_us": strconv.Itoa(rtPeriodBefore), }) - helper.CgroupData.config.Resources.CpuQuota = quotaAfter - helper.CgroupData.config.Resources.CpuPeriod = periodAfter - helper.CgroupData.config.Resources.CpuRtRuntime = rtRuntimeAfter - helper.CgroupData.config.Resources.CpuRtPeriod = rtPeriodAfter + r := &configs.Resources{ + CpuQuota: quotaAfter, + CpuPeriod: periodAfter, + CpuRtRuntime: rtRuntimeAfter, + CpuRtPeriod: rtPeriodAfter, + } cpu := &CpuGroup{} - if err := cpu.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := cpu.Set(path, r); err != nil { t.Fatal(err) } - quota, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.cfs_quota_us") + quota, err := fscommon.GetCgroupParamUint(path, "cpu.cfs_quota_us") if err != nil { t.Fatal(err) } @@ -74,7 +79,7 @@ func TestCpuSetBandWidth(t *testing.T) { t.Fatal("Got the wrong value, set cpu.cfs_quota_us failed.") } - period, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.cfs_period_us") + period, err := fscommon.GetCgroupParamUint(path, "cpu.cfs_period_us") if err != nil { t.Fatal(err) } @@ -82,7 +87,7 @@ func TestCpuSetBandWidth(t *testing.T) { t.Fatal("Got the wrong value, set cpu.cfs_period_us failed.") } - rtRuntime, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.rt_runtime_us") + rtRuntime, err := fscommon.GetCgroupParamUint(path, "cpu.rt_runtime_us") if err != nil { t.Fatal(err) } @@ -90,7 +95,7 @@ func TestCpuSetBandWidth(t *testing.T) { t.Fatal("Got the wrong value, set cpu.rt_runtime_us failed.") } - rtPeriod, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.rt_period_us") + rtPeriod, err := fscommon.GetCgroupParamUint(path, "cpu.rt_period_us") if err != nil { t.Fatal(err) } @@ -100,7 +105,7 @@ func TestCpuSetBandWidth(t *testing.T) { } func TestCpuStats(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") const ( nrPeriods = 2000 @@ -110,13 +115,13 @@ func TestCpuStats(t *testing.T) { cpuStatContent := fmt.Sprintf("nr_periods %d\nnr_throttled %d\nthrottled_time %d\n", nrPeriods, nrThrottled, throttledTime) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpu.stat": cpuStatContent, }) cpu := &CpuGroup{} actualStats := *cgroups.NewStats() - err := cpu.GetStats(helper.CgroupPath, &actualStats) + err := cpu.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -131,36 +136,36 @@ func TestCpuStats(t *testing.T) { } func TestNoCpuStatFile(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") cpu := &CpuGroup{} actualStats := *cgroups.NewStats() - err := cpu.GetStats(helper.CgroupPath, &actualStats) + err := cpu.GetStats(path, &actualStats) if err != nil { t.Fatal("Expected not to fail, but did") } } func TestInvalidCpuStat(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") cpuStatContent := `nr_periods 2000 nr_throttled 200 throttled_time fortytwo` - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpu.stat": cpuStatContent, }) cpu := &CpuGroup{} actualStats := *cgroups.NewStats() - err := cpu.GetStats(helper.CgroupPath, &actualStats) + err := cpu.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failed stat parsing.") } } func TestCpuSetRtSchedAtApply(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") const ( rtRuntimeBefore = 0 @@ -169,21 +174,22 @@ func TestCpuSetRtSchedAtApply(t *testing.T) { rtPeriodAfter = 7000 ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpu.rt_runtime_us": strconv.Itoa(rtRuntimeBefore), "cpu.rt_period_us": strconv.Itoa(rtPeriodBefore), }) - helper.CgroupData.config.Resources.CpuRtRuntime = rtRuntimeAfter - helper.CgroupData.config.Resources.CpuRtPeriod = rtPeriodAfter + r := &configs.Resources{ + CpuRtRuntime: rtRuntimeAfter, + CpuRtPeriod: rtPeriodAfter, + } cpu := &CpuGroup{} - helper.CgroupData.pid = 1234 - if err := cpu.Apply(helper.CgroupPath, helper.CgroupData); err != nil { + if err := cpu.Apply(path, r, 1234); err != nil { t.Fatal(err) } - rtRuntime, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.rt_runtime_us") + rtRuntime, err := fscommon.GetCgroupParamUint(path, "cpu.rt_runtime_us") if err != nil { t.Fatal(err) } @@ -191,7 +197,7 @@ func TestCpuSetRtSchedAtApply(t *testing.T) { t.Fatal("Got the wrong value, set cpu.rt_runtime_us failed.") } - rtPeriod, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.rt_period_us") + rtPeriod, err := fscommon.GetCgroupParamUint(path, "cpu.rt_period_us") if err != nil { t.Fatal(err) } @@ -199,7 +205,7 @@ func TestCpuSetRtSchedAtApply(t *testing.T) { t.Fatal("Got the wrong value, set cpu.rt_period_us failed.") } - pid, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cgroup.procs") + pid, err := fscommon.GetCgroupParamUint(path, "cgroup.procs") if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/cpuacct.go b/libcontainer/cgroups/fs/cpuacct.go index 537bada66..d3bd7e111 100644 --- a/libcontainer/cgroups/fs/cpuacct.go +++ b/libcontainer/cgroups/fs/cpuacct.go @@ -34,8 +34,8 @@ func (s *CpuacctGroup) Name() string { return "cpuacct" } -func (s *CpuacctGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *CpuacctGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *CpuacctGroup) Set(_ string, _ *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/cpuacct_test.go b/libcontainer/cgroups/fs/cpuacct_test.go index 753a0194b..70b237a0b 100644 --- a/libcontainer/cgroups/fs/cpuacct_test.go +++ b/libcontainer/cgroups/fs/cpuacct_test.go @@ -24,8 +24,8 @@ const ( ) func TestCpuacctStats(t *testing.T) { - helper := NewCgroupTestUtil("cpuacct.", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "cpuacct") + writeFileContents(t, path, map[string]string{ "cpuacct.usage": cpuAcctUsageContents, "cpuacct.usage_percpu": cpuAcctUsagePerCPUContents, "cpuacct.stat": cpuAcctStatContents, @@ -34,7 +34,7 @@ func TestCpuacctStats(t *testing.T) { cpuacct := &CpuacctGroup{} actualStats := *cgroups.NewStats() - err := cpuacct.GetStats(helper.CgroupPath, &actualStats) + err := cpuacct.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -64,8 +64,8 @@ func TestCpuacctStats(t *testing.T) { } func TestCpuacctStatsWithoutUsageAll(t *testing.T) { - helper := NewCgroupTestUtil("cpuacct.", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "cpuacct") + writeFileContents(t, path, map[string]string{ "cpuacct.usage": cpuAcctUsageContents, "cpuacct.usage_percpu": cpuAcctUsagePerCPUContents, "cpuacct.stat": cpuAcctStatContents, @@ -73,7 +73,7 @@ func TestCpuacctStatsWithoutUsageAll(t *testing.T) { cpuacct := &CpuacctGroup{} actualStats := *cgroups.NewStats() - err := cpuacct.GetStats(helper.CgroupPath, &actualStats) + err := cpuacct.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/cpuset.go b/libcontainer/cgroups/fs/cpuset.go index d4edfaafa..550baa427 100644 --- a/libcontainer/cgroups/fs/cpuset.go +++ b/libcontainer/cgroups/fs/cpuset.go @@ -20,8 +20,8 @@ func (s *CpusetGroup) Name() string { return "cpuset" } -func (s *CpusetGroup) Apply(path string, d *cgroupData) error { - return s.ApplyDir(path, d.config.Resources, d.pid) +func (s *CpusetGroup) Apply(path string, r *configs.Resources, pid int) error { + return s.ApplyDir(path, r, pid) } func (s *CpusetGroup) Set(path string, r *configs.Resources) error { @@ -166,9 +166,8 @@ func (s *CpusetGroup) ApplyDir(dir string, r *configs.Resources, pid int) error if err := s.ensureCpusAndMems(dir, r); 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 + // Since we are not using apply(), we need to place the pid + // into the procs file. return cgroups.WriteCgroupProc(dir, pid) } diff --git a/libcontainer/cgroups/fs/cpuset_test.go b/libcontainer/cgroups/fs/cpuset_test.go index 4918aaa28..8933b3ca3 100644 --- a/libcontainer/cgroups/fs/cpuset_test.go +++ b/libcontainer/cgroups/fs/cpuset_test.go @@ -6,6 +6,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" ) const ( @@ -37,24 +38,26 @@ var cpusetTestFiles = map[string]string{ } func TestCPUSetSetCpus(t *testing.T) { - helper := NewCgroupTestUtil("cpuset", t) + path := tempDir(t, "cpuset") const ( cpusBefore = "0" cpusAfter = "1-3" ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpuset.cpus": cpusBefore, }) - helper.CgroupData.config.Resources.CpusetCpus = cpusAfter + r := &configs.Resources{ + CpusetCpus: cpusAfter, + } cpuset := &CpusetGroup{} - if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := cpuset.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "cpuset.cpus") + value, err := fscommon.GetCgroupParamString(path, "cpuset.cpus") if err != nil { t.Fatal(err) } @@ -64,24 +67,26 @@ func TestCPUSetSetCpus(t *testing.T) { } func TestCPUSetSetMems(t *testing.T) { - helper := NewCgroupTestUtil("cpuset", t) + path := tempDir(t, "cpuset") const ( memsBefore = "0" memsAfter = "1" ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpuset.mems": memsBefore, }) - helper.CgroupData.config.Resources.CpusetMems = memsAfter + r := &configs.Resources{ + CpusetMems: memsAfter, + } cpuset := &CpusetGroup{} - if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := cpuset.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "cpuset.mems") + value, err := fscommon.GetCgroupParamString(path, "cpuset.mems") if err != nil { t.Fatal(err) } @@ -91,12 +96,12 @@ func TestCPUSetSetMems(t *testing.T) { } func TestCPUSetStatsCorrect(t *testing.T) { - helper := NewCgroupTestUtil("cpuset", t) - helper.writeFileContents(cpusetTestFiles) + path := tempDir(t, "cpuset") + writeFileContents(t, path, cpusetTestFiles) cpuset := &CpusetGroup{} actualStats := *cgroups.NewStats() - err := cpuset.GetStats(helper.CgroupPath, &actualStats) + err := cpuset.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -205,7 +210,7 @@ func TestCPUSetStatsMissingFiles(t *testing.T) { }, } { t.Run(testCase.desc, func(t *testing.T) { - helper := NewCgroupTestUtil("cpuset", t) + path := tempDir(t, "cpuset") tempCpusetTestFiles := map[string]string{} for i, v := range cpusetTestFiles { @@ -214,19 +219,19 @@ func TestCPUSetStatsMissingFiles(t *testing.T) { if testCase.removeFile { delete(tempCpusetTestFiles, testCase.filename) - helper.writeFileContents(tempCpusetTestFiles) + writeFileContents(t, path, tempCpusetTestFiles) cpuset := &CpusetGroup{} actualStats := *cgroups.NewStats() - err := cpuset.GetStats(helper.CgroupPath, &actualStats) + err := cpuset.GetStats(path, &actualStats) if err != nil { t.Errorf("failed unexpectedly: %q", err) } } else { tempCpusetTestFiles[testCase.filename] = testCase.contents - helper.writeFileContents(tempCpusetTestFiles) + writeFileContents(t, path, tempCpusetTestFiles) cpuset := &CpusetGroup{} actualStats := *cgroups.NewStats() - err := cpuset.GetStats(helper.CgroupPath, &actualStats) + err := cpuset.GetStats(path, &actualStats) if err == nil { t.Error("failed to return expected error") diff --git a/libcontainer/cgroups/fs/devices.go b/libcontainer/cgroups/fs/devices.go index 20a11e8c6..2476186a6 100644 --- a/libcontainer/cgroups/fs/devices.go +++ b/libcontainer/cgroups/fs/devices.go @@ -20,8 +20,8 @@ func (s *DevicesGroup) Name() string { return "devices" } -func (s *DevicesGroup) Apply(path string, d *cgroupData) error { - if d.config.SkipDevices { +func (s *DevicesGroup) Apply(path string, r *configs.Resources, pid int) error { + if r.SkipDevices { return nil } if path == "" { @@ -29,7 +29,8 @@ func (s *DevicesGroup) Apply(path string, d *cgroupData) error { // is a hard requirement for container's security. return errSubsystemDoesNotExist } - return apply(path, d.pid) + + return apply(path, pid) } func loadEmulator(path string) (*cgroupdevices.Emulator, error) { diff --git a/libcontainer/cgroups/fs/devices_test.go b/libcontainer/cgroups/fs/devices_test.go index b2d1624ce..4819eee53 100644 --- a/libcontainer/cgroups/fs/devices_test.go +++ b/libcontainer/cgroups/fs/devices_test.go @@ -4,35 +4,38 @@ import ( "testing" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/devices" ) func TestDevicesSetAllow(t *testing.T) { - helper := NewCgroupTestUtil("devices", t) + path := tempDir(t, "devices") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "devices.allow": "", "devices.deny": "", "devices.list": "a *:* rwm", }) - helper.CgroupData.config.Resources.Devices = []*devices.Rule{ - { - Type: devices.CharDevice, - Major: 1, - Minor: 5, - Permissions: devices.Permissions("rwm"), - Allow: true, + r := &configs.Resources{ + Devices: []*devices.Rule{ + { + Type: devices.CharDevice, + Major: 1, + Minor: 5, + Permissions: devices.Permissions("rwm"), + Allow: true, + }, }, } d := &DevicesGroup{testingSkipFinalCheck: true} - if err := d.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := d.Set(path, r); err != nil { t.Fatal(err) } // The default deny rule must be written. - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "devices.deny") + value, err := fscommon.GetCgroupParamString(path, "devices.deny") if err != nil { t.Fatal(err) } @@ -41,7 +44,7 @@ func TestDevicesSetAllow(t *testing.T) { } // Permitted rule must be written. - if value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "devices.allow"); err != nil { + if value, err := fscommon.GetCgroupParamString(path, "devices.allow"); err != nil { t.Fatal(err) } else if value != "c 1:5 rwm" { t.Errorf("Got the wrong value (%q), set devices.allow failed.", value) diff --git a/libcontainer/cgroups/fs/freezer.go b/libcontainer/cgroups/fs/freezer.go index 1dc4dd9fc..987f1bf5e 100644 --- a/libcontainer/cgroups/fs/freezer.go +++ b/libcontainer/cgroups/fs/freezer.go @@ -19,8 +19,8 @@ func (s *FreezerGroup) Name() string { return "freezer" } -func (s *FreezerGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *FreezerGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *FreezerGroup) Set(path string, r *configs.Resources) (Err error) { diff --git a/libcontainer/cgroups/fs/freezer_test.go b/libcontainer/cgroups/fs/freezer_test.go index b441bf74b..bbdd3717d 100644 --- a/libcontainer/cgroups/fs/freezer_test.go +++ b/libcontainer/cgroups/fs/freezer_test.go @@ -8,19 +8,21 @@ import ( ) func TestFreezerSetState(t *testing.T) { - helper := NewCgroupTestUtil("freezer", t) + path := tempDir(t, "freezer") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "freezer.state": string(configs.Frozen), }) - helper.CgroupData.config.Resources.Freezer = configs.Thawed + r := &configs.Resources{ + Freezer: configs.Thawed, + } freezer := &FreezerGroup{} - if err := freezer.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := freezer.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "freezer.state") + value, err := fscommon.GetCgroupParamString(path, "freezer.state") if err != nil { t.Fatal(err) } @@ -30,15 +32,15 @@ func TestFreezerSetState(t *testing.T) { } func TestFreezerSetInvalidState(t *testing.T) { - helper := NewCgroupTestUtil("freezer", t) + path := tempDir(t, "freezer") - const ( - invalidArg configs.FreezerState = "Invalid" - ) + const invalidArg configs.FreezerState = "Invalid" - helper.CgroupData.config.Resources.Freezer = invalidArg + r := &configs.Resources{ + Freezer: invalidArg, + } freezer := &FreezerGroup{} - if err := freezer.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err == nil { + if err := freezer.Set(path, r); 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 391c9532b..36e0c76c4 100644 --- a/libcontainer/cgroups/fs/fs.go +++ b/libcontainer/cgroups/fs/fs.go @@ -38,10 +38,12 @@ var errSubsystemDoesNotExist = errors.New("cgroup: subsystem does not exist") type subsystem interface { // Name returns the name of the subsystem. Name() string - // Returns the stats, as 'stats', corresponding to the cgroup under 'path'. + // GetStats fills in the stats for the subsystem. GetStats(path string, stats *cgroups.Stats) error - // Creates and joins the cgroup represented by 'cgroupData'. - Apply(path string, c *cgroupData) error + // Apply creates and joins a cgroup, adding pid into it. Some + // subsystems use resources to pre-configure the cgroup parents + // before creating or joining it. + Apply(path string, r *configs.Resources, pid int) error // Set sets the cgroup resources. Set(path string, r *configs.Resources) error } @@ -94,14 +96,16 @@ func (m *manager) Apply(pid int) (err error) { return cgroups.ErrV1NoUnified } - d, err := getCgroupData(m.cgroups, pid) + root, err := rootPath() if err != nil { return err } + inner, err := innerPath(c) + m.paths = make(map[string]string) for _, sys := range subsystems { - p, err := d.path(sys.Name()) + p, err := subsysPath(root, inner, sys.Name()) if err != nil { // The non-presence of the devices subsystem is // considered fatal for security reasons. @@ -112,7 +116,7 @@ func (m *manager) Apply(pid int) (err error) { } m.paths[sys.Name()] = p - if err := sys.Apply(p, d); err != nil { + if err := sys.Apply(p, c.Resources, pid); err != nil { // In the case of rootless (including euid=0 in userns), where an // explicit cgroup path hasn't been set, we don't bail on error in // case of permission problems. Cases where limits have been set diff --git a/libcontainer/cgroups/fs/hugetlb.go b/libcontainer/cgroups/fs/hugetlb.go index f95e3be25..86650128c 100644 --- a/libcontainer/cgroups/fs/hugetlb.go +++ b/libcontainer/cgroups/fs/hugetlb.go @@ -14,8 +14,8 @@ func (s *HugetlbGroup) Name() string { return "hugetlb" } -func (s *HugetlbGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *HugetlbGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *HugetlbGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/hugetlb_test.go b/libcontainer/cgroups/fs/hugetlb_test.go index ac4ec1533..ba54f35a1 100644 --- a/libcontainer/cgroups/fs/hugetlb_test.go +++ b/libcontainer/cgroups/fs/hugetlb_test.go @@ -24,7 +24,7 @@ const ( ) func TestHugetlbSetHugetlb(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) + path := tempDir(t, "hugetlb") const ( hugetlbBefore = 256 @@ -32,27 +32,28 @@ func TestHugetlbSetHugetlb(t *testing.T) { ) for _, pageSize := range HugePageSizes { - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ fmt.Sprintf(limit, pageSize): strconv.Itoa(hugetlbBefore), }) } + r := &configs.Resources{} for _, pageSize := range HugePageSizes { - helper.CgroupData.config.Resources.HugetlbLimit = []*configs.HugepageLimit{ + r.HugetlbLimit = []*configs.HugepageLimit{ { Pagesize: pageSize, Limit: hugetlbAfter, }, } hugetlb := &HugetlbGroup{} - if err := hugetlb.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := hugetlb.Set(path, r); err != nil { t.Fatal(err) } } for _, pageSize := range HugePageSizes { limit := fmt.Sprintf(limit, pageSize) - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, limit) + value, err := fscommon.GetCgroupParamUint(path, limit) if err != nil { t.Fatal(err) } @@ -63,9 +64,9 @@ func TestHugetlbSetHugetlb(t *testing.T) { } func TestHugetlbStats(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) + path := tempDir(t, "hugetlb") for _, pageSize := range HugePageSizes { - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ fmt.Sprintf(usage, pageSize): hugetlbUsageContents, fmt.Sprintf(maxUsage, pageSize): hugetlbMaxUsageContents, fmt.Sprintf(failcnt, pageSize): hugetlbFailcnt, @@ -74,7 +75,7 @@ func TestHugetlbStats(t *testing.T) { hugetlb := &HugetlbGroup{} actualStats := *cgroups.NewStats() - err := hugetlb.GetStats(helper.CgroupPath, &actualStats) + err := hugetlb.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -85,39 +86,39 @@ func TestHugetlbStats(t *testing.T) { } func TestHugetlbStatsNoUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "hugetlb") + writeFileContents(t, path, map[string]string{ maxUsage: hugetlbMaxUsageContents, }) hugetlb := &HugetlbGroup{} actualStats := *cgroups.NewStats() - err := hugetlb.GetStats(helper.CgroupPath, &actualStats) + err := hugetlb.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestHugetlbStatsNoMaxUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) + path := tempDir(t, "hugetlb") for _, pageSize := range HugePageSizes { - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ fmt.Sprintf(usage, pageSize): hugetlbUsageContents, }) } hugetlb := &HugetlbGroup{} actualStats := *cgroups.NewStats() - err := hugetlb.GetStats(helper.CgroupPath, &actualStats) + err := hugetlb.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestHugetlbStatsBadUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) + path := tempDir(t, "hugetlb") for _, pageSize := range HugePageSizes { - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ fmt.Sprintf(usage, pageSize): "bad", maxUsage: hugetlbMaxUsageContents, }) @@ -125,22 +126,22 @@ func TestHugetlbStatsBadUsageFile(t *testing.T) { hugetlb := &HugetlbGroup{} actualStats := *cgroups.NewStats() - err := hugetlb.GetStats(helper.CgroupPath, &actualStats) + err := hugetlb.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestHugetlbStatsBadMaxUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "hugetlb") + writeFileContents(t, path, map[string]string{ usage: hugetlbUsageContents, maxUsage: "bad", }) hugetlb := &HugetlbGroup{} actualStats := *cgroups.NewStats() - err := hugetlb.GetStats(helper.CgroupPath, &actualStats) + err := hugetlb.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go index a349ad4b6..b7c75f941 100644 --- a/libcontainer/cgroups/fs/memory.go +++ b/libcontainer/cgroups/fs/memory.go @@ -30,8 +30,8 @@ func (s *MemoryGroup) Name() string { return "memory" } -func (s *MemoryGroup) Apply(path string, d *cgroupData) (err error) { - return apply(path, d.pid) +func (s *MemoryGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func setMemory(path string, val int64) error { diff --git a/libcontainer/cgroups/fs/memory_test.go b/libcontainer/cgroups/fs/memory_test.go index 7d13d2501..d305a62a3 100644 --- a/libcontainer/cgroups/fs/memory_test.go +++ b/libcontainer/cgroups/fs/memory_test.go @@ -6,6 +6,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" ) const ( @@ -38,7 +39,7 @@ whatever=100 N0=0 ) func TestMemorySetMemory(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") const ( memoryBefore = 314572800 // 300M @@ -47,19 +48,21 @@ func TestMemorySetMemory(t *testing.T) { reservationAfter = 314572800 // 300M ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.limit_in_bytes": strconv.Itoa(memoryBefore), "memory.soft_limit_in_bytes": strconv.Itoa(reservationBefore), }) - helper.CgroupData.config.Resources.Memory = memoryAfter - helper.CgroupData.config.Resources.MemoryReservation = reservationAfter + r := &configs.Resources{ + Memory: memoryAfter, + MemoryReservation: reservationAfter, + } memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.limit_in_bytes") + value, err := fscommon.GetCgroupParamUint(path, "memory.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -67,7 +70,7 @@ func TestMemorySetMemory(t *testing.T) { t.Fatal("Got the wrong value, set memory.limit_in_bytes failed.") } - value, err = fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.soft_limit_in_bytes") + value, err = fscommon.GetCgroupParamUint(path, "memory.soft_limit_in_bytes") if err != nil { t.Fatal(err) } @@ -77,24 +80,26 @@ func TestMemorySetMemory(t *testing.T) { } func TestMemorySetMemoryswap(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") const ( memoryswapBefore = 314572800 // 300M memoryswapAfter = 524288000 // 500M ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.memsw.limit_in_bytes": strconv.Itoa(memoryswapBefore), }) - helper.CgroupData.config.Resources.MemorySwap = memoryswapAfter + r := &configs.Resources{ + MemorySwap: memoryswapAfter, + } memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.memsw.limit_in_bytes") + value, err := fscommon.GetCgroupParamUint(path, "memory.memsw.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -104,7 +109,7 @@ func TestMemorySetMemoryswap(t *testing.T) { } func TestMemorySetMemoryLargerThanSwap(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") const ( memoryBefore = 314572800 // 300M @@ -113,7 +118,7 @@ func TestMemorySetMemoryLargerThanSwap(t *testing.T) { memoryswapAfter = 838860800 // 800M ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.limit_in_bytes": strconv.Itoa(memoryBefore), "memory.memsw.limit_in_bytes": strconv.Itoa(memoryswapBefore), // Set will call getMemoryData when memory and swap memory are @@ -123,14 +128,16 @@ func TestMemorySetMemoryLargerThanSwap(t *testing.T) { "memory.failcnt": "0", }) - helper.CgroupData.config.Resources.Memory = memoryAfter - helper.CgroupData.config.Resources.MemorySwap = memoryswapAfter + r := &configs.Resources{ + Memory: memoryAfter, + MemorySwap: memoryswapAfter, + } memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.limit_in_bytes") + value, err := fscommon.GetCgroupParamUint(path, "memory.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -138,7 +145,7 @@ func TestMemorySetMemoryLargerThanSwap(t *testing.T) { t.Fatal("Got the wrong value, set memory.limit_in_bytes failed.") } - value, err = fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.memsw.limit_in_bytes") + value, err = fscommon.GetCgroupParamUint(path, "memory.memsw.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -148,7 +155,7 @@ func TestMemorySetMemoryLargerThanSwap(t *testing.T) { } func TestMemorySetSwapSmallerThanMemory(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") const ( memoryBefore = 629145600 // 600M @@ -157,19 +164,21 @@ func TestMemorySetSwapSmallerThanMemory(t *testing.T) { memoryswapAfter = 524288000 // 500M ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.limit_in_bytes": strconv.Itoa(memoryBefore), "memory.memsw.limit_in_bytes": strconv.Itoa(memoryswapBefore), }) - helper.CgroupData.config.Resources.Memory = memoryAfter - helper.CgroupData.config.Resources.MemorySwap = memoryswapAfter + r := &configs.Resources{ + Memory: memoryAfter, + MemorySwap: memoryswapAfter, + } memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.limit_in_bytes") + value, err := fscommon.GetCgroupParamUint(path, "memory.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -177,7 +186,7 @@ func TestMemorySetSwapSmallerThanMemory(t *testing.T) { t.Fatalf("Got the wrong value (%d != %d), set memory.limit_in_bytes failed", value, memoryAfter) } - value, err = fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.memsw.limit_in_bytes") + value, err = fscommon.GetCgroupParamUint(path, "memory.memsw.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -187,22 +196,24 @@ func TestMemorySetSwapSmallerThanMemory(t *testing.T) { } func TestMemorySetMemorySwappinessDefault(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") swappinessBefore := 60 // default is 60 swappinessAfter := uint64(0) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.swappiness": strconv.Itoa(swappinessBefore), }) - helper.CgroupData.config.Resources.MemorySwappiness = &swappinessAfter + r := &configs.Resources{ + MemorySwappiness: &swappinessAfter, + } memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.swappiness") + value, err := fscommon.GetCgroupParamUint(path, "memory.swappiness") if err != nil { t.Fatal(err) } @@ -212,8 +223,8 @@ func TestMemorySetMemorySwappinessDefault(t *testing.T) { } func TestMemoryStats(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": memoryUsageContents, "memory.limit_in_bytes": memoryLimitContents, @@ -233,7 +244,7 @@ func TestMemoryStats(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -263,8 +274,8 @@ func TestMemoryStats(t *testing.T) { } func TestMemoryStatsNoStatFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.usage_in_bytes": memoryUsageContents, "memory.max_usage_in_bytes": memoryMaxUsageContents, "memory.limit_in_bytes": memoryLimitContents, @@ -272,15 +283,15 @@ func TestMemoryStatsNoStatFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } } func TestMemoryStatsNoUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.max_usage_in_bytes": memoryMaxUsageContents, "memory.limit_in_bytes": memoryLimitContents, @@ -288,15 +299,15 @@ func TestMemoryStatsNoUsageFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsNoMaxUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": memoryUsageContents, "memory.limit_in_bytes": memoryLimitContents, @@ -304,15 +315,15 @@ func TestMemoryStatsNoMaxUsageFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsNoLimitInBytesFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": memoryUsageContents, "memory.max_usage_in_bytes": memoryMaxUsageContents, @@ -320,15 +331,15 @@ func TestMemoryStatsNoLimitInBytesFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsBadStatFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": "rss rss", "memory.usage_in_bytes": memoryUsageContents, "memory.max_usage_in_bytes": memoryMaxUsageContents, @@ -337,15 +348,15 @@ func TestMemoryStatsBadStatFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsBadUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": "bad", "memory.max_usage_in_bytes": memoryMaxUsageContents, @@ -354,15 +365,15 @@ func TestMemoryStatsBadUsageFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsBadMaxUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": memoryUsageContents, "memory.max_usage_in_bytes": "bad", @@ -371,15 +382,15 @@ func TestMemoryStatsBadMaxUsageFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsBadLimitInBytesFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": memoryUsageContents, "memory.max_usage_in_bytes": memoryMaxUsageContents, @@ -388,29 +399,30 @@ func TestMemoryStatsBadLimitInBytesFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemorySetOomControl(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") const ( oomKillDisable = 1 // disable oom killer, default is 0 ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.oom_control": strconv.Itoa(oomKillDisable), }) memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + r := &configs.Resources{} + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.oom_control") + value, err := fscommon.GetCgroupParamUint(path, "memory.oom_control") if err != nil { t.Fatal(err) } @@ -420,12 +432,12 @@ func TestMemorySetOomControl(t *testing.T) { } func TestNoHierarchicalNumaStat(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.numa_stat": memoryNUMAStatNoHierarchyContents + memoryNUMAStatExtraContents, }) - actualStats, err := getPageUsageByNUMA(helper.CgroupPath) + actualStats, err := getPageUsageByNUMA(path) if err != nil { t.Fatal(err) } @@ -470,13 +482,13 @@ anon=183 N0=12 badone `, }, } - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") for _, c := range memoryNUMAStatBadContents { - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.numa_stat": c.contents, }) - _, err := getPageUsageByNUMA(helper.CgroupPath) + _, err := getPageUsageByNUMA(path) if err == nil { t.Errorf("case %q: expected error, got nil", c.desc) } @@ -484,9 +496,9 @@ anon=183 N0=12 badone } func TestWithoutNumaStat(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") - actualStats, err := getPageUsageByNUMA(helper.CgroupPath) + actualStats, err := getPageUsageByNUMA(path) if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/name.go b/libcontainer/cgroups/fs/name.go index 056bf825d..b8d5d849c 100644 --- a/libcontainer/cgroups/fs/name.go +++ b/libcontainer/cgroups/fs/name.go @@ -14,10 +14,10 @@ func (s *NameGroup) Name() string { return s.GroupName } -func (s *NameGroup) Apply(path string, d *cgroupData) error { +func (s *NameGroup) Apply(path string, _ *configs.Resources, pid int) error { if s.Join { - // ignore errors if the named cgroup does not exist - _ = apply(path, d.pid) + // Ignore errors if the named cgroup does not exist. + _ = apply(path, pid) } return nil } diff --git a/libcontainer/cgroups/fs/net_cls.go b/libcontainer/cgroups/fs/net_cls.go index ef9514fb8..abfd09ce8 100644 --- a/libcontainer/cgroups/fs/net_cls.go +++ b/libcontainer/cgroups/fs/net_cls.go @@ -13,8 +13,8 @@ func (s *NetClsGroup) Name() string { return "net_cls" } -func (s *NetClsGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *NetClsGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *NetClsGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/net_cls_test.go b/libcontainer/cgroups/fs/net_cls_test.go index 6c36f0555..085c06151 100644 --- a/libcontainer/cgroups/fs/net_cls_test.go +++ b/libcontainer/cgroups/fs/net_cls_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" ) const ( @@ -13,22 +14,24 @@ const ( ) func TestNetClsSetClassid(t *testing.T) { - helper := NewCgroupTestUtil("net_cls", t) + path := tempDir(t, "net_cls") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "net_cls.classid": strconv.FormatUint(classidBefore, 10), }) - helper.CgroupData.config.Resources.NetClsClassid = classidAfter + r := &configs.Resources{ + NetClsClassid: classidAfter, + } netcls := &NetClsGroup{} - if err := netcls.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := netcls.Set(path, r); err != nil { t.Fatal(err) } // As we are in mock environment, we can't get correct value of classid from // net_cls.classid. // So. we just judge if we successfully write classid into file - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "net_cls.classid") + value, err := fscommon.GetCgroupParamUint(path, "net_cls.classid") if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/net_prio.go b/libcontainer/cgroups/fs/net_prio.go index 38c26989a..da74d3779 100644 --- a/libcontainer/cgroups/fs/net_prio.go +++ b/libcontainer/cgroups/fs/net_prio.go @@ -11,8 +11,8 @@ func (s *NetPrioGroup) Name() string { return "net_prio" } -func (s *NetPrioGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *NetPrioGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *NetPrioGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/net_prio_test.go b/libcontainer/cgroups/fs/net_prio_test.go index 7983041a4..453ff363b 100644 --- a/libcontainer/cgroups/fs/net_prio_test.go +++ b/libcontainer/cgroups/fs/net_prio_test.go @@ -16,15 +16,17 @@ var prioMap = []*configs.IfPrioMap{ } func TestNetPrioSetIfPrio(t *testing.T) { - helper := NewCgroupTestUtil("net_prio", t) + path := tempDir(t, "net_prio") - helper.CgroupData.config.Resources.NetPrioIfpriomap = prioMap + r := &configs.Resources{ + NetPrioIfpriomap: prioMap, + } netPrio := &NetPrioGroup{} - if err := netPrio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := netPrio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "net_prio.ifpriomap") + value, err := fscommon.GetCgroupParamString(path, "net_prio.ifpriomap") if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/paths.go b/libcontainer/cgroups/fs/paths.go index 9556f8b4c..fb4ff8b80 100644 --- a/libcontainer/cgroups/fs/paths.go +++ b/libcontainer/cgroups/fs/paths.go @@ -69,8 +69,8 @@ func tryDefaultCgroupRoot() string { return defaultCgroupRoot } -// Gets the cgroupRoot. -func getCgroupRoot() (string, error) { +// rootPath finds and returns path to the root of the cgroup hierarchies. +func rootPath() (string, error) { cgroupRootLock.Lock() defer cgroupRootLock.Unlock() @@ -105,21 +105,9 @@ func getCgroupRoot() (string, error) { return cgroupRoot, nil } -type cgroupData struct { - root string - innerPath string - config *configs.Cgroup - pid int -} - -func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { - root, err := getCgroupRoot() - if err != nil { - return nil, err - } - +func innerPath(c *configs.Cgroup) (string, error) { if (c.Name != "" || c.Parent != "") && c.Path != "" { - return nil, errors.New("cgroup: either Path or Name and Parent should be used") + return "", errors.New("cgroup: either Path or Name and Parent should be used") } // XXX: Do not remove CleanPath. Path safety is important! -- cyphar @@ -130,25 +118,20 @@ func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { innerPath = filepath.Join(cgParent, cgName) } - return &cgroupData{ - root: root, - innerPath: innerPath, - config: c, - pid: pid, - }, nil + return innerPath, nil } -func (raw *cgroupData) path(subsystem string) (string, error) { +func subsysPath(root, inner, subsystem string) (string, error) { // If the cgroup name/path is absolute do not look relative to the cgroup of the init process. - if filepath.IsAbs(raw.innerPath) { - mnt, err := cgroups.FindCgroupMountpoint(raw.root, subsystem) + if filepath.IsAbs(inner) { + mnt, err := cgroups.FindCgroupMountpoint(root, subsystem) // If we didn't mount the subsystem, there is no point we make the path. if err != nil { return "", err } // Sometimes subsystems can be mounted together as 'cpu,cpuacct'. - return filepath.Join(raw.root, filepath.Base(mnt), raw.innerPath), nil + return filepath.Join(root, filepath.Base(mnt), inner), nil } // Use GetOwnCgroupPath instead of GetInitCgroupPath, because the creating @@ -159,7 +142,7 @@ func (raw *cgroupData) path(subsystem string) (string, error) { return "", err } - return filepath.Join(parentPath, raw.innerPath), nil + return filepath.Join(parentPath, inner), nil } func apply(path string, pid int) error { diff --git a/libcontainer/cgroups/fs/paths_test.go b/libcontainer/cgroups/fs/paths_test.go index 58ebfc7c4..3a4d45fc2 100644 --- a/libcontainer/cgroups/fs/paths_test.go +++ b/libcontainer/cgroups/fs/paths_test.go @@ -14,7 +14,7 @@ func TestInvalidCgroupPath(t *testing.T) { t.Skip("cgroup v2 is not supported") } - root, err := getCgroupRoot() + root, err := rootPath() if err != nil { t.Fatalf("couldn't get cgroup root: %v", err) } @@ -67,19 +67,19 @@ func TestInvalidCgroupPath(t *testing.T) { t.Run(tc.test, func(t *testing.T) { config := &configs.Cgroup{Path: tc.path, Name: tc.name, Parent: tc.parent} - data, err := getCgroupData(config, 0) + inner, err := innerPath(config) if err != nil { t.Fatalf("couldn't get cgroup data: %v", err) } - // Make sure the final innerPath doesn't go outside the cgroup mountpoint. - if strings.HasPrefix(data.innerPath, "..") { + // Make sure the final inner path doesn't go outside the cgroup mountpoint. + if strings.HasPrefix(inner, "..") { t.Errorf("SECURITY: cgroup innerPath is outside cgroup mountpoint!") } // Double-check, using an actual cgroup. deviceRoot := filepath.Join(root, "devices") - devicePath, err := data.path("devices") + devicePath, err := subsysPath(root, inner, "devices") if err != nil { t.Fatalf("couldn't get cgroup path: %v", err) } diff --git a/libcontainer/cgroups/fs/perf_event.go b/libcontainer/cgroups/fs/perf_event.go index 486c7fe45..b86955c8f 100644 --- a/libcontainer/cgroups/fs/perf_event.go +++ b/libcontainer/cgroups/fs/perf_event.go @@ -11,8 +11,8 @@ func (s *PerfEventGroup) Name() string { return "perf_event" } -func (s *PerfEventGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *PerfEventGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *PerfEventGroup) Set(_ string, _ *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/pids.go b/libcontainer/cgroups/fs/pids.go index 33bb64c9b..1f13532a5 100644 --- a/libcontainer/cgroups/fs/pids.go +++ b/libcontainer/cgroups/fs/pids.go @@ -15,8 +15,8 @@ func (s *PidsGroup) Name() string { return "pids" } -func (s *PidsGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *PidsGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *PidsGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/pids_test.go b/libcontainer/cgroups/fs/pids_test.go index d05ec7215..9d9a7ce8f 100644 --- a/libcontainer/cgroups/fs/pids_test.go +++ b/libcontainer/cgroups/fs/pids_test.go @@ -6,6 +6,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" ) const ( @@ -14,19 +15,21 @@ const ( ) func TestPidsSetMax(t *testing.T) { - helper := NewCgroupTestUtil("pids", t) + path := tempDir(t, "pids") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "pids.max": "max", }) - helper.CgroupData.config.Resources.PidsLimit = maxLimited + r := &configs.Resources{ + PidsLimit: maxLimited, + } pids := &PidsGroup{} - if err := pids.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := pids.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "pids.max") + value, err := fscommon.GetCgroupParamUint(path, "pids.max") if err != nil { t.Fatal(err) } @@ -36,19 +39,21 @@ func TestPidsSetMax(t *testing.T) { } func TestPidsSetUnlimited(t *testing.T) { - helper := NewCgroupTestUtil("pids", t) + path := tempDir(t, "pids") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "pids.max": strconv.Itoa(maxLimited), }) - helper.CgroupData.config.Resources.PidsLimit = maxUnlimited + r := &configs.Resources{ + PidsLimit: maxUnlimited, + } pids := &PidsGroup{} - if err := pids.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := pids.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "pids.max") + value, err := fscommon.GetCgroupParamString(path, "pids.max") if err != nil { t.Fatal(err) } @@ -58,16 +63,16 @@ func TestPidsSetUnlimited(t *testing.T) { } func TestPidsStats(t *testing.T) { - helper := NewCgroupTestUtil("pids", t) + path := tempDir(t, "pids") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "pids.current": strconv.Itoa(1337), "pids.max": strconv.Itoa(maxLimited), }) pids := &PidsGroup{} stats := *cgroups.NewStats() - if err := pids.GetStats(helper.CgroupPath, &stats); err != nil { + if err := pids.GetStats(path, &stats); err != nil { t.Fatal(err) } @@ -81,16 +86,16 @@ func TestPidsStats(t *testing.T) { } func TestPidsStatsUnlimited(t *testing.T) { - helper := NewCgroupTestUtil("pids", t) + path := tempDir(t, "pids") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "pids.current": strconv.Itoa(4096), "pids.max": "max", }) pids := &PidsGroup{} stats := *cgroups.NewStats() - if err := pids.GetStats(helper.CgroupPath, &stats); err != nil { + if err := pids.GetStats(path, &stats); err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/rdma.go b/libcontainer/cgroups/fs/rdma.go index 3548db3b6..5bbe0f35f 100644 --- a/libcontainer/cgroups/fs/rdma.go +++ b/libcontainer/cgroups/fs/rdma.go @@ -12,8 +12,8 @@ func (s *RdmaGroup) Name() string { return "rdma" } -func (s *RdmaGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *RdmaGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *RdmaGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/util_test.go b/libcontainer/cgroups/fs/util_test.go index 4a4013774..85842b735 100644 --- a/libcontainer/cgroups/fs/util_test.go +++ b/libcontainer/cgroups/fs/util_test.go @@ -11,45 +11,29 @@ import ( "testing" "github.com/opencontainers/runc/libcontainer/cgroups" - "github.com/opencontainers/runc/libcontainer/configs" ) func init() { cgroups.TestMode = true } -type cgroupTestUtil struct { - // cgroup data to use in tests. - CgroupData *cgroupData - - // Path to the mock cgroup directory. - CgroupPath string - - t *testing.T -} - -// Creates a new test util for the specified subsystem -func NewCgroupTestUtil(subsystem string, t *testing.T) *cgroupTestUtil { - d := &cgroupData{ - config: &configs.Cgroup{ - Resources: &configs.Resources{}, - }, - root: t.TempDir(), - } - testCgroupPath := filepath.Join(d.root, subsystem) +// tempDir creates a new test directory for the specified subsystem. +func tempDir(t *testing.T, subsystem string) string { + path := filepath.Join(t.TempDir(), subsystem) // Ensure the full mock cgroup path exists. - if err := os.MkdirAll(testCgroupPath, 0o755); err != nil { + if err := os.Mkdir(path, 0o755); err != nil { t.Fatal(err) } - return &cgroupTestUtil{CgroupData: d, CgroupPath: testCgroupPath, t: t} + return path } -// Write the specified contents on the mock of the specified cgroup files. -func (c *cgroupTestUtil) writeFileContents(fileContents map[string]string) { +// writeFileContents writes the specified contents on the mock of the specified +// cgroup files. +func writeFileContents(t *testing.T, path string, fileContents map[string]string) { for file, contents := range fileContents { - err := cgroups.WriteFile(c.CgroupPath, file, contents) + err := cgroups.WriteFile(path, file, contents) if err != nil { - c.t.Fatal(err) + t.Fatal(err) } } }