Repository: opencontainers/runc
Pull request #2727 (2021-01-08 20:28:52)

libc/cg: convert r.CPU.Cpus/Mems to systemd props
Support for systemd properties AllowedCPUs and AllowedMemoryNodes
was added by commit 13afa58d0e46ceb0, but only for unified resources
of systemd v2 driver.

This adds support for Cpu.Cpus and Cpu.Mems resources to
both systemd v1 and v2 cgroup drivers. If systemd is not new
enough to support the properties, a  warning is logged.

An integration test is added to check that the settings work.

While at it:
 - log a warning in a different place checking systemd version;
 - add version check to systemd unified support handling.

kolyshkin: This one is ready
kolyshkin: > time="2021-01-14T01:13:19Z" level=error msg="container_linux.go:367: starting container process caused: process_linux.go:340: applying cgroup configuration for process caused: Cannot set property AllowedCPUs, or unknown property."
150

So we need systemd version check...

Patch:
diff --git a/libcontainer/cgroups/systemd/common.go b/libcontainer/cgroups/systemd/common.go
index 9a30e4e1..008b1c25 100644
--- a/libcontainer/cgroups/systemd/common.go
+++ b/libcontainer/cgroups/systemd/common.go
@@ -34,7 +34,6 @@ var (
 
 	versionOnce sync.Once
 	version     int
-	versionErr  error
 
 	isRunningSystemdOnce sync.Once
 	isRunningSystemd     bool
@@ -372,19 +371,20 @@ func stopUnit(dbusConnection *systemdDbus.Conn, unitName string) error {
 	return nil
 }
 
-func systemdVersion(conn *systemdDbus.Conn) (int, error) {
+func systemdVersion(conn *systemdDbus.Conn) int {
 	versionOnce.Do(func() {
 		version = -1
 		verStr, err := conn.GetManagerProperty("Version")
-		if err != nil {
-			versionErr = err
-			return
+		if err == nil {
+			version, err = systemdVersionAtoi(verStr)
 		}
 
-		version, versionErr = systemdVersionAtoi(verStr)
+		if err != nil {
+			logrus.WithError(err).Error("unable to get systemd version")
+		}
 	})
 
-	return version, versionErr
+	return version
 }
 
 func systemdVersionAtoi(verStr string) (int, error) {
@@ -405,12 +405,13 @@ func systemdVersionAtoi(verStr string) (int, error) {
 func addCpuQuota(conn *systemdDbus.Conn, properties *[]systemdDbus.Property, quota int64, period uint64) {
 	if period != 0 {
 		// systemd only supports CPUQuotaPeriodUSec since v242
-		sdVer, err := systemdVersion(conn)
-		if err != nil {
-			logrus.Warnf("systemdVersion: %s", err)
-		} else if sdVer >= 242 {
+		sdVer := systemdVersion(conn)
+		if sdVer >= 242 {
 			*properties = append(*properties,
 				newProp("CPUQuotaPeriodUSec", period))
+		} else {
+			logrus.Warnf("systemd v%d is too old to support CPUQuotaPeriodSec "+
+				" (setting will still be applied to cgroupfs)", sdVer)
 		}
 	}
 	if quota != 0 || period != 0 {
@@ -434,3 +435,37 @@ func addCpuQuota(conn *systemdDbus.Conn, properties *[]systemdDbus.Property, quo
 			newProp("CPUQuotaPerSecUSec", cpuQuotaPerSecUSec))
 	}
 }
+
+func addCpuset(conn *systemdDbus.Conn, props *[]systemdDbus.Property, cpus, mems string) error {
+	if cpus == "" && mems == "" {
+		return nil
+	}
+
+	// systemd only supports AllowedCPUs/AllowedMemoryNodes since v244
+	sdVer := systemdVersion(conn)
+	if sdVer < 244 {
+		logrus.Warnf("systemd v%d is too old to support AllowedCPUs/AllowedMemoryNodes"+
+			" (settings will still be applied to cgroupfs)", sdVer)
+		return nil
+	}
+
+	if cpus != "" {
+		bits, err := rangeToBits(cpus)
+		if err != nil {
+			return fmt.Errorf("resources.CPU.Cpus=%q conversion error: %w",
+				cpus, err)
+		}
+		*props = append(*props,
+			newProp("AllowedCPUs", bits))
+	}
+	if mems != "" {
+		bits, err := rangeToBits(mems)
+		if err != nil {
+			return fmt.Errorf("resources.CPU.Mems=%q conversion error: %w",
+				mems, err)
+		}
+		*props = append(*props,
+			newProp("AllowedMemoryNodes", bits))
+	}
+	return nil
+}
diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go
index 2679b999..64af1d94 100644
--- a/libcontainer/cgroups/systemd/v1.go
+++ b/libcontainer/cgroups/systemd/v1.go
@@ -90,6 +90,11 @@ func genV1ResourcesProperties(c *configs.Cgroup, conn *systemdDbus.Conn) ([]syst
 			newProp("TasksMax", uint64(r.PidsLimit)))
 	}
 
+	err = addCpuset(conn, &properties, r.CpusetCpus, r.CpusetMems)
+	if err != nil {
+		return nil, err
+	}
+
 	return properties, nil
 }
 
diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go
index 5691b43d..c1b0d737 100644
--- a/libcontainer/cgroups/systemd/v2.go
+++ b/libcontainer/cgroups/systemd/v2.go
@@ -102,8 +102,16 @@ func unifiedResToSystemdProps(conn *systemdDbus.Conn, res map[string]string) (pr
 				"cpuset.cpus": "AllowedCPUs",
 				"cpuset.mems": "AllowedMemoryNodes",
 			}
-			props = append(props,
-				newProp(m[k], bits))
+			// systemd only supports these properties since v244
+			sdVer := systemdVersion(conn)
+			if sdVer >= 244 {
+				props = append(props,
+					newProp(m[k], bits))
+			} else {
+				logrus.Warnf("systemd v%d is too old to support %s"+
+					" (setting will still be applied to cgroupfs)",
+					sdVer, m[k])
+			}
 
 		case "memory.high", "memory.low", "memory.min", "memory.max", "memory.swap.max":
 			num := uint64(math.MaxUint64)
@@ -201,6 +209,11 @@ func genV2ResourcesProperties(c *configs.Cgroup, conn *systemdDbus.Conn) ([]syst
 			newProp("TasksMax", uint64(r.PidsLimit)))
 	}
 
+	err = addCpuset(conn, &properties, r.CpusetCpus, r.CpusetMems)
+	if err != nil {
+		return nil, err
+	}
+
 	// ignore r.KernelMemory
 
 	// convert Resources.Unified map to systemd properties


Test patch:
diff --git a/tests/integration/update.bats b/tests/integration/update.bats
index 908ce0cc..c03e7c6d 100644
--- a/tests/integration/update.bats
+++ b/tests/integration/update.bats
@@ -392,7 +392,63 @@ EOF
 	check_systemd_value "TasksMax" 10
 }
 
+@test "update cpuset parameters via resources.CPU" {
+	[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
+	requires smp
+
+	local AllowedCPUs='AllowedCPUs' AllowedMemoryNodes='AllowedMemoryNodes'
+	# these properties require systemd >= v244
+	if [ "$(systemd_version)" -lt 244 ]; then
+		# a hack to skip checks, see check_systemd_value()
+		AllowedCPUs='unsupported'
+		AllowedMemoryNodes='unsupported'
+	fi
+
+	update_config ' .linux.resources.CPU |= {
+				"Cpus": "0",
+				"Mems": "0"
+			}' "${BUSYBOX_BUNDLE}"
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_update
+	[ "$status" -eq 0 ]
+
+	# check that initial values were properly set
+	check_systemd_value "$AllowedCPUs" 0
+	check_systemd_value "$AllowedMemoryNodes" 0
+
+	runc update -r - test_update <<EOF
+{
+  "CPU": {
+    "Cpus": "1"
+  }
+}
+EOF
+	[ "$status" -eq 0 ]
+
+	# check the updated systemd unit properties
+	check_systemd_value "$AllowedCPUs" 1
+
+	# More than 1 numa memory node is required to test this
+	file="/sys/fs/cgroup/cpuset.mems.effective"
+	if ! test -r $file || grep -q '^0$' $file; then
+		# skip the rest of it
+		return 0
+	fi
+
+	runc update -r - test_update <<EOF
+{
+  "CPU": {
+    "Mems": "1"
+  }
+}
+EOF
+	[ "$status" -eq 0 ]
+
+	# check the updated systemd unit properties
+	check_systemd_value "$AllowedMemoryNodes" 1
+}
+
 @test "update cpuset parameters via v2 unified map" {
+	# This test assumes systemd >= v244
 	[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
 	requires cgroups_v2 smp