-
Notifications
You must be signed in to change notification settings - Fork 361
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
Upgrade to isolate v2 #1222
Upgrade to isolate v2 #1222
Conversation
Cgroup v2 support is now present in the latest release of Isolate. See also #1257. |
619a895
to
9a5c27c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1222 +/- ##
==========================================
+ Coverage 69.39% 69.55% +0.15%
==========================================
Files 328 328
Lines 26201 26201
==========================================
+ Hits 18182 18223 +41
+ Misses 8019 7978 -41
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
9a5c27c
to
4495a45
Compare
To clarify, until this gets merged we should still use the kernel flags as detailed in ioi/isolate#35, correct? At the moment, using Failed to create control group /sys/fs/cgroup/memory/box-0/: No such file or directory
Traceback (most recent call last):
File "/home/cms/cms_venv/bin/cmsMake", line 11, in <module>
load_entry_point('cms==1.4rc1', 'console_scripts', 'cmsMake')()
File "/home/cms/cms_venv/lib/python3.6/site-packages/cms-1.4rc1-py3.6.egg/cmstaskenv/cmsMake.py", line 755, in main
assume=assume)
File "/home/cms/cms_venv/lib/python3.6/site-packages/cms-1.4rc1-py3.6.egg/cmstaskenv/cmsMake.py", line 692, in execute_multiple_targets
already_executed, debug=debug, assume=assume)
File "/home/cms/cms_venv/lib/python3.6/site-packages/cms-1.4rc1-py3.6.egg/cmstaskenv/cmsMake.py", line 682, in execute_target
action(assume=assume)
File "/home/cms/cms_venv/lib/python3.6/site-packages/cms-1.4rc1-py3.6.egg/cmstaskenv/cmsMake.py", line 269, in test_src
assume=assume)
File "/home/cms/cms_venv/lib/python3.6/site-packages/cms-1.4rc1-py3.6.egg/cmstaskenv/Test.py", line 163, in test_testcases
tasktype.evaluate(job, file_cacher)
File "/home/cms/cms_venv/lib/python3.6/site-packages/cms-1.4rc1-py3.6.egg/cms/grading/tasktypes/Batch.py", line 290, in evaluate
sandbox = create_sandbox(file_cacher, name="evaluate")
File "/home/cms/cms_venv/lib/python3.6/site-packages/cms-1.4rc1-py3.6.egg/cms/grading/tasktypes/util.py", line 71, in create_sandbox
sandbox = Sandbox(file_cacher, name=name)
File "/home/cms/cms_venv/lib/python3.6/site-packages/cms-1.4rc1-py3.6.egg/cms/grading/Sandbox.py", line 944, in __init__
self.initialize_isolate()
File "/home/cms/cms_venv/lib/python3.6/site-packages/cms-1.4rc1-py3.6.egg/cms/grading/Sandbox.py", line 1420, in initialize_isolate
"(error %d)" % (pretty_print_cmdline(init_cmd), ret))
cms.grading.Sandbox.SandboxInterfaceException: Failed to initialize sandbox with command: isolate --cg --box-id=0 --init (error 2) |
139cecb
to
738971e
Compare
Co-authored-by: Filippo Casarin <[email protected]>
738971e
to
a491864
Compare
Maybe I am misunderstanding something, but as per https://systemd.io/CGROUP_DELEGATION/ the root cgroup shouldn't not be touched by anything other than systemd. So even if it appears to work now, using @gollux ? |
I completely agree that overriding I would very much like CMS to stop installing isolate. It's a separate project and users should run its own installation mechanism. I would like to provide Debian/Ubuntu packages of Isolate, which install the systemd service following Debian conventions. I welcome other people to contribute packaging for their favorite distributions. |
Thank you @gollux that would be very helpful. Let me know how you plan to provide the .deb file and we will then integrate it as part of CMS's installation. A good way I guess would be to use Github "Releases" in https://github.com/ioi/isolate/releases and upload the .deb as an artifact of each release, what do you think? |
In order to upgrade to isolate v2 we need to:
cgroup: host
to the docker-compose file to run the container in the host's cgroup namespace (docs).prerequisites.py
with default settings. In the default settings, isolate expects/run/isolate/cgroup
to exist. This normally gets created byisolate-cg-keeper
, which is what theisolate.service
systemd unit is meant for. If we want users to install and enable that unit, then we should add it to the documentation (and possibly makeprerequisites.py
install the unit). I'm not sure however if it's really needed to run that, since just runningecho /sys/fs/cgroup > /run/isolate/group
manually seems to also work. Maybe we can instead configure isolate to always use/sys/fs/cgroup
(I think we can do it by changing the default configuration in/usr/local/etc/isolate
) and document this instead? We need to figure out which of these approaches are OK, and add it to the documentation.UPDATE: After discussion with @veluca93, we decided to create a local
./config/isolate.conf.sample
file (alongside thecms.conf.sample
,nginx.conf.sample
, etc) withcg_root
overridden to/sys/fs/cgroup
, and install that file as part of theprerequisites.py install
step instead of the default isolate config file.Fixes #1257