Skip to content

Commit b0b58d6

Browse files
johnstcnjohnstcn
authored
fix(envbox): work with cgroupv2 (#34)
* Envbox now warns instead of failing when trying to copy outer container quota to inner * Adds logic for cgroupv2 * Adds more unit tests around all of the above for both cgroupv1 and cgroupv2
1 parent 4a9fdbf commit b0b58d6

File tree

6 files changed

+321
-60
lines changed

6 files changed

+321
-60
lines changed

‎cli/clitest/fs.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ func FakeSysboxManagerReady(t *testing.T, fs afero.Fs) {
1616
}
1717

1818
func FakeCPUGroups(t *testing.T, fs afero.Fs, quota, period string) {
19-
err := afero.WriteFile(fs, xunix.CPUPeriodPath, []byte(period), 0o600)
19+
err := afero.WriteFile(fs, xunix.CPUPeriodPathCGroupV1, []byte(period), 0o600)
2020
require.NoError(t, err)
2121

22-
err = afero.WriteFile(fs, xunix.CPUQuotaPath, []byte(quota), 0o600)
22+
err = afero.WriteFile(fs, xunix.CPUQuotaPathCGroupV1, []byte(quota), 0o600)
2323
require.NoError(t, err)
2424
}

‎cli/docker.go

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -637,21 +637,22 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Docker
637637
return xerrors.Errorf("make bootstrap dir: %w", err)
638638
}
639639

640-
cpuQuota, err := xunix.ReadCPUQuota(ctx)
640+
cpuQuota, err := xunix.ReadCPUQuota(ctx, log)
641641
if err != nil {
642-
return xerrors.Errorf("read CPU quota: %w", err)
643-
}
644-
645-
log.Debug(ctx, "setting CPU quota",
646-
slog.F("quota", cpuQuota.Quota),
647-
slog.F("period", cpuQuota.Period),
648-
)
642+
blog.Infof("Unable to read CPU quota: %s", err.Error())
643+
} else {
644+
log.Debug(ctx, "setting CPU quota",
645+
slog.F("quota", cpuQuota.Quota),
646+
slog.F("period", cpuQuota.Period),
647+
slog.F("cgroup", cpuQuota.CGroup.String()),
648+
)
649649

650-
// We want the inner container to have the same limits as the outer container
651-
// so that processes inside the container know what they're working with.
652-
err = dockerutil.SetContainerCPUQuota(ctx, containerID, cpuQuota.Quota, cpuQuota.Period)
653-
if err != nil {
654-
return xerrors.Errorf("set inner container CPU quota: %w", err)
650+
// We want the inner container to have the same limits as the outer container
651+
// so that processes inside the container know what they're working with.
652+
if err := dockerutil.SetContainerQuota(ctx, containerID, cpuQuota); err != nil {
653+
blog.Infof("Unable to set quota for inner container: %s", err.Error())
654+
blog.Info("This is not a fatal error, but it may cause cgroup-aware applications to misbehave.")
655+
}
655656
}
656657

657658
blog.Info("Envbox startup complete!")

‎dockerutil/container.go

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -137,26 +137,61 @@ func BootstrapContainer(ctx context.Context, client DockerClient, conf Bootstrap
137137
return nil
138138
}
139139

140-
// copyCPUQuotaToInnerCGroup writes the contents of the following files to
141-
// their corresponding locations under cgroupBase:
142-
// - /sys/fs/cgroup/cpu,cpuacct/cpu.cfs_period_us
143-
// - /sys/fs/cgroup/cpu,cpuacct/cpu.cfs_quota_us
140+
// SetContainerQuota writes a quota to its correct location for the inner container.
141+
// HACK: until https://github.com/nestybox/sysbox/issues/582 is resolved, we need to copy
142+
// the CPU quota and period from the outer container to the inner container to ensure
143+
// that applications inside the container know how much CPU they have to work with.
144144
//
145-
// HACK: until https://github.com/nestybox/sysbox/issues/582 is resolved, we need to set cfs_quota_us
146-
// and cfs_period_us inside the container to ensure that applications inside the container know how much
147-
// CPU they have to work with.
148-
func SetContainerCPUQuota(ctx context.Context, containerID string, quota, period int) error {
145+
// For cgroupv2:
146+
// - /sys/fs/cgroup/<subpath>/init.scope/cpu.max
147+
//
148+
// For cgroupv1:
149+
// - /sys/fs/cgroup/cpu,cpuacct/<subpath>/syscont-cgroup-root/cpu.cfs_quota_us
150+
// - /sys/fs/cgroup/cpu,cpuacct/<subpath>/syscont-cgroup-root/cpu.cfs_period_us
151+
func SetContainerQuota(ctx context.Context, containerID string, quota xunix.CPUQuota) error {
152+
switch quota.CGroup {
153+
case xunix.CGroupV2:
154+
return setContainerQuotaCGroupV2(ctx, containerID, quota)
155+
case xunix.CGroupV1:
156+
return setContainerQuotaCGroupV1(ctx, containerID, quota)
157+
default:
158+
return xerrors.Errorf("Unknown cgroup %d", quota.CGroup)
159+
}
160+
}
161+
162+
func setContainerQuotaCGroupV2(ctx context.Context, containerID string, quota xunix.CPUQuota) error {
163+
var (
164+
fs = xunix.GetFS(ctx)
165+
cgroupBase = fmt.Sprintf("/sys/fs/cgroup/docker/%s/init.scope/", containerID)
166+
)
167+
168+
var content string
169+
if quota.Quota < 0 {
170+
content = fmt.Sprintf("max %d\n", quota.Period)
171+
} else {
172+
content = fmt.Sprintf("%d %d\n", quota.Quota, quota.Period)
173+
}
174+
175+
err := afero.WriteFile(fs, filepath.Join(cgroupBase, "cpu.max"), []byte(content), 0o644)
176+
if err != nil {
177+
return xerrors.Errorf("write cpu.max to inner container cgroup: %w", err)
178+
}
179+
180+
return nil
181+
}
182+
183+
func setContainerQuotaCGroupV1(ctx context.Context, containerID string, quota xunix.CPUQuota) error {
149184
var (
150185
fs = xunix.GetFS(ctx)
151186
cgroupBase = fmt.Sprintf("/sys/fs/cgroup/cpu,cpuacct/docker/%s/syscont-cgroup-root/", containerID)
152187
)
153188

154-
err := afero.WriteFile(fs, filepath.Join(cgroupBase, "cpu.cfs_period_us"), []byte(strconv.Itoa(period)), 0o644)
189+
err := afero.WriteFile(fs, filepath.Join(cgroupBase, "cpu.cfs_period_us"), []byte(strconv.Itoa(quota.Period)), 0o644)
155190
if err != nil {
156191
return xerrors.Errorf("write cpu.cfs_period_us to inner container cgroup: %w", err)
157192
}
158193

159-
err = afero.WriteFile(fs, filepath.Join(cgroupBase, "cpu.cfs_quota_us"), []byte(strconv.Itoa(quota)), 0o644)
194+
err = afero.WriteFile(fs, filepath.Join(cgroupBase, "cpu.cfs_quota_us"), []byte(strconv.Itoa(quota.Quota)), 0o644)
160195
if err != nil {
161196
return xerrors.Errorf("write cpu.cfs_quota_us to inner container cgroup: %w", err)
162197
}

‎dockerutil/container_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package dockerutil_test
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"testing"
7+
8+
"github.com/spf13/afero"
9+
"github.com/stretchr/testify/require"
10+
11+
"github.com/coder/envbox/dockerutil"
12+
"github.com/coder/envbox/xunix"
13+
"github.com/coder/envbox/xunix/xunixfake"
14+
)
15+
16+
func TestSetContainerQuota(t *testing.T) {
17+
t.Parallel()
18+
19+
for _, tc := range []struct {
20+
Name string
21+
Quota xunix.CPUQuota
22+
ContainerID string
23+
ExpectedFS map[string]string
24+
Error string
25+
}{
26+
{
27+
Name: "CGroupV1",
28+
Quota: xunix.CPUQuota{
29+
Quota: 150000,
30+
Period: 100000,
31+
CGroup: xunix.CGroupV1,
32+
},
33+
ContainerID: "dummy",
34+
ExpectedFS: map[string]string{
35+
"/sys/fs/cgroup/cpu,cpuacct/docker/dummy/syscont-cgroup-root/cpu.cfs_quota_us": "150000",
36+
"/sys/fs/cgroup/cpu,cpuacct/docker/dummy/syscont-cgroup-root/cpu.cfs_period_us": "100000",
37+
},
38+
},
39+
{
40+
Name: "CGroupV2",
41+
Quota: xunix.CPUQuota{
42+
Quota: 150000,
43+
Period: 100000,
44+
CGroup: xunix.CGroupV2,
45+
},
46+
ContainerID: "dummy",
47+
ExpectedFS: map[string]string{
48+
"/sys/fs/cgroup/docker/dummy/init.scope/cpu.max": "150000 100000",
49+
},
50+
},
51+
{
52+
Name: "CGroupV2Max",
53+
Quota: xunix.CPUQuota{
54+
Quota: -1,
55+
Period: 100000,
56+
CGroup: xunix.CGroupV2,
57+
},
58+
ContainerID: "dummy",
59+
ExpectedFS: map[string]string{
60+
"/sys/fs/cgroup/docker/dummy/init.scope/cpu.max": "max 100000",
61+
},
62+
},
63+
} {
64+
tc := tc
65+
t.Run(tc.Name, func(t *testing.T) {
66+
t.Parallel()
67+
tmpfs := &xunixfake.MemFS{MemMapFs: &afero.MemMapFs{}}
68+
ctx := xunix.WithFS(context.Background(), tmpfs)
69+
err := dockerutil.SetContainerQuota(ctx, tc.ContainerID, tc.Quota)
70+
if tc.Error == "" {
71+
require.NoError(t, err)
72+
} else {
73+
require.ErrorContains(t, err, tc.Error)
74+
}
75+
for path, content := range tc.ExpectedFS {
76+
actualContent, err := afero.ReadFile(tmpfs, path)
77+
require.NoError(t, err)
78+
require.Equal(t, content, string(bytes.TrimSpace(actualContent)))
79+
}
80+
})
81+
}
82+
}

‎xunix/sys.go

Lines changed: 108 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,40 +3,116 @@ package xunix
33
import (
44
"bytes"
55
"context"
6+
"path/filepath"
67
"strconv"
8+
"strings"
79

810
"github.com/spf13/afero"
911
"golang.org/x/xerrors"
12+
13+
"cdr.dev/slog"
1014
)
1115

1216
type CPUQuota struct {
1317
Quota int
1418
Period int
19+
CGroup CGroup
20+
}
21+
22+
const (
23+
CPUPeriodPathCGroupV1 = "/sys/fs/cgroup/cpu,cpuacct/cpu.cfs_period_us"
24+
CPUQuotaPathCGroupV1 = "/sys/fs/cgroup/cpu,cpuacct/cpu.cfs_quota_us"
25+
)
26+
27+
type CGroup int
28+
29+
func (c CGroup) String() string {
30+
return [...]string{"cgroupv1", "cgroupv2"}[c]
1531
}
1632

1733
const (
18-
CPUPeriodPath = "/sys/fs/cgroup/cpu,cpuacct/cpu.cfs_period_us"
19-
CPUQuotaPath = "/sys/fs/cgroup/cpu,cpuacct/cpu.cfs_quota_us"
34+
CGroupV1 CGroup = iota
35+
CGroupV2
2036
)
2137

22-
func ReadCPUQuota(ctx context.Context) (CPUQuota, error) {
38+
// ReadCPUQuota attempts to read the CFS CPU quota and period from the current
39+
// container context. It first attempts to read the paths relevant to cgroupv2
40+
// and falls back to reading the paths relevant go cgroupv1
41+
//
42+
// Relevant paths for cgroupv2:
43+
// - /proc/self/cgroup
44+
// - /sys/fs/cgroup/<self>/cpu.max
45+
//
46+
// Relevant paths for cgroupv1:
47+
// - /sys/fs/cgroup/cpu,cpuacct/cpu.cfs_quota_us
48+
// - /sys/fs/cgroup/cpu,cpuacct/cpu.cfs_period_us
49+
func ReadCPUQuota(ctx context.Context, log slog.Logger) (CPUQuota, error) {
50+
quota, err := readCPUQuotaCGroupV2(ctx)
51+
if err == nil {
52+
return quota, nil
53+
}
54+
55+
log.Info(ctx, "Unable to read cgroupv2 quota, falling back to cgroupv1", slog.Error(err))
56+
return readCPUQuotaCGroupV1(ctx)
57+
}
58+
59+
func readCPUQuotaCGroupV2(ctx context.Context) (CPUQuota, error) {
60+
fs := GetFS(ctx)
61+
self, err := ReadCGroupSelf(ctx)
62+
if err != nil {
63+
return CPUQuota{}, xerrors.Errorf("determine own cgroup: %w", err)
64+
}
65+
66+
maxStr, err := afero.ReadFile(fs, filepath.Join("/sys/fs/cgroup/", self, "cpu.max"))
67+
if err != nil {
68+
return CPUQuota{}, xerrors.Errorf("read cpu.max outside container: %w", err)
69+
}
70+
71+
list := strings.Split(string(bytes.TrimSpace(maxStr)), " ")
72+
if len(list) != 2 {
73+
return CPUQuota{}, xerrors.Errorf("expected cpu.max to have exactly two entries, got: %s", string(maxStr))
74+
}
75+
76+
var quota int
77+
var period int
78+
79+
if list[0] == "max" {
80+
quota = -1
81+
} else {
82+
quota, err = strconv.Atoi(list[0])
83+
if err != nil {
84+
return CPUQuota{}, xerrors.Errorf("quota %s not an int: %w", list[0], err)
85+
}
86+
}
87+
88+
period, err = strconv.Atoi(list[1])
89+
if err != nil {
90+
return CPUQuota{}, xerrors.Errorf("period %s not an int: %w", list[1], err)
91+
}
92+
93+
return CPUQuota{Quota: quota, Period: period, CGroup: CGroupV2}, nil
94+
}
95+
96+
func readCPUQuotaCGroupV1(ctx context.Context) (CPUQuota, error) {
2397
fs := GetFS(ctx)
24-
periodStr, err := afero.ReadFile(fs, "/sys/fs/cgroup/cpu,cpuacct/cpu.cfs_period_us")
98+
periodRaw, err := afero.ReadFile(fs, CPUPeriodPathCGroupV1)
2599
if err != nil {
26100
return CPUQuota{}, xerrors.Errorf("read cpu.cfs_period_us outside container: %w", err)
27101
}
28102

29-
quotaStr, err := afero.ReadFile(fs, "/sys/fs/cgroup/cpu,cpuacct/cpu.cfs_quota_us")
103+
quotaRaw, err := afero.ReadFile(fs, CPUQuotaPathCGroupV1)
30104
if err != nil {
31105
return CPUQuota{}, xerrors.Errorf("read cpu.cfs_quota_us outside container: %w", err)
32106
}
33107

34-
period, err := strconv.Atoi(string(bytes.TrimSpace(periodStr)))
108+
periodStr := string(bytes.TrimSpace(periodRaw))
109+
period, err := strconv.Atoi(periodStr)
35110
if err != nil {
36111
return CPUQuota{}, xerrors.Errorf("period %s not an int: %w", periodStr, err)
37112
}
38113

39-
quota, err := strconv.Atoi(string(bytes.TrimSpace(quotaStr)))
114+
quotaStr := string(bytes.TrimSpace(quotaRaw))
115+
quota, err := strconv.Atoi(quotaStr)
40116
if err != nil {
41117
return CPUQuota{}, xerrors.Errorf("quota %s not an int: %w", quotaStr, err)
42118
}
@@ -46,3 +122,28 @@ func ReadCPUQuota(ctx context.Context) (CPUQuota, error) {
46122
Period: period,
47123
}, nil
48124
}
125+
126+
// readCGroup attempts to determine the cgroup for the container by
127+
// reading the fields of /proc/self/cgroup (third field)
128+
// We currently only check the first line of /proc/self/cgroup.
129+
func ReadCGroupSelf(ctx context.Context) (string, error) {
130+
fs := GetFS(ctx)
131+
raw, err := afero.ReadFile(fs, "/proc/self/cgroup")
132+
if err != nil {
133+
return "", xerrors.Errorf("read /proc/self/cgroup: %w", err)
134+
}
135+
136+
lines := bytes.Split(raw, []byte("\n"))
137+
if len(lines) == 0 {
138+
return "", xerrors.Errorf("unexpected content of /proc/self/cgroup: %s", string(raw))
139+
}
140+
141+
// Just pick the first line.
142+
line := lines[0]
143+
fields := bytes.Split(line, []byte(":"))
144+
if len(fields) != 3 {
145+
return "", xerrors.Errorf("expected 3 fields in last line of /proc/self/cgroup: %s", string(raw))
146+
}
147+
148+
return string(fields[2]), nil
149+
}

0 commit comments

Comments
 (0)