diff --git a/libcontainer/cgroups/fs/blkio.go b/libcontainer/cgroups/fs/blkio.go index 2d764001ead..c81b6562ac5 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 join(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 eb6257c0d41..7810a67fc31 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 cd81cabc185..6c79f899b48 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 958e6040e72..bbdd45a7118 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 076704ab742..d3bd7e111c7 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 join(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 753a0194b71..70b237a0b61 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 d4edfaafad2..550baa42756 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 4918aaa2867..8933b3ca351 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 2c8917049ea..2476186a640 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 join(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 b2d1624cec8..4819eee5366 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 cf1c5efb0a5..987f1bf5e74 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 join(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 b441bf74b69..bbdd3717dbe 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 1c1e3f3e63e..36e0c76c447 100644 --- a/libcontainer/cgroups/fs/fs.go +++ b/libcontainer/cgroups/fs/fs.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "os" - "path/filepath" "sync" "golang.org/x/sys/unix" @@ -12,7 +11,6 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" "github.com/opencontainers/runc/libcontainer/configs" - libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils" ) var ( @@ -40,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 } @@ -63,105 +63,6 @@ func NewManager(cg *configs.Cgroup, paths map[string]string, rootless bool) cgro } } -// The absolute path to the root of the cgroup hierarchies. -var ( - cgroupRootLock sync.Mutex - cgroupRoot string -) - -const defaultCgroupRoot = "/sys/fs/cgroup" - -func tryDefaultCgroupRoot() string { - var st, pst unix.Stat_t - - // (1) it should be a directory... - err := unix.Lstat(defaultCgroupRoot, &st) - if err != nil || st.Mode&unix.S_IFDIR == 0 { - return "" - } - - // (2) ... and a mount point ... - err = unix.Lstat(filepath.Dir(defaultCgroupRoot), &pst) - if err != nil { - return "" - } - - if st.Dev == pst.Dev { - // parent dir has the same dev -- not a mount point - return "" - } - - // (3) ... of 'tmpfs' fs type. - var fst unix.Statfs_t - err = unix.Statfs(defaultCgroupRoot, &fst) - if err != nil || fst.Type != unix.TMPFS_MAGIC { - return "" - } - - // (4) it should have at least 1 entry ... - dir, err := os.Open(defaultCgroupRoot) - if err != nil { - return "" - } - names, err := dir.Readdirnames(1) - if err != nil { - return "" - } - if len(names) < 1 { - return "" - } - // ... which is a cgroup mount point. - err = unix.Statfs(filepath.Join(defaultCgroupRoot, names[0]), &fst) - if err != nil || fst.Type != unix.CGROUP_SUPER_MAGIC { - return "" - } - - return defaultCgroupRoot -} - -// Gets the cgroupRoot. -func getCgroupRoot() (string, error) { - cgroupRootLock.Lock() - defer cgroupRootLock.Unlock() - - if cgroupRoot != "" { - return cgroupRoot, nil - } - - // fast path - cgroupRoot = tryDefaultCgroupRoot() - if cgroupRoot != "" { - return cgroupRoot, nil - } - - // slow path: parse mountinfo - mi, err := cgroups.GetCgroupMounts(false) - if err != nil { - return "", err - } - if len(mi) < 1 { - return "", errors.New("no cgroup mount found in mountinfo") - } - - // Get the first cgroup mount (e.g. "/sys/fs/cgroup/memory"), - // use its parent directory. - root := filepath.Dir(mi[0].Mountpoint) - - if _, err := os.Stat(root); err != nil { - return "", err - } - - cgroupRoot = root - return cgroupRoot, nil -} - -type cgroupData struct { - root string - innerPath string - config *configs.Cgroup - pid int -} - // isIgnorableError returns whether err is a permission error (in the loose // sense of the word). This includes EROFS (which for an unprivileged user is // basically a permission error) and EACCES (for similar reasons) as well as @@ -195,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. @@ -213,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 @@ -315,68 +218,6 @@ func (m *manager) GetAllPids() ([]int, error) { return cgroups.GetAllPids(m.Path("devices")) } -func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { - root, err := getCgroupRoot() - if err != nil { - return nil, err - } - - if (c.Name != "" || c.Parent != "") && c.Path != "" { - return nil, errors.New("cgroup: either Path or Name and Parent should be used") - } - - // XXX: Do not remove this code. Path safety is important! -- cyphar - cgPath := libcontainerUtils.CleanPath(c.Path) - cgParent := libcontainerUtils.CleanPath(c.Parent) - cgName := libcontainerUtils.CleanPath(c.Name) - - innerPath := cgPath - if innerPath == "" { - innerPath = filepath.Join(cgParent, cgName) - } - - return &cgroupData{ - root: root, - innerPath: innerPath, - config: c, - pid: pid, - }, nil -} - -func (raw *cgroupData) path(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 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 - } - - // Use GetOwnCgroupPath instead of GetInitCgroupPath, because the creating - // process could in container and shared pid namespace with host, and - // /proc/1/cgroup could point to whole other world of cgroups. - parentPath, err := cgroups.GetOwnCgroupPath(subsystem) - if err != nil { - return "", err - } - - return filepath.Join(parentPath, raw.innerPath), nil -} - -func join(path string, pid int) error { - if path == "" { - return nil - } - if err := os.MkdirAll(path, 0o755); err != nil { - return err - } - return cgroups.WriteCgroupProc(path, pid) -} - func (m *manager) GetPaths() map[string]string { m.mu.Lock() defer m.mu.Unlock() diff --git a/libcontainer/cgroups/fs/fs_test.go b/libcontainer/cgroups/fs/fs_test.go index b4430603f1e..7a91768602b 100644 --- a/libcontainer/cgroups/fs/fs_test.go +++ b/libcontainer/cgroups/fs/fs_test.go @@ -1,108 +1,12 @@ package fs import ( - "path/filepath" - "strings" "testing" "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/configs" ) -func TestInvalidCgroupPath(t *testing.T) { - if cgroups.IsCgroup2UnifiedMode() { - t.Skip("cgroup v2 is not supported") - } - - root, err := getCgroupRoot() - if err != nil { - t.Fatalf("couldn't get cgroup root: %v", err) - } - - testCases := []struct { - test string - path, name, parent string - }{ - { - test: "invalid cgroup path", - path: "../../../../../../../../../../some/path", - }, - { - test: "invalid absolute cgroup path", - path: "/../../../../../../../../../../some/path", - }, - { - test: "invalid cgroup parent", - parent: "../../../../../../../../../../some/path", - name: "name", - }, - { - test: "invalid absolute cgroup parent", - parent: "/../../../../../../../../../../some/path", - name: "name", - }, - { - test: "invalid cgroup name", - parent: "parent", - name: "../../../../../../../../../../some/path", - }, - { - test: "invalid absolute cgroup name", - parent: "parent", - name: "/../../../../../../../../../../some/path", - }, - { - test: "invalid cgroup name and parent", - parent: "../../../../../../../../../../some/path", - name: "../../../../../../../../../../some/path", - }, - { - test: "invalid absolute cgroup name and parent", - parent: "/../../../../../../../../../../some/path", - name: "/../../../../../../../../../../some/path", - }, - } - - for _, tc := range testCases { - 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) - 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, "..") { - 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") - if err != nil { - t.Fatalf("couldn't get cgroup path: %v", err) - } - if !strings.HasPrefix(devicePath, deviceRoot) { - t.Errorf("SECURITY: cgroup path() is outside cgroup mountpoint!") - } - }) - } -} - -func TestTryDefaultCgroupRoot(t *testing.T) { - res := tryDefaultCgroupRoot() - exp := defaultCgroupRoot - if cgroups.IsCgroup2UnifiedMode() { - // checking that tryDefaultCgroupRoot does return "" - // in case /sys/fs/cgroup is not cgroup v1 root dir. - exp = "" - } - if res != exp { - t.Errorf("tryDefaultCgroupRoot: want %q, got %q", exp, res) - } -} - func BenchmarkGetStats(b *testing.B) { if cgroups.IsCgroup2UnifiedMode() { b.Skip("cgroup v2 is not supported") diff --git a/libcontainer/cgroups/fs/hugetlb.go b/libcontainer/cgroups/fs/hugetlb.go index 8477fa6b356..86650128c8d 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 join(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 ac4ec1533f5..ba54f35a16c 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 8e10c22b2e7..b7c75f94138 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 join(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 7d13d250111..d305a62a393 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 ce9f4a0a5ff..b8d5d849c52 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 - _ = join(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 d1364292697..abfd09ce83a 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 join(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 6c36f055597..085c0615123 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 cdc810baea6..da74d377989 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 join(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 7983041a41e..453ff363b81 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 new file mode 100644 index 00000000000..fb4ff8b80d5 --- /dev/null +++ b/libcontainer/cgroups/fs/paths.go @@ -0,0 +1,156 @@ +package fs + +import ( + "errors" + "os" + "path/filepath" + "sync" + + "golang.org/x/sys/unix" + + "github.com/opencontainers/runc/libcontainer/cgroups" + "github.com/opencontainers/runc/libcontainer/configs" + "github.com/opencontainers/runc/libcontainer/utils" +) + +// The absolute path to the root of the cgroup hierarchies. +var ( + cgroupRootLock sync.Mutex + cgroupRoot string +) + +const defaultCgroupRoot = "/sys/fs/cgroup" + +func tryDefaultCgroupRoot() string { + var st, pst unix.Stat_t + + // (1) it should be a directory... + err := unix.Lstat(defaultCgroupRoot, &st) + if err != nil || st.Mode&unix.S_IFDIR == 0 { + return "" + } + + // (2) ... and a mount point ... + err = unix.Lstat(filepath.Dir(defaultCgroupRoot), &pst) + if err != nil { + return "" + } + + if st.Dev == pst.Dev { + // parent dir has the same dev -- not a mount point + return "" + } + + // (3) ... of 'tmpfs' fs type. + var fst unix.Statfs_t + err = unix.Statfs(defaultCgroupRoot, &fst) + if err != nil || fst.Type != unix.TMPFS_MAGIC { + return "" + } + + // (4) it should have at least 1 entry ... + dir, err := os.Open(defaultCgroupRoot) + if err != nil { + return "" + } + names, err := dir.Readdirnames(1) + if err != nil { + return "" + } + if len(names) < 1 { + return "" + } + // ... which is a cgroup mount point. + err = unix.Statfs(filepath.Join(defaultCgroupRoot, names[0]), &fst) + if err != nil || fst.Type != unix.CGROUP_SUPER_MAGIC { + return "" + } + + return defaultCgroupRoot +} + +// rootPath finds and returns path to the root of the cgroup hierarchies. +func rootPath() (string, error) { + cgroupRootLock.Lock() + defer cgroupRootLock.Unlock() + + if cgroupRoot != "" { + return cgroupRoot, nil + } + + // fast path + cgroupRoot = tryDefaultCgroupRoot() + if cgroupRoot != "" { + return cgroupRoot, nil + } + + // slow path: parse mountinfo + mi, err := cgroups.GetCgroupMounts(false) + if err != nil { + return "", err + } + if len(mi) < 1 { + return "", errors.New("no cgroup mount found in mountinfo") + } + + // Get the first cgroup mount (e.g. "/sys/fs/cgroup/memory"), + // use its parent directory. + root := filepath.Dir(mi[0].Mountpoint) + + if _, err := os.Stat(root); err != nil { + return "", err + } + + cgroupRoot = root + return cgroupRoot, nil +} + +func innerPath(c *configs.Cgroup) (string, error) { + if (c.Name != "" || c.Parent != "") && c.Path != "" { + return "", errors.New("cgroup: either Path or Name and Parent should be used") + } + + // XXX: Do not remove CleanPath. Path safety is important! -- cyphar + innerPath := utils.CleanPath(c.Path) + if innerPath == "" { + cgParent := utils.CleanPath(c.Parent) + cgName := utils.CleanPath(c.Name) + innerPath = filepath.Join(cgParent, cgName) + } + + return innerPath, nil +} + +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(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(root, filepath.Base(mnt), inner), nil + } + + // Use GetOwnCgroupPath instead of GetInitCgroupPath, because the creating + // process could in container and shared pid namespace with host, and + // /proc/1/cgroup could point to whole other world of cgroups. + parentPath, err := cgroups.GetOwnCgroupPath(subsystem) + if err != nil { + return "", err + } + + return filepath.Join(parentPath, inner), nil +} + +func apply(path string, pid int) error { + if path == "" { + return nil + } + if err := os.MkdirAll(path, 0o755); err != nil { + return err + } + return cgroups.WriteCgroupProc(path, pid) +} diff --git a/libcontainer/cgroups/fs/paths_test.go b/libcontainer/cgroups/fs/paths_test.go new file mode 100644 index 00000000000..3a4d45fc212 --- /dev/null +++ b/libcontainer/cgroups/fs/paths_test.go @@ -0,0 +1,104 @@ +package fs + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/opencontainers/runc/libcontainer/cgroups" + "github.com/opencontainers/runc/libcontainer/configs" +) + +func TestInvalidCgroupPath(t *testing.T) { + if cgroups.IsCgroup2UnifiedMode() { + t.Skip("cgroup v2 is not supported") + } + + root, err := rootPath() + if err != nil { + t.Fatalf("couldn't get cgroup root: %v", err) + } + + testCases := []struct { + test string + path, name, parent string + }{ + { + test: "invalid cgroup path", + path: "../../../../../../../../../../some/path", + }, + { + test: "invalid absolute cgroup path", + path: "/../../../../../../../../../../some/path", + }, + { + test: "invalid cgroup parent", + parent: "../../../../../../../../../../some/path", + name: "name", + }, + { + test: "invalid absolute cgroup parent", + parent: "/../../../../../../../../../../some/path", + name: "name", + }, + { + test: "invalid cgroup name", + parent: "parent", + name: "../../../../../../../../../../some/path", + }, + { + test: "invalid absolute cgroup name", + parent: "parent", + name: "/../../../../../../../../../../some/path", + }, + { + test: "invalid cgroup name and parent", + parent: "../../../../../../../../../../some/path", + name: "../../../../../../../../../../some/path", + }, + { + test: "invalid absolute cgroup name and parent", + parent: "/../../../../../../../../../../some/path", + name: "/../../../../../../../../../../some/path", + }, + } + + for _, tc := range testCases { + t.Run(tc.test, func(t *testing.T) { + config := &configs.Cgroup{Path: tc.path, Name: tc.name, Parent: tc.parent} + + inner, err := innerPath(config) + if err != nil { + t.Fatalf("couldn't get cgroup data: %v", err) + } + + // 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 := subsysPath(root, inner, "devices") + if err != nil { + t.Fatalf("couldn't get cgroup path: %v", err) + } + if !strings.HasPrefix(devicePath, deviceRoot) { + t.Errorf("SECURITY: cgroup path() is outside cgroup mountpoint!") + } + }) + } +} + +func TestTryDefaultCgroupRoot(t *testing.T) { + res := tryDefaultCgroupRoot() + exp := defaultCgroupRoot + if cgroups.IsCgroup2UnifiedMode() { + // checking that tryDefaultCgroupRoot does return "" + // in case /sys/fs/cgroup is not cgroup v1 root dir. + exp = "" + } + if res != exp { + t.Errorf("tryDefaultCgroupRoot: want %q, got %q", exp, res) + } +} diff --git a/libcontainer/cgroups/fs/perf_event.go b/libcontainer/cgroups/fs/perf_event.go index 8ffa83e094e..b86955c8f31 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 join(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 55bb91fa6e8..1f13532a5ad 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 join(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 d05ec7215e6..9d9a7ce8f18 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 5fb25abe766..5bbe0f35f81 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 join(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 4a4013774df..85842b73532 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) } } } diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index 54266901ac1..470453f3316 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -99,6 +99,47 @@ func genV1ResourcesProperties(r *configs.Resources, cm *dbusConnManager) ([]syst return properties, nil } +// initPaths initializes m.paths. Supposed to be called under m.mu held. +func (m *legacyManager) initPaths() error { + if m.paths != nil { + return nil + } + + c := m.cgroups + + slice := "system.slice" + if c.Parent != "" { + var err error + slice, err = ExpandSlice(c.Parent) + if err != nil { + return err + } + } + + unit := getUnitName(c) + + paths := make(map[string]string) + for _, s := range legacySubsystems { + subsystemPath, err := getSubsystemPath(slice, unit, s.Name()) + if err != nil { + // Even if it's `not found` error, we'll return err + // because devices cgroup is hard requirement for + // container security. + if s.Name() == "devices" { + return err + } + // Don't fail if a cgroup hierarchy was not found, just skip this subsystem + if cgroups.IsNotFound(err) { + continue + } + return err + } + paths[s.Name()] = subsystemPath + } + m.paths = paths + return nil +} + func (m *legacyManager) Apply(pid int) error { var ( c = m.cgroups @@ -120,12 +161,14 @@ func (m *legacyManager) Apply(pid int) error { properties = append(properties, systemdDbus.PropDescription("libcontainer container "+c.Name)) - // if we create a slice, the parent is defined via a Wants= if strings.HasSuffix(unitName, ".slice") { + // If we create a slice, the parent is defined via a Wants=. properties = append(properties, systemdDbus.PropWants(slice)) } else { - // otherwise, we use Slice= + // Otherwise it's a scope, which we put into a Slice=. properties = append(properties, systemdDbus.PropSlice(slice)) + // Assume scopes always support delegation (supported since systemd v218). + properties = append(properties, newProp("Delegate", true)) } // only add pid if its valid, -1 is used w/ general slice creation. @@ -133,12 +176,6 @@ func (m *legacyManager) Apply(pid int) error { properties = append(properties, newProp("PIDs", []uint32{uint32(pid)})) } - // Check if we can delegate. This is only supported on systemd versions 218 and above. - if !strings.HasSuffix(unitName, ".slice") { - // Assume scopes always support delegation. - properties = append(properties, newProp("Delegate", true)) - } - // Always enable accounting, this gets us the same behaviour as the fs implementation, // plus the kernel has some problems with joining the memory cgroup at a later time. properties = append(properties, @@ -158,26 +195,9 @@ func (m *legacyManager) Apply(pid int) error { return err } - paths := make(map[string]string) - for _, s := range legacySubsystems { - subsystemPath, err := getSubsystemPath(m.cgroups, s.Name()) - if err != nil { - // Even if it's `not found` error, we'll return err - // because devices cgroup is hard requirement for - // container security. - if s.Name() == "devices" { - return err - } - // Don't fail if a cgroup hierarchy was not found, just skip this subsystem - if cgroups.IsNotFound(err) { - continue - } - return err - } - paths[s.Name()] = subsystemPath + if err := m.initPaths(); err != nil { + return err } - m.paths = paths - if err := m.joinCgroups(pid); err != nil { return err } @@ -235,7 +255,7 @@ func (m *legacyManager) joinCgroups(pid int) error { return nil } -func getSubsystemPath(c *configs.Cgroup, subsystem string) (string, error) { +func getSubsystemPath(slice, unit, subsystem string) (string, error) { mountpoint, err := cgroups.FindCgroupMountpoint("", subsystem) if err != nil { return "", err @@ -248,17 +268,7 @@ func getSubsystemPath(c *configs.Cgroup, subsystem string) (string, error) { // if pid 1 is systemd 226 or later, it will be in init.scope, not the root initPath = strings.TrimSuffix(filepath.Clean(initPath), "init.scope") - slice := "system.slice" - if c.Parent != "" { - slice = c.Parent - } - - slice, err = ExpandSlice(slice) - if err != nil { - return "", err - } - - return filepath.Join(mountpoint, initPath, slice, getUnitName(c)), nil + return filepath.Join(mountpoint, initPath, slice, unit), nil } func (m *legacyManager) Freeze(state configs.FreezerState) error { diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go index 106720d84b2..5af4500ead2 100644 --- a/libcontainer/cgroups/systemd/v2.go +++ b/libcontainer/cgroups/systemd/v2.go @@ -242,12 +242,14 @@ func (m *unifiedManager) Apply(pid int) error { properties = append(properties, systemdDbus.PropDescription("libcontainer container "+c.Name)) - // if we create a slice, the parent is defined via a Wants= if strings.HasSuffix(unitName, ".slice") { + // If we create a slice, the parent is defined via a Wants=. properties = append(properties, systemdDbus.PropWants(slice)) } else { - // otherwise, we use Slice= + // Otherwise it's a scope, which we put into a Slice=. properties = append(properties, systemdDbus.PropSlice(slice)) + // Assume scopes always support delegation (supported since systemd v218). + properties = append(properties, newProp("Delegate", true)) } // only add pid if its valid, -1 is used w/ general slice creation. @@ -255,12 +257,6 @@ func (m *unifiedManager) Apply(pid int) error { properties = append(properties, newProp("PIDs", []uint32{uint32(pid)})) } - // Check if we can delegate. This is only supported on systemd versions 218 and above. - if !strings.HasSuffix(unitName, ".slice") { - // Assume scopes always support delegation. - properties = append(properties, newProp("Delegate", true)) - } - // Always enable accounting, this gets us the same behaviour as the fs implementation, // plus the kernel has some problems with joining the memory cgroup at a later time. properties = append(properties,