Merge pull request #3749 from Zheaoli/manjusaka/carry-burst

[Carry #3205] libct/cg: add CFS bandwidth burst for CPU

contrib/completions/bash/runc

@@ -735,6 +735,7 @@ _runc_update() {
 	   --blkio-weight
 	   --cpu-period
 	   --cpu-quota
+	   --cpu-burst
 	   --cpu-rt-period
 	   --cpu-rt-runtime
 	   --cpu-share

docs/spec-conformance.md

@@ -10,7 +10,6 @@ Spec version | Feature                                  | PR
 v1.0.0       | `SCMP_ARCH_PARISC`                       | Unplanned, due to lack of users
 v1.0.0       | `SCMP_ARCH_PARISC64`                     | Unplanned, due to lack of users
 v1.0.2       | `.linux.personality`                     | [#3126](https://github.com/opencontainers/runc/pull/3126)
-v1.1.0       | `.linux.resources.cpu.burst`             | [#3749](https://github.com/opencontainers/runc/pull/3749)
 v1.1.0       | `SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV` | [#3862](https://github.com/opencontainers/runc/pull/3862)
 v1.1.0       | time namespaces                          | [#3876](https://github.com/opencontainers/runc/pull/3876)
 v1.1.0       | rsvd hugetlb cgroup                      | TODO ([#3859](https://github.com/opencontainers/runc/issues/3859))

libcontainer/cgroups/fs/cpu.go

@@ -84,6 +84,28 @@ func (s *CpuGroup) Set(path string, r *configs.Resources) error {
 			period = ""
 		}
 	}
+
+	var burst string
+	if r.CpuBurst != nil {
+		burst = strconv.FormatUint(*r.CpuBurst, 10)
+		if err := cgroups.WriteFile(path, "cpu.cfs_burst_us", burst); err != nil {
+			// this is a special trick for burst feature, the current systemd and low version of kernel will not support it.
+			// So, an `no such file or directory` error would be raised, and we can ignore it .
+			if !errors.Is(err, unix.ENOENT) {
+				// Sometimes when the burst to be set is larger
+				// than the current one, it is rejected by the kernel
+				// (EINVAL) as old_quota/new_burst exceeds the parent
+				// cgroup quota limit. If this happens and the quota is
+				// going to be set, ignore the error for now and retry
+				// after setting the quota.
+				if !errors.Is(err, unix.EINVAL) || r.CpuQuota == 0 {
+					return err
+				}
+			}
+		} else {
+			burst = ""
+		}
+	}
 	if r.CpuQuota != 0 {
 		if err := cgroups.WriteFile(path, "cpu.cfs_quota_us", strconv.FormatInt(r.CpuQuota, 10)); err != nil {
 			return err
@@ -93,6 +115,13 @@ func (s *CpuGroup) Set(path string, r *configs.Resources) error {
 				return err
 			}
 		}
+		if burst != "" {
+			if err := cgroups.WriteFile(path, "cpu.cfs_burst_us", burst); err != nil {
+				if !errors.Is(err, unix.ENOENT) {
+					return err
+				}
+			}
+		}
 	}
 
 	if r.CPUIdle != nil {

libcontainer/cgroups/fs/cpu_test.go

@@ -45,23 +45,27 @@ func TestCpuSetBandWidth(t *testing.T) {
 	const (
 		quotaBefore     = 8000
 		quotaAfter      = 5000
+		burstBefore     = 2000
 		periodBefore    = 10000
 		periodAfter     = 7000
 		rtRuntimeBefore = 8000
 		rtRuntimeAfter  = 5000
 		rtPeriodBefore  = 10000
 		rtPeriodAfter   = 7000
 	)
+	burstAfter := uint64(1000)
 
 	writeFileContents(t, path, map[string]string{
 		"cpu.cfs_quota_us":  strconv.Itoa(quotaBefore),
+		"cpu.cfs_burst_us":  strconv.Itoa(burstBefore),
 		"cpu.cfs_period_us": strconv.Itoa(periodBefore),
 		"cpu.rt_runtime_us": strconv.Itoa(rtRuntimeBefore),
 		"cpu.rt_period_us":  strconv.Itoa(rtPeriodBefore),
 	})
 
 	r := &configs.Resources{
 		CpuQuota:     quotaAfter,
+		CpuBurst:     &burstAfter,
 		CpuPeriod:    periodAfter,
 		CpuRtRuntime: rtRuntimeAfter,
 		CpuRtPeriod:  rtPeriodAfter,
@@ -79,6 +83,14 @@ func TestCpuSetBandWidth(t *testing.T) {
 		t.Fatal("Got the wrong value, set cpu.cfs_quota_us failed.")
 	}
 
+	burst, err := fscommon.GetCgroupParamUint(path, "cpu.cfs_burst_us")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if burst != burstAfter {
+		t.Fatal("Got the wrong value, set cpu.cfs_burst_us failed.")
+	}
+
 	period, err := fscommon.GetCgroupParamUint(path, "cpu.cfs_period_us")
 	if err != nil {
 		t.Fatal(err)

libcontainer/cgroups/fs/fs.go

@@ -191,7 +191,7 @@ func (m *Manager) Set(r *configs.Resources) error {
 			if path == "" {
 				// We never created a path for this cgroup, so we cannot set
 				// limits for it (though we have already tried at this point).
-				return fmt.Errorf("cannot set %s limit: container could not join or create cgroup", sys.Name())
+				return fmt.Errorf("cannot set %s limit: container could not join or create cgroup, and the error is %w", sys.Name(), err)
 			}
 			return err
 		}

libcontainer/cgroups/fs2/cpu.go

@@ -2,16 +2,19 @@ package fs2
 
 import (
 	"bufio"
+	"errors"
 	"os"
 	"strconv"
 
+	"golang.org/x/sys/unix"
+
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
 func isCpuSet(r *configs.Resources) bool {
-	return r.CpuWeight != 0 || r.CpuQuota != 0 || r.CpuPeriod != 0 || r.CPUIdle != nil
+	return r.CpuWeight != 0 || r.CpuQuota != 0 || r.CpuPeriod != 0 || r.CPUIdle != nil || r.CpuBurst != nil
 }
 
 func setCpu(dirPath string, r *configs.Resources) error {
@@ -32,6 +35,23 @@ func setCpu(dirPath string, r *configs.Resources) error {
 		}
 	}
 
+	var burst string
+	if r.CpuBurst != nil {
+		burst = strconv.FormatUint(*r.CpuBurst, 10)
+		if err := cgroups.WriteFile(dirPath, "cpu.max.burst", burst); err != nil {
+			// Sometimes when the burst to be set is larger
+			// than the current one, it is rejected by the kernel
+			// (EINVAL) as old_quota/new_burst exceeds the parent
+			// cgroup quota limit. If this happens and the quota is
+			// going to be set, ignore the error for now and retry
+			// after setting the quota.
+			if !errors.Is(err, unix.EINVAL) || r.CpuQuota == 0 {
+				return err
+			}
+		} else {
+			burst = ""
+		}
+	}
 	if r.CpuQuota != 0 || r.CpuPeriod != 0 {
 		str := "max"
 		if r.CpuQuota > 0 {
@@ -47,6 +67,11 @@ func setCpu(dirPath string, r *configs.Resources) error {
 		if err := cgroups.WriteFile(dirPath, "cpu.max", str); err != nil {
 			return err
 		}
+		if burst != "" {
+			if err := cgroups.WriteFile(dirPath, "cpu.max.burst", burst); err != nil {
+				return err
+			}
+		}
 	}
 
 	return nil

libcontainer/configs/cgroup_linux.go

@@ -69,6 +69,9 @@ type Resources struct {
 	// CPU hardcap limit (in usecs). Allowed cpu time in a given period.
 	CpuQuota int64 `json:"cpu_quota"`
 
+	// CPU hardcap burst limit (in usecs). Allowed accumulated cpu time additionally for burst in a given period.
+	CpuBurst *uint64 `json:"cpu_burst"` //nolint:revive
+
 	// CPU period to be used for hardcapping (in usecs). 0 to use system default.
 	CpuPeriod uint64 `json:"cpu_period"`
 

libcontainer/specconv/spec_linux.go

@@ -755,6 +755,9 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
 				if r.CPU.Quota != nil {
 					c.Resources.CpuQuota = *r.CPU.Quota
 				}
+				if r.CPU.Burst != nil {
+					c.Resources.CpuBurst = r.CPU.Burst
+				}
 				if r.CPU.Period != nil {
 					c.Resources.CpuPeriod = *r.CPU.Period
 				}

man/runc-update.8.md

@@ -28,6 +28,7 @@ In case **-r** is used, the JSON format is like this:
 			"cpu": {
 				"shares": 0,
 				"quota": 0,
+				"burst": 0,
 				"period": 0,
 				"realtimeRuntime": 0,
 				"realtimePeriod": 0,
@@ -53,6 +54,9 @@ stdin. If this option is used, all other options are ignored.
 **--cpu-quota** _num_
 : Set CPU usage limit within a given period (in microseconds).
 
+**--cpu-burst** _num_
+: Set CPU burst limit within a given period (in microseconds).
+
 **--cpu-rt-period** _num_
 : Set CPU realtime period to be used for hardcapping (in microseconds).
 

tests/integration/helpers.bash

@@ -260,11 +260,16 @@ function get_cgroup_value() {
 # Helper to check a if value in a cgroup file matches the expected one.
 function check_cgroup_value() {
 	local current
+	local cgroup
+	cgroup="$(get_cgroup_path "$1")"
+	if [ ! -f "$cgroup/$1" ]; then
+		skip "$cgroup/$1 does not exist"
+	fi
 	current="$(get_cgroup_value "$1")"
 	local expected=$2
 
 	echo "current $current !? $expected"
-	[ "$current" = "$expected" ]
+	[ "$current" = "$expected" ] || [ "$current" = "$((expected / 1000))" ]
 }
 
 # Helper to check a value in systemd.
@@ -310,6 +315,15 @@ function check_cpu_quota() {
 	check_systemd_value "CPUQuotaPeriodUSec" $sd_period $sd_infinity
 }
 
+function check_cpu_burst() {
+	local burst=$1
+	if [ -v CGROUP_V2 ]; then
+		check_cgroup_value "cpu.max.burst" "$burst"
+	else
+		check_cgroup_value "cpu.cfs_burst_us" "$burst"
+	fi
+}
+
 # Works for cgroup v1 and v2, accepts v1 shares as an argument.
 function check_cpu_shares() {
 	local shares=$1

tests/integration/update.bats

@@ -288,6 +288,12 @@ EOF
 	runc update test_update --cpu-share 200
 	[ "$status" -eq 0 ]
 	check_cpu_shares 200
+	runc update test_update --cpu-period 900000 --cpu-burst 500000
+	[ "$status" -eq 0 ]
+	check_cpu_burst 500000
+	runc update test_update --cpu-period 900000 --cpu-burst 0
+	[ "$status" -eq 0 ]
+	check_cpu_burst 0
 
 	# Revert to the test initial value via json on stding
 	runc update -r - test_update <<EOF

update.go

@@ -44,6 +44,7 @@ The accepted format is as follow (unchanged values can be omitted):
   "cpu": {
     "shares": 0,
     "quota": 0,
+    "burst": 0,
     "period": 0,
     "realtimeRuntime": 0,
     "realtimePeriod": 0,
@@ -73,6 +74,10 @@ other options are ignored.
 			Name:  "cpu-quota",
 			Usage: "CPU CFS hardcap limit (in usecs). Allowed cpu time in a given period",
 		},
+		cli.StringFlag{
+			Name:  "cpu-burst",
+			Usage: "CPU CFS hardcap burst limit (in usecs). Allowed accumulated cpu time additionally for burst a given period",
+		},
 		cli.StringFlag{
 			Name:  "cpu-share",
 			Usage: "CPU shares (relative weight vs. other containers)",
@@ -153,6 +158,7 @@ other options are ignored.
 			CPU: &specs.LinuxCPU{
 				Shares:          u64Ptr(0),
 				Quota:           i64Ptr(0),
+				Burst:           u64Ptr(0),
 				Period:          u64Ptr(0),
 				RealtimeRuntime: i64Ptr(0),
 				RealtimePeriod:  u64Ptr(0),
@@ -210,6 +216,7 @@ other options are ignored.
 				opt  string
 				dest *uint64
 			}{
+				{"cpu-burst", r.CPU.Burst},
 				{"cpu-period", r.CPU.Period},
 				{"cpu-rt-period", r.CPU.RealtimePeriod},
 				{"cpu-share", r.CPU.Shares},
@@ -299,6 +306,7 @@ other options are ignored.
 			}
 		}
 
+		config.Cgroups.Resources.CpuBurst = r.CPU.Burst
 		config.Cgroups.Resources.CpuShares = *r.CPU.Shares
 		// CpuWeight is used for cgroupv2 and should be converted
 		config.Cgroups.Resources.CpuWeight = cgroups.ConvertCPUSharesToCgroupV2Value(*r.CPU.Shares)