cgroups-rs icon indicating copy to clipboard operation
cgroups-rs copied to clipboard

Builder pattern for creating control groups

Open levex opened this issue 5 years ago • 5 comments

I've started working on an API that looks like this:

let cgroup: Cgroup = CgroupBuilder::new("hello", V1)
    .memory()
        .kernel_memory_limit(1024 * 1024)
        .memory_hard_limit(1024 * 1024)
        .done()
    .cpu()
        .shares(100)
        .done()
    .build();

Thoughts?

In particular, I'd prefer to leave out an API that adds a pid to the control group. That's racy and it might introduce subtle race conditions into applications depending on this crate. Instead, the API will likely include an include_command() build that starts the Command in the control group. This would be done via a trait ideally, so people can extend it.

levex avatar Oct 19 '18 22:10 levex

It does look a lot cleaner! One thing I've noticed about the current API is that if you only set up memory, but you try to add a PID to the Cgroup, it'll attempt to add to all types of cgroup, even the ones you haven't setup yet, which often fails. I think setting it up this way will be more intuitive!

You'll need a function for moving the current process into a cgroup as well.

I'd do something like this for spawning commands:

use std::process::Command;
use std::os::unix::process::CommandExt;

pub trait ComandCgroup {
    fn cgroup(&mut self, &Cgroup) -> &mut Self;
}

impl CommandCgroup for Command {
    fn cgroup(&mut self, cgroup: &Cgroup) -> &mut Self {
        self.before_exec(|| {
            unimplemented!()
        })
    }
}

tecywiz121 avatar Oct 21 '18 14:10 tecywiz121

Yeah, lazily creating the controllers is on my TODO list (which I really really should convert into Issues here).

Your suggestion for the trait seems great, I'll add that.

BTW, the current state-of-the-art for the builder pattern is on builder-pattern branch of this repo.

levex avatar Oct 21 '18 15:10 levex

@levex I would love to use the new API. Are there any roadblocks? Can I help with sorting them out?

frol avatar Dec 06 '18 21:12 frol

Hi @frol, great! No there aren't any. I just need to make a cargo release. I'll make one tomorrow, as I'm unable to do that from this machine.

levex avatar Dec 07 '18 23:12 levex

@levex I want to let you know that I ended up implementing my own cgroups-fs crate due to the following issues:

  • I could not attach a child process to the cgroup (you cannot move Cgroup without V1, and I don't actually want to move, but .before_exec captures them).
  • This crate parses/touches too many control files while I only need a few.

Here is what I, essentially, wanted (and built):

let my_cgroup = cgroups_fs::CgroupName::new("my-cgroup");
let my_memory_cgroup = cgroups_fs::AutomanagedCgroup::init(&my_cgroup, "memory").unwrap();

use cgroups_fs::CgroupsCommandExt;
let output = std::process::Command::new("echo")
    .arg("Hello world")
    .cgroups(&[&my_memory_cgroup])
    .output()
    .expect("Failed to execute command");

println!(
    "The echo process used {} bytes of RAM.",
    my_memory_cgroup.get_value::<u64>("memory.max_usage_in_bytes").unwrap()
);

frol avatar Dec 27 '18 00:12 frol