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)