Add support for systemd managed cgroups #46

nimrodshn · 2021-05-29T10:39:15Z

What this does?

Adds initial support for systemd to manage cgroups.

Why do this?

The option to have systemd manage cgroups (via a systemd-cgroup flag) is supported both in runc and in crun, and is a candidate to be added to the oci runtim-spec, so much so that conmon is relying on this flag to be present which means that in order for youki to support podman we must first add support for systemd-cgroup (as well as a --force flag).

cc: @utam0k

Related issue: #24 .

GIF:

utam0k · 2021-05-29T10:46:34Z

So far, This PR looks good!

utam0k · 2021-05-29T10:48:26Z

@Furisto
Maybe this PR will affect the work you are doing to support cgoups v2.

nimrodshn · 2021-05-29T10:51:34Z

@utam0k Sorry for some of the unrelated changes here, I've thrown in a cargo fmt as well, hoping you're OK with it 🐱

utam0k · 2021-05-29T10:54:34Z

@nimrodshn np. I'm rather grateful.

src/create.rs

utam0k · 2021-05-29T11:02:15Z

I've recently started adding comments for docs.rs on Youki, if you'd like to add some comments?

nimrodshn · 2021-05-29T11:05:36Z

I've recently started adding comments for docs.rs on Youki, if you'd like to add some comments?

@utam0k Sure. what would you like me to write? I dont see docs.rs locally perhaps it was just pushed. (Did you mean doc-draft.md?)

utam0k · 2021-05-29T11:14:09Z

@nimrodshn
Sorry, my explanation was a little bit confusing, docs.rs means that you should write your comments according to this format.
https://doc.rust-lang.org/book/ch14-02-publishing-to-crates-io.html

At the very least, it would be helpful if you could provide a brief description of SystemDCGroupManager. If you have a good reference, you may want to link to it.
I'd like you to add inline comments as you like to make it easier for other contributors to understand.

nimrodshn · 2021-05-29T11:17:54Z

@utam0k Ah of course! I will be sure to document (basically my points of reference are [1][2]).

utam0k · 2021-06-02T00:24:49Z

@nimrodshn This PR had a big conflict with the inclusion of cgroups v2 support. Sorry about that.
utam0k#48

nimrodshn · 2021-06-02T06:47:59Z

@utam0k No worries. Sorry for taking so long, I'm learning this topic as I write the code.

utam0k · 2021-06-02T06:56:01Z

@nimrodshn
No problem at all.
I think this is a great opportunity to learn more about this topic and I welcome it. If you have any problems, please feel free to ask for help.

@utam0k No worries. Sorry for taking so long, I'm learning this topic as I write the code.

src/cgroups/v2/systemd_manager.rs

nimrodshn · 2021-06-19T17:51:36Z

@Furisto @utam0k Please review.

Not entirely sure why CI is failing 😿

utam0k

I have confirmed that this PR works correctly.
@Furisto is more detailed, so please provide additional review.

Furisto · 2021-06-21T21:01:56Z

src/cgroups/common.rs

@@ -109,6 +112,16 @@ pub fn create_cgroup_manager<P: Into<PathBuf>>(cgroup_path: P) -> Result<Box<dyn
        }
        (None, Some(cgroup2)) => {
            log::info!("cgroup manager V2 will be used");
+            if systemd_cgroup {
+                if !booted()? {


This seems to be the only place where the systemd crate is used. You do not need to do this now, but think about if we can check the availability of systemd another way and avoid another dependency. I

https://docs.rs/libsystemd/0.3.1/libsystemd/daemon/fn.booted.html

src/cgroups/v2/systemd_manager.rs

Furisto · 2021-06-21T21:07:19Z

src/cgroups/v2/systemd_manager.rs

+        let full_cgroup_path = self.create_unified_cgroup(pid)?;
+
+        for controller in CONTROLLER_TYPES {
+            match controller {


I assume this is temporary until you tell systemd to do this for you?

No. this still has to be done here (I might be mistaken though)... the only thing that would be different is adding the unit (via dbus) to systemd - that way systemd manages the container and can do different things with it (like start the container on boot etc.). I might be mistaken though. I will try and look at what runc does.

@Furisto Hmm looking at the runc docs it seems that that the above resource limitations wont be needed and indeed they would to be set as properties on the unit created. I will do that in a follow up PR.

See also https://www.freedesktop.org/software/systemd/man/systemd.resource-control.html. I would recommend to split this into controllers again, that then can be done by other people like we did with cgroup v1 and v2.

Sounds good. Thanks!

src/cgroups/v2/systemd_manager.rs

Furisto · 2021-06-22T09:01:55Z

@nimrodshn Looks good. Thanks!

add default handling when there isn't cgroup path in config.json. organize the logger. Merge pull request youki-dev#47 from utam0k/organize-log organize the logger. Merge pull request youki-dev#45 from utam0k/no-cgroup-path add default handling when there isn't cgroup path in config.json. ignore submodule artifacts. add a comment in oci_spec. make sure that config.json can be read regardless of where it is executed. add the tutorial on using youki. Merge pull request youki-dev#49 from utam0k/tutorial add the tutorial on using youki. update README. Merge pull request youki-dev#50 from utam0k/edit-readme update README. Reorganize cgroup modules cpu and mems is optional according to spec Spike cgroupv2 implementation Move CgroupManager to common module Implement cgroup manager trait Detect group version and create manager Improve file write options Implement string trait for controller types style Support cpu controller Add test module Style Enable controllers along the hierarchy Add tests for cpu controller Add modules for controllers Add tests for cpuset controller Use Result consistently Rework cgroup fs detection Support removal of cgroup Remove types module Formatting only Prefer v1 over v2 Fix integration test failure Control selection of cgroup fs with env variable Get rid of warnings Merge pull request youki-dev#48 from Furisto/cgroupv2 Initial support for cgroups v2 cargo clippy. make log level debug to get more information when ci failed. Merge pull request youki-dev#53 from utam0k/fix-unstable-ci make log level debug to get more information when ci failed. 4b260b0 Merge branch 'main' of github.com:utam0k/youki into HEAD Merge pull request youki-dev#52 from utam0k/cargo-clippy cargo clippy. Align cgroup controller implementations - Use common method to write to cgroup file - Ensure that pid is always written to the cgroup Merge pull request youki-dev#54 from Furisto/cg-common Align cgroup controller implementations add spec cli command resolve conflicts impl defaults for lots of structs Consolidate cgroup test methods Merge pull request youki-dev#57 from Furisto/cg-tests Consolidate cgroup test methods Update README.md - Change comment out sign `//` to `#` in shell script - Add new line at end of file formatting add back default attributes Update comment Merge branch 'main' of github.com:utam0k/youki into main Update comment Merge branch 'main' of github.com:YJDoc2/youki into main Add 'Community' section to README.md Merge pull request youki-dev#59 from nimrodshn/add_community_section_to_readme Add 'Community' section to README.md Update README: Split code section in tutorial - Split code section in tutorial. Merge pull request youki-dev#58 from aoki/main Update README.md Update comment, fix cargo-clippy warning Merge pull request youki-dev#43 from YJDoc2/main Add comments to create.rs utam0k -> containers Signed-off-by: Sora Morimoto <sora@morimoto.io> Merge pull request youki-dev#61 from smorimoto/utam0k-containers utam0k -> containers Add cgroup v1 cpu controller Add cgroup v1 cpuset controller Unit tests for cgroup v1 cpu controller Unit tests for cgroup v1 cpuset controller Cut down on boilerplate Activate cpu integration test Fix error while moving task into cgroup If a task is moved into the cgroup and a value has not been set for cpus and mems Errno 28 (no space left on device) will be returned. Therefore we set the value from the parent if required. Improve reliability of cgroup removal Merge pull request youki-dev#63 from Furisto/cg-cpu-v1 Support for cgroup v1 cpu and cpuset subsystem Add comments to process module and minor refactoring Changed hard coded mentions of poll time and event numbers to references of constants Merge pull request youki-dev#64 from YJDoc2/main Add comments to process module and minor refactoring Added install command for prerequisite in README Merge pull request youki-dev#66 from PeterYordanov/add-install-prerequisite Added install command for prerequisite in README Fixed spelling mistake in src/rootfs.rs Merge pull request youki-dev#67 from PeterYordanov/spelling-mistake-rootfs Fixed spelling mistake in src/rootfs.rs add handling of WouldBlock error. Use init pid instead of child pid Ensure controllers are enabled at the root Logging Added folder structure section in README Added folder structure section in README Added resources to folder structure section Fixed spelling mistakes Added documentation comment for cgroup module Added doc comments for modules Removed folder structure section Update doc comment Merge pull request youki-dev#68 from utam0k/handle_wouldblock add handling of WouldBlock error. Create a template for integration tests. refactore Run fmt Merge branch 'main' into add-spec-arg Added doc on how to run integration tests Update youki path Change build command Merge pull request youki-dev#73 from minakawa-daiki/fix-youki-path Change execution path and fix CI Merge branch 'main' into add-integration-template Merge pull request youki-dev#69 from Furisto/cg-escape Fix issues with cgroup v1 and v2 Merge pull request youki-dev#71 from minakawa-daiki/add-integration-template Added Integration test template basic device tests property testing on devices property testing hugetbl and memory controllers fix comount checks Updated doc comments migrate LinuxCapabilityType to enum Update caps_migrations.rs Fixed typo Unfinished sentence Merge pull request youki-dev#70 from PeterYordanov/added-doc-comments-modules Added doc comments modules Add comments to container module make LinuxCapabilityType add some widgets to README.md Merge pull request youki-dev#76 from utam0k/widget add some widgets to README.md Handle relative cgroup paths Ensure parents have value Merge pull request youki-dev#74 from Furisto/cg-relative Handle relative cgroup paths address requested changes Merge pull request youki-dev#75 from tsturzl/main Improved testing, property testing, device tests impl From for Capabilities and add tests Add comments to command module Add draft-doc for container and command module Merge branch 'main' of github.com:containers/youki into main Merge pull request youki-dev#79 from YJDoc2/main Document Container and Command modules Fix README badges fix github ci badge typo... Merge pull request youki-dev#80 from tsturzl/main Fix badges in README add create kill delete state in integration test Merge pull request youki-dev#81 from duduainankai/add-integration-test add create kill delete state in integration test Provide better error messages Merge pull request youki-dev#84 from Furisto/cg-better-errors Provide better error messages limit scope of unsafe get rid of unneeded unsafes make sure any values with `=` are not affected Merge pull request youki-dev#85 from tsturzl/main Clean up use of unsafe Add info command Merge pull request youki-dev#83 from Furisto/info-cmd Add info command Merge remote-tracking branch 'upstream/main' into add-spec-arg resolve conflict add lots of comments Add CODE-OF-CONDUCT.md and SECURITY.md Fix README link typo Merge pull request youki-dev#88 from sasurau4/fix/readme-link Fix README link typo Merge pull request youki-dev#86 from utam0k/coc-security Add CODE-OF-CONDUCT.md and SECURITY.md docs clean up around the tty. Renamed Cond to Pipe Merge pull request youki-dev#89 from utam0k/tty-refactor clean up around the tty. Merge pull request youki-dev#90 from YJDoc2/main Rename Cond to Pipe make sure to log any unimplemented controllers. Merge pull request youki-dev#91 from utam0k/unknown-controller make sure to log any unimplemented controllers. Add support for cpuacct in cgroup v1. Remove unnecessary processing. use bail! insted of anyhow Merge pull request youki-dev#92 from yjuba/v1-cpuacct [WIP] Add support for cpuacct in cgroup v1. Merge pull request youki-dev#94 from utam0k/bail use bail! insted of anyhow Add cgroup v1 freezer controller Add a test for applying CpuAcct. Merge pull request youki-dev#96 from yjuba/add-cpuacct-test Add a test for applying CpuAcct. improve build time in CI Check if rootless container is required and ensure prerequisites Ensure map binaries are available Implement protocol for identifier mapping Ensure root directory can be written by non root user Identifier mapping names were not correct Only one mapping needs to match Prevent panic when resources are not specified Fix wrong mapping Clippy and fmt Merge pull request youki-dev#93 from duduainankai/add-freezer Add cgroup v1 freezer controller remove the cargo-when dependency. Address review comments - should_use_rootless doesn't need Result type - add warning regarding current rootless limitations - make lookup_map_binary more concise Merge pull request youki-dev#98 from Furisto/rootless Experimental support for rootless containers Add unit tests for tty module Reuse tmpdir implementation from cgroup Extend info cmd with version and os Merge pull request youki-dev#102 from constreference/tty Add unit tests for tty module Merge pull request youki-dev#101 from Furisto/info-ex Extend info cmd with version and os Use `assert!` instead of `assert_eq!` when comparing a boolean. Merge pull request youki-dev#104 from utam0k/fix-clippy Use `assert!` instead of `assert_eq!` when comparing a boolean. Add support for systemd managed cgroups Merge pull request youki-dev#46 from nimrodshn/add_support_for_systemd_cgroups Add support for systemd managed cgroups update README.md Merge pull request youki-dev#105 from utam0k/update-README update README.md clean up comments Fix README.md Fedora & Centos instructions Merge pull request youki-dev#107 from nimrodshn/fix_readme Fix README.md Fedora & Centos instructions Add list command Merge pull request youki-dev#108 from Furisto/list-cmd Add list command fix conflicts. Merge pull request youki-dev#97 from utam0k/actions-cache improve build time in CI split the subcommands into their own files. Merge pull request youki-dev#110 from utam0k/refactor-cli-commands split the subcommands into their own files. Seperate adding tasks and applying resource restrictions Shorten names Update README.md Change `dnf` to `apt-get` for Debian based systems Address review comments - Add comments for functions - Use better naming in systemd cgroup manager Merge pull request youki-dev#112 from bkochendorfer/patch-1 Update README.md Merge pull request youki-dev#111 from Furisto/cg-add Seperate adding tasks to cgroups and applying resource restrictions Require only cgroups that are needed to fullfill the resource restrictions Merge pull request youki-dev#114 from Furisto/cg-only-required Require only requested cgroups to be present force delete container if it is running or created Merge pull request youki-dev#115 from TinySong/main force delete container if it is running or created add comments in intergration_test.sh about test case that runc no paas Merge pull request youki-dev#116 from TinySong/main add comments in intergration_test.sh about test case that runc no paas remove unnecessary clone() in create.rs Merge pull request youki-dev#117 from utam0k/refactor-create-remove-unnecessary-clone remove unnecessary clone() in create.rs Allow wider range of arguments for spec loading Rename command Provide context in case of errors during dir creation Ensure file info is captured Modularize create code add cgroup v2 pid controller Merge pull request youki-dev#119 from TinySong/main add cgroup v2 pids controller make String to signal conversion more user friendly by using a Trait. add tests for the signal. Split container builder into dedicated init and tenant builders The current monolithic builder provides options that should only be called during init and not when creating a tenant and vice versa. This puts the burden on the user of the builder to know which methods are safe to call. Now the ContainerBuilder can be used to specify options that are common to both scenarios and afterwards as_init/as_tenant can be called to provide scenario specific options. This also simplifies the whole "if init then else" branching logic during container build. Add documentation Remove tests Renaming Fix kill cmd test failures Execute doc tests Merge pull request youki-dev#122 from utam0k/refactor-signal make String to signal conversion more simplify by using a Trait. Review feedback and fmt Reduce binary size Merge pull request youki-dev#124 from Furisto/release Reduce size of binary Add cgroup v2 freezer controller Merge pull request youki-dev#123 from duduainankai/add-freezer-v2 Add cgroup v2 freezer controller Merge pull request youki-dev#121 from Furisto/builder Modularize container creation fix the warnings shown by cargo clippy support cgroupv2 io controller Merge pull request youki-dev#128 from TinySong/cgroupv2-io-controller Cgroupv2 io controller Merge pull request youki-dev#127 from utam0k/cargo-clippy-2 fix the warnings shown by cargo clippy Add code format check in CI format code to pass CI check Merge pull request youki-dev#129 from duduainankai/add-format-check-ci Add format check ci Fix spec path in delete Merge pull request youki-dev#130 from duduainankai/fix-path-in-delete Fix spec path in delete Document Capabilities and refactor its drop_privileges function Merge branch 'main' of github.com:containers/youki into comment-capabilities Fix same tmp dir in freezer v2 tests Merge pull request youki-dev#133 from duduainankai/fix-tmp-dir-in-test Fix same tmp dir in freezer v2 tests Merge pull request youki-dev#131 from YJDoc2/comment-capabilities Document capabilities rs and refactor its drop_privileges function cgroupsv2 hugetlb remove regex usage from hugetlb v2 use different temp dir for hugetlbv2 tests Document Info module Update doc-draft.md Merge pull request youki-dev#136 from YJDoc2/main Document Info module Document list module Merge pull request youki-dev#135 from 0xdco/cgroups-v2-hugetlb cgroupsv2 hugetlb Comment and modify Log module: Now the default log level is set according to type of compilation debug/production Merge branch 'main' of github.com:containers/youki into main Merge pull request youki-dev#137 from YJDoc2/main Document list and logger modules Implement exec command Support calling exec multiple times Fix test failure Remove todo Add comments Merge pull request youki-dev#138 from Furisto/exec Implement exec command Add pause and resume command conflicts Merge branch 'main' into add-spec-arg final issue split into multiple files add serde defaults to fields Merge pull request youki-dev#139 from duduainankai/support-pause-resume Add pause and resume command Merge branch 'main' into add-spec-arg

nimrodshn force-pushed the add_support_for_systemd_cgroups branch 2 times, most recently from 60e81cc to 9e17d5d Compare May 29, 2021 10:46

nimrodshn force-pushed the add_support_for_systemd_cgroups branch from 9e17d5d to e3f4c5f Compare May 29, 2021 10:48

nimrodshn commented May 29, 2021

View reviewed changes

src/create.rs Outdated Show resolved Hide resolved

nimrodshn force-pushed the add_support_for_systemd_cgroups branch 4 times, most recently from 00d7c7a to 52180d5 Compare June 1, 2021 19:20

nimrodshn force-pushed the add_support_for_systemd_cgroups branch 3 times, most recently from 79108b3 to 5998cde Compare June 2, 2021 06:39

nimrodshn force-pushed the add_support_for_systemd_cgroups branch 8 times, most recently from 987a312 to 27f7175 Compare June 5, 2021 09:53

nimrodshn force-pushed the add_support_for_systemd_cgroups branch 3 times, most recently from 701f2d9 to d51eaa5 Compare June 15, 2021 17:07

nimrodshn changed the title ~~[WIP] Add support for systemd managed cgroups~~ Add support for systemd managed cgroups Jun 16, 2021

utam0k reviewed Jun 16, 2021

View reviewed changes

src/cgroups/v2/systemd_manager.rs Outdated Show resolved Hide resolved

nimrodshn force-pushed the add_support_for_systemd_cgroups branch 2 times, most recently from 46edd7f to a42781d Compare June 16, 2021 16:59

Furisto reviewed Jun 16, 2021

View reviewed changes

src/cgroups/v2/systemd_manager.rs Show resolved Hide resolved

nimrodshn force-pushed the add_support_for_systemd_cgroups branch 4 times, most recently from f06827c to e50e73d Compare June 19, 2021 17:39

nimrodshn force-pushed the add_support_for_systemd_cgroups branch from e50e73d to 8c7aa11 Compare June 20, 2021 03:21

utam0k assigned Furisto and utam0k Jun 20, 2021

utam0k approved these changes Jun 20, 2021

View reviewed changes

nimrodshn force-pushed the add_support_for_systemd_cgroups branch from 8c7aa11 to aa867f9 Compare June 20, 2021 17:10

Furisto reviewed Jun 21, 2021

View reviewed changes

src/cgroups/v2/systemd_manager.rs Outdated Show resolved Hide resolved

Furisto reviewed Jun 21, 2021

View reviewed changes

src/cgroups/v2/systemd_manager.rs Outdated Show resolved Hide resolved

Furisto reviewed Jun 21, 2021

View reviewed changes

src/cgroups/v2/systemd_manager.rs Outdated Show resolved Hide resolved

Furisto reviewed Jun 21, 2021

View reviewed changes

src/cgroups/v2/systemd_manager.rs Outdated Show resolved Hide resolved

Furisto reviewed Jun 21, 2021

View reviewed changes

src/cgroups/v2/systemd_manager.rs Outdated Show resolved Hide resolved

Add support for systemd managed cgroups

8abb9f5

nimrodshn force-pushed the add_support_for_systemd_cgroups branch from aa867f9 to 8abb9f5 Compare June 22, 2021 06:34

Furisto merged commit 37bcf46 into youki-dev:main Jun 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for systemd managed cgroups #46

Add support for systemd managed cgroups #46

nimrodshn commented May 29, 2021 •

edited

Loading

utam0k commented May 29, 2021 •

edited

Loading

utam0k commented May 29, 2021

nimrodshn commented May 29, 2021

utam0k commented May 29, 2021

utam0k commented May 29, 2021

nimrodshn commented May 29, 2021 •

edited

Loading

utam0k commented May 29, 2021 •

edited

Loading

nimrodshn commented May 29, 2021 •

edited

Loading

utam0k commented Jun 2, 2021

nimrodshn commented Jun 2, 2021

utam0k commented Jun 2, 2021

nimrodshn commented Jun 19, 2021

utam0k left a comment

Furisto Jun 21, 2021

nimrodshn Jun 22, 2021

lucab Sep 28, 2021

Furisto Jun 21, 2021

nimrodshn Jun 22, 2021 •

edited

Loading

nimrodshn Jun 22, 2021 •

edited

Loading

Furisto Jun 22, 2021

nimrodshn Jun 22, 2021

Furisto commented Jun 22, 2021

Add support for systemd managed cgroups #46

Add support for systemd managed cgroups #46

Conversation

nimrodshn commented May 29, 2021 • edited Loading

What this does?

Why do this?

utam0k commented May 29, 2021 • edited Loading

utam0k commented May 29, 2021

nimrodshn commented May 29, 2021

utam0k commented May 29, 2021

utam0k commented May 29, 2021

nimrodshn commented May 29, 2021 • edited Loading

utam0k commented May 29, 2021 • edited Loading

nimrodshn commented May 29, 2021 • edited Loading

utam0k commented Jun 2, 2021

nimrodshn commented Jun 2, 2021

utam0k commented Jun 2, 2021

nimrodshn commented Jun 19, 2021

utam0k left a comment

Choose a reason for hiding this comment

Furisto Jun 21, 2021

Choose a reason for hiding this comment

nimrodshn Jun 22, 2021

Choose a reason for hiding this comment

lucab Sep 28, 2021

Choose a reason for hiding this comment

Furisto Jun 21, 2021

Choose a reason for hiding this comment

nimrodshn Jun 22, 2021 • edited Loading

Choose a reason for hiding this comment

nimrodshn Jun 22, 2021 • edited Loading

Choose a reason for hiding this comment

Furisto Jun 22, 2021

Choose a reason for hiding this comment

nimrodshn Jun 22, 2021

Choose a reason for hiding this comment

Furisto commented Jun 22, 2021

nimrodshn commented May 29, 2021 •

edited

Loading

utam0k commented May 29, 2021 •

edited

Loading

nimrodshn commented May 29, 2021 •

edited

Loading

utam0k commented May 29, 2021 •

edited

Loading

nimrodshn commented May 29, 2021 •

edited

Loading

nimrodshn Jun 22, 2021 •

edited

Loading

nimrodshn Jun 22, 2021 •

edited

Loading