From 407b883106c062973f7c91db3704c3dd61392d4c Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Mon, 12 Jun 2023 17:57:56 +0000 Subject: [PATCH 01/29] Update youki to the latest main branch and oci-spec to 0.6.1. Since youki now uses thiserror instead of anyhow, this PR also updates the containerd-shim-wasmedge to properly map the error. Signed-off-by: jiaxiao zhou --- Cargo.lock | 122 ++++++------------ Cargo.toml | 4 +- .../containerd-shim-wasmedge/src/executor.rs | 33 +++-- .../containerd-shim-wasmedge/src/instance.rs | 49 ++++--- 4 files changed, 96 insertions(+), 112 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5f68c02fe..692613564 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -333,11 +333,8 @@ checksum = "ec837a71355b28f6556dbd569b37b3f363091c0bd4b2e735674521b4c5fd9bc5" dependencies = [ "android-tzdata", "iana-time-zone", - "js-sys", "num-traits", "serde", - "time 0.1.45", - "wasm-bindgen", "winapi", ] @@ -476,7 +473,7 @@ dependencies = [ "serde_json", "signal-hook", "thiserror", - "time 0.3.22", + "time", "uuid 0.8.2", ] @@ -491,7 +488,7 @@ dependencies = [ "containerd-shim-wasmtime", "criterion", "libc", - "oci-spec 0.6.0", + "oci-spec 0.6.1", "serde", "serde_json", "tempfile", @@ -521,7 +518,7 @@ dependencies = [ "libc", "log", "nix 0.26.2", - "oci-spec 0.6.0", + "oci-spec 0.6.1", "pretty_assertions", "proc-mounts", "protobuf 2.28.0", @@ -548,7 +545,7 @@ dependencies = [ "libcontainer", "log", "nix 0.26.2", - "oci-spec 0.6.0", + "oci-spec 0.6.1", "pretty_assertions", "serde", "serde_json", @@ -571,7 +568,7 @@ dependencies = [ "libc", "log", "nix 0.26.2", - "oci-spec 0.6.0", + "oci-spec 0.6.1", "pretty_assertions", "serde_json", "tempfile", @@ -1075,6 +1072,12 @@ dependencies = [ "instant", ] +[[package]] +name = "fastrand" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6999dc1837253364c2ebb0704ba97994bd874e8f195d665c50b7548f6ea92764" + [[package]] name = "fd-lock" version = "3.0.12" @@ -1286,7 +1289,7 @@ checksum = "be4136b2a15dd319360be1c07d9933517ccf0be8f16bf62a3bee4f0d618df427" dependencies = [ "cfg-if 1.0.0", "libc", - "wasi 0.11.0+wasi-snapshot-preview1", + "wasi", ] [[package]] @@ -1568,45 +1571,44 @@ checksum = "f92be4933c13fd498862a9e02a3055f8a8d9c039ce33db97306fd5a6caa7f29b" [[package]] name = "libcgroups" version = "0.0.5" -source = "git+https://github.com/containers/youki?rev=edd63c84f903aa09510ef758ea6f617e0cb8b7e1#edd63c84f903aa09510ef758ea6f617e0cb8b7e1" +source = "git+https://github.com/containers/youki?rev=1a6d1f4bd7553e971d6d787698a9732836188444#1a6d1f4bd7553e971d6d787698a9732836188444" dependencies = [ - "anyhow", "dbus", "fixedbitset 0.4.2", - "log", "nix 0.26.2", - "oci-spec 0.6.0", + "oci-spec 0.6.1", "procfs", "serde", + "thiserror", + "tracing", ] [[package]] name = "libcontainer" version = "0.0.5" -source = "git+https://github.com/containers/youki?rev=edd63c84f903aa09510ef758ea6f617e0cb8b7e1#edd63c84f903aa09510ef758ea6f617e0cb8b7e1" +source = "git+https://github.com/containers/youki?rev=1a6d1f4bd7553e971d6d787698a9732836188444#1a6d1f4bd7553e971d6d787698a9732836188444" dependencies = [ - "anyhow", "bitflags 2.3.1", "caps", "chrono", "clone3", - "crossbeam-channel", - "fastrand", + "fastrand 2.0.0", "futures", "libc", "libcgroups", "libseccomp", - "log", - "mio", "nix 0.26.2", - "oci-spec 0.6.0", - "path-clean", + "oci-spec 0.6.1", + "once_cell", "prctl", "procfs", + "regex", "rust-criu", + "safe-path", "serde", "serde_json", - "syscalls", + "thiserror", + "tracing", ] [[package]] @@ -1746,18 +1748,6 @@ dependencies = [ "adler", ] -[[package]] -name = "mio" -version = "0.8.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "927a765cd3fc26206e66b296465fa9d3e5ab003e651c1b3c060e7956d96b19d2" -dependencies = [ - "libc", - "log", - "wasi 0.11.0+wasi-snapshot-preview1", - "windows-sys 0.48.0", -] - [[package]] name = "multimap" version = "0.8.3" @@ -1871,9 +1861,9 @@ dependencies = [ [[package]] name = "oci-spec" -version = "0.6.0" +version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "214b837f7dde5026f2028ead5ae720073277c19f82ff85623b142c39d4b843e7" +checksum = "cf77d2eace1f4909b081d231d1ad3570ba3448ae95290ab701314faaee1b611a" dependencies = [ "derive_builder 0.12.0", "getset", @@ -1890,7 +1880,7 @@ dependencies = [ "clap", "env_logger", "log", - "oci-spec 0.6.0", + "oci-spec 0.6.1", "serde", "serde_json", "sha256", @@ -1956,12 +1946,6 @@ version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f746c4065a8fa3fe23974dd82f15431cc8d40779821001404d10d2e79ca7d79" -[[package]] -name = "path-clean" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17359afc20d7ab31fdb42bb844c8b3bb1dabd7dcf7e68428492da7f16966fcef" - [[package]] name = "peeking_take_while" version = "0.1.2" @@ -2450,6 +2434,15 @@ version = "1.0.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f91339c0467de62360649f8d3e185ca8de4224ff281f66000de5eb2a77a79041" +[[package]] +name = "safe-path" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "980abdd3220aa19b67ca3ea07b173ca36383f18ae48cde696d90c8af39447ffb" +dependencies = [ + "libc", +] + [[package]] name = "same-file" version = "1.0.6" @@ -2496,17 +2489,6 @@ dependencies = [ "serde", ] -[[package]] -name = "serde_repr" -version = "0.1.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bcec881020c684085e55a25f7fd888954d56609ef363479dc5a1305eb0d40cab" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.18", -] - [[package]] name = "serial_test" version = "2.0.0" @@ -2648,17 +2630,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "syscalls" -version = "0.6.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41a292bad4a4fdbab06bb5399d2d578e4afed89e6701b47a75cf952d510c1473" -dependencies = [ - "cc", - "serde", - "serde_repr", -] - [[package]] name = "system-interface" version = "0.25.7" @@ -2700,7 +2671,7 @@ checksum = "31c0432476357e58790aaa47a8efb0c5138f137343f3b5f23bd36a27e3b0a6d6" dependencies = [ "autocfg", "cfg-if 1.0.0", - "fastrand", + "fastrand 1.9.0", "redox_syscall 0.3.5", "rustix 0.37.19", "windows-sys 0.48.0", @@ -2735,17 +2706,6 @@ dependencies = [ "syn 2.0.18", ] -[[package]] -name = "time" -version = "0.1.45" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b797afad3f312d1c66a56d11d0316f916356d11bd158fbc6ca6389ff6bf805a" -dependencies = [ - "libc", - "wasi 0.10.0+wasi-snapshot-preview1", - "winapi", -] - [[package]] name = "time" version = "0.3.22" @@ -2994,12 +2954,6 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "wasi" -version = "0.10.0+wasi-snapshot-preview1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a143597ca7c7793eff794def352d41792a93c481eb1042423ff7ff72ba2c31f" - [[package]] name = "wasi" version = "0.11.0+wasi-snapshot-preview1" @@ -3057,7 +3011,7 @@ dependencies = [ "anyhow", "env_logger", "log", - "oci-spec 0.6.0", + "oci-spec 0.6.1", "oci-tar-builder", "sha256", "tar", diff --git a/Cargo.toml b/Cargo.toml index af4168f30..457628a1c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,10 +30,10 @@ nix = "0.26" cap-std = "1.0" thiserror = "1.0" libc = "0.2.145" -oci-spec = { version = "0.6.0", features = ["runtime"] } +oci-spec = { version = "0.6.1", features = ["runtime"] } sha256 = "1.1" # TODO: Wait for the release and use the crates.io one. -libcontainer = { git = "https://github.com/containers/youki", rev = "edd63c84f903aa09510ef758ea6f617e0cb8b7e1" } +libcontainer = { git = "https://github.com/containers/youki", rev = "1a6d1f4bd7553e971d6d787698a9732836188444" } [profile.release] panic = "abort" diff --git a/crates/containerd-shim-wasmedge/src/executor.rs b/crates/containerd-shim-wasmedge/src/executor.rs index 169e11095..3d3318255 100644 --- a/crates/containerd-shim-wasmedge/src/executor.rs +++ b/crates/containerd-shim-wasmedge/src/executor.rs @@ -1,8 +1,8 @@ -use anyhow::{bail, Result}; +use anyhow::Result; use oci_spec::runtime::Spec; use libc::{dup, dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; -use libcontainer::workload::Executor; +use libcontainer::workload::{Executor, ExecutorError}; use std::os::unix::io::RawFd; use wasmedge_sdk::{ @@ -23,11 +23,11 @@ pub struct WasmEdgeExecutor { } impl Executor for WasmEdgeExecutor { - fn exec(&self, spec: &Spec) -> Result<()> { + fn exec(&self, spec: &Spec) -> Result<(), ExecutorError> { // parse wasi parameters let args = get_args(spec); if args.is_empty() { - bail!("args should not be empty") + return Err(ExecutorError::InvalidArg); } let mut cmd = args[0].clone(); @@ -39,22 +39,35 @@ impl Executor for WasmEdgeExecutor { // create configuration with `wasi` option enabled let config = ConfigBuilder::new(CommonConfigOptions::default()) .with_host_registration_config(HostRegistrationConfigOptions::default().wasi(true)) - .build()?; + .build() + .map_err(|err| { + ExecutorError::Other(format!("failed to create wasmedge config: {}", err)) + })?; // create a vm with the config settings - let mut vm = VmBuilder::new().with_config(config).build()?; + let mut vm = VmBuilder::new() + .with_config(config) + .build() + .map_err(|err| { + ExecutorError::Other(format!("failed to create wasmedge vm: {}", err)) + })?; // initialize the wasi module with the parsed parameters let wasi_module = vm .wasi_module_mut() - .ok_or_else(|| anyhow::Error::msg("Not found wasi module"))?; + .ok_or_else(|| ExecutorError::Other("Not found wasi module".into()))?; wasi_module.initialize( Some(args.iter().map(|s| s as &str).collect()), Some(envs.iter().map(|s| s as &str).collect()), None, ); - let vm = vm.register_module_from_file("main", cmd)?; + let vm = vm.register_module_from_file("main", cmd).map_err(|err| { + ExecutorError::Other(format!( + "failed to register wasmedge module from the file: {}", + err + )) + })?; if let Some(stdin) = self.stdin { unsafe { @@ -83,8 +96,8 @@ impl Executor for WasmEdgeExecutor { }; } - fn can_handle(&self, _spec: &Spec) -> Result { - Ok(true) + fn can_handle(&self, _spec: &Spec) -> bool { + true } fn name(&self) -> &'static str { diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index 457b53778..ed92e4d9a 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -5,7 +5,7 @@ use std::os::unix::io::{IntoRawFd, RawFd}; use std::sync::{Arc, Condvar, Mutex}; use std::thread; -use anyhow::{bail, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; use chrono::{DateTime, Utc}; use containerd_shim_wasm::sandbox::error::Error; use containerd_shim_wasm::sandbox::instance::Wait; @@ -217,23 +217,12 @@ impl Instance for Wasi { fn start(&self) -> Result { debug!("preparing module"); - let syscall = create_syscall(); - fs::create_dir_all(&self.rootdir)?; let stdin = maybe_open_stdio(self.stdin.as_str()).context("could not open stdin")?; let stdout = maybe_open_stdio(self.stdout.as_str()).context("could not open stdout")?; let stderr = maybe_open_stdio(self.stderr.as_str()).context("could not open stderr")?; - let mut container = ContainerBuilder::new(self.id.clone(), syscall.as_ref()) - .with_executor(vec![Box::new(WasmEdgeExecutor { - stdin, - stdout, - stderr, - })])? - .with_root_path(self.rootdir.clone())? - .as_init(&self.bundle) - .with_systemd(false) - .build()?; + let mut container = self.build_container(stdin, stdout, stderr)?; let code = self.exit_code.clone(); let pid = container.pid().unwrap(); @@ -244,7 +233,9 @@ impl Instance for Wasi { stdout.map(close); stderr.map(close); - container.start()?; + container + .start() + .map_err(|err| Error::Any(anyhow!("failed to start container: {}", err)))?; thread::spawn(move || { let (lock, cvar) = &*code; @@ -278,7 +269,9 @@ impl Instance for Wasi { } let mut container = load_container(&self.rootdir, self.id.as_str())?; - match container.kill(Signal::try_from(signal as i32)?, true) { + let signal = Signal::try_from(signal as i32) + .map_err(|err| Error::InvalidArgument(format!("invalid signal number: {}", err)))?; + match container.kill(signal, true) { Ok(_) => Ok(()), Err(e) => { if container.status() == ContainerStatus::Stopped { @@ -302,7 +295,9 @@ impl Instance for Wasi { } } match load_container(&self.rootdir, self.id.as_str()) { - Ok(mut container) => container.delete(true)?, + Ok(mut container) => container.delete(true).map_err(|err| { + Error::Any(anyhow!("failed to delete container {}: {}", self.id, err)) + })?, Err(err) => { error!("could not find the container, skipping cleanup: {}", err); return Ok(()); @@ -318,6 +313,28 @@ impl Instance for Wasi { } } +impl Wasi { + fn build_container( + &self, + stdin: Option, + stdout: Option, + stderr: Option, + ) -> anyhow::Result { + let syscall = create_syscall(); + let container = ContainerBuilder::new(self.id.clone(), syscall.as_ref()) + .with_executor(vec![Box::new(WasmEdgeExecutor { + stdin, + stdout, + stderr, + })])? + .with_root_path(self.rootdir.clone())? + .as_init(&self.bundle) + .with_systemd(false) + .build()?; + Ok(container) + } +} + #[cfg(test)] mod wasitest { use std::borrow::Cow; From c555e3f94aed737cf556e293a7b1da6dfaf707b2 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Tue, 13 Jun 2023 22:16:14 +0000 Subject: [PATCH 02/29] Removed some uses of unsafe in wasmedge shim Signed-off-by: jiaxiao zhou --- .../containerd-shim-wasmedge/src/executor.rs | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/crates/containerd-shim-wasmedge/src/executor.rs b/crates/containerd-shim-wasmedge/src/executor.rs index ffe20acac..1158271bc 100644 --- a/crates/containerd-shim-wasmedge/src/executor.rs +++ b/crates/containerd-shim-wasmedge/src/executor.rs @@ -1,7 +1,8 @@ use anyhow::Result; +use nix::unistd::{dup, dup2}; use oci_spec::runtime::Spec; -use libc::{dup, dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; +use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use libcontainer::workload::{Executor, ExecutorError}; use std::os::unix::io::RawFd; @@ -10,10 +11,6 @@ use wasmedge_sdk::{ params, VmBuilder, }; -static mut STDIN_FD: Option = None; -static mut STDOUT_FD: Option = None; -static mut STDERR_FD: Option = None; - const EXECUTOR_NAME: &str = "wasmedge"; pub struct WasmEdgeExecutor { @@ -65,14 +62,16 @@ impl Executor for WasmEdgeExecutor { .map_err(|err| ExecutorError::Execution(err))?; if let Some(stdin) = self.stdin { - unsafe { - STDIN_FD = Some(dup(STDIN_FILENO)); - } + let _ = dup(STDIN_FILENO); + let _ = dup2(stdin, STDIN_FILENO); + } + if let Some(stdout) = self.stdout { + let _ = dup(STDOUT_FILENO); + let _ = dup2(stdout, STDOUT_FILENO); } if let Some(stderr) = self.stderr { - unsafe { - dup2(stderr, STDERR_FILENO); - } + let _ = dup(STDERR_FILENO); + let _ = dup2(stderr, STDERR_FILENO); } // TODO: How to get exit code? From fc1bb2e02d83d2c6599a87d89b0cfadf91866087 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Tue, 13 Jun 2023 23:46:54 +0000 Subject: [PATCH 03/29] Use youki's libcontainer APIs to implement wasmtime shim. - Implement Executor trait for wasmtime executor. - Use libcontainer's lifecycle APIs to implement runwasi's Instance trait. Signed-off-by: jiaxiao zhou --- Cargo.lock | 2 + .../containerd-shim-wasmedge/src/executor.rs | 15 +- crates/containerd-shim-wasmtime/Cargo.toml | 3 + .../containerd-shim-wasmtime/src/executor.rs | 133 ++++++ .../containerd-shim-wasmtime/src/instance.rs | 392 +++++++++--------- crates/containerd-shim-wasmtime/src/lib.rs | 1 + 6 files changed, 340 insertions(+), 206 deletions(-) create mode 100644 crates/containerd-shim-wasmtime/src/executor.rs diff --git a/Cargo.lock b/Cargo.lock index 6788875c0..11aa4bd16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -566,10 +566,12 @@ dependencies = [ "containerd-shim", "containerd-shim-wasm", "libc", + "libcontainer", "log", "nix 0.26.2", "oci-spec 0.6.1", "pretty_assertions", + "serde", "serde_json", "tempfile", "thiserror", diff --git a/crates/containerd-shim-wasmedge/src/executor.rs b/crates/containerd-shim-wasmedge/src/executor.rs index 1158271bc..5bbac9663 100644 --- a/crates/containerd-shim-wasmedge/src/executor.rs +++ b/crates/containerd-shim-wasmedge/src/executor.rs @@ -1,4 +1,5 @@ use anyhow::Result; +use containerd_shim_wasm::sandbox::oci; use nix::unistd::{dup, dup2}; use oci_spec::runtime::Spec; @@ -22,7 +23,7 @@ pub struct WasmEdgeExecutor { impl Executor for WasmEdgeExecutor { fn exec(&self, spec: &Spec) -> Result<(), ExecutorError> { // parse wasi parameters - let args = get_args(spec); + let args = oci::get_args(spec); if args.is_empty() { return Err(ExecutorError::InvalidArg); } @@ -91,18 +92,6 @@ impl Executor for WasmEdgeExecutor { } } -fn get_args(spec: &Spec) -> &[String] { - let p = match spec.process() { - None => return &[], - Some(p) => p, - }; - - match p.args() { - None => &[], - Some(args) => args.as_slice(), - } -} - fn env_to_wasi(spec: &Spec) -> Vec { let default = vec![]; let env = spec diff --git a/crates/containerd-shim-wasmtime/Cargo.toml b/crates/containerd-shim-wasmtime/Cargo.toml index 5222fb453..028a3532f 100644 --- a/crates/containerd-shim-wasmtime/Cargo.toml +++ b/crates/containerd-shim-wasmtime/Cargo.toml @@ -33,6 +33,9 @@ oci-spec = { workspace = true, features = ["runtime"] } thiserror = { workspace = true } serde_json = { workspace = true } nix = { workspace = true } +libcontainer = { workspace = true } +serde = { workspace = true } +libc = { workspace = true } [dev-dependencies] tempfile = "3.0" diff --git a/crates/containerd-shim-wasmtime/src/executor.rs b/crates/containerd-shim-wasmtime/src/executor.rs new file mode 100644 index 000000000..fce5049fb --- /dev/null +++ b/crates/containerd-shim-wasmtime/src/executor.rs @@ -0,0 +1,133 @@ +use std::{fs::OpenOptions, os::fd::RawFd, path::PathBuf}; + +use anyhow::{anyhow, Result}; +use containerd_shim_wasm::sandbox::oci; +use libc::{dup, dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; +use libcontainer::workload::{Executor, ExecutorError}; +use oci_spec::runtime::Spec; + +use wasmtime::{Engine, Linker, Module, Store}; +use wasmtime_wasi::WasiCtxBuilder; + +use crate::oci_wasmtime::{self, wasi_dir, wasi_file}; + +const EXECUTOR_NAME: &str = "wasmtime"; + +static mut STDIN_FD: Option = None; +static mut STDOUT_FD: Option = None; +static mut STDERR_FD: Option = None; + +pub struct WasmtimeExecutor { + pub stdin: Option, + pub stdout: Option, + pub stderr: Option, + pub engine: Engine, +} + +impl Executor for WasmtimeExecutor { + fn exec(&self, spec: &Spec) -> Result<(), ExecutorError> { + let args = oci::get_args(spec); + if args.is_empty() { + return Err(ExecutorError::InvalidArg); + } + + let (mut store, f) = self + .prepare_function(spec, args) + .map_err(|err| ExecutorError::Other(format!("failed to prepare function: {}", err)))?; + + log::info!("calling start function"); + match f.call(&mut store, &[], &mut []) { + Ok(_) => std::process::exit(0), + Err(_) => std::process::exit(137), + }; + } + + fn can_handle(&self, _spec: &Spec) -> bool { + true + } + + fn name(&self) -> &'static str { + EXECUTOR_NAME + } +} + +impl WasmtimeExecutor { + fn prepare_function( + &self, + spec: &Spec, + args: &[String], + ) -> anyhow::Result<(Store, wasmtime::Func)> { + // already in the cgroup + let env = oci_wasmtime::env_to_wasi(spec); + log::info!("setting up wasi"); + + let path = wasi_dir(".", OpenOptions::new().read(true))?; + let mut wasi_builder = WasiCtxBuilder::new() + .args(args)? + .envs(env.as_slice())? + .preopened_dir(path, "/")?; + + if let Some(stdin) = self.stdin { + unsafe { + STDIN_FD = Some(dup(STDIN_FILENO)); + dup2(stdin, STDIN_FILENO); + } + } + if let Some(stdout) = self.stdout { + unsafe { + STDOUT_FD = Some(dup(STDOUT_FILENO)); + dup2(stdout, STDOUT_FILENO); + } + } + if let Some(stderr) = self.stderr { + unsafe { + STDERR_FD = Some(dup(STDERR_FILENO)); + dup2(stderr, STDERR_FILENO); + } + } + log::info!("opening stdin"); + let stdin_path = PathBuf::from("/dev/stdin"); + let stdin_wasi_file = wasi_file(stdin_path, OpenOptions::new().read(true))?; + wasi_builder = wasi_builder.stdin(Box::new(stdin_wasi_file)); + + log::info!("opening stdout"); + let stdout_path = PathBuf::from("/dev/stdout"); + let stdout_wasi_file = wasi_file(stdout_path, OpenOptions::new().write(true))?; + wasi_builder = wasi_builder.stdout(Box::new(stdout_wasi_file)); + + log::info!("opening stderr"); + let stderr_path = PathBuf::from("/dev/stderr"); + let stderr_wasi_file = wasi_file(stderr_path, OpenOptions::new().write(true))?; + wasi_builder = wasi_builder.stderr(Box::new(stderr_wasi_file)); + + log::info!("building wasi context"); + let wctx = wasi_builder.build(); + + log::info!("wasi context ready"); + let start = args[0].clone(); + let mut iterator = start.split('#'); + let mut cmd = iterator.next().unwrap().to_string(); + let stripped = cmd.strip_prefix(std::path::MAIN_SEPARATOR); + if let Some(strpd) = stripped { + cmd = strpd.to_string(); + } + let method = iterator.next().unwrap_or("_start"); + let mod_path = cmd; + + log::info!("loading module from file"); + let module = Module::from_file(&self.engine, mod_path)?; + let mut linker = Linker::new(&self.engine); + + wasmtime_wasi::add_to_linker(&mut linker, |s| s)?; + let mut store = Store::new(&self.engine, wctx); + + log::info!("instantiating instance"); + let i = linker.instantiate(&mut store, &module)?; + + log::info!("getting start function"); + let f = i + .get_func(&mut store, method) + .ok_or_else(|| anyhow!("module does not have a wasi start function".to_string()))?; + Ok((store, f)) + } +} diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index 4fbaa7c9a..474d47670 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -1,26 +1,34 @@ -use std::fs::OpenOptions; -use std::path::Path; +use anyhow::{bail, Result}; +use libcontainer::container::builder::ContainerBuilder; +use libcontainer::container::{Container, ContainerStatus}; +use nix::errno::Errno; +use nix::sys::wait::waitid; +use std::fs::{self, OpenOptions}; +use std::io::ErrorKind; +use std::os::fd::{IntoRawFd, RawFd}; +use std::path::{Path, PathBuf}; use std::sync::{Arc, Condvar, Mutex}; use std::thread; use anyhow::Context; use chrono::{DateTime, Utc}; use containerd_shim_wasm::sandbox::error::Error; -use containerd_shim_wasm::sandbox::exec; use containerd_shim_wasm::sandbox::instance::Wait; -use containerd_shim_wasm::sandbox::oci; -use containerd_shim_wasm::sandbox::{EngineGetter, Instance, InstanceConfig}; -use log::{debug, error}; -use nix::sys::signal::SIGKILL; -use wasmtime::{Engine, Linker, Module, Store}; -use wasmtime_wasi::{sync::file::File as WasiFile, WasiCtx, WasiCtxBuilder}; +use containerd_shim_wasm::sandbox::{oci, EngineGetter, Instance, InstanceConfig}; +use libc::{SIGINT, SIGKILL}; +use libcontainer::syscall::syscall::create_syscall; +use log::error; +use nix::sys::wait::{Id as WaitID, WaitPidFlag, WaitStatus}; +use serde::{Deserialize, Serialize}; +use wasmtime::Engine; -use super::error::WasmtimeError; -use super::oci_wasmtime; +use crate::executor::WasmtimeExecutor; +use libcontainer::signal::Signal; + +type ExitCode = Arc<(Mutex)>>, Condvar)>; -type ExitCode = (Mutex)>>, Condvar); pub struct Wasi { - exit_code: Arc, + exit_code: ExitCode, engine: wasmtime::Engine, stdin: String, @@ -28,243 +36,214 @@ pub struct Wasi { stderr: String, bundle: String, - pidfd: Arc>>, -} + rootdir: PathBuf, -#[cfg(test)] -mod tests { - use std::fs::File; + id: String, +} - use tempfile::tempdir; +fn construct_container_root>(root_path: P, container_id: &str) -> Result { + let root_path = fs::canonicalize(&root_path).with_context(|| { + format!( + "failed to canonicalize {} for container {}", + root_path.as_ref().display(), + container_id + ) + })?; + Ok(root_path.join(container_id)) +} - use super::*; +fn load_container>(root_path: P, container_id: &str) -> Result { + let container_root = construct_container_root(root_path, container_id)?; + if !container_root.exists() { + bail!("container {} does not exist.", container_id) + } - #[test] - fn test_maybe_open_stdio() -> Result<(), Error> { - let f = maybe_open_stdio("")?; - assert!(f.is_none()); + Container::load(container_root) + .with_context(|| format!("could not load state for container {container_id}")) +} - let f = maybe_open_stdio("/some/nonexistent/path")?; - assert!(f.is_none()); +fn container_exists>(root_path: P, container_id: &str) -> Result { + let container_root = construct_container_root(root_path, container_id)?; + Ok(container_root.exists()) +} - let dir = tempdir()?; - let temp = File::create(dir.path().join("testfile"))?; - drop(temp); - let f = maybe_open_stdio(dir.path().join("testfile").as_path().to_str().unwrap())?; - assert!(f.is_some()); - drop(f); +#[derive(Serialize, Deserialize)] +struct Options { + root: Option, +} - Ok(()) - } +fn load_spec(bundle: String) -> Result { + let mut spec = oci::load(Path::new(&bundle).join("config.json").to_str().unwrap())?; + spec.canonicalize_rootfs(&bundle) + .map_err(|e| Error::Others(format!("error canonicalizing rootfs in spec: {}", e)))?; + Ok(spec) } /// containerd can send an empty path or a non-existant path /// In both these cases we should just assume that the stdio stream was not setup (intentionally) /// Any other error is a real error. -pub fn maybe_open_stdio(path: &str) -> Result, Error> { +fn maybe_open_stdio(path: &str) -> Result, Error> { if path.is_empty() { return Ok(None); } - match oci_wasmtime::wasi_file(path, OpenOptions::new().read(true).write(true)) { - Ok(f) => Ok(Some(f)), + match OpenOptions::new().read(true).write(true).open(path) { + Ok(f) => Ok(Some(f.into_raw_fd())), Err(err) => match err.kind() { - std::io::ErrorKind::NotFound => Ok(None), + ErrorKind::NotFound => Ok(None), _ => Err(err.into()), }, } } -fn load_spec(bundle: String) -> Result { - let mut spec = oci::load(Path::new(&bundle).join("config.json").to_str().unwrap())?; - spec.canonicalize_rootfs(&bundle) - .map_err(|e| Error::Others(format!("error canonicalizing rootfs in spec: {}", e)))?; - Ok(spec) -} - -pub fn prepare_module( - engine: wasmtime::Engine, - spec: &oci::Spec, - stdin_path: String, - stdout_path: String, - stderr_path: String, -) -> Result<(WasiCtx, Module, String), WasmtimeError> { - debug!("opening rootfs"); - let rootfs = oci_wasmtime::get_rootfs(spec)?; - let args = oci::get_args(spec); - let env = oci_wasmtime::env_to_wasi(spec); - - debug!("setting up wasi"); - let mut wasi_builder = WasiCtxBuilder::new() - .args(args)? - .envs(env.as_slice())? - .preopened_dir(rootfs, "/")?; - - debug!("opening stdin"); - let stdin = maybe_open_stdio(&stdin_path).context("could not open stdin")?; - if let Some(sin) = stdin { - wasi_builder = wasi_builder.stdin(Box::new(sin)); - } - - debug!("opening stdout"); - let stdout = maybe_open_stdio(&stdout_path).context("could not open stdout")?; - if let Some(sout) = stdout { - wasi_builder = wasi_builder.stdout(Box::new(sout)); - } - - debug!("opening stderr"); - let stderr = maybe_open_stdio(&stderr_path).context("could not open stderr")?; - if let Some(serr) = stderr { - wasi_builder = wasi_builder.stderr(Box::new(serr)); - } - - debug!("building wasi context"); - let wctx = wasi_builder.build(); - debug!("wasi context ready"); - - let start = args[0].clone(); - let mut iterator = start.split('#'); - let mut cmd = iterator.next().unwrap().to_string(); - - let stripped = cmd.strip_prefix(std::path::MAIN_SEPARATOR); - if let Some(strpd) = stripped { - cmd = strpd.to_string(); - } - let method = iterator.next().unwrap_or("_start"); - - let mod_path = oci::get_root(spec).join(cmd); - debug!("loading module from file"); - let module = Module::from_file(&engine, mod_path) - .map_err(|err| Error::Others(format!("could not load module from file: {}", err)))?; - - Ok((wctx, module, method.to_string())) -} - impl Instance for Wasi { type E = wasmtime::Engine; - fn new(_id: String, cfg: Option<&InstanceConfig>) -> Self { - let cfg = cfg.unwrap(); // TODO: handle error + fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { + log::info!("creating new instance: {}", id); + let cfg = cfg.unwrap(); + let bundle = cfg.get_bundle().unwrap_or_default(); + let spec = load_spec(bundle.clone()).unwrap(); + let rootdir = oci::get_root(&spec); Wasi { + id, exit_code: Arc::new((Mutex::new(None), Condvar::new())), engine: cfg.get_engine(), stdin: cfg.get_stdin().unwrap_or_default(), stdout: cfg.get_stdout().unwrap_or_default(), stderr: cfg.get_stderr().unwrap_or_default(), - bundle: cfg.get_bundle().unwrap_or_default(), - pidfd: Arc::new(Mutex::new(None)), + bundle: bundle.clone(), + rootdir: rootdir.clone(), } } fn start(&self) -> Result { + log::info!("starting instance: {}", self.id); let engine = self.engine.clone(); - let stdin = self.stdin.clone(); - let stdout = self.stdout.clone(); - let stderr = self.stderr.clone(); - - debug!("starting instance"); - let mut linker = Linker::new(&engine); - - wasmtime_wasi::add_to_linker(&mut linker, |s| s) - .map_err(|err| Error::Others(format!("error adding to linker: {}", err)))?; - - debug!("preparing module"); - let spec = load_spec(self.bundle.clone())?; - let m = prepare_module(engine.clone(), &spec, stdin, stdout, stderr) - .map_err(|e| Error::Others(format!("error setting up module: {}", e)))?; + let mut container = self.build_container( + self.stdin.as_str(), + self.stdout.as_str(), + self.stderr.as_str(), + engine, + )?; - let mut store = Store::new(&engine, m.0); - - debug!("instantiating instance"); - let i = linker - .instantiate(&mut store, &m.1) - .map_err(|err| Error::Others(format!("error instantiating module: {}", err)))?; - - debug!("getting start function"); - let f = i.get_func(&mut store, &m.2).ok_or_else(|| { - Error::InvalidArgument("module does not have a wasi start function".to_string()) - })?; - - debug!("starting wasi instance"); - - let cg = oci::get_cgroup(&spec)?; - - oci::setup_cgroup(cg.as_ref(), &spec) - .map_err(|e| Error::Others(format!("error setting up cgroups: {}", e)))?; - - let res = unsafe { exec::fork(Some(cg.as_ref())) }?; - match res { - exec::Context::Parent(tid, pidfd) => { - let mut lr = self.pidfd.lock().unwrap(); - *lr = Some(pidfd.clone()); - - debug!("started wasi instance with tid {}", tid); - - let code = self.exit_code.clone(); - - let _ = thread::spawn(move || { - let (lock, cvar) = &*code; - let status = match pidfd.wait() { - Ok(status) => status, - Err(e) => { - error!("error waiting for pid {}: {}", tid, e); - cvar.notify_all(); - return; - } - }; - - debug!("wasi instance exited with status {}", status.status); - let mut ec = lock.lock().unwrap(); - *ec = Some((status.status, Utc::now())); - drop(ec); - cvar.notify_all(); - }); - Ok(tid) - } - exec::Context::Child => { - // child process - - // TODO: How to get exit code? - // This was relatively straight forward in go, but wasi and wasmtime are totally separate things in rust. - match f.call(&mut store, &[], &mut []) { - Ok(_) => std::process::exit(0), - Err(_) => std::process::exit(137), - }; - } - } + log::info!("created container: {}", self.id); + let code = self.exit_code.clone(); + let pid = container.pid().unwrap(); + + container + .start() + .map_err(|err| Error::Any(anyhow::anyhow!("failed to start container: {}", err)))?; + + thread::spawn(move || { + let (lock, cvar) = &*code; + + let status = match waitid(WaitID::Pid(pid), WaitPidFlag::WEXITED) { + Ok(WaitStatus::Exited(_, status)) => status, + Ok(WaitStatus::Signaled(_, sig, _)) => sig as i32, + Ok(_) => 0, + Err(e) => { + if e == Errno::ECHILD { + 0 + } else { + panic!("waitpid failed: {}", e); + } + } + } as u32; + let mut ec = lock.lock().unwrap(); + *ec = Some((status, Utc::now())); + drop(ec); + cvar.notify_all(); + }); + + Ok(pid.as_raw() as u32) } fn kill(&self, signal: u32) -> Result<(), Error> { - if signal != SIGKILL as u32 { + log::info!("killing instance: {}", self.id); + if signal as i32 != SIGKILL && signal as i32 != SIGINT { return Err(Error::InvalidArgument( - "only SIGKILL is supported".to_string(), + "only SIGKILL and SIGINT are supported".to_string(), )); } - let lr = self.pidfd.lock().unwrap(); - let fd = lr - .as_ref() - .ok_or_else(|| Error::FailedPrecondition("module is not running".to_string()))?; - fd.kill(signal as i32) + let mut container = load_container(&self.rootdir, self.id.as_str())?; + let signal = Signal::try_from(signal as i32) + .map_err(|err| Error::InvalidArgument(format!("invalid signal number: {}", err)))?; + match container.kill(signal, true) { + Ok(_) => Ok(()), + Err(e) => { + if container.status() == ContainerStatus::Stopped { + return Err(Error::Others("container not running".into())); + } + Err(Error::Others(e.to_string())) + } + } } fn delete(&self) -> Result<(), Error> { - let spec = match load_spec(self.bundle.clone()) { - Ok(spec) => spec, + log::info!("deleting instance: {}", self.id); + match container_exists(&self.rootdir, self.id.as_str()) { + Ok(exists) => { + if !exists { + return Ok(()); + } + } Err(err) => { - error!("Could not load spec, skipping cgroup cleanup: {}", err); + error!("could not find the container, skipping cleanup: {}", err); return Ok(()); } - }; - let cg = oci::get_cgroup(&spec)?; - cg.delete()?; + } + match load_container(&self.rootdir, self.id.as_str()) { + Ok(mut container) => container.delete(true).map_err(|err| { + Error::Any(anyhow::anyhow!( + "failed to delete container {}: {}", + self.id, + err + )) + })?, + Err(err) => { + error!("could not find the container, skipping cleanup: {}", err); + return Ok(()); + } + } + Ok(()) } fn wait(&self, waiter: &Wait) -> Result<(), Error> { + log::info!("waiting for instance: {}", self.id); let code = self.exit_code.clone(); waiter.set_up_exit_code_wait(code) } } +impl Wasi { + fn build_container( + &self, + stdin: &str, + stdout: &str, + stderr: &str, + engine: Engine, + ) -> anyhow::Result { + let syscall = create_syscall(); + let stdin = maybe_open_stdio(stdin).context("could not open stdin")?; + let stdout = maybe_open_stdio(stdout).context("could not open stdout")?; + let stderr = maybe_open_stdio(stderr).context("could not open stderr")?; + + let container = ContainerBuilder::new(self.id.clone(), syscall.as_ref()) + .with_executor(vec![Box::new(WasmtimeExecutor { + stdin, + stdout, + stderr, + engine, + })])? + .with_root_path(self.rootdir.clone())? + .as_init(&self.bundle) + .with_systemd(false) + .build()?; + Ok(container) + } +} + #[cfg(test)] mod wasitest { use std::fs::{create_dir, read_to_string, File}; @@ -384,3 +363,30 @@ impl EngineGetter for Wasi { Ok(engine) } } + +#[cfg(test)] +mod tests { + use std::fs::File; + + use tempfile::tempdir; + + use super::*; + + #[test] + fn test_maybe_open_stdio() -> Result<(), Error> { + let f = maybe_open_stdio("")?; + assert!(f.is_none()); + + let f = maybe_open_stdio("/some/nonexistent/path")?; + assert!(f.is_none()); + + let dir = tempdir()?; + let temp = File::create(dir.path().join("testfile"))?; + drop(temp); + let f = maybe_open_stdio(dir.path().join("testfile").as_path().to_str().unwrap())?; + assert!(f.is_some()); + drop(f); + + Ok(()) + } +} diff --git a/crates/containerd-shim-wasmtime/src/lib.rs b/crates/containerd-shim-wasmtime/src/lib.rs index 48495d071..312731e44 100644 --- a/crates/containerd-shim-wasmtime/src/lib.rs +++ b/crates/containerd-shim-wasmtime/src/lib.rs @@ -1,3 +1,4 @@ pub mod error; +pub mod executor; pub mod instance; pub mod oci_wasmtime; From 83ed1a2aba5d6697c00090d3a549f65a270b63be Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Wed, 14 Jun 2023 00:05:07 +0000 Subject: [PATCH 04/29] Fix some clippy warnings Signed-off-by: jiaxiao zhou --- .../benches/webassembly-benchmarks.rs | 3 +-- crates/containerd-shim-wasmtime/src/instance.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/benches/containerd-shim-benchmarks/benches/webassembly-benchmarks.rs b/benches/containerd-shim-benchmarks/benches/webassembly-benchmarks.rs index 484f0864f..7079b442e 100644 --- a/benches/containerd-shim-benchmarks/benches/webassembly-benchmarks.rs +++ b/benches/containerd-shim-benchmarks/benches/webassembly-benchmarks.rs @@ -117,10 +117,9 @@ fn run_wasmtime_test_with_spec( wasi.start()?; - let w = wasi.clone(); let (tx, rx) = channel(); let waiter = Wait::new(tx); - w.wait(&waiter).unwrap(); + wasi.wait(&waiter).unwrap(); let res = match rx.recv_timeout(Duration::from_secs(60)) { Ok(res) => Ok(res), diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index 474d47670..b145bc05d 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -110,7 +110,7 @@ impl Instance for Wasi { stdin: cfg.get_stdin().unwrap_or_default(), stdout: cfg.get_stdout().unwrap_or_default(), stderr: cfg.get_stderr().unwrap_or_default(), - bundle: bundle.clone(), + bundle, rootdir: rootdir.clone(), } } @@ -385,7 +385,6 @@ mod tests { drop(temp); let f = maybe_open_stdio(dir.path().join("testfile").as_path().to_str().unwrap())?; assert!(f.is_some()); - drop(f); Ok(()) } From d3b7f6915e93eae147f95ccff6d046d514fbaed4 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Wed, 21 Jun 2023 17:56:22 +0000 Subject: [PATCH 05/29] Passed the test_delete_after_create test Signed-off-by: jiaxiao zhou --- .../containerd-shim-wasmtime/src/instance.rs | 76 ++++++++++--------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index b145bc05d..2baf9bbe8 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -98,6 +98,8 @@ fn maybe_open_stdio(path: &str) -> Result, Error> { impl Instance for Wasi { type E = wasmtime::Engine; fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { + // TODO: there are failure cases e.x. parsing cfg, loading spec, etc. + // thus should make `new` return `Result` instead of `Self` log::info!("creating new instance: {}", id); let cfg = cfg.unwrap(); let bundle = cfg.get_bundle().unwrap_or_default(); @@ -251,9 +253,12 @@ mod wasitest { use std::sync::mpsc::channel; use std::time::Duration; + use containerd_shim_wasm::function; + use containerd_shim_wasm::sandbox::exec::has_cap_sys_admin; use containerd_shim_wasm::sandbox::instance::Wait; + use containerd_shim_wasm::sandbox::testutil::run_test_with_sudo; use oci_spec::runtime::{ProcessBuilder, RootBuilder, SpecBuilder}; - use tempfile::tempdir; + use tempfile::{tempdir, TempDir}; use super::*; @@ -288,46 +293,27 @@ mod wasitest { "#.as_bytes(); #[test] - fn test_delete_after_create() { - let i = Wasi::new( - "".to_string(), - Some(&InstanceConfig::new( - Engine::default(), - "test_namespace".into(), - )), - ); - i.delete().unwrap(); + fn test_delete_after_create() -> Result<()> { + let dir = tempdir()?; + create_dir(dir.path().join("rootfs"))?; + let cfg = prepare_cfg(&dir)?; + + let i = Wasi::new("".to_string(), Some(&cfg)); + i.delete()?; + Ok(()) } #[test] fn test_wasi() -> Result<(), Error> { + if !has_cap_sys_admin() { + println!("running test with sudo: {}", function!()); + return run_test_with_sudo(function!()); + } let dir = tempdir()?; create_dir(dir.path().join("rootfs"))?; + let cfg = prepare_cfg(&dir)?; - let mut f = File::create(dir.path().join("rootfs/hello.wat"))?; - f.write_all(WASI_HELLO_WAT)?; - - let stdout = File::create(dir.path().join("stdout"))?; - drop(stdout); - - let spec = SpecBuilder::default() - .root(RootBuilder::default().path("rootfs").build()?) - .process( - ProcessBuilder::default() - .cwd("/") - .args(vec!["hello.wat".to_string()]) - .build()?, - ) - .build()?; - - spec.save(dir.path().join("config.json"))?; - - let mut cfg = InstanceConfig::new(Engine::default(), "test_namespace".into()); - let cfg = cfg - .set_bundle(dir.path().to_str().unwrap().to_string()) - .set_stdout(dir.path().join("stdout").to_str().unwrap().to_string()); - - let wasi = Wasi::new("test".to_string(), Some(cfg)); + let wasi = Wasi::new("test".to_string(), Some(&cfg)); wasi.start()?; @@ -354,6 +340,28 @@ mod wasitest { Ok(()) } + + fn prepare_cfg(dir: &TempDir) -> Result> { + let mut f = File::create(dir.path().join("rootfs/hello.wat"))?; + f.write_all(WASI_HELLO_WAT)?; + let stdout = File::create(dir.path().join("stdout"))?; + drop(stdout); + let spec = SpecBuilder::default() + .root(RootBuilder::default().path("rootfs").build()?) + .process( + ProcessBuilder::default() + .cwd("/") + .args(vec!["hello.wat".to_string()]) + .build()?, + ) + .build()?; + spec.save(dir.path().join("config.json"))?; + let mut cfg = InstanceConfig::new(Engine::default(), "test_namespace".into()); + let cfg = cfg + .set_bundle(dir.path().to_str().unwrap().to_string()) + .set_stdout(dir.path().join("stdout").to_str().unwrap().to_string()); + Ok(cfg.to_owned()) + } } impl EngineGetter for Wasi { From feb93fc0803539c8dc20a7ee6463e06effe62553 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Wed, 21 Jun 2023 18:17:42 +0000 Subject: [PATCH 06/29] This commit adds a new crate called "containerd-shim-common". The purpose of this crate is to provide common helper functionalities to shim implementors that use youki's libcontainer crate. Notice that the "containerd-shim-wasm" crate does not depend on libcontainer. This commit refactors shared implementation from the wasmtime and wasmedge shims to the newly created common crate. Signed-off-by: jiaxiao zhou --- Cargo.lock | 11 + Cargo.toml | 3 + crates/common/Cargo.toml | 11 + crates/common/src/lib.rs | 55 +++++ crates/containerd-shim-wasmedge/Cargo.toml | 5 +- .../containerd-shim-wasmedge/src/instance.rs | 224 ++++++++---------- crates/containerd-shim-wasmtime/Cargo.toml | 3 +- .../containerd-shim-wasmtime/src/instance.rs | 72 +----- 8 files changed, 190 insertions(+), 194 deletions(-) create mode 100644 crates/common/Cargo.toml create mode 100644 crates/common/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index cdbc97545..f78a4002a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -494,6 +494,15 @@ dependencies = [ "tempfile", ] +[[package]] +name = "containerd-shim-common" +version = "0.1.0" +dependencies = [ + "anyhow", + "containerd-shim-wasm", + "libcontainer", +] + [[package]] name = "containerd-shim-protos" version = "0.2.0" @@ -540,6 +549,7 @@ dependencies = [ "cap-std", "chrono", "containerd-shim", + "containerd-shim-common", "containerd-shim-wasm", "libc", "libcontainer", @@ -564,6 +574,7 @@ dependencies = [ "cap-std", "chrono", "containerd-shim", + "containerd-shim-common", "containerd-shim-wasm", "libc", "libcontainer", diff --git a/Cargo.toml b/Cargo.toml index 8d14b7823..b9a291417 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,6 +6,7 @@ members = [ "crates/containerd-shim-wasmedge", "crates/containerd-shim-wasmtime", "benches/containerd-shim-benchmarks", + "crates/common", ] [workspace.package] @@ -18,6 +19,8 @@ homepage = "https://github.com/containerd/runwasi" [workspace.dependencies] anyhow = "1.0" +containerd-shim-common = { path = "crates/common" } +containerd-shim-wasm = { path = "crates/containerd-shim-wasm" } serde = "1.0" serde_json = "1.0" env_logger = "0.10" diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml new file mode 100644 index 000000000..45d15b007 --- /dev/null +++ b/crates/common/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "containerd-shim-common" +version.workspace = true +edition.workspace = true + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +anyhow = "1" +containerd-shim-wasm = { workspace = true } +libcontainer = { workspace = true } \ No newline at end of file diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs new file mode 100644 index 000000000..c5ee76635 --- /dev/null +++ b/crates/common/src/lib.rs @@ -0,0 +1,55 @@ +//! Common utilities for the containerd shims, including wasmtime and wasmedge shim. + +use anyhow::{bail, Context, Result}; +use containerd_shim_wasm::sandbox::error::Error; +use libcontainer::container::Container; +use std::{ + fs::{self, OpenOptions}, + io::ErrorKind, + os::fd::{IntoRawFd, RawFd}, + path::{Path, PathBuf}, +}; + +/// Loads the container state from the given root path. +pub fn load_container>(root_path: P, container_id: &str) -> Result { + let container_root = construct_container_root(root_path, container_id)?; + if !container_root.exists() { + bail!("container {} does not exist.", container_id) + } + + Container::load(container_root) + .with_context(|| format!("could not load state for container {container_id}")) +} + +/// Checks if the container exists. +pub fn container_exists>(root_path: P, container_id: &str) -> Result { + let container_root = construct_container_root(root_path, container_id)?; + Ok(container_root.exists()) +} + +/// containerd can send an empty path or a non-existant path +/// In both these cases we should just assume that the stdio stream was not setup (intentionally) +/// Any other error is a real error. +pub fn maybe_open_stdio(path: &str) -> Result, Error> { + if path.is_empty() { + return Ok(None); + } + match OpenOptions::new().read(true).write(true).open(path) { + Ok(f) => Ok(Some(f.into_raw_fd())), + Err(err) => match err.kind() { + ErrorKind::NotFound => Ok(None), + _ => Err(err.into()), + }, + } +} + +fn construct_container_root>(root_path: P, container_id: &str) -> Result { + let root_path = fs::canonicalize(&root_path).with_context(|| { + format!( + "failed to canonicalize {} for container {}", + root_path.as_ref().display(), + container_id + ) + })?; + Ok(root_path.join(container_id)) +} diff --git a/crates/containerd-shim-wasmedge/Cargo.toml b/crates/containerd-shim-wasmedge/Cargo.toml index afc6e271d..8bb08cc3b 100644 --- a/crates/containerd-shim-wasmedge/Cargo.toml +++ b/crates/containerd-shim-wasmedge/Cargo.toml @@ -4,8 +4,9 @@ version.workspace = true edition.workspace = true [dependencies] -containerd-shim ={ workspace = true } -containerd-shim-wasm = { path = "../containerd-shim-wasm" } +containerd-shim = { workspace = true } +containerd-shim-wasm = { workspace = true } +containerd-shim-common = { workspace = true } log = { workspace = true } ttrpc = { workspace = true } wasmedge-sdk = "0.8.1" diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index ee3bb0ede..2e51d6aeb 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -1,12 +1,15 @@ -use std::fs::{File, OpenOptions}; +use std::fs::File; use std::io::prelude::*; use std::io::ErrorKind; -use std::os::unix::io::{IntoRawFd, RawFd}; +use std::os::unix::io::RawFd; use std::sync::{Arc, Condvar, Mutex}; use std::thread; -use anyhow::{anyhow, bail, Context, Result}; +use anyhow::Context; +use anyhow::{anyhow, Result}; use chrono::{DateTime, Utc}; +use containerd_shim_common::maybe_open_stdio; +use containerd_shim_common::{container_exists, load_container}; use containerd_shim_wasm::sandbox::error::Error; use containerd_shim_wasm::sandbox::instance::Wait; use containerd_shim_wasm::sandbox::{EngineGetter, Instance, InstanceConfig}; @@ -55,73 +58,6 @@ pub struct Wasi { rootdir: PathBuf, } -fn construct_container_root>(root_path: P, container_id: &str) -> Result { - let root_path = fs::canonicalize(&root_path).with_context(|| { - format!( - "failed to canonicalize {} for container {}", - root_path.as_ref().display(), - container_id - ) - })?; - Ok(root_path.join(container_id)) -} - -fn load_container>(root_path: P, container_id: &str) -> Result { - let container_root = construct_container_root(root_path, container_id)?; - if !container_root.exists() { - bail!("container {} does not exist.", container_id) - } - - Container::load(container_root) - .with_context(|| format!("could not load state for container {container_id}")) -} - -fn container_exists>(root_path: P, container_id: &str) -> Result { - let container_root = construct_container_root(root_path, container_id)?; - Ok(container_root.exists()) -} - -#[cfg(test)] -mod tests { - use std::fs::File; - - use tempfile::tempdir; - - use super::*; - - #[test] - fn test_maybe_open_stdio() -> Result<(), Error> { - let f = maybe_open_stdio("")?; - assert!(f.is_none()); - - let f = maybe_open_stdio("/some/nonexistent/path")?; - assert!(f.is_none()); - - let dir = tempdir()?; - let temp = File::create(dir.path().join("testfile"))?; - drop(temp); - let f = maybe_open_stdio(dir.path().join("testfile").as_path().to_str().unwrap())?; - assert!(f.is_some()); - Ok(()) - } -} - -/// containerd can send an empty path or a non-existant path -/// In both these cases we should just assume that the stdio stream was not setup (intentionally) -/// Any other error is a real error. -fn maybe_open_stdio(path: &str) -> Result, Error> { - if path.is_empty() { - return Ok(None); - } - match OpenOptions::new().read(true).write(true).open(path) { - Ok(f) => Ok(Some(f.into_raw_fd())), - Err(err) => match err.kind() { - ErrorKind::NotFound => Ok(None), - _ => Err(err.into()), - }, - } -} - pub fn reset_stdio() { unsafe { if STDIN_FD.is_some() { @@ -160,45 +96,6 @@ fn determine_rootdir>(bundle: P, namespace: String) -> Result Result<(), Error> { - let namespace = "test_namespace"; - let dir = tempdir()?; - let rootdir = dir.path().join("runwasi"); - let opts = Options { - root: Some(rootdir.clone()), - }; - let opts_file = OpenOptions::new() - .read(true) - .create(true) - .truncate(true) - .write(true) - .open(dir.path().join("options.json"))?; - write!(&opts_file, "{}", serde_json::to_string(&opts)?)?; - let root = determine_rootdir(dir.path(), namespace.into())?; - assert_eq!(root, rootdir.join(namespace)); - Ok(()) - } - - #[test] - fn test_determine_rootdir_without_options_file() -> Result<(), Error> { - let dir = tempdir()?; - let namespace = "test_namespace"; - let root = determine_rootdir(dir.path(), namespace.into())?; - assert!(root.is_absolute()); - assert_eq!( - root, - PathBuf::from(DEFAULT_CONTAINER_ROOT_DIR).join(namespace) - ); - Ok(()) - } -} - impl Instance for Wasi { type E = Vm; fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { @@ -337,12 +234,33 @@ impl Wasi { } } +impl EngineGetter for Wasi { + type E = Vm; + fn new_engine() -> Result { + PluginManager::load(None).unwrap(); + let mut host_options = HostRegistrationConfigOptions::default(); + host_options = host_options.wasi(true); + #[cfg(all(target_os = "linux", feature = "wasi_nn", target_arch = "x86_64"))] + { + host_options = host_options.wasi_nn(true); + } + let config = ConfigBuilder::new(CommonConfigOptions::default()) + .with_host_registration_config(host_options) + .build() + .map_err(anyhow::Error::msg)?; + let vm = VmBuilder::new() + .with_config(config) + .build() + .map_err(anyhow::Error::msg)?; + Ok(vm) + } +} + #[cfg(test)] mod wasitest { use std::borrow::Cow; - use std::fs::{create_dir, read_to_string, File}; - use std::io::prelude::*; - use std::os::unix::fs::OpenOptionsExt; + use std::fs::{create_dir, read_to_string, File, OpenOptions}; + use std::os::unix::prelude::OpenOptionsExt; use std::sync::mpsc::channel; use std::time::Duration; @@ -523,24 +441,68 @@ mod wasitest { } } -impl EngineGetter for Wasi { - type E = Vm; - fn new_engine() -> Result { - PluginManager::load(None).unwrap(); - let mut host_options = HostRegistrationConfigOptions::default(); - host_options = host_options.wasi(true); - #[cfg(all(target_os = "linux", feature = "wasi_nn", target_arch = "x86_64"))] - { - host_options = host_options.wasi_nn(true); - } - let config = ConfigBuilder::new(CommonConfigOptions::default()) - .with_host_registration_config(host_options) - .build() - .map_err(anyhow::Error::msg)?; - let vm = VmBuilder::new() - .with_config(config) - .build() - .map_err(anyhow::Error::msg)?; - Ok(vm) +#[cfg(test)] +mod tests { + use std::fs::File; + + use tempfile::tempdir; + + use super::*; + + #[test] + fn test_maybe_open_stdio() -> Result<(), Error> { + let f = maybe_open_stdio("")?; + assert!(f.is_none()); + + let f = maybe_open_stdio("/some/nonexistent/path")?; + assert!(f.is_none()); + + let dir = tempdir()?; + let temp = File::create(dir.path().join("testfile"))?; + drop(temp); + let f = maybe_open_stdio(dir.path().join("testfile").as_path().to_str().unwrap())?; + assert!(f.is_some()); + Ok(()) + } +} + +#[cfg(test)] +mod rootdirtest { + use std::fs::OpenOptions; + + use super::*; + use tempfile::tempdir; + + #[test] + fn test_determine_rootdir_with_options_file() -> Result<(), Error> { + let namespace = "test_namespace"; + let dir = tempdir()?; + let rootdir = dir.path().join("runwasi"); + let opts = Options { + root: Some(rootdir.clone()), + }; + let opts_file = OpenOptions::new() + .read(true) + .create(true) + .truncate(true) + .write(true) + .open(dir.path().join("options.json"))?; + write!(&opts_file, "{}", serde_json::to_string(&opts)?)?; + let root = determine_rootdir(dir.path(), namespace.into())?; + assert_eq!(root, rootdir.join(namespace)); + Ok(()) + } + + #[test] + fn test_determine_rootdir_without_options_file() -> Result<(), Error> { + let dir = tempdir()?; + let namespace = "test_namespace"; + let root = determine_rootdir(dir.path(), namespace.into())?; + assert!(root.is_absolute()); + assert_eq!( + root, + PathBuf::from(DEFAULT_CONTAINER_ROOT_DIR).join(namespace) + ); + Ok(()) } } diff --git a/crates/containerd-shim-wasmtime/Cargo.toml b/crates/containerd-shim-wasmtime/Cargo.toml index 028a3532f..eac9b91f6 100644 --- a/crates/containerd-shim-wasmtime/Cargo.toml +++ b/crates/containerd-shim-wasmtime/Cargo.toml @@ -5,7 +5,8 @@ edition.workspace = true [dependencies] containerd-shim = { workspace = true } -containerd-shim-wasm = { path = "../containerd-shim-wasm" } +containerd-shim-wasm = { workspace = true } +containerd-shim-common = { workspace = true } log = { workspace = true } ttrpc = { workspace = true } diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index 2baf9bbe8..d41e39ae7 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -1,11 +1,9 @@ -use anyhow::{bail, Result}; +use anyhow::Result; +use containerd_shim_common::{container_exists, load_container, maybe_open_stdio}; use libcontainer::container::builder::ContainerBuilder; use libcontainer::container::{Container, ContainerStatus}; use nix::errno::Errno; use nix::sys::wait::waitid; -use std::fs::{self, OpenOptions}; -use std::io::ErrorKind; -use std::os::fd::{IntoRawFd, RawFd}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Condvar, Mutex}; use std::thread; @@ -19,7 +17,7 @@ use libc::{SIGINT, SIGKILL}; use libcontainer::syscall::syscall::create_syscall; use log::error; use nix::sys::wait::{Id as WaitID, WaitPidFlag, WaitStatus}; -use serde::{Deserialize, Serialize}; + use wasmtime::Engine; use crate::executor::WasmtimeExecutor; @@ -41,37 +39,6 @@ pub struct Wasi { id: String, } -fn construct_container_root>(root_path: P, container_id: &str) -> Result { - let root_path = fs::canonicalize(&root_path).with_context(|| { - format!( - "failed to canonicalize {} for container {}", - root_path.as_ref().display(), - container_id - ) - })?; - Ok(root_path.join(container_id)) -} - -fn load_container>(root_path: P, container_id: &str) -> Result { - let container_root = construct_container_root(root_path, container_id)?; - if !container_root.exists() { - bail!("container {} does not exist.", container_id) - } - - Container::load(container_root) - .with_context(|| format!("could not load state for container {container_id}")) -} - -fn container_exists>(root_path: P, container_id: &str) -> Result { - let container_root = construct_container_root(root_path, container_id)?; - Ok(container_root.exists()) -} - -#[derive(Serialize, Deserialize)] -struct Options { - root: Option, -} - fn load_spec(bundle: String) -> Result { let mut spec = oci::load(Path::new(&bundle).join("config.json").to_str().unwrap())?; spec.canonicalize_rootfs(&bundle) @@ -79,22 +46,6 @@ fn load_spec(bundle: String) -> Result { Ok(spec) } -/// containerd can send an empty path or a non-existant path -/// In both these cases we should just assume that the stdio stream was not setup (intentionally) -/// Any other error is a real error. -fn maybe_open_stdio(path: &str) -> Result, Error> { - if path.is_empty() { - return Ok(None); - } - match OpenOptions::new().read(true).write(true).open(path) { - Ok(f) => Ok(Some(f.into_raw_fd())), - Err(err) => match err.kind() { - ErrorKind::NotFound => Ok(None), - _ => Err(err.into()), - }, - } -} - impl Instance for Wasi { type E = wasmtime::Engine; fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { @@ -246,6 +197,14 @@ impl Wasi { } } +impl EngineGetter for Wasi { + type E = wasmtime::Engine; + fn new_engine() -> Result { + let engine = Engine::default(); + Ok(engine) + } +} + #[cfg(test)] mod wasitest { use std::fs::{create_dir, read_to_string, File}; @@ -364,18 +323,11 @@ mod wasitest { } } -impl EngineGetter for Wasi { - type E = wasmtime::Engine; - fn new_engine() -> Result { - let engine = Engine::default(); - Ok(engine) - } -} - #[cfg(test)] mod tests { use std::fs::File; + use containerd_shim_common::maybe_open_stdio; use tempfile::tempdir; use super::*; From 47ce17d0ac32627f82579545968744b1d9b2d48a Mon Sep 17 00:00:00 2001 From: Jiaxiao Zhou Date: Fri, 23 Jun 2023 11:38:27 -0700 Subject: [PATCH 07/29] Apply suggestions from code review Co-authored-by: Toru Komatsu Signed-off-by: Jiaxiao Zhou --- crates/containerd-shim-wasmtime/src/executor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/containerd-shim-wasmtime/src/executor.rs b/crates/containerd-shim-wasmtime/src/executor.rs index fce5049fb..0bb88a3f1 100644 --- a/crates/containerd-shim-wasmtime/src/executor.rs +++ b/crates/containerd-shim-wasmtime/src/executor.rs @@ -52,7 +52,7 @@ impl Executor for WasmtimeExecutor { } impl WasmtimeExecutor { - fn prepare_function( + fn prepare( &self, spec: &Spec, args: &[String], From 363f1cd79d24bfa739bf186fde480116c8b11d79 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Fri, 23 Jun 2023 18:47:48 +0000 Subject: [PATCH 08/29] Resolved some review comments Signed-off-by: jiaxiao zhou --- Cargo.lock | 1 + crates/common/Cargo.toml | 5 +++- crates/common/src/lib.rs | 25 +++++++++++++++++ .../containerd-shim-wasmtime/src/executor.rs | 2 +- .../containerd-shim-wasmtime/src/instance.rs | 27 ------------------- 5 files changed, 31 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f78a4002a..26283acd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -501,6 +501,7 @@ dependencies = [ "anyhow", "containerd-shim-wasm", "libcontainer", + "tempfile", ] [[package]] diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index 45d15b007..521bc92aa 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -8,4 +8,7 @@ edition.workspace = true [dependencies] anyhow = "1" containerd-shim-wasm = { workspace = true } -libcontainer = { workspace = true } \ No newline at end of file +libcontainer = { workspace = true } + +[dev-dependencies] +tempfile = "3.0" \ No newline at end of file diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index c5ee76635..6771b68d0 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -53,3 +53,28 @@ fn construct_container_root>(root_path: P, container_id: &str) -> })?; Ok(root_path.join(container_id)) } + +#[cfg(test)] +mod tests { + use std::fs::File; + + use tempfile::tempdir; + + use super::*; + + #[test] + fn test_maybe_open_stdio() -> Result<(), Error> { + let f = maybe_open_stdio("")?; + assert!(f.is_none()); + + let f = maybe_open_stdio("/some/nonexistent/path")?; + assert!(f.is_none()); + + let dir = tempdir()?; + let temp = File::create(dir.path().join("testfile"))?; + drop(temp); + let f = maybe_open_stdio(dir.path().join("testfile").as_path().to_str().unwrap())?; + assert!(f.is_some()); + Ok(()) + } +} diff --git a/crates/containerd-shim-wasmtime/src/executor.rs b/crates/containerd-shim-wasmtime/src/executor.rs index 0bb88a3f1..443b4c2ed 100644 --- a/crates/containerd-shim-wasmtime/src/executor.rs +++ b/crates/containerd-shim-wasmtime/src/executor.rs @@ -32,7 +32,7 @@ impl Executor for WasmtimeExecutor { } let (mut store, f) = self - .prepare_function(spec, args) + .prepare(spec, args) .map_err(|err| ExecutorError::Other(format!("failed to prepare function: {}", err)))?; log::info!("calling start function"); diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index d41e39ae7..9776878e7 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -322,30 +322,3 @@ mod wasitest { Ok(cfg.to_owned()) } } - -#[cfg(test)] -mod tests { - use std::fs::File; - - use containerd_shim_common::maybe_open_stdio; - use tempfile::tempdir; - - use super::*; - - #[test] - fn test_maybe_open_stdio() -> Result<(), Error> { - let f = maybe_open_stdio("")?; - assert!(f.is_none()); - - let f = maybe_open_stdio("/some/nonexistent/path")?; - assert!(f.is_none()); - - let dir = tempdir()?; - let temp = File::create(dir.path().join("testfile"))?; - drop(temp); - let f = maybe_open_stdio(dir.path().join("testfile").as_path().to_str().unwrap())?; - assert!(f.is_some()); - - Ok(()) - } -} From 6882dd5bd7c1dba629bd97af5ede1c0a67b39c91 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Fri, 23 Jun 2023 20:50:49 +0000 Subject: [PATCH 09/29] Removed common crate and moved the library to containerd-shim-wasm and added libcontainer feature Signed-off-by: jiaxiao zhou --- Cargo.lock | 184 +----------------- Cargo.toml | 3 - crates/common/Cargo.toml | 14 -- crates/containerd-shim-wasm/Cargo.toml | 2 + crates/containerd-shim-wasm/src/lib.rs | 3 + .../src/wasm_libcontainer/mod.rs} | 2 +- crates/containerd-shim-wasmedge/Cargo.toml | 3 +- .../containerd-shim-wasmedge/src/instance.rs | 29 +-- crates/containerd-shim-wasmtime/Cargo.toml | 3 +- .../containerd-shim-wasmtime/src/executor.rs | 4 +- .../containerd-shim-wasmtime/src/instance.rs | 2 +- 11 files changed, 14 insertions(+), 235 deletions(-) delete mode 100644 crates/common/Cargo.toml rename crates/{common/src/lib.rs => containerd-shim-wasm/src/wasm_libcontainer/mod.rs} (98%) diff --git a/Cargo.lock b/Cargo.lock index 26283acd8..6c0fafe1d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -58,12 +58,6 @@ dependencies = [ "libc", ] -[[package]] -name = "anes" -version = "0.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4b46cbb362ab8752921c97e041f5e366ee6297bd428a31275b9fcf1e380f7299" - [[package]] name = "anstream" version = "0.3.2" @@ -289,12 +283,6 @@ dependencies = [ "thiserror", ] -[[package]] -name = "cast" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" - [[package]] name = "cc" version = "1.0.79" @@ -338,33 +326,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "ciborium" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "effd91f6c78e5a4ace8a5d3c0b6bfaec9e2baaef55f3efc00e45fb2e477ee926" -dependencies = [ - "ciborium-io", - "ciborium-ll", - "serde", -] - -[[package]] -name = "ciborium-io" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cdf919175532b369853f5d5e20b26b43112613fd6fe7aee757e35f7a44642656" - -[[package]] -name = "ciborium-ll" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "defaa24ecc093c77630e6c15e17c51f5e187bf35ee514f4e2d67baaa96dae22b" -dependencies = [ - "ciborium-io", - "half", -] - [[package]] name = "clang-sys" version = "1.6.1" @@ -477,33 +438,6 @@ dependencies = [ "uuid 0.8.2", ] -[[package]] -name = "containerd-shim-benchmarks" -version = "0.1.0" -dependencies = [ - "anyhow", - "chrono", - "containerd-shim-wasm", - "containerd-shim-wasmedge", - "containerd-shim-wasmtime", - "criterion", - "libc", - "oci-spec 0.6.1", - "serde", - "serde_json", - "tempfile", -] - -[[package]] -name = "containerd-shim-common" -version = "0.1.0" -dependencies = [ - "anyhow", - "containerd-shim-wasm", - "libcontainer", - "tempfile", -] - [[package]] name = "containerd-shim-protos" version = "0.2.0" @@ -526,6 +460,7 @@ dependencies = [ "containerd-shim", "env_logger", "libc", + "libcontainer", "log", "nix 0.26.2", "oci-spec 0.6.1", @@ -550,7 +485,6 @@ dependencies = [ "cap-std", "chrono", "containerd-shim", - "containerd-shim-common", "containerd-shim-wasm", "libc", "libcontainer", @@ -575,7 +509,6 @@ dependencies = [ "cap-std", "chrono", "containerd-shim", - "containerd-shim-common", "containerd-shim-wasm", "libc", "libcontainer", @@ -734,42 +667,6 @@ dependencies = [ "cfg-if 1.0.0", ] -[[package]] -name = "criterion" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2b12d017a929603d80db1831cd3a24082f8137ce19c69e6447f54f5fc8d692f" -dependencies = [ - "anes", - "cast", - "ciborium", - "clap", - "criterion-plot", - "is-terminal", - "itertools", - "num-traits", - "once_cell", - "oorandom", - "plotters", - "rayon", - "regex", - "serde", - "serde_derive", - "serde_json", - "tinytemplate", - "walkdir", -] - -[[package]] -name = "criterion-plot" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b50826342786a51a89e2da3a28f1c32b06e387201bc2d19791f622c673706b1" -dependencies = [ - "cast", - "itertools", -] - [[package]] name = "crossbeam-channel" version = "0.5.8" @@ -1344,12 +1241,6 @@ dependencies = [ "cfg-if 0.1.10", ] -[[package]] -name = "half" -version = "1.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eabb4a44450da02c90444cf74558da904edde8fb4e9035a9a6a4e15445af0bd7" - [[package]] name = "hashbrown" version = "0.12.3" @@ -1916,12 +1807,6 @@ version = "1.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dd8b5dd2ae5ed71462c540258bedcb51965123ad7e7ccf4b9a8cafaa4a63576d" -[[package]] -name = "oorandom" -version = "11.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ab1bc2a289d34bd04a330323ac98a1b4bc82c9d9fcb1e66b63caa84da26b575" - [[package]] name = "output_vt100" version = "0.1.3" @@ -2009,34 +1894,6 @@ version = "0.3.27" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "26072860ba924cbfa98ea39c8c19b4dd6a4a25423dbdf219c1eca91aa0cf6964" -[[package]] -name = "plotters" -version = "0.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2538b639e642295546c50fcd545198c9d64ee2a38620a628724a3b266d5fbf97" -dependencies = [ - "num-traits", - "plotters-backend", - "plotters-svg", - "wasm-bindgen", - "web-sys", -] - -[[package]] -name = "plotters-backend" -version = "0.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "193228616381fecdc1224c62e96946dfbc73ff4384fba576e052ff8c1bea8142" - -[[package]] -name = "plotters-svg" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f9a81d2759aae1dae668f783c308bc5c8ebd191ff4184aaa1b37f65a6ae5a56f" -dependencies = [ - "plotters-backend", -] - [[package]] name = "ppv-lite86" version = "0.2.17" @@ -2466,15 +2323,6 @@ dependencies = [ "libc", ] -[[package]] -name = "same-file" -version = "1.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" -dependencies = [ - "winapi-util", -] - [[package]] name = "scopeguard" version = "1.1.0" @@ -2755,16 +2603,6 @@ dependencies = [ "time-core", ] -[[package]] -name = "tinytemplate" -version = "1.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be4d6b5f19ff7664e8c98d03e2139cb510db9b0a60b55f8e8709b689d939b6bc" -dependencies = [ - "serde", - "serde_json", -] - [[package]] name = "tinyvec" version = "1.6.0" @@ -2967,16 +2805,6 @@ version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" -[[package]] -name = "walkdir" -version = "2.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "36df944cda56c7d8d8b7496af378e6b16de9284591917d307c9b4d313c44e698" -dependencies = [ - "same-file", - "winapi-util", -] - [[package]] name = "wasi" version = "0.11.0+wasi-snapshot-preview1" @@ -3415,16 +3243,6 @@ dependencies = [ "wast 60.0.0", ] -[[package]] -name = "web-sys" -version = "0.3.64" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b85cbef8c220a6abc02aefd892dfc0fc23afb1c6a426316ec33253a3877249b" -dependencies = [ - "js-sys", - "wasm-bindgen", -] - [[package]] name = "which" version = "4.4.0" diff --git a/Cargo.toml b/Cargo.toml index b9a291417..0fdd706ca 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,8 +5,6 @@ members = [ "crates/oci-tar-builder", "crates/containerd-shim-wasmedge", "crates/containerd-shim-wasmtime", - "benches/containerd-shim-benchmarks", - "crates/common", ] [workspace.package] @@ -19,7 +17,6 @@ homepage = "https://github.com/containerd/runwasi" [workspace.dependencies] anyhow = "1.0" -containerd-shim-common = { path = "crates/common" } containerd-shim-wasm = { path = "crates/containerd-shim-wasm" } serde = "1.0" serde_json = "1.0" diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml deleted file mode 100644 index 521bc92aa..000000000 --- a/crates/common/Cargo.toml +++ /dev/null @@ -1,14 +0,0 @@ -[package] -name = "containerd-shim-common" -version.workspace = true -edition.workspace = true - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - -[dependencies] -anyhow = "1" -containerd-shim-wasm = { workspace = true } -libcontainer = { workspace = true } - -[dev-dependencies] -tempfile = "3.0" \ No newline at end of file diff --git a/crates/containerd-shim-wasm/Cargo.toml b/crates/containerd-shim-wasm/Cargo.toml index 594f2d741..3ed2a5a6f 100644 --- a/crates/containerd-shim-wasm/Cargo.toml +++ b/crates/containerd-shim-wasm/Cargo.toml @@ -28,6 +28,7 @@ clone3 = "0.2" libc = { workspace = true } caps = "0.5" proc-mounts = "0.3" +libcontainer = { workspace = true, optional = true} [build-dependencies] ttrpc-codegen = { version = "0.3", optional = true } @@ -43,3 +44,4 @@ rand = "0.8" default = [] generate_bindings = ["ttrpc-codegen"] generate_doc = [] +libcontainer = ["dep:libcontainer"] \ No newline at end of file diff --git a/crates/containerd-shim-wasm/src/lib.rs b/crates/containerd-shim-wasm/src/lib.rs index b4a324107..3c657a0f5 100644 --- a/crates/containerd-shim-wasm/src/lib.rs +++ b/crates/containerd-shim-wasm/src/lib.rs @@ -6,3 +6,6 @@ pub mod sandbox; pub mod services; + +#[cfg(feature = "libcontainer")] +pub mod wasm_libcontainer; diff --git a/crates/common/src/lib.rs b/crates/containerd-shim-wasm/src/wasm_libcontainer/mod.rs similarity index 98% rename from crates/common/src/lib.rs rename to crates/containerd-shim-wasm/src/wasm_libcontainer/mod.rs index 6771b68d0..2459031f1 100644 --- a/crates/common/src/lib.rs +++ b/crates/containerd-shim-wasm/src/wasm_libcontainer/mod.rs @@ -1,7 +1,7 @@ //! Common utilities for the containerd shims, including wasmtime and wasmedge shim. +use crate::sandbox::error::Error; use anyhow::{bail, Context, Result}; -use containerd_shim_wasm::sandbox::error::Error; use libcontainer::container::Container; use std::{ fs::{self, OpenOptions}, diff --git a/crates/containerd-shim-wasmedge/Cargo.toml b/crates/containerd-shim-wasmedge/Cargo.toml index 8bb08cc3b..43aec1d3a 100644 --- a/crates/containerd-shim-wasmedge/Cargo.toml +++ b/crates/containerd-shim-wasmedge/Cargo.toml @@ -5,8 +5,7 @@ edition.workspace = true [dependencies] containerd-shim = { workspace = true } -containerd-shim-wasm = { workspace = true } -containerd-shim-common = { workspace = true } +containerd-shim-wasm = { workspace = true, features = ["libcontainer"]} log = { workspace = true } ttrpc = { workspace = true } wasmedge-sdk = "0.8.1" diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index 2e51d6aeb..56412e64d 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -8,11 +8,11 @@ use std::thread; use anyhow::Context; use anyhow::{anyhow, Result}; use chrono::{DateTime, Utc}; -use containerd_shim_common::maybe_open_stdio; -use containerd_shim_common::{container_exists, load_container}; use containerd_shim_wasm::sandbox::error::Error; use containerd_shim_wasm::sandbox::instance::Wait; use containerd_shim_wasm::sandbox::{EngineGetter, Instance, InstanceConfig}; +use containerd_shim_wasm::wasm_libcontainer::maybe_open_stdio; +use containerd_shim_wasm::wasm_libcontainer::{container_exists, load_container}; use libc::{dup2, SIGINT, SIGKILL, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use log::{debug, error}; use nix::errno::Errno; @@ -441,31 +441,6 @@ mod wasitest { } } -#[cfg(test)] -mod tests { - use std::fs::File; - - use tempfile::tempdir; - - use super::*; - - #[test] - fn test_maybe_open_stdio() -> Result<(), Error> { - let f = maybe_open_stdio("")?; - assert!(f.is_none()); - - let f = maybe_open_stdio("/some/nonexistent/path")?; - assert!(f.is_none()); - - let dir = tempdir()?; - let temp = File::create(dir.path().join("testfile"))?; - drop(temp); - let f = maybe_open_stdio(dir.path().join("testfile").as_path().to_str().unwrap())?; - assert!(f.is_some()); - Ok(()) - } -} - #[cfg(test)] mod rootdirtest { use std::fs::OpenOptions; diff --git a/crates/containerd-shim-wasmtime/Cargo.toml b/crates/containerd-shim-wasmtime/Cargo.toml index eac9b91f6..2730d20a9 100644 --- a/crates/containerd-shim-wasmtime/Cargo.toml +++ b/crates/containerd-shim-wasmtime/Cargo.toml @@ -5,8 +5,7 @@ edition.workspace = true [dependencies] containerd-shim = { workspace = true } -containerd-shim-wasm = { workspace = true } -containerd-shim-common = { workspace = true } +containerd-shim-wasm = { workspace = true, features = ["libcontainer"] } log = { workspace = true } ttrpc = { workspace = true } diff --git a/crates/containerd-shim-wasmtime/src/executor.rs b/crates/containerd-shim-wasmtime/src/executor.rs index 443b4c2ed..e117d702b 100644 --- a/crates/containerd-shim-wasmtime/src/executor.rs +++ b/crates/containerd-shim-wasmtime/src/executor.rs @@ -1,8 +1,8 @@ -use std::{fs::OpenOptions, os::fd::RawFd, path::PathBuf}; +use std::{fs::OpenOptions, path::PathBuf}; use anyhow::{anyhow, Result}; use containerd_shim_wasm::sandbox::oci; -use libc::{dup, dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; + use libcontainer::workload::{Executor, ExecutorError}; use oci_spec::runtime::Spec; diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index 9776878e7..37a7a9b02 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -1,5 +1,5 @@ use anyhow::Result; -use containerd_shim_common::{container_exists, load_container, maybe_open_stdio}; +use containerd_shim_wasm::wasm_libcontainer::{container_exists, load_container, maybe_open_stdio}; use libcontainer::container::builder::ContainerBuilder; use libcontainer::container::{Container, ContainerStatus}; use nix::errno::Errno; From fc333582bf7742d1678044a2b63c6381ad1f106a Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Fri, 23 Jun 2023 20:57:14 +0000 Subject: [PATCH 10/29] Rollback accidentally deleted deps Signed-off-by: jiaxiao zhou --- crates/containerd-shim-wasmtime/src/executor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/containerd-shim-wasmtime/src/executor.rs b/crates/containerd-shim-wasmtime/src/executor.rs index e117d702b..443b4c2ed 100644 --- a/crates/containerd-shim-wasmtime/src/executor.rs +++ b/crates/containerd-shim-wasmtime/src/executor.rs @@ -1,8 +1,8 @@ -use std::{fs::OpenOptions, path::PathBuf}; +use std::{fs::OpenOptions, os::fd::RawFd, path::PathBuf}; use anyhow::{anyhow, Result}; use containerd_shim_wasm::sandbox::oci; - +use libc::{dup, dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use libcontainer::workload::{Executor, ExecutorError}; use oci_spec::runtime::Spec; From b606b25373ddd3b933099064dd49fc18fe280359 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Fri, 23 Jun 2023 23:26:05 +0000 Subject: [PATCH 11/29] Removed the libcontainer feature This Commit removes the libcontainer feature form the containerd-wasm-shim create by moving the invocation of Container::load() function to shim implmenetaiton, hence reducing the need to depend on libcontainer entirely. It adds a new mod called `instance_utils` to hold common shim utility functions. Signed-off-by: jiaxiao zhou --- Cargo.lock | 1 - crates/containerd-shim-wasm/Cargo.toml | 4 +-- crates/containerd-shim-wasm/src/lib.rs | 3 -- .../src/sandbox/instance.rs | 10 ++++-- .../mod.rs => sandbox/instance_utils.rs} | 33 ++++++++++--------- .../containerd-shim-wasm/src/sandbox/mod.rs | 1 + crates/containerd-shim-wasmedge/Cargo.toml | 2 +- .../containerd-shim-wasmedge/src/instance.rs | 24 +++++++++++--- crates/containerd-shim-wasmtime/Cargo.toml | 2 +- .../containerd-shim-wasmtime/src/instance.rs | 24 +++++++++++--- 10 files changed, 68 insertions(+), 36 deletions(-) rename crates/containerd-shim-wasm/src/{wasm_libcontainer/mod.rs => sandbox/instance_utils.rs} (65%) diff --git a/Cargo.lock b/Cargo.lock index 6c0fafe1d..f4c9ba359 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -460,7 +460,6 @@ dependencies = [ "containerd-shim", "env_logger", "libc", - "libcontainer", "log", "nix 0.26.2", "oci-spec 0.6.1", diff --git a/crates/containerd-shim-wasm/Cargo.toml b/crates/containerd-shim-wasm/Cargo.toml index 3ed2a5a6f..0c5d834c3 100644 --- a/crates/containerd-shim-wasm/Cargo.toml +++ b/crates/containerd-shim-wasm/Cargo.toml @@ -28,7 +28,6 @@ clone3 = "0.2" libc = { workspace = true } caps = "0.5" proc-mounts = "0.3" -libcontainer = { workspace = true, optional = true} [build-dependencies] ttrpc-codegen = { version = "0.3", optional = true } @@ -43,5 +42,4 @@ rand = "0.8" [features] default = [] generate_bindings = ["ttrpc-codegen"] -generate_doc = [] -libcontainer = ["dep:libcontainer"] \ No newline at end of file +generate_doc = [] \ No newline at end of file diff --git a/crates/containerd-shim-wasm/src/lib.rs b/crates/containerd-shim-wasm/src/lib.rs index 3c657a0f5..b4a324107 100644 --- a/crates/containerd-shim-wasm/src/lib.rs +++ b/crates/containerd-shim-wasm/src/lib.rs @@ -6,6 +6,3 @@ pub mod sandbox; pub mod services; - -#[cfg(feature = "libcontainer")] -pub mod wasm_libcontainer; diff --git a/crates/containerd-shim-wasm/src/sandbox/instance.rs b/crates/containerd-shim-wasm/src/sandbox/instance.rs index 4007edfb1..3cc2941df 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance.rs @@ -19,7 +19,7 @@ pub struct InstanceConfig where E: Send + Sync + Clone, { - /// The wasm engine to use. + /// The WASI engine to use. /// This should be cheap to clone. engine: E, /// Optional stdin named pipe path. @@ -104,21 +104,27 @@ where } } -/// Represents a wasi module(s). +/// Represents a WASI module(s). /// Instance is a trait that gets implemented by consumers of this library. pub trait Instance { + /// The WASI engine type type E: Send + Sync + Clone; + /// Create a new instance fn new(id: String, cfg: Option<&InstanceConfig>) -> Self; + /// Start the instance /// The returned value should be a unique ID (such as a PID) for the instance. /// Nothing internally should be using this ID, but it is returned to containerd where a user may want to use it. fn start(&self) -> Result; + /// Send a signal to the instance fn kill(&self, signal: u32) -> Result<(), Error>; + /// Delete any reference to the instance /// This is called after the instance has exited. fn delete(&self) -> Result<(), Error>; + /// Set up waiting for the instance to exit /// The Wait struct is used to send the exit code and time back to the /// caller. The recipient is expected to call function diff --git a/crates/containerd-shim-wasm/src/wasm_libcontainer/mod.rs b/crates/containerd-shim-wasm/src/sandbox/instance_utils.rs similarity index 65% rename from crates/containerd-shim-wasm/src/wasm_libcontainer/mod.rs rename to crates/containerd-shim-wasm/src/sandbox/instance_utils.rs index 2459031f1..14d046199 100644 --- a/crates/containerd-shim-wasm/src/wasm_libcontainer/mod.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance_utils.rs @@ -1,8 +1,6 @@ -//! Common utilities for the containerd shims, including wasmtime and wasmedge shim. - +//! Common utilities for the containerd shims. use crate::sandbox::error::Error; use anyhow::{bail, Context, Result}; -use libcontainer::container::Container; use std::{ fs::{self, OpenOptions}, io::ErrorKind, @@ -10,21 +8,26 @@ use std::{ path::{Path, PathBuf}, }; -/// Loads the container state from the given root path. -pub fn load_container>(root_path: P, container_id: &str) -> Result { - let container_root = construct_container_root(root_path, container_id)?; - if !container_root.exists() { - bail!("container {} does not exist.", container_id) +/// Return the root path for the instance. +/// +/// The root path is the path to the directory containing the container's state. +pub fn get_instance_root>( + root_path: P, + instance_id: &str, +) -> Result { + let instance_root = construct_instance_root(root_path, instance_id)?; + if !instance_root.exists() { + bail!("container {} does not exist.", instance_id) } - - Container::load(container_root) - .with_context(|| format!("could not load state for container {container_id}")) + Ok(instance_root) } /// Checks if the container exists. -pub fn container_exists>(root_path: P, container_id: &str) -> Result { - let container_root = construct_container_root(root_path, container_id)?; - Ok(container_root.exists()) +/// +/// The root path is the path to the directory containing the container's state. +pub fn instance_exists>(root_path: P, container_id: &str) -> Result { + let instance_root = construct_instance_root(root_path, container_id)?; + Ok(instance_root.exists()) } /// containerd can send an empty path or a non-existant path @@ -43,7 +46,7 @@ pub fn maybe_open_stdio(path: &str) -> Result, Error> { } } -fn construct_container_root>(root_path: P, container_id: &str) -> Result { +fn construct_instance_root>(root_path: P, container_id: &str) -> Result { let root_path = fs::canonicalize(&root_path).with_context(|| { format!( "failed to canonicalize {} for container {}", diff --git a/crates/containerd-shim-wasm/src/sandbox/mod.rs b/crates/containerd-shim-wasm/src/sandbox/mod.rs index 72d7ac2f3..7494815a7 100644 --- a/crates/containerd-shim-wasm/src/sandbox/mod.rs +++ b/crates/containerd-shim-wasm/src/sandbox/mod.rs @@ -6,6 +6,7 @@ pub mod cgroups; pub mod error; pub mod exec; pub mod instance; +pub mod instance_utils; pub mod manager; pub mod shim; diff --git a/crates/containerd-shim-wasmedge/Cargo.toml b/crates/containerd-shim-wasmedge/Cargo.toml index 43aec1d3a..5d8f2745f 100644 --- a/crates/containerd-shim-wasmedge/Cargo.toml +++ b/crates/containerd-shim-wasmedge/Cargo.toml @@ -5,7 +5,7 @@ edition.workspace = true [dependencies] containerd-shim = { workspace = true } -containerd-shim-wasm = { workspace = true, features = ["libcontainer"]} +containerd-shim-wasm = { workspace = true } log = { workspace = true } ttrpc = { workspace = true } wasmedge-sdk = "0.8.1" diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index 56412e64d..eb71244c1 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -10,9 +10,10 @@ use anyhow::{anyhow, Result}; use chrono::{DateTime, Utc}; use containerd_shim_wasm::sandbox::error::Error; use containerd_shim_wasm::sandbox::instance::Wait; +use containerd_shim_wasm::sandbox::instance_utils::{ + get_instance_root, instance_exists, maybe_open_stdio, +}; use containerd_shim_wasm::sandbox::{EngineGetter, Instance, InstanceConfig}; -use containerd_shim_wasm::wasm_libcontainer::maybe_open_stdio; -use containerd_shim_wasm::wasm_libcontainer::{container_exists, load_container}; use libc::{dup2, SIGINT, SIGKILL, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use log::{debug, error}; use nix::errno::Errno; @@ -169,7 +170,13 @@ impl Instance for Wasi { ))?, }; - let mut container = load_container(&self.rootdir, self.id.as_str())?; + let container_root = get_instance_root(&self.rootdir, self.id.as_str())?; + let mut container = Container::load(container_root).with_context(|| { + format!( + "could not load state for container {id}", + id = self.id.as_str() + ) + })?; match container.kill(signal, true) { Ok(_) => Ok(()), Err(e) => { @@ -182,7 +189,7 @@ impl Instance for Wasi { } fn delete(&self) -> Result<(), Error> { - match container_exists(&self.rootdir, self.id.as_str()) { + match instance_exists(&self.rootdir, self.id.as_str()) { Ok(exists) => { if !exists { return Ok(()); @@ -193,7 +200,14 @@ impl Instance for Wasi { return Ok(()); } } - match load_container(&self.rootdir, self.id.as_str()) { + let container_root = get_instance_root(&self.rootdir, self.id.as_str())?; + let container = Container::load(container_root).with_context(|| { + format!( + "could not load state for container {id}", + id = self.id.as_str() + ) + }); + match container { Ok(mut container) => container.delete(true).map_err(|err| { Error::Any(anyhow!("failed to delete container {}: {}", self.id, err)) })?, diff --git a/crates/containerd-shim-wasmtime/Cargo.toml b/crates/containerd-shim-wasmtime/Cargo.toml index 2730d20a9..c47eef6b7 100644 --- a/crates/containerd-shim-wasmtime/Cargo.toml +++ b/crates/containerd-shim-wasmtime/Cargo.toml @@ -5,7 +5,7 @@ edition.workspace = true [dependencies] containerd-shim = { workspace = true } -containerd-shim-wasm = { workspace = true, features = ["libcontainer"] } +containerd-shim-wasm = { workspace = true } log = { workspace = true } ttrpc = { workspace = true } diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index 37a7a9b02..cc8d81c91 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -1,5 +1,7 @@ use anyhow::Result; -use containerd_shim_wasm::wasm_libcontainer::{container_exists, load_container, maybe_open_stdio}; +use containerd_shim_wasm::sandbox::instance_utils::{ + get_instance_root, instance_exists, maybe_open_stdio, +}; use libcontainer::container::builder::ContainerBuilder; use libcontainer::container::{Container, ContainerStatus}; use nix::errno::Errno; @@ -117,8 +119,13 @@ impl Instance for Wasi { "only SIGKILL and SIGINT are supported".to_string(), )); } - - let mut container = load_container(&self.rootdir, self.id.as_str())?; + let container_root = get_instance_root(&self.rootdir, self.id.as_str())?; + let mut container = Container::load(container_root).with_context(|| { + format!( + "could not load state for container {id}", + id = self.id.as_str() + ) + })?; let signal = Signal::try_from(signal as i32) .map_err(|err| Error::InvalidArgument(format!("invalid signal number: {}", err)))?; match container.kill(signal, true) { @@ -134,7 +141,7 @@ impl Instance for Wasi { fn delete(&self) -> Result<(), Error> { log::info!("deleting instance: {}", self.id); - match container_exists(&self.rootdir, self.id.as_str()) { + match instance_exists(&self.rootdir, self.id.as_str()) { Ok(exists) => { if !exists { return Ok(()); @@ -145,7 +152,14 @@ impl Instance for Wasi { return Ok(()); } } - match load_container(&self.rootdir, self.id.as_str()) { + let container_root = get_instance_root(&self.rootdir, self.id.as_str())?; + let container = Container::load(container_root).with_context(|| { + format!( + "could not load state for container {id}", + id = self.id.as_str() + ) + }); + match container { Ok(mut container) => container.delete(true).map_err(|err| { Error::Any(anyhow::anyhow!( "failed to delete container {}: {}", From 8c9e43dccd40e3c7ce05b1a30ad57e517a3b691b Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Wed, 28 Jun 2023 00:16:58 +0000 Subject: [PATCH 12/29] Fixed the test_wasi test This commit fixes the test_wasi test --- Cargo.lock | 1 + crates/containerd-shim-wasmtime/Cargo.toml | 1 + .../containerd-shim-wasmtime/src/instance.rs | 20 +++++++++++++++---- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f4c9ba359..d8973db19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -509,6 +509,7 @@ dependencies = [ "chrono", "containerd-shim", "containerd-shim-wasm", + "env_logger", "libc", "libcontainer", "log", diff --git a/crates/containerd-shim-wasmtime/Cargo.toml b/crates/containerd-shim-wasmtime/Cargo.toml index c47eef6b7..0a7e0b691 100644 --- a/crates/containerd-shim-wasmtime/Cargo.toml +++ b/crates/containerd-shim-wasmtime/Cargo.toml @@ -41,6 +41,7 @@ libc = { workspace = true } tempfile = "3.0" libc = { workspace = true } pretty_assertions = "1" +env_logger = "0.10" [[bin]] name = "containerd-shim-wasmtime-v1" diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index cc8d81c91..dff6a05b7 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -221,8 +221,9 @@ impl EngineGetter for Wasi { #[cfg(test)] mod wasitest { - use std::fs::{create_dir, read_to_string, File}; + use std::fs::{create_dir, read_to_string, File, OpenOptions}; use std::io::prelude::*; + use std::os::unix::prelude::OpenOptionsExt; use std::sync::mpsc::channel; use std::time::Duration; @@ -282,6 +283,9 @@ mod wasitest { println!("running test with sudo: {}", function!()); return run_test_with_sudo(function!()); } + // start logging + let _ = env_logger::try_init(); + let dir = tempdir()?; create_dir(dir.path().join("rootfs"))?; let cfg = prepare_cfg(&dir)?; @@ -315,16 +319,23 @@ mod wasitest { } fn prepare_cfg(dir: &TempDir) -> Result> { - let mut f = File::create(dir.path().join("rootfs/hello.wat"))?; + let mut f = OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o755) + .open(dir.path().join("rootfs/hello.wat"))?; f.write_all(WASI_HELLO_WAT)?; let stdout = File::create(dir.path().join("stdout"))?; + let stderr = File::create(dir.path().join("stderr"))?; drop(stdout); + drop(stderr); let spec = SpecBuilder::default() .root(RootBuilder::default().path("rootfs").build()?) .process( ProcessBuilder::default() .cwd("/") - .args(vec!["hello.wat".to_string()]) + .args(vec!["./hello.wat".to_string()]) .build()?, ) .build()?; @@ -332,7 +343,8 @@ mod wasitest { let mut cfg = InstanceConfig::new(Engine::default(), "test_namespace".into()); let cfg = cfg .set_bundle(dir.path().to_str().unwrap().to_string()) - .set_stdout(dir.path().join("stdout").to_str().unwrap().to_string()); + .set_stdout(dir.path().join("stdout").to_str().unwrap().to_string()) + .set_stderr(dir.path().join("stderr").to_str().unwrap().to_string()); Ok(cfg.to_owned()) } } From 11c2e20a11c11d127a5ab4715803c7f6b760559a Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Wed, 28 Jun 2023 07:02:37 +0000 Subject: [PATCH 13/29] Add trace to "cargo test" in CI Signed-off-by: jiaxiao zhou --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c78c10c08..4585ea006 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ fix: .PHONY: test test: - cargo test --all --verbose + RUST_LOG=trace cargo test --all --verbose -- --nocapture .PHONY: install install: From 80cd7427d2473030ffb101fbcb8bad41fdd1fc3f Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Wed, 28 Jun 2023 22:20:10 +0000 Subject: [PATCH 14/29] Temp commit to run tests on ubuntu 22.04 only This commit runs the wasi_test tests on ubuntu 22.04 CI, and it meant to be a testing only commit. Will rollback once the experiment has finished. Signed-off-by: jiaxiao zhou --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 160ba2b52..55b641e35 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,7 +35,7 @@ jobs: build: strategy: matrix: - os: ["ubuntu-20.04", "ubuntu-22.04"] + os: ["ubuntu-22.04"] runs-on: ${{ matrix.os }} steps: - uses: Swatinem/rust-cache@v2 From ebad2827e997ad95c4790383ac1f17ed6fb05ab4 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 29 Jun 2023 22:10:39 +0000 Subject: [PATCH 15/29] Rollback ubuntu 20.02 test in CI Signed-off-by: jiaxiao zhou --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 55b641e35..160ba2b52 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,7 +35,7 @@ jobs: build: strategy: matrix: - os: ["ubuntu-22.04"] + os: ["ubuntu-20.04", "ubuntu-22.04"] runs-on: ${{ matrix.os }} steps: - uses: Swatinem/rust-cache@v2 From 560356f01d04370d71f768eb9bd3617dd367b1fd Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 29 Jun 2023 22:11:13 +0000 Subject: [PATCH 16/29] Use the same determine root logic from wasmedge shim Signed-off-by: jiaxiao zhou --- .../src/services/sandbox_ttrpc.rs | 2 +- .../containerd-shim-wasmtime/src/instance.rs | 59 +++++++++++++++---- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/crates/containerd-shim-wasm/src/services/sandbox_ttrpc.rs b/crates/containerd-shim-wasm/src/services/sandbox_ttrpc.rs index 19f31cf3f..ab3161923 100644 --- a/crates/containerd-shim-wasm/src/services/sandbox_ttrpc.rs +++ b/crates/containerd-shim-wasm/src/services/sandbox_ttrpc.rs @@ -29,7 +29,7 @@ pub struct ManagerClient { impl ManagerClient { pub fn new(client: ::ttrpc::Client) -> Self { ManagerClient { - client, + client: client, } } diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index dff6a05b7..c0a197976 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -6,6 +6,9 @@ use libcontainer::container::builder::ContainerBuilder; use libcontainer::container::{Container, ContainerStatus}; use nix::errno::Errno; use nix::sys::wait::waitid; +use serde::{Deserialize, Serialize}; +use std::fs::File; +use std::io::{ErrorKind, Read}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Condvar, Mutex}; use std::thread; @@ -25,6 +28,7 @@ use wasmtime::Engine; use crate::executor::WasmtimeExecutor; use libcontainer::signal::Signal; +static DEFAULT_CONTAINER_ROOT_DIR: &str = "/run/containerd/wasmtime"; type ExitCode = Arc<(Mutex)>>, Condvar)>; pub struct Wasi { @@ -41,11 +45,34 @@ pub struct Wasi { id: String, } -fn load_spec(bundle: String) -> Result { - let mut spec = oci::load(Path::new(&bundle).join("config.json").to_str().unwrap())?; - spec.canonicalize_rootfs(&bundle) - .map_err(|e| Error::Others(format!("error canonicalizing rootfs in spec: {}", e)))?; - Ok(spec) +#[derive(Serialize, Deserialize)] +struct Options { + root: Option, +} + +fn determine_rootdir>(bundle: P, namespace: String) -> Result { + log::info!( + "determining rootdir for bundle: {}", + bundle.as_ref().display() + ); + let mut file = match File::open(bundle.as_ref().join("options.json")) { + Ok(f) => f, + Err(err) => match err.kind() { + ErrorKind::NotFound => { + return Ok(<&str as Into>::into(DEFAULT_CONTAINER_ROOT_DIR).join(namespace)) + } + _ => return Err(err.into()), + }, + }; + let mut data = String::new(); + file.read_to_string(&mut data)?; + let options: Options = serde_json::from_str(&data)?; + let path = options + .root + .unwrap_or(PathBuf::from(DEFAULT_CONTAINER_ROOT_DIR)) + .join(namespace); + log::info!("youki root path is: {}", path.display()); + Ok(path) } impl Instance for Wasi { @@ -56,8 +83,7 @@ impl Instance for Wasi { log::info!("creating new instance: {}", id); let cfg = cfg.unwrap(); let bundle = cfg.get_bundle().unwrap_or_default(); - let spec = load_spec(bundle.clone()).unwrap(); - let rootdir = oci::get_root(&spec); + let rootdir = determine_rootdir(bundle.as_str(), cfg.get_namespace()).unwrap(); Wasi { id, exit_code: Arc::new((Mutex::new(None), Condvar::new())), @@ -269,7 +295,6 @@ mod wasitest { #[test] fn test_delete_after_create() -> Result<()> { let dir = tempdir()?; - create_dir(dir.path().join("rootfs"))?; let cfg = prepare_cfg(&dir)?; let i = Wasi::new("".to_string(), Some(&cfg)); @@ -287,7 +312,6 @@ mod wasitest { let _ = env_logger::try_init(); let dir = tempdir()?; - create_dir(dir.path().join("rootfs"))?; let cfg = prepare_cfg(&dir)?; let wasi = Wasi::new("test".to_string(), Some(&cfg)); @@ -319,13 +343,28 @@ mod wasitest { } fn prepare_cfg(dir: &TempDir) -> Result> { + create_dir(dir.path().join("rootfs"))?; + + let opts = Options { + root: Some(dir.path().join("runwasi")), + }; + let opts_file = OpenOptions::new() + .read(true) + .create(true) + .truncate(true) + .write(true) + .open(dir.path().join("options.json"))?; + write!(&opts_file, "{}", serde_json::to_string(&opts)?)?; + + let wasm_path = dir.path().join("rootfs/hello.wat"); let mut f = OpenOptions::new() .write(true) .create(true) .truncate(true) .mode(0o755) - .open(dir.path().join("rootfs/hello.wat"))?; + .open(wasm_path)?; f.write_all(WASI_HELLO_WAT)?; + let stdout = File::create(dir.path().join("stdout"))?; let stderr = File::create(dir.path().join("stderr"))?; drop(stdout); From 4b106b2e0091a60fbc369c0a8a8c898e6c0fa2a1 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 29 Jun 2023 22:16:16 +0000 Subject: [PATCH 17/29] Fixed clippy issues Signed-off-by: jiaxiao zhou --- crates/containerd-shim-wasm/src/services/sandbox_ttrpc.rs | 2 +- crates/containerd-shim-wasmtime/src/instance.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/containerd-shim-wasm/src/services/sandbox_ttrpc.rs b/crates/containerd-shim-wasm/src/services/sandbox_ttrpc.rs index ab3161923..19f31cf3f 100644 --- a/crates/containerd-shim-wasm/src/services/sandbox_ttrpc.rs +++ b/crates/containerd-shim-wasm/src/services/sandbox_ttrpc.rs @@ -29,7 +29,7 @@ pub struct ManagerClient { impl ManagerClient { pub fn new(client: ::ttrpc::Client) -> Self { ManagerClient { - client: client, + client, } } diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index c0a197976..9c465573a 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -17,7 +17,7 @@ use anyhow::Context; use chrono::{DateTime, Utc}; use containerd_shim_wasm::sandbox::error::Error; use containerd_shim_wasm::sandbox::instance::Wait; -use containerd_shim_wasm::sandbox::{oci, EngineGetter, Instance, InstanceConfig}; +use containerd_shim_wasm::sandbox::{EngineGetter, Instance, InstanceConfig}; use libc::{SIGINT, SIGKILL}; use libcontainer::syscall::syscall::create_syscall; use log::error; @@ -92,7 +92,7 @@ impl Instance for Wasi { stdout: cfg.get_stdout().unwrap_or_default(), stderr: cfg.get_stderr().unwrap_or_default(), bundle, - rootdir: rootdir.clone(), + rootdir, } } fn start(&self) -> Result { From 51182c6acd80fe193802e5657bf711e15528c837 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 29 Jun 2023 22:36:37 +0000 Subject: [PATCH 18/29] Reset stdio at the end of the tests Signed-off-by: jiaxiao zhou --- .../containerd-shim-wasmtime/src/instance.rs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index 9c465573a..7bbf67733 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -9,6 +9,7 @@ use nix::sys::wait::waitid; use serde::{Deserialize, Serialize}; use std::fs::File; use std::io::{ErrorKind, Read}; +use std::os::fd::RawFd; use std::path::{Path, PathBuf}; use std::sync::{Arc, Condvar, Mutex}; use std::thread; @@ -18,6 +19,7 @@ use chrono::{DateTime, Utc}; use containerd_shim_wasm::sandbox::error::Error; use containerd_shim_wasm::sandbox::instance::Wait; use containerd_shim_wasm::sandbox::{EngineGetter, Instance, InstanceConfig}; +use libc::{dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use libc::{SIGINT, SIGKILL}; use libcontainer::syscall::syscall::create_syscall; use log::error; @@ -31,6 +33,10 @@ use libcontainer::signal::Signal; static DEFAULT_CONTAINER_ROOT_DIR: &str = "/run/containerd/wasmtime"; type ExitCode = Arc<(Mutex)>>, Condvar)>; +static mut STDIN_FD: Option = None; +static mut STDOUT_FD: Option = None; +static mut STDERR_FD: Option = None; + pub struct Wasi { exit_code: ExitCode, engine: wasmtime::Engine, @@ -45,6 +51,20 @@ pub struct Wasi { id: String, } +pub fn reset_stdio() { + unsafe { + if STDIN_FD.is_some() { + dup2(STDIN_FD.unwrap(), STDIN_FILENO); + } + if STDOUT_FD.is_some() { + dup2(STDOUT_FD.unwrap(), STDOUT_FILENO); + } + if STDERR_FD.is_some() { + dup2(STDERR_FD.unwrap(), STDERR_FILENO); + } + } +} + #[derive(Serialize, Deserialize)] struct Options { root: Option, @@ -299,6 +319,7 @@ mod wasitest { let i = Wasi::new("".to_string(), Some(&cfg)); i.delete()?; + reset_stdio(); Ok(()) } @@ -339,6 +360,7 @@ mod wasitest { wasi.delete()?; + reset_stdio(); Ok(()) } From 7ca42a1ab7b06b6ee15dbec0701d46eec277e2da Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 29 Jun 2023 22:55:54 +0000 Subject: [PATCH 19/29] Need rollback: commenting out the stdio thing Signed-off-by: jiaxiao zhou --- .../containerd-shim-wasmtime/src/executor.rs | 65 ++++++++----------- 1 file changed, 28 insertions(+), 37 deletions(-) diff --git a/crates/containerd-shim-wasmtime/src/executor.rs b/crates/containerd-shim-wasmtime/src/executor.rs index 443b4c2ed..e43727a40 100644 --- a/crates/containerd-shim-wasmtime/src/executor.rs +++ b/crates/containerd-shim-wasmtime/src/executor.rs @@ -1,8 +1,9 @@ use std::{fs::OpenOptions, os::fd::RawFd, path::PathBuf}; +use nix::unistd::{dup, dup2}; use anyhow::{anyhow, Result}; use containerd_shim_wasm::sandbox::oci; -use libc::{dup, dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; +use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use libcontainer::workload::{Executor, ExecutorError}; use oci_spec::runtime::Spec; @@ -13,10 +14,6 @@ use crate::oci_wasmtime::{self, wasi_dir, wasi_file}; const EXECUTOR_NAME: &str = "wasmtime"; -static mut STDIN_FD: Option = None; -static mut STDOUT_FD: Option = None; -static mut STDERR_FD: Option = None; - pub struct WasmtimeExecutor { pub stdin: Option, pub stdout: Option, @@ -67,38 +64,32 @@ impl WasmtimeExecutor { .envs(env.as_slice())? .preopened_dir(path, "/")?; - if let Some(stdin) = self.stdin { - unsafe { - STDIN_FD = Some(dup(STDIN_FILENO)); - dup2(stdin, STDIN_FILENO); - } - } - if let Some(stdout) = self.stdout { - unsafe { - STDOUT_FD = Some(dup(STDOUT_FILENO)); - dup2(stdout, STDOUT_FILENO); - } - } - if let Some(stderr) = self.stderr { - unsafe { - STDERR_FD = Some(dup(STDERR_FILENO)); - dup2(stderr, STDERR_FILENO); - } - } - log::info!("opening stdin"); - let stdin_path = PathBuf::from("/dev/stdin"); - let stdin_wasi_file = wasi_file(stdin_path, OpenOptions::new().read(true))?; - wasi_builder = wasi_builder.stdin(Box::new(stdin_wasi_file)); - - log::info!("opening stdout"); - let stdout_path = PathBuf::from("/dev/stdout"); - let stdout_wasi_file = wasi_file(stdout_path, OpenOptions::new().write(true))?; - wasi_builder = wasi_builder.stdout(Box::new(stdout_wasi_file)); - - log::info!("opening stderr"); - let stderr_path = PathBuf::from("/dev/stderr"); - let stderr_wasi_file = wasi_file(stderr_path, OpenOptions::new().write(true))?; - wasi_builder = wasi_builder.stderr(Box::new(stderr_wasi_file)); + // if let Some(stdin) = self.stdin { + // let _ = dup(STDIN_FILENO); + // let _ = dup2(stdin, STDIN_FILENO); + // } + // if let Some(stdout) = self.stdout { + // let _ = dup(STDOUT_FILENO); + // let _ = dup2(stdout, STDOUT_FILENO); + // } + // if let Some(stderr) = self.stderr { + // let _ = dup(STDERR_FILENO); + // let _ = dup2(stderr, STDERR_FILENO); + // } + // log::info!("opening stdin"); + // let stdin_path = PathBuf::from("/dev/stdin"); + // let stdin_wasi_file = wasi_file(stdin_path, OpenOptions::new().read(true))?; + // wasi_builder = wasi_builder.stdin(Box::new(stdin_wasi_file)); + + // log::info!("opening stdout"); + // let stdout_path = PathBuf::from("/dev/stdout"); + // let stdout_wasi_file = wasi_file(stdout_path, OpenOptions::new().write(true))?; + // wasi_builder = wasi_builder.stdout(Box::new(stdout_wasi_file)); + + // log::info!("opening stderr"); + // let stderr_path = PathBuf::from("/dev/stderr"); + // let stderr_wasi_file = wasi_file(stderr_path, OpenOptions::new().write(true))?; + // wasi_builder = wasi_builder.stderr(Box::new(stderr_wasi_file)); log::info!("building wasi context"); let wctx = wasi_builder.build(); From 8d5c858199955dc602d65dc3cfa165fc78e2ad1e Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 29 Jun 2023 23:08:19 +0000 Subject: [PATCH 20/29] Use inherit_stdio() Signed-off-by: jiaxiao zhou --- .../containerd-shim-wasmtime/src/executor.rs | 45 +++++++------------ 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/crates/containerd-shim-wasmtime/src/executor.rs b/crates/containerd-shim-wasmtime/src/executor.rs index e43727a40..79be0db56 100644 --- a/crates/containerd-shim-wasmtime/src/executor.rs +++ b/crates/containerd-shim-wasmtime/src/executor.rs @@ -1,5 +1,5 @@ -use std::{fs::OpenOptions, os::fd::RawFd, path::PathBuf}; use nix::unistd::{dup, dup2}; +use std::{fs::OpenOptions, os::fd::RawFd}; use anyhow::{anyhow, Result}; use containerd_shim_wasm::sandbox::oci; @@ -10,7 +10,7 @@ use oci_spec::runtime::Spec; use wasmtime::{Engine, Linker, Module, Store}; use wasmtime_wasi::WasiCtxBuilder; -use crate::oci_wasmtime::{self, wasi_dir, wasi_file}; +use crate::oci_wasmtime::{self, wasi_dir}; const EXECUTOR_NAME: &str = "wasmtime"; @@ -59,37 +59,24 @@ impl WasmtimeExecutor { log::info!("setting up wasi"); let path = wasi_dir(".", OpenOptions::new().read(true))?; - let mut wasi_builder = WasiCtxBuilder::new() + let wasi_builder = WasiCtxBuilder::new() .args(args)? .envs(env.as_slice())? + .inherit_stdio() .preopened_dir(path, "/")?; - // if let Some(stdin) = self.stdin { - // let _ = dup(STDIN_FILENO); - // let _ = dup2(stdin, STDIN_FILENO); - // } - // if let Some(stdout) = self.stdout { - // let _ = dup(STDOUT_FILENO); - // let _ = dup2(stdout, STDOUT_FILENO); - // } - // if let Some(stderr) = self.stderr { - // let _ = dup(STDERR_FILENO); - // let _ = dup2(stderr, STDERR_FILENO); - // } - // log::info!("opening stdin"); - // let stdin_path = PathBuf::from("/dev/stdin"); - // let stdin_wasi_file = wasi_file(stdin_path, OpenOptions::new().read(true))?; - // wasi_builder = wasi_builder.stdin(Box::new(stdin_wasi_file)); - - // log::info!("opening stdout"); - // let stdout_path = PathBuf::from("/dev/stdout"); - // let stdout_wasi_file = wasi_file(stdout_path, OpenOptions::new().write(true))?; - // wasi_builder = wasi_builder.stdout(Box::new(stdout_wasi_file)); - - // log::info!("opening stderr"); - // let stderr_path = PathBuf::from("/dev/stderr"); - // let stderr_wasi_file = wasi_file(stderr_path, OpenOptions::new().write(true))?; - // wasi_builder = wasi_builder.stderr(Box::new(stderr_wasi_file)); + if let Some(stdin) = self.stdin { + let _ = dup(STDIN_FILENO); + let _ = dup2(stdin, STDIN_FILENO); + } + if let Some(stdout) = self.stdout { + let _ = dup(STDOUT_FILENO); + let _ = dup2(stdout, STDOUT_FILENO); + } + if let Some(stderr) = self.stderr { + let _ = dup(STDERR_FILENO); + let _ = dup2(stderr, STDERR_FILENO); + } log::info!("building wasi context"); let wctx = wasi_builder.build(); From 65177c33d8a72aba2c1ef12fed25d39136441a92 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Fri, 30 Jun 2023 00:05:17 +0000 Subject: [PATCH 21/29] Setup CI build env for wasmtimie e2e Signed-off-by: jiaxiao zhou --- .github/workflows/ci.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 160ba2b52..3f10e842a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,8 +79,13 @@ jobs: - uses: actions/checkout@v3 - name: setup rust-wasm target run: rustup target add wasm32-wasi + - name: Setup OCI runtime build env + run: | + sudo apt -y update + sudo apt install -y pkg-config libsystemd-dev libdbus-glib-1-dev build-essential libelf-dev libseccomp-dev libclang-dev - name: run - run: make test/k8s + run: | + make test/k8s - name: cleanup if: always() run: make test/k8s/clean From bb1e940e921bc3b39bb45763abaa5bfe852d2010 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Fri, 30 Jun 2023 22:50:47 +0000 Subject: [PATCH 22/29] Add inpsection step to e2e-wasmtime Signed-off-by: jiaxiao zhou --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3f10e842a..3679e2c86 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -86,6 +86,12 @@ jobs: - name: run run: | make test/k8s + # only runs when the previous step fails + - name: inspect failed pods + if: failure() + run: | + kubectl get pods --all-namespaces + kubectl describe pods --all-namespaces - name: cleanup if: always() run: make test/k8s/clean From e120093db744cb8c23f8d935fc873bb494c1fbaf Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Wed, 5 Jul 2023 16:37:03 +0000 Subject: [PATCH 23/29] Add deps to kind nodes that run k8s tests Signed-off-by: jiaxiao zhou --- test/k8s/Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/test/k8s/Dockerfile b/test/k8s/Dockerfile index e95149eab..7cffedfbc 100644 --- a/test/k8s/Dockerfile +++ b/test/k8s/Dockerfile @@ -33,6 +33,7 @@ FROM scratch AS kind COPY --from=kind-fetch /root/kind /kind FROM kind-base +RUN apt-get update && apt-get install --no-install-recommends -y build-essential git clang pkg-config libsystemd-dev libdbus-glib-1-dev build-essential libelf-dev libseccomp-dev libclang-dev RUN <> /etc/containerd/config.toml From f7472fb069a589987845d6fd5231749899b4281a Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 6 Jul 2023 21:53:09 +0000 Subject: [PATCH 24/29] Handle Result types from dup and dup2 in both wasmtime and wasmedge shims Signed-off-by: jiaxiao zhou --- .../containerd-shim-wasmedge/src/executor.rs | 60 +++++++++---------- .../containerd-shim-wasmtime/src/executor.rs | 12 ++-- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/crates/containerd-shim-wasmedge/src/executor.rs b/crates/containerd-shim-wasmedge/src/executor.rs index 5bbac9663..21ab3bf76 100644 --- a/crates/containerd-shim-wasmedge/src/executor.rs +++ b/crates/containerd-shim-wasmedge/src/executor.rs @@ -28,67 +28,67 @@ impl Executor for WasmEdgeExecutor { return Err(ExecutorError::InvalidArg); } + let vm = self + .prepare(args, spec) + .map_err(|err| ExecutorError::Other(format!("failed to prepare function: {}", err)))?; + + // TODO: How to get exit code? + // This was relatively straight forward in go, but wasi and wasmtime are totally separate things in rust + match vm.run_func(Some("main"), "_start", params!()) { + Ok(_) => std::process::exit(0), + Err(_) => std::process::exit(137), + }; + } + + fn can_handle(&self, _spec: &Spec) -> bool { + true + } + + fn name(&self) -> &'static str { + EXECUTOR_NAME + } +} + +impl WasmEdgeExecutor { + fn prepare(&self, args: &[String], spec: &Spec) -> anyhow::Result { let mut cmd = args[0].clone(); if let Some(stripped) = args[0].strip_prefix(std::path::MAIN_SEPARATOR) { cmd = stripped.to_string(); } let envs = env_to_wasi(spec); - - // create configuration with `wasi` option enabled let config = ConfigBuilder::new(CommonConfigOptions::default()) .with_host_registration_config(HostRegistrationConfigOptions::default().wasi(true)) .build() .map_err(|err| ExecutorError::Execution(err))?; - - // create a vm with the config settings let mut vm = VmBuilder::new() .with_config(config) .build() .map_err(|err| ExecutorError::Execution(err))?; - - // initialize the wasi module with the parsed parameters let wasi_module = vm .wasi_module_mut() .ok_or_else(|| anyhow::Error::msg("Not found wasi module")) .map_err(|err| ExecutorError::Execution(err.into()))?; - wasi_module.initialize( Some(args.iter().map(|s| s as &str).collect()), Some(envs.iter().map(|s| s as &str).collect()), None, ); - let vm = vm .register_module_from_file("main", cmd) .map_err(|err| ExecutorError::Execution(err))?; - if let Some(stdin) = self.stdin { - let _ = dup(STDIN_FILENO); - let _ = dup2(stdin, STDIN_FILENO); + dup(STDIN_FILENO)?; + dup2(stdin, STDIN_FILENO)?; } if let Some(stdout) = self.stdout { - let _ = dup(STDOUT_FILENO); - let _ = dup2(stdout, STDOUT_FILENO); + dup(STDOUT_FILENO)?; + dup2(stdout, STDOUT_FILENO)?; } if let Some(stderr) = self.stderr { - let _ = dup(STDERR_FILENO); - let _ = dup2(stderr, STDERR_FILENO); + dup(STDERR_FILENO)?; + dup2(stderr, STDERR_FILENO)?; } - - // TODO: How to get exit code? - // This was relatively straight forward in go, but wasi and wasmtime are totally separate things in rust - match vm.run_func(Some("main"), "_start", params!()) { - Ok(_) => std::process::exit(0), - Err(_) => std::process::exit(137), - }; - } - - fn can_handle(&self, _spec: &Spec) -> bool { - true - } - - fn name(&self) -> &'static str { - EXECUTOR_NAME + Ok(vm) } } diff --git a/crates/containerd-shim-wasmtime/src/executor.rs b/crates/containerd-shim-wasmtime/src/executor.rs index 79be0db56..f5a5f90d4 100644 --- a/crates/containerd-shim-wasmtime/src/executor.rs +++ b/crates/containerd-shim-wasmtime/src/executor.rs @@ -66,16 +66,16 @@ impl WasmtimeExecutor { .preopened_dir(path, "/")?; if let Some(stdin) = self.stdin { - let _ = dup(STDIN_FILENO); - let _ = dup2(stdin, STDIN_FILENO); + dup(STDIN_FILENO)?; + dup2(stdin, STDIN_FILENO)?; } if let Some(stdout) = self.stdout { - let _ = dup(STDOUT_FILENO); - let _ = dup2(stdout, STDOUT_FILENO); + dup(STDOUT_FILENO)?; + dup2(stdout, STDOUT_FILENO)?; } if let Some(stderr) = self.stderr { - let _ = dup(STDERR_FILENO); - let _ = dup2(stderr, STDERR_FILENO); + dup(STDERR_FILENO)?; + dup2(stderr, STDERR_FILENO)?; } log::info!("building wasi context"); From d807601cd358278a49d4c7d38423782c4d18cadd Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 6 Jul 2023 22:10:10 +0000 Subject: [PATCH 25/29] Small refactoring Signed-off-by: jiaxiao zhou --- crates/containerd-shim-wasmtime/src/instance.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index 7bbf67733..c08fa816f 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -117,7 +117,7 @@ impl Instance for Wasi { } fn start(&self) -> Result { log::info!("starting instance: {}", self.id); - let engine = self.engine.clone(); + let engine: Engine = self.engine.clone(); let mut container = self.build_container( self.stdin.as_str(), @@ -260,8 +260,7 @@ impl Wasi { impl EngineGetter for Wasi { type E = wasmtime::Engine; fn new_engine() -> Result { - let engine = Engine::default(); - Ok(engine) + Ok(Engine::default()) } } From 3f31397c7e4cd5837b85a278569fd02570898b54 Mon Sep 17 00:00:00 2001 From: Jiaxiao Zhou Date: Thu, 6 Jul 2023 15:21:03 -0700 Subject: [PATCH 26/29] Update crates/containerd-shim-wasmtime/src/executor.rs Co-authored-by: Dan Chiarlone Signed-off-by: Jiaxiao Zhou --- crates/containerd-shim-wasmtime/src/executor.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/containerd-shim-wasmtime/src/executor.rs b/crates/containerd-shim-wasmtime/src/executor.rs index 79be0db56..62d55bf6b 100644 --- a/crates/containerd-shim-wasmtime/src/executor.rs +++ b/crates/containerd-shim-wasmtime/src/executor.rs @@ -100,12 +100,12 @@ impl WasmtimeExecutor { let mut store = Store::new(&self.engine, wctx); log::info!("instantiating instance"); - let i = linker.instantiate(&mut store, &module)?; + let instance = linker.instantiate(&mut store, &module)?; log::info!("getting start function"); - let f = i + let start_func = instance .get_func(&mut store, method) - .ok_or_else(|| anyhow!("module does not have a wasi start function".to_string()))?; - Ok((store, f)) + .ok_or_else(|| anyhow!("module does not have a WASI start function".to_string()))?; + Ok((store, start_func)) } } From 729c3b4f49c7c22c2e17429d67961594e2158885 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 6 Jul 2023 22:26:35 +0000 Subject: [PATCH 27/29] Add a log to when Errno::ECHILD was returned Signed-off-by: jiaxiao zhou --- crates/containerd-shim-wasmedge/src/instance.rs | 1 + crates/containerd-shim-wasmtime/src/instance.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index eb71244c1..33609343a 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -146,6 +146,7 @@ impl Instance for Wasi { Ok(_) => 0, Err(e) => { if e == Errno::ECHILD { + log::info!("no child process"); 0 } else { panic!("waitpid failed: {}", e); diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index c08fa816f..5b5be474b 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -143,6 +143,7 @@ impl Instance for Wasi { Ok(_) => 0, Err(e) => { if e == Errno::ECHILD { + log::info!("no child process"); 0 } else { panic!("waitpid failed: {}", e); From aaaea73f5d34659245b49fdce4dd6df78ba180e6 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Sun, 9 Jul 2023 19:55:36 +0000 Subject: [PATCH 28/29] Resolve comments Signed-off-by: jiaxiao zhou --- crates/containerd-shim-wasmtime/src/executor.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/containerd-shim-wasmtime/src/executor.rs b/crates/containerd-shim-wasmtime/src/executor.rs index 92f3a3d83..135ed1188 100644 --- a/crates/containerd-shim-wasmtime/src/executor.rs +++ b/crates/containerd-shim-wasmtime/src/executor.rs @@ -1,7 +1,7 @@ use nix::unistd::{dup, dup2}; use std::{fs::OpenOptions, os::fd::RawFd}; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Result, Context}; use containerd_shim_wasm::sandbox::oci; use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use libcontainer::workload::{Executor, ExecutorError}; @@ -24,7 +24,7 @@ pub struct WasmtimeExecutor { impl Executor for WasmtimeExecutor { fn exec(&self, spec: &Spec) -> Result<(), ExecutorError> { let args = oci::get_args(spec); - if args.is_empty() { + if args.len() != 1 { return Err(ExecutorError::InvalidArg); } @@ -82,8 +82,10 @@ impl WasmtimeExecutor { let wctx = wasi_builder.build(); log::info!("wasi context ready"); - let start = args[0].clone(); - let mut iterator = start.split('#'); + let mut iterator = args + .first() + .context("args must have at least one argument.")? + .split('#'); let mut cmd = iterator.next().unwrap().to_string(); let stripped = cmd.strip_prefix(std::path::MAIN_SEPARATOR); if let Some(strpd) = stripped { From dfc680daa6bd5036323d6fddaf762eab2acd900c Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Sun, 9 Jul 2023 20:08:44 +0000 Subject: [PATCH 29/29] Rustfmt Signed-off-by: jiaxiao zhou --- crates/containerd-shim-wasmtime/src/executor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/containerd-shim-wasmtime/src/executor.rs b/crates/containerd-shim-wasmtime/src/executor.rs index 135ed1188..cf21ffeb4 100644 --- a/crates/containerd-shim-wasmtime/src/executor.rs +++ b/crates/containerd-shim-wasmtime/src/executor.rs @@ -1,7 +1,7 @@ use nix::unistd::{dup, dup2}; use std::{fs::OpenOptions, os::fd::RawFd}; -use anyhow::{anyhow, Result, Context}; +use anyhow::{anyhow, Context, Result}; use containerd_shim_wasm::sandbox::oci; use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use libcontainer::workload::{Executor, ExecutorError};