From b439330dc1bc261d9aaa1ad2dadb53a887c13c78 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 27 Mar 2023 10:59:40 -0700 Subject: [PATCH] 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] https://github.com/systemd/systemd/pull/23299 [2] https://github.com/opencontainers/runc/issues/3786 Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/systemd/systemd_test.go | 84 ++++++++++++++++++++ libcontainer/cgroups/systemd/v2.go | 42 +++++++++- 2 files changed, 124 insertions(+), 2 deletions(-) diff --git a/libcontainer/cgroups/systemd/systemd_test.go b/libcontainer/cgroups/systemd/systemd_test.go index 40584f78e..8e333a006 100644 --- 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 index e8c471233..c5576441c 100644 --- 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)