fix: handling of cgroup driver setting #864
Conversation
|
It looks like one of the test failed with the following error: Error: failed to stop unit youki-test.scope: dbus error: dbus function call error: Unit youki-test.scope not loaded.
Caused by:
failed to stop unit youki-test.scope: dbus error: dbus function call error: Unit youki-test.scope not loaded.
Stack backtrace:
0: anyhow::error::<impl core::convert::From<E> for anyhow::Error>::from
1: <core::result::Result<T,F> as core::ops::try_trait::FromResidual<core::result::Result<core::convert::Infallible,E>>>::from_residual
2: containerd_shim_wasm::testing::WasiTest<WasiInstance>::wait
3: containerd_shim_wasmedge::wasmedge_tests::test_seccomp::{{closure}}
4: core::ops::function::FnOnce::call_once
5: serial_test::serial_code_lock::local_serial_core_with_return
6: containerd_shim_wasmedge::wasmedge_tests::test_seccomp
7: containerd_shim_wasmedge::wasmedge_tests::test_seccomp::{{closure}}
8: core::ops::function::FnOnce::call_once
9: test::__rust_begin_short_backtrace
10: test::run_test::{{closure}}
11: std::sys::backtrace::__rust_begin_short_backtrace
12: core::ops::function::FnOnce::call_once{{vtable.shim}}
13: std::sys::pal::unix::thread::Thread::new::thread_start
test wasmedge_tests::test_seccomp ... FAILED
test wasmedge_tests::test_unreachable ... ok
failures:
failures:
wasmedge_tests::test_seccomp
|
z63d
force-pushed
the
fix/cgroup-driver
branch
from
79004b2 to
b7ba5e9
Compare
|
hmm... But other problems are occurring... |
test/k8s/Dockerfile
Outdated
| @@ -17,5 +17,5 @@ ADD dist/bin/* /usr/local/bin/ | |||
| ARG RUNTIME | |||
| RUN cat <<EOF >> /etc/containerd/config.toml | |||
| [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.wasm] | |||
| runtime_type = "io.containerd.${RUNTIME}.v1" | |||
| runtime_path = "io.containerd.${RUNTIME}.v1" | |||
There was a problem hiding this comment.
I didn't fully follow the discussion in Slack about this, but I would expect runtime_path to be a path, while this is not a path.
There was a problem hiding this comment.
Why is runtime_type a path in k3s but not in k8s? I am confused.
Should runtime_type always be in the form of io.containerd.${RUNTIME}.{version} for k8s and k3s?
z63d
force-pushed
the
fix/cgroup-driver
branch
from
1a6b2a1 to
4665961
Compare
| // Deserialization in the following way doesn't work. | ||
| // https://github.com/containerd/rust-extensions/blob/shim-v0.8.0/crates/runc-shim/src/runc.rs#L85-L88 | ||
| // Because req.options gives us the following string: "\u{1a}\u{15}SystemdCgroup = true\n" | ||
| // So use regex to work around this issue. | ||
| fn parse_systemd_cgroup(input: &str) -> Option<bool> { | ||
| let regex: Regex = Regex::new(r#"\u{1a}\u{15}SystemdCgroup = (true|false)\n"#).unwrap(); | ||
| if let Some(captures) = regex.captures(input) { | ||
| if let Some(value_match) = captures.get(1) { | ||
| let value_str = value_match.as_str(); | ||
| return Some(value_str.parse().unwrap_or(true)); | ||
| } | ||
| } | ||
| None | ||
| } | ||
| let systemd_cgroup = match req.options.as_ref() { | ||
| Some(any) => match String::from_utf8(any.value.to_vec()) { | ||
| Ok(opts_str) => parse_systemd_cgroup(opts_str.as_str()).unwrap_or(true), | ||
| Err(_) => true, | ||
| }, | ||
| None => true, | ||
| }; | ||
|
|
There was a problem hiding this comment.
Would you mind trying this?
It needs 2 new dependencies
prost = "0.13"
toml = "0.8" #[derive(Deserialize, Default, Clone, PartialEq)]
struct Config {
#[serde(alias = "SystemdCgroup")]
systemd_cgroup: bool,
}
fn get_config(options: Option<&Any>) -> anyhow::Result<Config> {
let Some(opts) = options else {
return Ok(Default::default());
};
ensure!(
opts.type_url == "runtimeoptions.v1.Options",
"Invalid options type {}",
opts.type_url
);
#[derive(Message, Clone, PartialEq)]
struct Options {
#[prost(string)]
type_url: String,
#[prost(string)]
config_path: String,
#[prost(string)]
config_body: String,
}
let opts = Options::decode(opts.value.as_slice())?;
let config = toml::from_str(opts.config_body.as_str()).map_err(|err| {
Error::InvalidArgument(format!("invalid shim options: {err}"))
})?;
Ok(config)
}
let config = get_config(req.options.as_ref()).map_err(|err| {
Error::InvalidArgument(format!("invalid shim options: {err}"))
})?;
let systemd_cgroup = config.systemd_cgroup;There was a problem hiding this comment.
This is great. Works perfectly!
Would it be better to separate theOptions and Config logic into different files?
z63d
force-pushed
the
fix/cgroup-driver
branch
3 times, most recently
from
4665961 to
79bebfc
Compare
z63d
force-pushed
the
fix/cgroup-driver
branch
from
79bebfc to
4764514
Compare
|
This error occurs randomly. Probably this happens too.
|
|
I am confused. From what you observe, it looks like the options have indeed But it also looks like the runc shim is not expecting an option like that, but rather a I don't understand why we receive different type of options, or how to tell to containerd the type of protobuf message that we are expecting (i.e., our own Also it looks like containerd sends us the @cpuguy83 are you familiar with any of this? |
| impl Default for Config { | ||
| fn default() -> Self { | ||
| Self { | ||
| systemd_cgroup: true, |
There was a problem hiding this comment.
IIUC what runc is doing, I think this should default to false.
Also, I think defaulting this to false would fix a few of the CI failures.
There was a problem hiding this comment.
If you set it to false, most k8s clusters will have metrics bugs unles explicitly specify SystemdCgroup = true in containerd/config.toml.
Is this OK?
Also, I think defaulting this to false would fix a few of the CI failures.
I think the kind problem will be solved.
But when using systemd cgroups, kind pod doesn't seem to work.
As you can see below, kind supports systemd cgroups. It's strange that it doesn't work.
#356 (comment)
There was a problem hiding this comment.
Is this OK?
IIUC, that would be the same behaviour as runc, so it´s fine with me.
There was a problem hiding this comment.
Also, I think it's better to err on the side of not using systemd.
If we wrongly DON'T use systemd, the metrincs don't work.
If we wrongly DO use systemd, the container doesn't even start.
z63d
force-pushed
the
fix/cgroup-driver
branch
4 times, most recently
from
660c53b to
9191061
Compare
@jprendes This is due to this code: https://github.com/containerd/containerd/blob/6f652853f01ef9ba340a860c2f39edf1701102d1/internal/cri/config/config.go#L658-L667 The runwasi shim is not of type |
z63d
force-pushed
the
fix/cgroup-driver
branch
from
9191061 to
cb21dc5
Compare
Signed-off-by: Kaita Nakamura <kaita.nakamura0830@gmail.com>
z63d
force-pushed
the
fix/cgroup-driver
branch
from
cb21dc5 to
8b2e146
Compare
|
@Mossaka @jprendes |
fix: #821
Fixes an issue where cgroupfs was always used as the cgroup driver.
Receives SystemdCgroup from containerd and allows you to configure the cgroup driver. (There is a deserialize issue, so we'll use a regular expression to work around it.)
Uses systemd as the default cgroup driver.