feat: Enable systemd cgroups management #540
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dtrudg
force-pushed
the
issue299
branch
9 times, most recently
from
06bca64
to
e963973
Compare
dtrudg
changed the title
dtrudg
force-pushed
the
issue299
branch
2 times, most recently
from
553b660
to
af15171
Compare
dtrudg
added a commit
to dtrudg/singularity
that referenced
this pull request
Mar 4, 2022
The Singularity e2e-tests were previously all run in a mount and PID namespace, to avoid polluting the host filesystem (critical), and process tree (less critical). These namespaces are set up in some CGO init code. The use of the PID namespace prevents testing with systemd as the cgroups manager, as systemd is aware of the host process tree, not the one in the new e2e PID namespace. This is a major omission since sylabs#540 switched to using systemd for cgroups management by default. This PR: * Modifies the e2e init CGO code, so that an env var `SINGULARITYE_E2E_NO_PID_NS` will cause it *not* to create a new PID namespace. * Modifies the Makefile so that the e2e suite is called twice, once with PID ns, once without. * Moves the INSTANCE cgroups test into a new package/topic e2e/cgroups, run in the e2e call without PID ns. * Moves the OCI tests into the e2e call without PID ns. * Modifies the CGROUPS and OCI tests so that they test with both systemd and cgroupfs management, using a convenience wrapper function. Fixes: 563 Note - this breaks the e2e CLI coverage, as it only supports collecting / analyzing one e2e run... where we now have two. The CLI coverage metrics are unused in practice, and flawed (don't consider combinations of flags required or possible), so I'm going to propose removing them in a separate issue.
dtrudg
added a commit
to dtrudg/singularity
that referenced
this pull request
Mar 4, 2022
The Singularity e2e-tests were previously all run in a mount and PID namespace, to avoid polluting the host filesystem (critical), and process tree (less critical). These namespaces are set up in some CGO init code. The use of the PID namespace prevents testing with systemd as the cgroups manager, as systemd is aware of the host process tree, not the one in the new e2e PID namespace. This is a major omission since sylabs#540 switched to using systemd for cgroups management by default. This PR: * Modifies the e2e init CGO code, so that an env var `SINGULARITYE_E2E_NO_PID_NS` will cause it *not* to create a new PID namespace. * Modifies the Makefile so that the e2e suite is called twice, once with PID ns, once without. * Moves the INSTANCE cgroups test into a new package/topic e2e/cgroups, run in the e2e call without PID ns. * Moves the OCI tests into the e2e call without PID ns. * Modifies the CGROUPS and OCI tests so that they test with both systemd and cgroupfs management, using a convenience wrapper function. Fixes: 563 Note - this breaks the e2e CLI coverage, as it only supports collecting / analyzing one e2e run... where we now have two. The CLI coverage metrics are unused in practice, and flawed (don't consider combinations of flags required or possible), so I'm going to propose removing them in a separate issue.
dtrudg
added a commit
to dtrudg/singularity
that referenced
this pull request
Mar 4, 2022
The Singularity e2e-tests were previously all run in a mount and PID namespace, to avoid polluting the host filesystem (critical), and process tree (less critical). These namespaces are set up in some CGO init code. The use of the PID namespace prevents testing with systemd as the cgroups manager, as systemd is aware of the host process tree, not the one in the new e2e PID namespace. This is a major omission since sylabs#540 switched to using systemd for cgroups management by default. This PR: * Modifies the e2e init CGO code, so that an env var `SINGULARITYE_E2E_NO_PID_NS` will cause it *not* to create a new PID namespace. * Modifies the Makefile so that the e2e suite is called twice, once with PID ns, once without. * Moves the INSTANCE cgroups test into a new package/topic e2e/cgroups, run in the e2e call without PID ns. * Moves the OCI tests into the e2e call without PID ns. * Modifies the CGROUPS and OCI tests so that they test with both systemd and cgroupfs management, using a convenience wrapper function. Fixes: 563 Note - this breaks the e2e CLI coverage, as it only supports collecting / analyzing one e2e run... where we now have two. The CLI coverage metrics are unused in practice, and flawed (don't consider combinations of flags required or possible), so I'm going to propose removing them in a separate issue.
This was referenced Mar 18, 2022
This was referenced Mar 23, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enable cgroups management via
systemd.--apply-cgroupsforshell/exec/runetc. will create asingularity-<pid>.scopein thesystem.slice.The OCI runtime will create a
singularity-oci-<name>.scopein thesystem.slice.This is a stepping stone toward allowing unpriv cgroups limits, via asking systemd to create and delegate scope in the user slice, on cgroups v2 systems.
Set systemd cgroups management as the default. It will be needed as we move toward rootless, and we are not directly supporting any non-systemd distributions at this time. It can be disabled, to fall back to direct manipulation of cgroupfs, by setting
systemd cgroups = noinsingularity.conf.NOTE...
There is reasonable unit test coverage of the cgroups functionality with both cgroupfs and systemd management.
There is currently NO e2e coverage of systemd cgroups management. Our e2e tests are run in a new PID namespace. We can't ask the host systemd to manage cgroups for PIDs in this namespace, as they are completely separate from the host PIDs that system sees.
I am going to capture a tech debt issue for the e2e to address separately, as it won't be a trivial change (#563).
Fixes #299