From 0cd36e6161802be5614e0fed0d509174fdb2fef0 Mon Sep 17 00:00:00 2001 From: Charlie Doern Date: Thu, 30 Jun 2022 15:16:41 -0400 Subject: [PATCH] additional cgroup testing and implementation of various systemd entities added support cpu-shares/weight and most Blkio throttle/weight devices. podman currently support ThrottleReadBps devices so I added those, writeBps, and weight devices. write/read iops seems to be lesser supported but will see if that is a possibility in the future. this involved some rewiring in godbus and some questions in systemd. Seems like systemd is pretty rigorous with their device paths, requiring a full path to either char or block as well as the major:minor. This was difficult to debug and will most likely lead to design limitations in the future. Signed-off-by: Charlie Doern --- pkg/cgroups/cgroups_linux_test.go | 86 +++++++++++++++++++++++++++--- pkg/cgroups/systemd_linux.go | 87 +++++++++++++++++++++++++++++-- 2 files changed, 161 insertions(+), 12 deletions(-) diff --git a/pkg/cgroups/cgroups_linux_test.go b/pkg/cgroups/cgroups_linux_test.go index b218b7007..43e58d201 100644 --- a/pkg/cgroups/cgroups_linux_test.go +++ b/pkg/cgroups/cgroups_linux_test.go @@ -5,6 +5,8 @@ package cgroups import ( "fmt" + "os" + "os/exec" "strconv" "testing" @@ -42,9 +44,38 @@ func TestResources(t *testing.T) { return } + wtDevices := []*configs.WeightDevice{} + devices := []*configs.ThrottleDevice{} + dev1 := &configs.ThrottleDevice{ + BlockIODevice: configs.BlockIODevice{ + Major: 1, + Minor: 3, + }, + Rate: 2097152, + } + dev2 := &configs.ThrottleDevice{ + BlockIODevice: configs.BlockIODevice{ + Major: 3, + Minor: 10, + }, + Rate: 2097152, + } + dev3 := &configs.WeightDevice{ + BlockIODevice: configs.BlockIODevice{ + Major: 5, + Minor: 9, + }, + Weight: 500, + } + devices = append(devices, dev1, dev2) + wtDevices = append(wtDevices, dev3) + var resources configs.Resources resources.CpuPeriod = 100000 resources.CpuQuota = 100000 + resources.CpuShares = 100 + resources.CpusetCpus = "0" + resources.CpusetMems = "0" resources.Memory = 900 resources.MemorySwap = 1000 resources.BlkioWeight = 300 @@ -53,6 +84,11 @@ func TestResources(t *testing.T) { if err != nil { t.Fatal(err) } + defer func() { + if err := cgr.Delete(); err != nil { + t.Fatal(err) + } + }() // TestMode is used in the runc packages for unit tests, works without this as well here. TestMode = true @@ -64,17 +100,13 @@ func TestResources(t *testing.T) { t.Fatal("Got the wrong value, set cpu.cfs_period_us failed.") } - if err := cgr.Delete(); err != nil { - t.Fatal(err) - } - cgr2, err := NewSystemd("machine2.slice", &resources) if err != nil { t.Fatal(err) } // test CPU Quota adjustment. - u, _, _, _ := resourcesToProps(&resources) + u, _, _, _, _ := resourcesToProps(&resources, true) val, ok := u["CPUQuotaPerSecUSec"] if !ok { @@ -84,13 +116,51 @@ func TestResources(t *testing.T) { t.Fatal("CPU Quota incorrect value expected 1000000 got " + strconv.FormatUint(val, 10)) } - // machine.slice = parent, libpod_pod_ID = path - err = cgr2.CreateSystemdUnit(fmt.Sprintf("%s/%s-%s%s", "machine2.slice", "machine2", "libpod_pod_1234", ".slice")) + err = os.Mkdir("/dev/foodevdir", os.ModePerm) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll("/dev/foodevdir") + + c := exec.Command("mknod", "/dev/foodevdir/null", "b", "1", "3") + c.Env = os.Environ() + c.Stderr = os.Stderr + c.Stdout = os.Stdout + err = c.Run() + if err != nil { + t.Fatal(err) + } + + c = exec.Command("mknod", "/dev/foodevdir/bar", "b", "3", "10") + c.Env = os.Environ() + c.Stderr = os.Stderr + c.Stdout = os.Stdout + err = c.Run() + if err != nil { + t.Fatal(err) + } + + c = exec.Command("mknod", "/dev/foodevdir/bat", "b", "5", "9") + c.Env = os.Environ() + c.Stderr = os.Stderr + c.Stdout = os.Stdout + err = c.Run() if err != nil { t.Fatal(err) } - if err := cgr2.Delete(); err != nil { + resources.BlkioThrottleReadBpsDevice = []*configs.ThrottleDevice{devices[0]} + resources.BlkioThrottleWriteBpsDevice = []*configs.ThrottleDevice{devices[1]} + resources.BlkioWeightDevice = wtDevices + + // machine.slice = parent, libpod_pod_ID = path + err = cgr2.CreateSystemdUnit(fmt.Sprintf("%s/%s-%s%s", "machine2.slice", "machine2", "libpod_pod_12345", ".slice")) + if err != nil { t.Fatal(err) } + defer func() { + if err := cgr2.Delete(); err != nil { + t.Fatal(err) + } + }() } diff --git a/pkg/cgroups/systemd_linux.go b/pkg/cgroups/systemd_linux.go index ee9f584de..2cbebc792 100644 --- a/pkg/cgroups/systemd_linux.go +++ b/pkg/cgroups/systemd_linux.go @@ -14,6 +14,11 @@ import ( "github.com/opencontainers/runc/libcontainer/configs" ) +type BlkioDev struct { + Device string + Bytes uint64 +} + func systemdCreate(resources *configs.Resources, path string, c *systemdDbus.Conn) error { slice, name := filepath.Split(path) slice = strings.TrimSuffix(slice, "/") @@ -24,11 +29,18 @@ func systemdCreate(resources *configs.Resources, path string, c *systemdDbus.Con systemdDbus.PropDescription(fmt.Sprintf("cgroup %s", name)), systemdDbus.PropWants(slice), } + ioString := "" + v2, _ := IsCgroup2UnifiedMode() + if v2 { + ioString = "IOAccounting" + } else { + ioString = "BlockIOAccounting" + } pMap := map[string]bool{ "DefaultDependencies": false, "MemoryAccounting": true, "CPUAccounting": true, - "BlockIOAccounting": true, + ioString: true, } if i == 0 { pMap["Delegate"] = true @@ -42,7 +54,7 @@ func systemdCreate(resources *configs.Resources, path string, c *systemdDbus.Con properties = append(properties, p) } - uMap, sMap, bMap, iMap := resourcesToProps(resources) + uMap, sMap, bMap, iMap, structMap := resourcesToProps(resources, v2) for k, v := range uMap { p := systemdDbus.Property{ Name: k, @@ -75,6 +87,14 @@ func systemdCreate(resources *configs.Resources, path string, c *systemdDbus.Con properties = append(properties, p) } + for k, v := range structMap { + p := systemdDbus.Property{ + Name: k, + Value: dbus.MakeVariant(v), + } + properties = append(properties, p) + } + ch := make(chan string) _, err := c.StartTransientUnitContext(context.TODO(), name, "replace", properties, ch) if err != nil { @@ -117,12 +137,13 @@ func systemdDestroyConn(path string, c *systemdDbus.Conn) error { return nil } -func resourcesToProps(res *configs.Resources) (map[string]uint64, map[string]string, map[string][]byte, map[string]int64) { +func resourcesToProps(res *configs.Resources, v2 bool) (map[string]uint64, map[string]string, map[string][]byte, map[string]int64, map[string][]BlkioDev) { bMap := make(map[string][]byte) // this array is not used but will be once more resource limits are added sMap := make(map[string]string) iMap := make(map[string]int64) uMap := make(map[string]uint64) + structMap := make(map[string][]BlkioDev) // CPU if res.CpuPeriod != 0 { @@ -140,6 +161,17 @@ func resourcesToProps(res *configs.Resources) (map[string]uint64, map[string]str uMap["CPUQuotaPerSecUSec"] = cpuQuotaPerSecUSec } + if res.CpuShares != 0 { + // convert from shares to weight. weight only supports 1-10000 + v2, _ := IsCgroup2UnifiedMode() + if v2 { + wt := (1 + ((res.CpuShares-2)*9999)/262142) + uMap["CPUWeight"] = wt + } else { + uMap["CPUShares"] = res.CpuShares + } + } + // CPUSet if res.CpusetCpus != "" { bits := []byte(res.CpusetCpus) @@ -163,5 +195,52 @@ func resourcesToProps(res *configs.Resources) (map[string]uint64, map[string]str uMap["BlockIOWeight"] = uint64(res.BlkioWeight) } - return uMap, sMap, bMap, iMap + // systemd requires the paths to be in the form /dev/{block, char}/major:minor + // this is difficult since runc's resources only store the major and minor, not the type of device + // therefore, assume all are block (I think this is a correct assumption) + if res.BlkioThrottleReadBpsDevice != nil { + for _, entry := range res.BlkioThrottleReadBpsDevice { + newThrottle := BlkioDev{ + Device: fmt.Sprintf("/dev/block/%d:%d", entry.Major, entry.Minor), + Bytes: entry.Rate, + } + fmt.Println(newThrottle.Device, newThrottle.Bytes) + if v2 { + structMap["IOReadBandwidthMax"] = append(structMap["IOReadBandwidthMax"], newThrottle) + } else { + structMap["BlockIOReadBandwidth"] = append(structMap["BlockIOReadBandwidth"], newThrottle) + } + } + } + + if res.BlkioThrottleWriteBpsDevice != nil { + for _, entry := range res.BlkioThrottleWriteBpsDevice { + newThrottle := BlkioDev{ + Device: fmt.Sprintf("/dev/block/%d:%d", entry.Major, entry.Minor), + Bytes: entry.Rate, + } + if v2 { + structMap["IOWriteBandwidthMax"] = append(structMap["IOWriteBandwidthMax"], newThrottle) + } else { + structMap["BlockIOWriteBandwidth"] = append(structMap["BlockIOWriteBandwidth"], newThrottle) + } + } + } + + if res.BlkioWeightDevice != nil { + for _, entry := range res.BlkioWeightDevice { + newWeight := BlkioDev{ + Device: fmt.Sprintf("/dev/block/%d:%d", entry.Major, entry.Minor), + Bytes: uint64(entry.Weight), + } + if v2 { + structMap["IODeviceWeight"] = append(structMap["IODeviceWeight"], newWeight) + } else { + structMap["BlockIODeviceWeight"] = append(structMap["BlockIODeviceWeight"], newWeight) + } + + } + } + + return uMap, sMap, bMap, iMap, structMap }