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

Support for cgroups v2 #32

Closed
utam0k opened this issue May 24, 2021 · 16 comments
Closed

Support for cgroups v2 #32

utam0k opened this issue May 24, 2021 · 16 comments
Assignees
Labels
help wanted Extra attention is needed
Milestone

Comments

@utam0k
Copy link
Member

utam0k commented May 24, 2021

I am looking into supporting cgroups v2.
However, I'm not an expert, so I'm looking for what I need, useful documentation, design suggestions, etc.
Also, if anyone would like to challenge me, I welcome you to challenge me with me.

References

@utam0k utam0k added the help wanted Extra attention is needed label May 24, 2021
@Furisto
Copy link
Member

Furisto commented May 24, 2021

Sure, would be happy to help. How do you want to proceed @utam0k ? I thought about getting through the references and then attempting a spike of the cgroup v2 manager.

@utam0k
Copy link
Member Author

utam0k commented May 24, 2021

@Furisto
It is a very good idea as a first step. I think an eventual implementation like the crun about it would be nice.
https://github.com/containers/crun/blob/master/crun.1.md#cgroup-v2

I thought about getting through the references and then attempting a spike of the cgroup v2 manager.

I'd love to hear other people's opinions as well as mine, so please let me know what you think.
If you are interested, you can try to implement a cgroups v2 manager?

@Furisto
Copy link
Member

Furisto commented May 25, 2021

Progress can be seen here: https://github.com/Furisto/youki/tree/cgroupv2. I am currently fleshing out the cgroups v2 manager and 1-2 subsystems.

@utam0k
Copy link
Member Author

utam0k commented May 25, 2021

@Furisto
I took a look at your commit. As a first step it was exactly what I imagined. I think it's a good policy.
Can I assign this problem to you?
If it looks like it needs to be handled by a variety of controllers, I think it would be a good idea to provide an example like in v1 to give others a chance to try.

@Furisto
Copy link
Member

Furisto commented May 25, 2021

@utam0k
You can assign it to me. Yes, the idea is to do the first one or two myself and then the rest can be done by other people.

@utam0k
Copy link
Member Author

utam0k commented May 25, 2021

@Furisto I assigned you. I look forward to your PR!

@utam0k utam0k added this to the First release milestone May 27, 2021
@utam0k utam0k self-assigned this May 27, 2021
@utam0k
Copy link
Member Author

utam0k commented May 27, 2021

@Furisto cc: @tsturzl
I decided to make cgroups v2 support a key feature for the first release of youki.
I'm looking forward to it.

@Furisto
Copy link
Member

Furisto commented May 27, 2021

@utam0k @tsturzl
I am planning to have a PR ready the day after tomorrow

@utam0k
Copy link
Member Author

utam0k commented May 27, 2021

@Furisto Wow! I am amazed at how fast you work.

@utam0k
Copy link
Member Author

utam0k commented Jun 1, 2021

@Furisto
Thanks for your hard work in supporting v2. It was very nice work!

BTW, I'd like to talk to you about how to proceed with v2 support for each subsystem.
I think it is better to proceed with the project in the same way as utam0k#9.
What do you think about it?

@Furisto
Copy link
Member

Furisto commented Jun 1, 2021

@utam0k
I think it is a good idea. I am investigating one open issue, once that is solved we can split up the work for the different subsystems. The issue is that the cgroup manager gets the pid of youki and not of the container process. If i do this

sudo docker run -d --rm -m 30m --runtime youki progrium/stress --cpu 2

then the process tree looks like this:

root       13519       1  0 00:19 ?        00:00:00 /usr/bin/containerd-shim-runc-v2 -namespace moby -id 509e2e3131d50c63d599a
root       13530   13519  0 00:19 pts/24   00:00:00  \_ /home/furisto/Projects/Existing/youki-me/target/x86_64-unknown-linux-gnu/d
root       13531   13530  0 00:19 pts/24   00:00:00      \_ /usr/bin/stress --verbose --cpu 2 
root       13538   13531 99 00:19 pts/24   00:00:03          \_ /usr/bin/stress --verbose --cpu 2 
root       13539   13531 99 00:19 pts/24   00:00:03          \_ /usr/bin/stress --verbose --cpu 2 

The cgroup manager gets pid 13530, not 13531. We also seem to call apply too late during create, because 13538, 13539 and 13531 end up in the containerd.slice not in the cgroup created by youki. That must mean that when these processes are forked their parent is not yet in the correct cgroup.

@utam0k
Copy link
Member Author

utam0k commented Jun 2, 2021

@Furisto
Maybe applying cmanager in fork_init() instead of fork_first() will fix it.
https://github.com/utam0k/youki/blob/52b9a15d5168b590770d5aa79f9cebdee04d7604/src/process/fork.rs#L56

The flow in this image is old, and some of it may be wrong, but it's about right.
image

@utam0k
Copy link
Member Author

utam0k commented Jun 9, 2021

@Furisto Hi! I think the problem mentioned above was solved by #69. So Can I create a tracking issue to implement some subsystems for cgv2?

@Furisto
Copy link
Member

Furisto commented Jun 9, 2021

@utam0k Yes, you can!

@utam0k
Copy link
Member Author

utam0k commented Jun 9, 2021

I created #78.
Let's continue with cgv2 support in this issue!

@utam0k
Copy link
Member Author

utam0k commented Jun 9, 2021

Special thanks to @Furisto

@utam0k utam0k closed this as completed Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants