Skip to content
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

memory cgv2 subsystem implemented #141

Merged
merged 2 commits into from
Jul 21, 2021

Conversation

tsturzl
Copy link
Collaborator

@tsturzl tsturzl commented Jul 16, 2021

After a lot of fumbling around with my weird local distro I finally was able to get cgroups v2 working properly to test this. I tested this by creating the container with youki directly(following the youki tutorial in the README) where I set the memory limits in the config.json directly. I was able to observe the values being set appropriately in the cgroup files.

Notice 1: I do not have systemd installed locally and I cannot get the systemd crate to build properly as I cannot get libsystemd installed locally without a lot of hassle. What I did was I made the systemd create an optional dependency and included it as a default feature, so for default builds things should work exactly the same for everyone else but this gives me the option of building and testing with the --no-default-options cargo flag. I implement the booted function from the crate inline with the only place it is used, which is the only use we have for this dependecy currently. The function will simply return an error indicating you are using a build that does not have this dependency. I think this will also be useful for cases where we want to create builds for platforms which are NOT using systemd, this would include Alpine Linux which is a very popular distro for small embedded devices and I think it's useful for youki to support these distros as systemd does not currently compile against alternative C libraries(eg musl) which are commonly used for these smaller embedded distros.

Notice 2: I had been away for a while and the history of my branch had lagged far behind that of the main branch. With that I decided to just reapply my changes over to a new branch which is directly branched off the main branch as of making this PR. I will therefore close PR #95 in favor of this PR.

@tsturzl tsturzl mentioned this pull request Jul 16, 2021
src/cgroups/v2/memory.rs Outdated Show resolved Hide resolved
src/cgroups/v2/memory.rs Outdated Show resolved Hide resolved
@tsturzl
Copy link
Collaborator Author

tsturzl commented Jul 19, 2021

requested changes have been made

@nimrodshn
Copy link
Contributor

nimrodshn commented Jul 21, 2021

LGTM 💯

@Furisto Furisto merged commit ffe2819 into containers:main Jul 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants