libct/cg/sd: support setting cpu.idle via systemd

Systemd v252 (available in CentOS Stream 9 in our CI) added support for setting cpu.idle (see [1]). The way it works is: - if CPUWeight == 0, cpu.idle is set to 1; - if CPUWeight != 0, cpu.idle is set to 0. This commit implements setting cpu.idle in systemd cgroup driver via a unit property. In case CPUIdle is set to non-zero value, the driver sets adds CPUWeight=0 property, which will result in systemd setting cpu.idle to 1. Unfortunately, there's no way to set cpu.idle to 0 without also changing the CPUWeight value, so the driver doesn't do anything if CPUIdle is explicitly set to 0. This case is handled by the fs driver which is always used as a followup to setting systemd unit properties. Also, handle cpu.idle set via unified map. In case it is set to non-zero value, add CPUWeight=0 property, and ignore cpu.weight (otherwise we'll get two different CPUWeight properties set). Add a unit test for new values in unified map, and an integration test case. [1] systemd/systemd#23299 [2] #3786 Signed-off-by: Kir Kolyshkin <[email protected]>
opencontainers · Apr 4, 2023 · ed9651b · ed9651b
1 parent b5ecad7
commit ed9651b
Show file tree

Hide file tree

Showing 4 changed files with 170 additions and 2 deletions.
diff --git a/docs/systemd.md b/docs/systemd.md
@@ -91,6 +91,7 @@ The following tables summarize which properties are translated.
 | cpu.mems                | AllowedMemoryNodes    | v244                |
 | unified.cpu.max         | CPUQuota, CPUQuotaPeriodSec | v242          |
 | unified.cpu.weight      | CPUWeight             |                     |
+| unified.cpu.idle        | CPUWeight             | v252                |
 | unified.cpuset.cpus     | AllowedCPUs           | v244                |
 | unified.cpuset.mems     | AllowedMemoryNodes    | v244                |
 | unified.memory.high     | MemoryHigh            |                     |

diff --git a/libcontainer/cgroups/systemd/systemd_test.go b/libcontainer/cgroups/systemd/systemd_test.go
@@ -2,8 +2,10 @@ package systemd
 
 import (
 	"os"
+	"reflect"
 	"testing"
 
+	systemdDbus "github.com/coreos/go-systemd/v22/dbus"
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
@@ -95,3 +97,85 @@ func TestUnitExistsIgnored(t *testing.T) {
 		}
 	}
 }
+
+func TestUnifiedResToSystemdProps(t *testing.T) {
+	if !IsRunningSystemd() {
+		t.Skip("Test requires systemd.")
+	}
+	if !cgroups.IsCgroup2UnifiedMode() {
+		t.Skip("cgroup v2 is required")
+	}
+
+	cm := newDbusConnManager(os.Geteuid() != 0)
+
+	testCases := []struct {
+		name     string
+		minVer   int
+		res      map[string]string
+		expError bool
+		expProps []systemdDbus.Property
+	}{
+		{
+			name: "empty map",
+			res:  map[string]string{},
+		},
+		{
+			name:   "only cpu.idle=1",
+			minVer: cpuIdleSupportedVersion,
+			res: map[string]string{
+				"cpu.idle": "1",
+			},
+			expProps: []systemdDbus.Property{
+				newProp("CPUWeight", uint64(0)),
+			},
+		},
+		{
+			name:   "only cpu.idle=0",
+			minVer: cpuIdleSupportedVersion,
+			res: map[string]string{
+				"cpu.idle": "0",
+			},
+		},
+		{
+			name:   "cpu.idle=1 and cpu.weight=1000",
+			minVer: cpuIdleSupportedVersion,
+			res: map[string]string{
+				"cpu.idle":   "1",
+				"cpu.weight": "1000",
+			},
+			expProps: []systemdDbus.Property{
+				newProp("CPUWeight", uint64(0)),
+			},
+		},
+		{
+			name:   "cpu.idle=0 and cpu.weight=1000",
+			minVer: cpuIdleSupportedVersion,
+			res: map[string]string{
+				"cpu.idle":   "0",
+				"cpu.weight": "1000",
+			},
+			expProps: []systemdDbus.Property{
+				newProp("CPUWeight", uint64(1000)),
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.minVer != 0 && systemdVersion(cm) < tc.minVer {
+				t.Skipf("requires systemd >= %d", tc.minVer)
+			}
+			props, err := unifiedResToSystemdProps(cm, tc.res)
+			if err != nil && !tc.expError {
+				t.Fatalf("expected no error, got: %v", err)
+			}
+			if err == nil && tc.expError {
+				t.Fatal("expected error, got nil")
+			}
+			if !reflect.DeepEqual(tc.expProps, props) {
+				t.Errorf("wrong properties (exp %+v, got %+v)", tc.expProps, props)
+			}
+		})
+	}
+}
diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go
@@ -20,6 +20,10 @@ import (
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
+const (
+	cpuIdleSupportedVersion = 252
+)
+
 type UnifiedManager struct {
 	mu      sync.Mutex
 	cgroups *configs.Cgroup
@@ -48,6 +52,14 @@ func NewUnifiedManager(config *configs.Cgroup, path string) (*UnifiedManager, er
 	return m, nil
 }
 
+func shouldSetCPUIdle(cm *dbusConnManager, v string) bool {
+	// The only valid values for cpu.idle are 0 and 1. As it is
+	// not possible to directly set cpu.idle to 0 via systemd,
+	// ignore 0. Ignore other values as we'll error out later
+	// in Set() while calling fsMgr.Set().
+	return v == "1" && systemdVersion(cm) >= cpuIdleSupportedVersion
+}
+
 // unifiedResToSystemdProps tries to convert from Cgroup.Resources.Unified
 // key/value map (where key is cgroupfs file name) to systemd unit properties.
 // This is on a best-effort basis, so the properties that are not known
@@ -72,6 +84,14 @@ func unifiedResToSystemdProps(cm *dbusConnManager, res map[string]string) (props
 		v = strings.TrimSpace(v)
 		// Please keep cases in alphabetical order.
 		switch k {
+		case "cpu.idle":
+			if shouldSetCPUIdle(cm, v) {
+				// Setting CPUWeight to 0 tells systemd
+				// to set cpu.idle to 1.
+				props = append(props,
+					newProp("CPUWeight", uint64(0)))
+			}
+
 		case "cpu.max":
 			// value: quota [period]
 			quota := int64(0) // 0 means "unlimited" for addCpuQuota, if period is set
@@ -97,6 +117,12 @@ func unifiedResToSystemdProps(cm *dbusConnManager, res map[string]string) (props
 			addCpuQuota(cm, &props, quota, period)
 
 		case "cpu.weight":
+			if shouldSetCPUIdle(cm, strings.TrimSpace(res["cpu.idle"])) {
+				// Do not add duplicate CPUWeight property
+				// (see case "cpu.idle" above).
+				logrus.Warn("unable to apply both cpu.weight and cpu.idle to systemd, ignoring cpu.weight")
+				continue
+			}
 			num, err := strconv.ParseUint(v, 10, 64)
 			if err != nil {
 				return nil, fmt.Errorf("unified resource %q value conversion error: %w", k, err)
@@ -212,9 +238,21 @@ func genV2ResourcesProperties(dirPath string, r *configs.Resources, cm *dbusConn
 			newProp("MemorySwapMax", uint64(swap)))
 	}
 
-	if r.CpuWeight != 0 {
+	idleSet := false
+	// The logic here is the same as in shouldSetCPUIdle.
+	if r.CPUIdle != nil && *r.CPUIdle == 1 && systemdVersion(cm) >= cpuIdleSupportedVersion {
 		properties = append(properties,
-			newProp("CPUWeight", r.CpuWeight))
+			newProp("CPUWeight", uint64(0)))
+		idleSet = true
+	}
+	if r.CpuWeight != 0 {
+		if idleSet {
+			// Ignore CpuWeight if CPUIdle is already set.
+			logrus.Warn("unable to apply both CPUWeight and CpuIdle to systemd, ignoring CPUWeight")
+		} else {
+			properties = append(properties,
+				newProp("CPUWeight", r.CpuWeight))
+		}
 	}
 
 	addCpuQuota(cm, &properties, r.CpuQuota, r.CpuPeriod)

diff --git a/tests/integration/update.bats b/tests/integration/update.bats
@@ -473,6 +473,51 @@ EOF
 	check_cgroup_value "cpu.idle" "1"
 }
 
+@test "update cgroup cpu.idle via systemd v252+" {
+	requires cgroups_v2 systemd cgroups_cpu_idle
+	[ $EUID -ne 0 ] && requires rootless_cgroup
+	if [ "$(systemd_version)" -lt 252 ]; then
+		skip "requires systemd >= v252"
+	fi
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_update
+	[ "$status" -eq 0 ]
+	check_cgroup_value "cpu.idle" "0"
+
+	# If cpu-idle is set, cpu-share (converted to CPUWeight) can't be set via systemd.
+	runc update --cpu-share 200 --cpu-idle 1 test_update
+	[[ "$output" == *"unable to apply both"* ]]
+	check_cgroup_value "cpu.idle" "1"
+
+	# Changing cpu-shares (converted to CPU weight) resets cpu.idle to 0.
+	runc update --cpu-share 200 test_update
+	check_cgroup_value "cpu.idle" "0"
+
+	# Setting values via unified map.
+
+	# If cpu.idle is set, cpu.weight is ignored.
+	runc update -r - test_update <<EOF
+{
+  "unified": {
+    "cpu.idle": "1",
+    "cpu.weight": "8"
+  }
+}
+EOF
+	[[ "$output" == *"unable to apply both"* ]]
+	check_cgroup_value "cpu.idle" "1"
+
+	# Setting any cpu.weight should reset cpu.idle to 0.
+	runc update -r - test_update <<EOF
+{
+  "unified": {
+    "cpu.weight": "8"
+  }
+}
+EOF
+	check_cgroup_value "cpu.idle" "0"
+}
+
 @test "update cgroup v2 resources via unified map" {
 	[ $EUID -ne 0 ] && requires rootless_cgroup
 	requires cgroups_v2