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) } } }