-
Notifications
You must be signed in to change notification settings - Fork 254
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
Implement cgroup v2 #2588
base: main
Are you sure you want to change the base?
Implement cgroup v2 #2588
Conversation
3793aa6
to
3c426ec
Compare
50b74aa
to
f65f320
Compare
Fixes DOMjudge#1072. Also move our integration tests from broken gitlab to working github actions.
There are three TODOs that I need to think a little bit more about, but in general it is ready for review (and works both on my laptop, a desktop and on CI). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We lose some of the other integration tests, can you leave the gitlab configs for now so we can add those back in the future?
You checked if the cgroupv1 also still worked?
steps: | ||
- uses: actions/checkout@v4 | ||
- name: info | ||
run: cat /proc/cmdline && echo && cat /proc/mounts && echo && ls -al /sys/fs/cgroup && echo && uname -a && echo && stat -fc %T /sys/fs/cgroup && echo && cat /proc/self/cgroup && cat /proc/cpuinfo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
run: cat /proc/cmdline && echo && cat /proc/mounts && echo && ls -al /sys/fs/cgroup && echo && uname -a && echo && stat -fc %T /sys/fs/cgroup && echo && cat /proc/self/cgroup && cat /proc/cpuinfo | |
run: | | |
cat /proc/cmdline && echo && | |
cat /proc/mounts && echo && | |
ls -al /sys/fs/cgroup && echo && | |
uname -a && echo && | |
stat -fc %T /sys/fs/cgroup && echo && | |
cat /proc/self/cgroup && | |
cat /proc/cpuinfo |
Happy to revert what needs to be reverted but I fail to see what we are losing.
Locally: yes. |
The GitLab version tested:
I think not all were copied over and would require a matrix job. |
Since judging was basically disabled on gitlab we didn't get a lot of use from the matrix |
I agree, but I think those tests found errors in the past. But if you think they're not needed let's leave it like this. |
The most important part is testing this PR to figure out where it works and where it doesn't. @nickygerritsen already tested on Mac plus docker and we have other people who committed to testing on Debian stable, fedora and Arch. |
I tried to run this on my Arch Linux laptop using
However, I do have When I change `exit 1` to `exit 0` in `judge/create_cgroups.in`, the webapp starts, but the judgedaemons fail to start.diff --git a/judge/create_cgroups.in b/judge/create_cgroups.in
index 56d1338a3..a36eb8c00 100755
--- a/judge/create_cgroups.in
+++ b/judge/create_cgroups.in
@@ -16,8 +16,9 @@ cgroup_error_and_usage () {
On modern distros (e.g. Debian bullseye and Ubuntu Jammy Jellyfish) which have cgroup v2 enabled by default,
you need to add 'systemd.unified_cgroup_hierarchy=0' as well.
2. Run update-grub
- 3. Reboot" >&2
- exit 1
+ 3. Reboot
+ (ignoring for now, hehe)" >&2
+ exit 0
}
for i in cpuset memory; do It looks like I cannot run cgroups v2 without also having support for v1, unless I'm missing some configuration setting somewhere 😛 I'm guessing that |
The create_cgroups script should not be needed. I commented out the whole thing on my docker setup and that worked. What error is the judgedaemon giving you? |
In my
However, when looking in However, when I try to upload a contest on this fresh DB state, I get three times the following judgehost error, on the first submission it tries to run for each of the languages C++, Python, and Java:
|
@mpsijm thanks for testing, can you add |
There you go, some debug output:
|
I did some initial testing and can reproduce the debug output in a container. As far as I understand, moving the process out of the container slice is not permitted. When mounting the complete cgroupfs, the processes are already running in a cgroup:
Manually creating a Processes in the container think that they are in the cgroup:
If we do not mount |
If you are running docker, you might need to pass |
The (podman in my setup) container is already running privileged, and I tried using the host cgroupns but that did not solve it. |
I've added I've checked for some submissions that should time out that they do, and for submissions that use too much memory that they are terminated, looks like it works 😄 I haven't tested the scenario where the compiler uses too much memory or times out, because I wouldn't know how to 😂 |
So we need to test if |
This is just to test whether it works on CI, it needs a lot of cleanup.