Deal with when cpu.cfs_quota_us is negative

components/ws-daemon/pkg/cpulimit/cfs.go

@@ -21,30 +21,27 @@ type CgroupCFSController string
 
 // Usage returns the cpuacct.usage value of the cgroup
 func (basePath CgroupCFSController) Usage() (usage CPUTime, err error) {
-
-	cpuTimeInNS, err := basePath.readUint64("cpuacct.usage")
+	cputime, err := basePath.readCpuUsage()
 	if err != nil {
 		return 0, xerrors.Errorf("cannot read cpuacct.usage: %w", err)
 	}
 
-	return CPUTime(time.Duration(cpuTimeInNS) * time.Nanosecond), nil
+	return CPUTime(cputime), nil
 }
 
 // SetQuota sets a new CFS quota on the cgroup
 func (basePath CgroupCFSController) SetLimit(limit Bandwidth) (changed bool, err error) {
-	p, err := basePath.readUint64("cpu.cfs_period_us")
+	period, err := basePath.readCfsPeriod()
 	if err != nil {
 		err = xerrors.Errorf("cannot parse CFS period: %w", err)
 		return
 	}
-	period := time.Duration(p) * time.Microsecond
 
-	q, err := basePath.readUint64("cpu.cfs_quota_us")
+	quota, err := basePath.readCfsQuota()
 	if err != nil {
 		err = xerrors.Errorf("cannot parse CFS quota: %w", err)
 		return
 	}
-	quota := time.Duration(q) * time.Microsecond
 	target := limit.Quota(period)
 	if quota == target {
 		return false, nil
@@ -60,31 +57,66 @@ func (basePath CgroupCFSController) SetLimit(limit Bandwidth) (changed bool, err
 
 func (basePath CgroupCFSController) readParentQuota() time.Duration {
 	parent := CgroupCFSController(filepath.Dir(string(basePath)))
-	pq, err := parent.readUint64("cpu.cfs_quota_us")
+	pq, err := parent.readCfsQuota()
 	if err != nil {
 		return time.Duration(0)
 	}
 
 	return time.Duration(pq) * time.Microsecond
 }
 
-func (basePath CgroupCFSController) readUint64(path string) (uint64, error) {
+func (basePath CgroupCFSController) readString(path string) (string, error) {
 	fn := filepath.Join(string(basePath), path)
 	fc, err := os.ReadFile(fn)
 	if err != nil {
-		return 0, err
+		return "", err
 	}
 
 	s := strings.TrimSpace(string(fc))
-	if s == "max" {
-		return math.MaxUint64, nil
+	return s, nil
+}
+
+func (basePath CgroupCFSController) readCfsPeriod() (time.Duration, error) {
+	s, err := basePath.readString("cpu.cfs_period_us")
+	if err != nil {
+		return 0, err
+	}
+
+	p, err := strconv.ParseInt(s, 10, 64)
+	if err != nil {
+		return 0, err
+	}
+	return time.Duration(uint(p)) * time.Microsecond, nil
+}
+
+func (basePath CgroupCFSController) readCfsQuota() (time.Duration, error) {
+	s, err := basePath.readString("cpu.cfs_quota_us")
+	if err != nil {
+		return 0, err
+	}
+
+	p, err := strconv.ParseInt(s, 10, 64)
+	if err != nil {
+		return 0, err
+	}
+
+	if p < 0 {
+		return time.Duration(math.MaxInt64), nil
+	}
+	return time.Duration(p) * time.Microsecond, nil
+}
+
+func (basePath CgroupCFSController) readCpuUsage() (time.Duration, error) {
+	s, err := basePath.readString("cpuacct.usage")
+	if err != nil {
+		return 0, err
 	}
 
 	p, err := strconv.ParseInt(s, 10, 64)
 	if err != nil {
 		return 0, err
 	}
-	return uint64(p), nil
+	return time.Duration(uint64(p)) * time.Nanosecond, nil
 }
 
 // NrThrottled returns the number of CFS periods the cgroup was throttled in

components/ws-daemon/pkg/cpulimit/cfs_test.go

@@ -0,0 +1,167 @@
+// Copyright (c) 2022 Gitpod GmbH. All rights reserved.
+// Licensed under the GNU Affero General Public License (AGPL).
+// See License-AGPL.txt in the project root for license information.
+
+package cpulimit
+
+import (
+	"math"
+	"os"
+	"path/filepath"
+	"strconv"
+	"testing"
+	"time"
+
+	"github.com/opencontainers/runc/libcontainer/cgroups"
+)
+
+func init() {
+	cgroups.TestMode = true
+}
+
+func createTempDir(t *testing.T, subsystem string) string {
+	path := filepath.Join(t.TempDir(), subsystem)
+	if err := os.Mkdir(path, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	return path
+}
+
+func TestCfsSetLimit(t *testing.T) {
+	type test struct {
+		beforeCfsPeriodUs int
+		beforeCfsQuotaUs  int
+		bandWidth         Bandwidth
+		cfsQuotaUs        int
+		changed           bool
+	}
+	tests := []test{
+		{
+			beforeCfsPeriodUs: 10000,
+			beforeCfsQuotaUs:  -1,
+			bandWidth:         Bandwidth(6000),
+			cfsQuotaUs:        60000,
+			changed:           true,
+		},
+		{
+			beforeCfsPeriodUs: 5000,
+			beforeCfsQuotaUs:  -1,
+			bandWidth:         Bandwidth(6000),
+			cfsQuotaUs:        30000,
+			changed:           true,
+		},
+		{
+			beforeCfsPeriodUs: 10000,
+			beforeCfsQuotaUs:  60000,
+			bandWidth:         Bandwidth(6000),
+			cfsQuotaUs:        60000,
+			changed:           false,
+		},
+	}
+	for _, tc := range tests {
+		tempdir := createTempDir(t, "cpu")
+		err := cgroups.WriteFile(tempdir, "cpu.cfs_period_us", strconv.Itoa(tc.beforeCfsPeriodUs))
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = cgroups.WriteFile(tempdir, "cpu.cfs_quota_us", strconv.Itoa(tc.beforeCfsQuotaUs))
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		cfs := CgroupCFSController(tempdir)
+		changed, err := cfs.SetLimit(tc.bandWidth)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if changed != tc.changed {
+			t.Fatalf("unexpected error: changed is '%v' but expected '%v'", changed, tc.changed)
+		}
+		cfsQuotaUs, err := cgroups.ReadFile(tempdir, "cpu.cfs_quota_us")
+		if err != nil {
+			t.Fatal(err)
+		}
+		if cfsQuotaUs != strconv.Itoa(tc.cfsQuotaUs) {
+			t.Fatalf("unexpected error: cfsQuotaUs is '%v' but expected '%v'", cfsQuotaUs, tc.cfsQuotaUs)
+		}
+	}
+}
+
+func TestReadCfsQuota(t *testing.T) {
+	type test struct {
+		value  int
+		expect int
+	}
+	tests := []test{
+		{
+			value:  100000,
+			expect: 100000,
+		},
+		{
+			value:  -1,
+			expect: int(time.Duration(math.MaxInt64).Microseconds()),
+		},
+	}
+
+	for _, tc := range tests {
+		tempdir := createTempDir(t, "cpu")
+		err := cgroups.WriteFile(tempdir, "cpu.cfs_quota_us", strconv.Itoa(tc.value))
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		cfs := CgroupCFSController(tempdir)
+		v, err := cfs.readCfsQuota()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if v.Microseconds() != int64(tc.expect) {
+			t.Fatalf("unexpected error: cfs quota is '%v' but expected '%v'", v, tc.expect)
+		}
+	}
+}
+
+func TestReadCfsPeriod(t *testing.T) {
+	tests := []int{
+		10000,
+	}
+	for _, tc := range tests {
+		tempdir := createTempDir(t, "cpu")
+		err := cgroups.WriteFile(tempdir, "cpu.cfs_period_us", strconv.Itoa(tc))
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		cfs := CgroupCFSController(tempdir)
+		v, err := cfs.readCfsPeriod()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if v.Microseconds() != int64(tc) {
+			t.Fatalf("unexpected error: cfs period is '%v' but expected '%v'", v, tc)
+		}
+	}
+}
+
+func TestReadCpuUsage(t *testing.T) {
+	tests := []int{
+		0,
+		100000,
+	}
+	for _, tc := range tests {
+		tempdir := createTempDir(t, "cpu")
+		err := cgroups.WriteFile(tempdir, "cpuacct.usage", strconv.Itoa(tc))
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		cfs := CgroupCFSController(tempdir)
+		v, err := cfs.readCpuUsage()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if v.Nanoseconds() != int64(tc) {
+			t.Fatalf("unexpected error: cpu usage is '%v' but expected '%v'", v, tc)
+		}
+	}
+}