From 88f59382934d07eb65dc5520119d792f5f272498 Mon Sep 17 00:00:00 2001 From: Austin Vazquez Date: Mon, 19 May 2025 22:12:56 +0000 Subject: [PATCH] Add cgroupV2 CPUQuotaPeriodUSec support Signed-off-by: Austin Vazquez --- cgroup2/cpu.go | 51 ++++++++++++++++++------ cgroup2/cpuv2_test.go | 82 +++++++++++++++++++++++++++++++-------- cgroup2/manager.go | 60 +++++++++++++++++++++++++++- cgroup2/manager_test.go | 47 ++++++++++++++++++++++ cgroup2/testutils_test.go | 12 ++++++ 5 files changed, 223 insertions(+), 29 deletions(-) diff --git a/cgroup2/cpu.go b/cgroup2/cpu.go index dcb253db..c7e5fef4 100644 --- a/cgroup2/cpu.go +++ b/cgroup2/cpu.go @@ -24,12 +24,26 @@ import ( type CPUMax string +const ( + // Default kernel value for cpu quota period is 100000 us (100 ms), same for v1 and v2. + // v1: https://www.kernel.org/doc/html/latest/scheduler/sched-bwc.html and + // v2: https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html + defaultCPUMax = "max" + defaultCPUMaxPeriod = 100000 + defaultCPUMaxPeriodStr = "100000" +) + func NewCPUMax(quota *int64, period *uint64) CPUMax { - max := "max" + max := defaultCPUMax if quota != nil { max = strconv.FormatInt(*quota, 10) } - return CPUMax(strings.Join([]string{max, strconv.FormatUint(*period, 10)}, " ")) + + duration := defaultCPUMaxPeriodStr + if period != nil { + duration = strconv.FormatUint(*period, 10) + } + return CPUMax(strings.Join([]string{max, duration}, " ")) } type CPU struct { @@ -39,19 +53,34 @@ type CPU struct { Mems string } -func (c CPUMax) extractQuotaAndPeriod() (int64, uint64) { +func (c CPUMax) extractQuotaAndPeriod() (int64, uint64, error) { var ( - quota int64 - period uint64 + quota int64 = math.MaxInt64 + period uint64 = defaultCPUMaxPeriod + err error ) + + // value: quota [period] values := strings.Split(string(c), " ") - if values[0] == "max" { - quota = math.MaxInt64 - } else { - quota, _ = strconv.ParseInt(values[0], 10, 64) + if len(values) < 1 || len(values) > 2 { + return 0, 0, ErrInvalidFormat } - period, _ = strconv.ParseUint(values[1], 10, 64) - return quota, period + + if strings.ToLower(values[0]) != defaultCPUMax { + quota, err = strconv.ParseInt(values[0], 10, 64) + if err != nil { + return 0, 0, err + } + } + + if len(values) == 2 { + period, err = strconv.ParseUint(values[1], 10, 64) + if err != nil { + return 0, 0, err + } + } + + return quota, period, nil } func (r *CPU) Values() (o []Value) { diff --git a/cgroup2/cpuv2_test.go b/cgroup2/cpuv2_test.go index 6751d23d..92d157d0 100644 --- a/cgroup2/cpuv2_test.go +++ b/cgroup2/cpuv2_test.go @@ -83,22 +83,72 @@ func TestSystemdCgroupCpuController_NilWeight(t *testing.T) { } func TestExtractQuotaAndPeriod(t *testing.T) { - var ( - period uint64 - quota int64 + const ( + defaultQuota int64 = math.MaxInt64 + defaultPeriod uint64 = 100000 ) - quota = 10000 - period = 8000 - cpuMax := NewCPUMax("a, &period) - tquota, tPeriod := cpuMax.extractQuotaAndPeriod() - - assert.Equal(t, quota, tquota) - assert.Equal(t, period, tPeriod) - - // case with nil quota which makes it "max" - max int val - cpuMax2 := NewCPUMax(nil, &period) - tquota2, tPeriod2 := cpuMax2.extractQuotaAndPeriod() - assert.Equal(t, int64(math.MaxInt64), tquota2) - assert.Equal(t, period, tPeriod2) + require.Equal(t, defaultCPUMaxPeriodStr, strconv.Itoa(defaultCPUMaxPeriod), "Constant for default period does not match its string type constant.") + + // Default "max 100000" + cpuMax := NewCPUMax(nil, nil) + assert.Equal(t, CPUMax("max 100000"), cpuMax) + quota, period, err := cpuMax.extractQuotaAndPeriod() + assert.NoError(t, err) + assert.Equal(t, defaultQuota, quota) + assert.Equal(t, defaultPeriod, period) + + // Only specifing limit is valid. + cpuMax = CPUMax("max") + quota, period, err = cpuMax.extractQuotaAndPeriod() + assert.NoError(t, err) + assert.Equal(t, defaultQuota, quota) + assert.Equal(t, defaultPeriod, period) + + tests := []struct { + cpuMax string + quota int64 + period uint64 + }{ + { + cpuMax: "0 0", + quota: 0, + period: 0, + }, + { + cpuMax: "10000 8000", + quota: 10000, + period: 8000, + }, + { + cpuMax: "42000 4200", + quota: 42000, + period: 4200, + }, + { + cpuMax: "9223372036854775807 18446744073709551615", + quota: 9223372036854775807, + period: 18446744073709551615, + }, + } + + for _, test := range tests { + t.Run(test.cpuMax, func(t *testing.T) { + cpuMax := NewCPUMax(&test.quota, &test.period) + assert.Equal(t, CPUMax(test.cpuMax), cpuMax) + + tquota, tPeriod, err := cpuMax.extractQuotaAndPeriod() + assert.NoError(t, err) + assert.Equal(t, test.quota, tquota) + assert.Equal(t, test.period, tPeriod) + }) + } + + // Negative test cases result in errors. + for i, cpuMax := range []string{"", " ", "max 100000 100000"} { + t.Run(fmt.Sprintf("negative-test-%d", i+1), func(t *testing.T) { + _, _, err = CPUMax(cpuMax).extractQuotaAndPeriod() + assert.Error(t, err) + }) + } } diff --git a/cgroup2/manager.go b/cgroup2/manager.go index b446939d..9daf2b7f 100644 --- a/cgroup2/manager.go +++ b/cgroup2/manager.go @@ -27,6 +27,7 @@ import ( "path/filepath" "strconv" "strings" + "sync" "time" "github.com/containerd/cgroups/v3/cgroup2/stats" @@ -45,9 +46,17 @@ const ( typeFile = "cgroup.type" defaultCgroup2Path = "/sys/fs/cgroup" defaultSlice = "system.slice" + + // systemd only supports CPUQuotaPeriodUSec since v2.42.0 + cpuQuotaPeriodUSecSupportedVersion = 242 ) -var canDelegate bool +var ( + canDelegate bool + + versionOnce sync.Once + version int +) type Event struct { Low uint64 @@ -875,7 +884,19 @@ func NewSystemd(slice, group string, pid int, resources *Resources) (*Manager, e } if resources.CPU != nil && resources.CPU.Max != "" { - quota, period := resources.CPU.Max.extractQuotaAndPeriod() + quota, period, err := resources.CPU.Max.extractQuotaAndPeriod() + if err != nil { + return &Manager{}, err + } + + if period != 0 { + if sdVer := systemdVersion(conn); sdVer >= cpuQuotaPeriodUSecSupportedVersion { + properties = append(properties, newSystemdProperty("CPUQuotaPeriodUSec", period)) + } else { + log.G(context.TODO()).WithField("version", sdVer).Debug("Systemd version is too old to support CPUQuotaPeriodUSec") + } + } + // cpu.cfs_quota_us and cpu.cfs_period_us are controlled by systemd. // corresponds to USEC_INFINITY in systemd // if USEC_INFINITY is provided, CPUQuota is left unbound by systemd @@ -915,6 +936,41 @@ func NewSystemd(slice, group string, pid int, resources *Resources) (*Manager, e }, nil } +// Adapted from https://github.com/opencontainers/cgroups/blob/9657f5a18b8d60a0f39fbb34d0cb7771e28e6278/systemd/common.go#L245-L281 +func systemdVersion(conn *systemdDbus.Conn) int { + versionOnce.Do(func() { + version = -1 + verStr, err := conn.GetManagerProperty("Version") + if err == nil { + version, err = systemdVersionAtoi(verStr) + } + + if err != nil { + log.G(context.TODO()).WithError(err).Error("Unable to get systemd version") + } + }) + + return version +} + +func systemdVersionAtoi(str string) (int, error) { + // Unconditionally remove the leading prefix ("v). + str = strings.TrimLeft(str, `"v`) + // Match on the first integer we can grab. + for i := range len(str) { + if str[i] < '0' || str[i] > '9' { + // First non-digit: cut the tail. + str = str[:i] + break + } + } + ver, err := strconv.Atoi(str) + if err != nil { + return -1, fmt.Errorf("can't parse version: %w", err) + } + return ver, nil +} + func startUnit(conn *systemdDbus.Conn, group string, properties []systemdDbus.Property, ignoreExists bool) error { ctx := context.TODO() diff --git a/cgroup2/manager_test.go b/cgroup2/manager_test.go index 76cbc3ff..dd9d2caa 100644 --- a/cgroup2/manager_test.go +++ b/cgroup2/manager_test.go @@ -17,6 +17,7 @@ package cgroup2 import ( + "context" "fmt" "os" "os/exec" @@ -24,6 +25,7 @@ import ( "testing" "time" + systemdDbus "github.com/coreos/go-systemd/v22/dbus" "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -343,6 +345,51 @@ func TestSystemdCgroupPSIController(t *testing.T) { } } +func TestCPUQuotaPeriodUSec(t *testing.T) { + checkCgroupMode(t) + requireSystemdVersion(t, cpuQuotaPeriodUSecSupportedVersion) + + pid := os.Getpid() + group := fmt.Sprintf("testing-cpu-period-%d.scope", pid) + + tests := []struct { + name string + cpuMax CPUMax + expectedCPUMax string + expectedPeriod uint64 + }{ + { + name: "default cpu.max", + cpuMax: "max 100000", + expectedCPUMax: "max 100000", + expectedPeriod: 100000, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + c, err := NewSystemd("", group, pid, &Resources{ + CPU: &CPU{ + Max: test.cpuMax, + }, + }) + require.NoError(t, err, "failed to init new cgroup systemd manager") + + conn, err := systemdDbus.NewWithContext(context.TODO()) + require.NoError(t, err, "failed to connect to systemd") + defer conn.Close() + + unitName := systemdUnitFromPath(c.path) + props, err := conn.GetAllPropertiesContext(context.TODO(), unitName) + require.NoError(t, err, "failed to get unit properties") + + periodUSec, ok := props["CPUQuotaPeriodUSec"] + require.True(t, ok, "CPUQuotaPeriodUSec property not found") + require.Equal(t, test.expectedPeriod, periodUSec.(uint64), "CPUQuotaPeriodUSec value doesn't match expected period") + }) + } +} + func BenchmarkStat(b *testing.B) { checkCgroupMode(b) group := "/stat-test-cg" diff --git a/cgroup2/testutils_test.go b/cgroup2/testutils_test.go index a4c4bc79..476d59e8 100644 --- a/cgroup2/testutils_test.go +++ b/cgroup2/testutils_test.go @@ -17,11 +17,13 @@ package cgroup2 import ( + "context" "os" "path/filepath" "strings" "testing" + systemdDbus "github.com/coreos/go-systemd/v22/dbus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/sys/unix" @@ -38,6 +40,16 @@ func checkCgroupMode(tb testing.TB) { } } +func requireSystemdVersion(tb testing.TB, requiredMinVersion int) { + conn, err := systemdDbus.NewWithContext(context.TODO()) + require.NoError(tb, err, "failed to connect to systemd") + defer conn.Close() + + if sdVer := systemdVersion(conn); sdVer < requiredMinVersion { + tb.Skipf("Skipping test; systemd version %d < required version %d", sdVer, requiredMinVersion) + } +} + func checkCgroupControllerSupported(t *testing.T, controller string) { b, err := os.ReadFile(filepath.Join(defaultCgroup2Path, controllersFile)) if err != nil || !strings.Contains(string(b), controller) {