Add cgroupV2 CPUQuotaPeriodUSec support

Signed-off-by: Austin Vazquez <[email protected]>

Author: austinvazquez

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) {

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(&quota, &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)
+		})
+	}
 }

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()
 

cgroup2/manager_test.go

@@ -17,13 +17,15 @@
 package cgroup2
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"os/exec"
 	"syscall"
 	"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"

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) {