Handle threaded cgroup types

[Bacto]

Resolve #3821

[AkihiroSuda]

Can we have an integration test?

[Bacto]

@AkihiroSuda what do you mean by integration test? Should I do something on my side?

[AkihiroSuda]

@AkihiroSuda what do you mean by integration test?

These tests
https://github.com/opencontainers/runc/tree/main/tests/integration

Should I do something on my side?

Yes, please.
Usually we expect an integration test to be accompanied with a bug fix so that we can catch potential regression in future.

[Bacto]

Here is the integration test result:

ok 1 runc exec (cgroup v2, ro cgroupfs, new cgroupns) does not chown cgroup # skip test requires systemd
ok 2 runc exec (cgroup v2, rw cgroupfs, inherit cgroupns) does not chown cgroup # skip test requires systemd
ok 3 runc exec (cgroup v2, rw cgroupfs, new cgroupns) does chown cgroup # skip test requires systemd
ok 4 runc create (no limits + no cgrouppath + no permission) succeeds
ok 5 runc create (rootless + no limits + cgrouppath + no permission) fails with permission error # skip test requires rootless
ok 6 runc create (rootless + limits + no cgrouppath + no permission) fails with informative error # skip test requires rootless
ok 7 runc create (limits + cgrouppath + permission on the cgroup dir) succeeds
ok 8 runc exec (limits + cgrouppath + permission on the cgroup dir) succeeds
ok 9 runc exec (cgroup v2 + init process in non-root cgroup) succeeds
ok 10 runc run (cgroup v1 + unified resources should fail) # skip test requires cgroups_v1
ok 11 runc run (blkio weight)
ok 12 runc run (per-device io weight for bfq)
ok 13 runc run (hugetlb limits)
ok 14 runc run (cgroup v2 resources.unified only)
ok 15 runc run (cgroup v2 resources.unified override)
ok 16 runc run (cgroupv2 mount inside container)
ok 17 runc exec (cgroup v1+hybrid joins correct cgroup) # skip test requires cgroups_hybrid
ok 18 runc exec should refuse a paused container
ok 19 runc exec --ignore-paused
ok 20 runc run/create should error/warn about a non-empty cgroup
ok 21 runc run/create should refuse pre-existing frozen cgroup
ok 22 checkpoint and restore
ok 23 checkpoint and restore (bind mount, destination is symlink)
ok 24 checkpoint and restore (with --debug)
ok 25 checkpoint and restore (cgroupns) # skip test requires cgroups_v1
ok 26 checkpoint --pre-dump (bad --parent-path)
not ok 27 checkpoint --pre-dump and restore
# (in test file tests/integration/checkpoint.bats, line 182)
#   `[ "$status" -eq 0 ]' failed
# runc spec (status=0):
#
# runc state test_busybox (status=0):
# {
#   "ociVersion": "1.0.2-dev",
#   "id": "test_busybox",
#   "pid": 6288,
#   "status": "running",
#   "bundle": "/tmp/bats-run-0qA2pz/runc.vxQsJ0/bundle",
#   "rootfs": "/tmp/bats-run-0qA2pz/runc.vxQsJ0/bundle/rootfs",
#   "created": "2024-01-10T08:59:26.819432257Z",
#   "owner": ""
# }
# runc checkpoint --pre-dump --image-path ./parent-dir test_busybox (status=1):
# time="2024-01-10T08:59:26Z" level=error msg="CRIU is missing features"
ok 28 checkpoint --lazy-pages and restore # skip this criu does not support lazy migration
not ok 29 checkpoint and restore in external network namespace
# (in test file tests/integration/checkpoint.bats, line 277)
#   `ip netns add "$ns_name"' failed with status 127
# runc spec (status=0):
#
# external_net_ns is supported
# /go/src/github.com/opencontainers/runc/tests/integration/checkpoint.bats: line 277: ip: command not found
ok 30 checkpoint and restore with container specific CRIU config
ok 31 checkpoint and restore with nested bind mounts
ok 32 runc create
ok 33 runc create exec
ok 34 runc create --pid-file
ok 35 runc create --pid-file with new CWD
ok 36 runc exec --user with no access to cwd
ok 37 runc create sets up user before chdir to cwd if needed # skip test requires rootless
ok 38 runc create can chdir if runc has access
ok 39 global --debug
ok 40 global --debug to --log
ok 41 global --debug to --log --log-format 'text'
ok 42 global --debug to --log --log-format 'json'
ok 43 runc delete
ok 44 runc delete --force
ok 45 runc delete --force ignore not exist
ok 46 runc delete --force [paused container]
ok 47 runc delete --force in cgroupv1 with subcgroups # skip test requires cgroups_v1
ok 48 runc delete --force in cgroupv2 with subcgroups
ok 49 runc delete removes failed systemd unit # skip requires systemd >= v244
ok 50 runc run [redundant default /dev/tty]
ok 51 runc run [redundant default /dev/ptmx]
ok 52 runc run/update [device cgroup deny]
ok 53 runc run [device cgroup allow rw char device]
ok 54 runc run [device cgroup allow rm block device]
ok 55 runc exec vs systemctl daemon-reload # skip test requires systemd
ok 56 events --stats
ok 57 events --interval default
ok 58 events --interval 1s
ok 59 events --interval 100ms
ok 60 events oom
ok 61 runc exec
ok 62 runc exec [exit codes]
ok 63 runc exec --pid-file
ok 64 runc exec --pid-file with new CWD
ok 65 runc exec ls -la
ok 66 runc exec ls -la with --cwd
ok 67 runc exec --env
ok 68 runc exec --user
ok 69 runc exec --user vs /dev/null ownership
ok 70 runc exec --additional-gids
ok 71 runc exec --preserve-fds
ok 72 runc --debug exec
ok 73 runc --debug --log exec
ok 74 runc exec --cgroup sub-cgroups [v1] # skip test requires cgroups_v1
ok 75 runc exec --cgroup subcgroup [v2]
ok 76 runc -h
ok 77 runc command -h
ok 78 runc foo -h
ok 79 runc run (hooks library tests)
ok 80 kill detached busybox
ok 81 list
ok 82 mask paths [file]
ok 83 mask paths [directory]
ok 84 mask paths [prohibit symlink /proc]
ok 85 mask paths [prohibit symlink /sys]
ok 86 runc run [tmpcopyup]
ok 87 runc run [bind mount]
ok 88 runc run [ro tmpfs mount]
ok 89 runc run [ro /dev mount]
ok 90 runc run [tmpfs mount with absolute symlink]
ok 91 runc run [ro /sys/fs/cgroup mounts]
ok 92 runc run [ro /sys/fs/cgroup mounts + cgroupns]
ok 93 runc run [rbind,ro mount is read-only but not recursively]
ok 94 runc run [rbind,rro mount is recursively read-only]
ok 95 runc run [rbind,ro,rro mount is recursively read-only too]
ok 96 runc run [rw bind mount of a ro fuse sshfs mount] # skip test requires working sshfs mounts
ok 97 runc run --no-pivot must not expose bare /proc
ok 98 runc pause and resume
ok 99 runc pause and resume with nonexist container
ok 100 ps
ok 101 ps -f json
ok 102 ps -e -x
ok 103 ps after the container stopped
ok 104 global --root
ok 105 runc run
ok 106 runc run --keep
ok 107 runc run --keep (check cgroup exists)
ok 108 runc run with tmpfs
ok 109 runc run with tmpfs perms
ok 110 runc run [seccomp] (SCMP_ACT_NOTIFY old kernel) # skip requires kernel less than 5.6
ok 111 runc run [seccomp] (SCMP_ACT_NOTIFY noNewPrivileges false) # skip test requires arch_x86_64
ok 112 runc run [seccomp] (SCMP_ACT_NOTIFY noNewPrivileges true) # skip test requires arch_x86_64
ok 113 runc exec [seccomp] (SCMP_ACT_NOTIFY noNewPrivileges false) # skip test requires arch_x86_64
ok 114 runc exec [seccomp] (SCMP_ACT_NOTIFY noNewPrivileges true) # skip test requires arch_x86_64
ok 115 runc run [seccomp] (SCMP_ACT_NOTIFY important syscalls noNewPrivileges false) # skip test requires arch_x86_64
ok 116 runc run [seccomp] (SCMP_ACT_NOTIFY important syscalls noNewPrivileges true) # skip test requires arch_x86_64
ok 117 runc run [seccomp] (empty listener path) # skip test requires arch_x86_64
ok 118 runc run [seccomp] (SCMP_ACT_NOTIFY empty listener path) # skip test requires arch_x86_64
ok 119 runc run [seccomp] (SCMP_ACT_NOTIFY wrong listener path) # skip test requires arch_x86_64
ok 120 runc run [seccomp] (SCMP_ACT_NOTIFY abstract listener path) # skip test requires arch_x86_64
ok 121 runc run [seccomp] (SCMP_ACT_NOTIFY kill seccompagent) # skip test requires arch_x86_64
ok 122 runc run [seccomp] (SCMP_ACT_NOTIFY no seccompagent) # skip test requires arch_x86_64
ok 123 runc run [seccomp] (SCMP_ACT_NOTIFY error chmod) # skip test requires arch_x86_64
ok 124 runc run [seccomp] (SCMP_ACT_NOTIFY write) # skip test requires arch_x86_64
ok 125 runc run [seccomp] (SCMP_ACT_NOTIFY startContainer hook) # skip test requires arch_x86_64
ok 126 runc run [seccomp] (SCMP_ACT_NOTIFY example config) # skip test requires arch_x86_64
ok 127 runc run [seccomp -ENOSYS handling]
ok 128 runc run [seccomp defaultErrnoRet=ENXIO]
not ok 129 runc run [seccomp] (SCMP_ACT_ERRNO default)
# (in test file tests/integration/seccomp.bats, line 51)
#   `[ "$status" -ne 0 ]' failed
# runc spec (status=0):
#
# runc run test_busybox (status=0):
#
not ok 130 runc run [seccomp] (SCMP_ACT_ERRNO explicit errno)
# (in test file tests/integration/seccomp.bats, line 65)
#   `[ "$status" -ne 0 ]' failed
# runc spec (status=0):
#
# runc run test_busybox (status=0):
#
not ok 131 runc run [seccomp] (SCMP_ACT_KILL)
# (in test file tests/integration/seccomp.bats, line 79)
#   `[ "$status" -ne 0 ]' failed
# runc spec (status=0):
#
# runc run test_busybox (status=0):
#
not ok 132 runc run [seccomp] (startContainer hook)
# (in test file tests/integration/seccomp.bats, line 98)
#   `[ "$status" -ne 0 ]' failed
# runc spec (status=0):
#
# runc run test_busybox (status=0):
#
ok 133 spec generation cwd
ok 134 spec generation --bundle
ok 135 spec validator
ok 136 runc start
ok 137 runc run detached
ok 138 runc run detached ({u,g}id != 0)
ok 139 runc run detached --pid-file
ok 140 runc run detached --pid-file with new CWD
ok 141 runc run
ok 142 runc run ({u,g}id != 0)
ok 143 runc run as user with no exec bit but CAP_DAC_OVERRIDE set
ok 144 runc run with rootfs set to .
ok 145 runc run --pid-file
ok 146 runc run [rootless with host pidns]
ok 147 runc run [redundant seccomp rules]
ok 148 state (kill + delete)
ok 149 state (pause + resume)
ok 150 runc run [stdin not a tty]
ok 151 runc run [tty ptsname]
ok 152 runc run [tty owner]
ok 153 runc run [tty owner] ({u,g}id != 0)
ok 154 runc exec [stdin not a tty]
ok 155 runc exec [tty ptsname]
ok 156 runc exec [tty owner]
ok 157 runc exec [tty owner] ({u,g}id != 0)
ok 158 runc exec [tty consolesize]
ok 159 runc create [terminal=false]
ok 160 runc run [terminal=false]
ok 161 runc run -d [terminal=false]
ok 162 umask
ok 163 update cgroup v1/v2 common limits
ok 164 update cgroup cpu limits
ok 165 set cpu period with no quota
ok 166 set cpu period with no quota (invalid period)
ok 167 set cpu quota with no period
ok 168 update cpu period with no previous period/quota set
ok 169 update cpu quota with no previous period/quota set
ok 170 update cpu period in a pod cgroup with pod limit set # skip test requires cgroups_v1
ok 171 update cgroup v2 resources via unified map
ok 172 update cpuset parameters via resources.CPU
ok 173 update cpuset parameters via v2 unified map
ok 174 update cpuset cpus range via v2 unified map # skip test requires systemd
ok 175 update rt period and runtime # skip test requires cgroups_v1
ok 176 update devices [minimal transition rules]
ok 177 update paused container
ok 178 userns with simple mount
ok 179 userns with 2 inaccessible mounts
ok 180 userns with inaccessible mount + exec
ok 181 userns with bind mount before a cgroupfs mount # skip test requires cgroups_v1
ok 182 runc version

I don't know why some tests failed but I run them on branch v1.1.10 too and got the same error, so it doesn't seem related to the patch.

Best,
Adrien

[kolyshkin]

@Bacto please squash your commits.

[kolyshkin]

Rebased to latest HEAD to get CI fixes

[kolyshkin]

We still don't have the integration test :(

[kolyshkin]

So, I think the way the code should work is try to read cgroup.procs and if there's "operation not supported" error, switch to cgroup.threads. This way it is faster than reading an extra file (cgroup.type).

[kolyshkin]

You are using public names here. Do you want to use these constants from other packages?

[kolyshkin]

There are couple of linter issues to be addressed as well.

[kolyshkin]

@Bacto please let us know if you want to keep working on this.