Merge pull request #2230 from Hitachi/fix-cgroup-cpuset

cgroups: fix incorrect cgroups cpuset via systemd

Author: openshift-merge-bot[bot]

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)

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
 }