From d63a212e13d9d3728df6827323bf15bfc13bc081 Mon Sep 17 00:00:00 2001 From: Yuta Maeda Date: Tue, 5 Nov 2024 16:56:43 +0900 Subject: [PATCH] cgroups: fix incorrect cgroups cpuset via systemd Signed-off-by: Yuta Maeda Signed-off-by: Tatsuya Ono --- pkg/cgroups/cgroups_linux_test.go | 23 +++++++++- pkg/cgroups/systemd_linux.go | 74 ++++++++++++++++++++++++++++--- 2 files changed, 90 insertions(+), 7 deletions(-) diff --git a/pkg/cgroups/cgroups_linux_test.go b/pkg/cgroups/cgroups_linux_test.go index db2036b08..0adaf93a0 100644 --- a/pkg/cgroups/cgroups_linux_test.go +++ b/pkg/cgroups/cgroups_linux_test.go @@ -3,7 +3,9 @@ package cgroups import ( + "bytes" "fmt" + "math/big" "os" "os/exec" "strconv" @@ -87,7 +89,7 @@ func TestResources(t *testing.T) { } // test CPU Quota adjustment. - u, _, _, _, _ := resourcesToProps(&resources, true) + u, _, b, _, _, _ := resourcesToProps(&resources, true) val, ok := u["CPUQuotaPerSecUSec"] if !ok { @@ -97,6 +99,25 @@ func TestResources(t *testing.T) { t.Fatal("CPU Quota incorrect value expected 1000000 got " + strconv.FormatUint(val, 10)) } + bits := new(big.Int) + cpuset_val := bits.SetBit(bits, 0, 1).Bytes() + + cpus, ok := b["AllowedCPUs"] + if !ok { + t.Fatal("Cpuset Cpus not parsed.") + } + if !bytes.Equal(cpus, cpuset_val) { + t.Fatal("Cpuset Cpus incorrect value expected " + string(cpuset_val) + " got " + string(cpus)) + } + + mems, ok := b["AllowedMemoryNodes"] + if !ok { + t.Fatal("Cpuset Mems not parsed.") + } + if !bytes.Equal(mems, cpuset_val) { + t.Fatal("Cpuset Mems incorrect value expected " + string(cpuset_val) + " got " + string(mems)) + } + err = os.Mkdir("/dev/foodevdir", os.ModePerm) if err != nil { t.Fatal(err) diff --git a/pkg/cgroups/systemd_linux.go b/pkg/cgroups/systemd_linux.go index b1529410f..b723b34cc 100644 --- a/pkg/cgroups/systemd_linux.go +++ b/pkg/cgroups/systemd_linux.go @@ -4,8 +4,12 @@ package cgroups import ( "context" + "errors" "fmt" + "math/big" "path/filepath" + "slices" + "strconv" "strings" systemdDbus "github.com/coreos/go-systemd/v22/dbus" @@ -53,7 +57,11 @@ func systemdCreate(resources *configs.Resources, path string, c *systemdDbus.Con properties = append(properties, p) } - uMap, sMap, bMap, iMap, structMap := resourcesToProps(resources, v2) + uMap, sMap, bMap, iMap, structMap, err := resourcesToProps(resources, v2) + if err != nil { + lastError = err + continue + } for k, v := range uMap { p := systemdDbus.Property{ Name: k, @@ -95,7 +103,7 @@ func systemdCreate(resources *configs.Resources, path string, c *systemdDbus.Con } ch := make(chan string) - _, err := c.StartTransientUnitContext(context.TODO(), name, "replace", properties, ch) + _, err = c.StartTransientUnitContext(context.TODO(), name, "replace", properties, ch) if err != nil { lastError = err continue @@ -142,7 +150,7 @@ func systemdDestroyConn(path string, c *systemdDbus.Conn) error { return nil } -func resourcesToProps(res *configs.Resources, v2 bool) (map[string]uint64, map[string]string, map[string][]byte, map[string]int64, map[string][]BlkioDev) { +func resourcesToProps(res *configs.Resources, v2 bool) (map[string]uint64, map[string]string, map[string][]byte, map[string]int64, map[string][]BlkioDev, error) { bMap := make(map[string][]byte) // this array is not used but will be once more resource limits are added sMap := make(map[string]string) @@ -179,11 +187,19 @@ func resourcesToProps(res *configs.Resources, v2 bool) (map[string]uint64, map[s // CPUSet if res.CpusetCpus != "" { - bits := []byte(res.CpusetCpus) + bits, err := rangeToBits(res.CpusetCpus) + if err != nil { + return nil, nil, nil, nil, nil, fmt.Errorf("resources.CpusetCpus=%q conversion error: %w", + res.CpusetCpus, err) + } bMap["AllowedCPUs"] = bits } if res.CpusetMems != "" { - bits := []byte(res.CpusetMems) + bits, err := rangeToBits(res.CpusetMems) + if err != nil { + return nil, nil, nil, nil, nil, fmt.Errorf("resources.CpusetMems=%q conversion error: %w", + res.CpusetMems, err) + } bMap["AllowedMemoryNodes"] = bits } @@ -258,5 +274,51 @@ func resourcesToProps(res *configs.Resources, v2 bool) (map[string]uint64, map[s } } - return uMap, sMap, bMap, iMap, structMap + return uMap, sMap, bMap, iMap, structMap, nil +} + +func rangeToBits(str string) ([]byte, error) { + bits := new(big.Int) + + for _, r := range strings.Split(str, ",") { + // allow extra spaces around + r = strings.TrimSpace(r) + // allow empty elements (extra commas) + if r == "" { + continue + } + startr, endr, ok := strings.Cut(r, "-") + if ok { + start, err := strconv.ParseUint(startr, 10, 32) + if err != nil { + return nil, err + } + end, err := strconv.ParseUint(endr, 10, 32) + if err != nil { + return nil, err + } + if start > end { + return nil, errors.New("invalid range: " + r) + } + for i := start; i <= end; i++ { + bits.SetBit(bits, int(i), 1) + } + } else { + val, err := strconv.ParseUint(startr, 10, 32) + if err != nil { + return nil, err + } + bits.SetBit(bits, int(val), 1) + } + } + + ret := bits.Bytes() + if len(ret) == 0 { + // do not allow empty values + return nil, errors.New("empty value") + } + + // fit cpuset parsing order in systemd + slices.Reverse(ret) + return ret, nil }