client: setting empty cpu cgroup is broken #11933

shoenig · 2022-01-25T22:53:45Z

Somewhere between the upgrade to runc v1.0.3 and testing on ubuntu 20.04, cpuset management got broken.

➜ sudo -E go test -v -run TestCpusetManager_AddAlloc
=== RUN   TestCpusetManager_AddAlloc
2022-01-25T16:46:21.938-0600 [INFO]  cgutil/cpuset_manager_linux.go:165: initialized cpuset cgroup manager: parent=/gotest-8d9ec808 cpuset=0-23
    cpuset_manager_linux_test.go:78: 
        	Error Trace:	cpuset_manager_linux_test.go:78
        	Error:      	Should be empty, but was [0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23]
        	Test:       	TestCpusetManager_AddAlloc
--- FAIL: TestCpusetManager_AddAlloc (0.02s)
FAIL
exit status 1
FAIL	github.com/hashicorp/nomad/client/lib/cgutil	0.029s

I notice that by manually setting value written to the shared/cpuset.cpus to a space character " " instead of empty string "", it works.

 func (_ *cpusetManager) setCgroupCpusetCPUs(path, cpus string) error {
        currentCpusRaw, err := fscommon.ReadFile(path, "cpuset.cpus")
        if err != nil {
                return err
        }
 
        if cpus != strings.TrimSpace(currentCpusRaw) {
+               if cpus == "" {
+                       cpus = " "
+               }
                if err := fscommon.WriteFile(path, "cpuset.cpus", cpus); err != nil {
                        return err
                }
        }

➜ sudo -E go test -v -run TestCpusetManager_AddAlloc
=== RUN   TestCpusetManager_AddAlloc
2022-01-25T16:48:43.574-0600 [INFO]  cgutil/cpuset_manager_linux.go:168: initialized cpuset cgroup manager: parent=/gotest-af715183 cpuset=0-23
--- PASS: TestCpusetManager_AddAlloc (0.03s)
PASS
ok  	github.com/hashicorp/nomad/client/lib/cgutil	0.033s

But I don't understand why. The implementation of fscommon.WriteFile changed significantly from runc v1.0.0-rc92 to runc v1.0.0-rc93, but AFAICT this feature was introduced with the later version.

The text was updated successfully, but these errors were encountered:

shoenig · 2022-01-25T23:33:23Z

Spun up a ubuntu 16.04 vm and indeed it seems to work there

ubuntu@ip-172-31-73-208:~/nomad/client/lib/cgutil$ cat /etc/os-release  | grep VERSION= 
VERSION="16.04.7 LTS (Xenial Xerus)"
ubuntu@ip-172-31-73-208:~/nomad/client/lib/cgutil$ git show | grep commit
commit 94b744c5394f9943158e737f4b53b99722122392
ubuntu@ip-172-31-73-208:~/nomad/client/lib/cgutil$ sudo -E /usr/local/go/bin/go test -v -run TestCpusetManager_AddAlloc
=== RUN   TestCpusetManager_AddAlloc
2022-01-25T23:32:09.086Z [INFO]  cgutil/cpuset_manager_linux.go:165: initialized cpuset cgroup manager: parent=/gotest-e7a6fdcd cpuset=0-3
--- PASS: TestCpusetManager_AddAlloc (0.00s)
PASS
ok  	github.com/hashicorp/nomad/client/lib/cgutil	0.014s

This PR tweaks the TestCpusetManager_AddAlloc unit test to not break when being run on a machine using cgroupsv2. The behavior of writing an empty cpuset.cpu changes in cgroupv2, where such a group now inherits the value of its parent group, rather than remaining empty. The test in question was written such that a task would consume all available cores shared on an alloc, causing the empty set to be written to the shared group, which works fine on cgroupsv1 but breaks on cgroupsv2. By adjusting the test to consume only 1 core instead of all cores, it no longer triggers that edge case. The actual fix for the new cgroupsv2 behavior will be in #11933

shoenig · 2022-01-26T18:21:26Z

Turns out this is a duplicate of #11289

This PR tweaks the TestCpusetManager_AddAlloc unit test to not break when being run on a machine using cgroupsv2. The behavior of writing an empty cpuset.cpu changes in cgroupv2, where such a group now inherits the value of its parent group, rather than remaining empty. The test in question was written such that a task would consume all available cores shared on an alloc, causing the empty set to be written to the shared group, which works fine on cgroupsv1 but breaks on cgroupsv2. By adjusting the test to consume only 1 core instead of all cores, it no longer triggers that edge case. The actual fix for the new cgroupsv2 behavior will be in #11933

This PR introduces support for using Nomad on systems with cgroups v2 [1] enabled as the cgroups controller mounted on /sys/fs/cgroups. Newer Linux distros like Ubuntu 21.10 are shipping with cgroups v2 only, causing problems for Nomad users. Nomad mostly "just works" with cgroups v2 due to the indirection via libcontainer, but not so for managing cpuset cgroups. Before, Nomad has been making use of a feature in v1 where a PID could be a member of more than one cgroup. In v2 this is no longer possible, and so the logic around computing cpuset values must be modified. When Nomad detects v2, it manages cpuset values in-process, rather than making use of cgroup heirarchy inheritence via shared/reserved parents. Nomad will only activate the v2 logic when it detects cgroups2 is mounted at /sys/fs/cgroups. This means on systems running in hybrid mode with cgroups2 mounted at /sys/fs/cgroups/unified (as is typical) Nomad will continue to use the v1 logic, and should operate as before. Systems that do not support cgroups v2 are also not affected. When v2 is activated, Nomad will create a parent called nomad.slice (unless otherwise configured in Client conifg), and create cgroups for tasks using naming convention <allocID>-<task>.scope. These follow the naming convention set by systemd and also used by Docker when cgroups v2 is detected. Client nodes now export a new fingerprint attribute, unique.cgroups.version which will be set to "v1" or "v2" to indicate the cgroups regime in use by Nomad. The new cpuset management strategy fixes #11705, where docker tasks that spawned processes on startup would "leak". In cgroups v2, the PIDs are started in the cgroup they will always live in, and thus the cause of the leak is eliminated. [1] https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html Closes #11289 Fixes #11705 #11773 #11933

This PR introduces support for using Nomad on systems with cgroups v2 [1] enabled as the cgroups controller mounted on /sys/fs/cgroups. Newer Linux distros like Ubuntu 21.10 are shipping with cgroups v2 only, causing problems for Nomad users. Nomad mostly "just works" with cgroups v2 due to the indirection via libcontainer, but not so for managing cpuset cgroups. Before, Nomad has been making use of a feature in v1 where a PID could be a member of more than one cgroup. In v2 this is no longer possible, and so the logic around computing cpuset values must be modified. When Nomad detects v2, it manages cpuset values in-process, rather than making use of cgroup heirarchy inheritence via shared/reserved parents. Nomad will only activate the v2 logic when it detects cgroups2 is mounted at /sys/fs/cgroups. This means on systems running in hybrid mode with cgroups2 mounted at /sys/fs/cgroups/unified (as is typical) Nomad will continue to use the v1 logic, and should operate as before. Systems that do not support cgroups v2 are also not affected. When v2 is activated, Nomad will create a parent called nomad.slice (unless otherwise configured in Client conifg), and create cgroups for tasks using naming convention <allocID>-<task>.scope. These follow the naming convention set by systemd and also used by Docker when cgroups v2 is detected. Client nodes now export a new fingerprint attribute, unique.cgroups.version which will be set to 'v1' or 'v2' to indicate the cgroups regime in use by Nomad. The new cpuset management strategy fixes #11705, where docker tasks that spawned processes on startup would "leak". In cgroups v2, the PIDs are started in the cgroup they will always live in, and thus the cause of the leak is eliminated. [1] https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html Closes #11289 Fixes #11705 #11773 #11933

github-actions · 2022-10-12T02:44:29Z

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

shoenig added the type/bug label Jan 25, 2022

shoenig added this to the 1.3.0 milestone Jan 25, 2022

shoenig added the theme/cgroups cgroups issues label Jan 25, 2022

jrasell added this to Needs Triage in Nomad - Community Issues Triage via automation Jan 26, 2022

jrasell moved this from Needs Triage to Needs Roadmapping in Nomad - Community Issues Triage Jan 26, 2022

shoenig closed this as completed Jan 26, 2022

Nomad - Community Issues Triage automation moved this from Needs Roadmapping to Done Jan 26, 2022

shoenig mentioned this issue Jan 27, 2022

client: change test to not poke cgroupv2 edge case #11951

Merged

shoenig mentioned this issue Mar 21, 2022

client: enable cpuset support for cgroups.v2 #12274

Merged

github-actions bot locked as resolved and limited conversation to collaborators Oct 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client: setting empty cpu cgroup is broken #11933

client: setting empty cpu cgroup is broken #11933

shoenig commented Jan 25, 2022

shoenig commented Jan 25, 2022

shoenig commented Jan 26, 2022

github-actions bot commented Oct 12, 2022

client: setting empty cpu cgroup is broken #11933

client: setting empty cpu cgroup is broken #11933

Comments

shoenig commented Jan 25, 2022

shoenig commented Jan 25, 2022

shoenig commented Jan 26, 2022

github-actions bot commented Oct 12, 2022