Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support running integration tests on cgroupv2 hosts #3143

Merged
merged 2 commits into from
Jul 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions build/integration.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,22 @@ while [ "$(curl -Gs http://localhost:8080/healthz)" != "ok" ]; do
sleep 1
done

if [[ "${DOCKER_IN_DOCKER_ENABLED:-}" == "true" ]]; then
# see https://github.com/moby/moby/blob/master/hack/dind
# cgroup v2: enable nesting
if [ -f /sys/fs/cgroup/cgroup.controllers ]; then
echo ">> configuring cgroupsv2 for docker in docker..."
# move the processes from the root group to the /init group,
# otherwise writing subtree_control fails with EBUSY.
# An error during moving non-existent process (i.e., "cat") is ignored.
mkdir -p /sys/fs/cgroup/init
xargs -rn1 < /sys/fs/cgroup/cgroup.procs > /sys/fs/cgroup/init/cgroup.procs || :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this xargs block do?

I can see that -r is do not run the command if there is no stdin, and -n1 is max args 1? but I don't see a command that its executing.

Copy link
Collaborator Author

@bobbypage bobbypage Jul 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this over from https://github.com/moby/moby/blob/master/hack/dind :)

So the idea here, is that for docker in docker for cgroupv2 we need to be move all the processes in a child cgroup or else we will hit the runc/cgroupv2 limitation mentioned here - containerd/containerd#6659 (comment)

The goal here is to first create a new cgroup (mkdir -p /sys/fs/cgroup/init). Then we want to copy all of pids one by one from /sys/fs/cgroup/cgroup.procs to the new child cgroup /sys/fs/cgroup/init/cgroup.procs (the xargs -rn1 ensures the list of pids is split one by one).

There is no command being passed to xargs, so as a result it will simply echo each pid entry (see https://unix.stackexchange.com/questions/110134/what-does-xargs-do-if-its-used-without-any-parameter)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat, so xargs allows you to copy the file but throw an error if the source doesn't have any lines in it.

The new comment is helpful thanks!

# enable controllers
sed -e 's/ / +/g' -e 's/^/+/' < /sys/fs/cgroup/cgroup.controllers \
> /sys/fs/cgroup/cgroup.subtree_control
fi
fi

echo ">> running integration tests against local cAdvisor"
if ! [ -f ./api.test ] || ! [ -f ./healthz.test ]; then
echo You must compile the ./api.test binary and ./healthz.test binary before
Expand Down
3 changes: 2 additions & 1 deletion container/common/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ func getSpecInternal(cgroupPaths map[string]string, machineInfoFactory info.Mach
if cgroup2UnifiedMode {
ioControllerName = "io"
}
if blkioRoot, ok := cgroupPaths[ioControllerName]; ok && utils.FileExists(blkioRoot) {

if blkioRoot, ok := GetControllerPath(cgroupPaths, ioControllerName, cgroup2UnifiedMode); ok && utils.FileExists(blkioRoot) {
spec.HasDiskIo = true
}

Expand Down
2 changes: 1 addition & 1 deletion container/common/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func TestGetSpecCgroupV2(t *testing.T) {
assert.EqualValues(t, spec.Processes.Limit, 1027)

assert.False(t, spec.HasHugetlb)
assert.False(t, spec.HasDiskIo)
assert.True(t, spec.HasDiskIo)
}

func TestGetSpecCgroupV2Max(t *testing.T) {
Expand Down
16 changes: 15 additions & 1 deletion integration/tests/api/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
v2 "github.com/google/cadvisor/info/v2"
"github.com/google/cadvisor/integration/framework"

"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -211,7 +212,20 @@ func TestDockerContainerSpec(t *testing.T) {
assert := assert.New(t)

assert.True(containerInfo.Spec.HasCpu, "CPU should be isolated")
assert.Equal(cpuShares, containerInfo.Spec.Cpu.Limit, "Container should have %d shares, has %d", cpuShares, containerInfo.Spec.Cpu.Limit)
if cgroups.IsCgroup2UnifiedMode() {
// cpu shares are rounded slighly on cgroupv2 due to conversion between cgroupv1 (cpu.shares) and cgroupv2 (cpu.weight)
// When container is created via docker, runc will convert cpu shares to cpu.weight https://github.com/opencontainers/runc/blob/d11f4d756e85ece5cdba8bb69f8bd4db3cdcbeab/libcontainer/cgroups/utils.go#L423-L428
// And cAdvisor will convert cpu.weight back to cpu shares in https://github.com/google/cadvisor/blob/24e7a9883d12f944fd4403861707f4bafcaf4f3d/container/common/helpers.go#L249-L260
// Worked example:
// cpuShares = 2048 (input to docker --cpu-shares)
// cpuWeight = int((1 + ((cpuShares-2)*9999)/262142))=79 (conversion done by runc)
// cpuWeight back to cpuShares = int(2 + ((cpuWeight-1)*262142)/9999)= 2046
var cgroupV2Shares uint64 = 2046
assert.Equal(cgroupV2Shares, containerInfo.Spec.Cpu.Limit, "Container should have %d shares, has %d", cgroupV2Shares, containerInfo.Spec.Cpu.Limit)
} else {
assert.Equal(cpuShares, containerInfo.Spec.Cpu.Limit, "Container should have %d shares, has %d", cpuShares, containerInfo.Spec.Cpu.Limit)
}

assert.Equal(cpuMask, containerInfo.Spec.Cpu.Mask, "Cpu mask should be %q, but is %q", cpuMask, containerInfo.Spec.Cpu.Mask)
assert.True(containerInfo.Spec.HasMemory, "Memory should be isolated")
assert.Equal(memoryLimit, containerInfo.Spec.Memory.Limit, "Container should have memory limit of %d, has %d", memoryLimit, containerInfo.Spec.Memory.Limit)
Expand Down
7 changes: 6 additions & 1 deletion integration/tests/api/machinestats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/google/cadvisor/integration/framework"
"github.com/opencontainers/runc/libcontainer/cgroups"
)

func TestMachineStatsIsReturned(t *testing.T) {
Expand All @@ -37,7 +38,11 @@ func TestMachineStatsIsReturned(t *testing.T) {
for _, stat := range machineStats {
as.NotEqual(stat.Timestamp, time.Time{})
as.True(stat.Cpu.Usage.Total > 0)
as.True(len(stat.Cpu.Usage.PerCpu) > 0)
// PerCPU CPU usage is not supported in cgroupv2 (cpuacct.usage_percpu)
// https://github.com/google/cadvisor/issues/3065
if !cgroups.IsCgroup2UnifiedMode() {
as.True(len(stat.Cpu.Usage.PerCpu) > 0)
}
if stat.CpuInst != nil {
as.True(stat.CpuInst.Usage.Total > 0)
}
Expand Down
18 changes: 13 additions & 5 deletions integration/tests/api/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (

info "github.com/google/cadvisor/info/v1"

"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/stretchr/testify/assert"
"k8s.io/klog/v2"
)
Expand All @@ -46,12 +47,19 @@ func checkCPUStats(t *testing.T, stat info.CpuStats) {
assert := assert.New(t)

assert.NotEqual(0, stat.Usage.Total, "Total CPU usage should not be zero")
assert.NotEmpty(stat.Usage.PerCpu, "Per-core usage should not be empty")
totalUsage := uint64(0)
for _, usage := range stat.Usage.PerCpu {
totalUsage += usage

// PerCPU CPU usage is not supported in cgroupv2 (cpuacct.usage_percpu)
// https://github.com/google/cadvisor/issues/3065
if !cgroups.IsCgroup2UnifiedMode() {
assert.NotEmpty(stat.Usage.PerCpu, "Per-core usage should not be empty")

totalUsage := uint64(0)
for _, usage := range stat.Usage.PerCpu {
totalUsage += usage
}
inDelta(t, stat.Usage.Total, totalUsage, uint64((5 * time.Millisecond).Nanoseconds()), "Per-core CPU usage")
}
inDelta(t, stat.Usage.Total, totalUsage, uint64((5 * time.Millisecond).Nanoseconds()), "Per-core CPU usage")

inDelta(t, stat.Usage.Total, stat.Usage.User+stat.Usage.System, uint64((500 * time.Millisecond).Nanoseconds()), "User + system CPU usage")
// TODO(rjnagal): Add verification for cpu load.
}
Expand Down