-
Notifications
You must be signed in to change notification settings - Fork 398
Upgrade to isolate v2 #1222
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
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? |
Co-authored-by: Filippo Casarin <[email protected]>
In order to upgrade to isolate v2 we need to:
cgroup: hostto the docker-compose file to run the container in the host's cgroup namespace (docs).prerequisites.pywith default settings. In the default settings, isolate expects/run/isolate/cgroupto exist. This normally gets created byisolate-cg-keeper, which is what theisolate.servicesystemd 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.pyinstall the unit). I'm not sure however if it's really needed to run that, since just runningecho /sys/fs/cgroup > /run/isolate/groupmanually 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.samplefile (alongside thecms.conf.sample,nginx.conf.sample, etc) withcg_rootoverridden to/sys/fs/cgroup, and install that file as part of theprerequisites.py installstep instead of the default isolate config file.Fixes #1257