From 99f1015df15039d4905e5b35f6e1ef58eda780f8 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 3 Aug 2023 01:31:35 +0000 Subject: [PATCH 01/11] refactor: add a new YoukiInstance for default implementation of Instance trait This commits adds a new YoukiInstance trait in the containerd-shim-wasm crate. It refactors the common youki Instance implementation into this trait and as a consequence, all the Instance implementation from both the wasmtime and wasmedge shims moved to a common place. This closes #131 Signed-off-by: jiaxiao zhou --- Cargo.lock | 214 +++++++++++++++++- Cargo.toml | 2 +- crates/containerd-shim-wasm/Cargo.toml | 10 +- .../src/sandbox/instance.rs | 151 +++++++++++- crates/containerd-shim-wasmedge/Cargo.toml | 2 +- .../containerd-shim-wasmedge/src/instance.rs | 202 ++++++----------- crates/containerd-shim-wasmtime/Cargo.toml | 2 +- .../containerd-shim-wasmtime/src/instance.rs | 174 ++++---------- 8 files changed, 472 insertions(+), 285 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e607ed2cc..e79f4654c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -128,6 +128,12 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e2d098ff73c1ca148721f37baad5ea6a465a13f9573aba8641fbbbae8164a54e" +[[package]] +name = "ascii" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ae7d751998c189c1d4468cf0a39bb2eae052a9c58d50ebb3b9591ee3813ad50" + [[package]] name = "async-trait" version = "0.1.71" @@ -160,6 +166,12 @@ dependencies = [ "rustc-demangle", ] +[[package]] +name = "base-x" +version = "0.2.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cbbc9d0964165b47557570cce6c952866c2678457aca742aafc9fb771d30270" + [[package]] name = "base64" version = "0.21.2" @@ -440,6 +452,16 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" +[[package]] +name = "combine" +version = "2.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1645a65a99c7c8d345761f4b75a6ffe5be3b3b27a93ee731fccc5050ba6be97c" +dependencies = [ + "ascii", + "byteorder", +] + [[package]] name = "command-fds" version = "0.2.2" @@ -450,6 +472,12 @@ dependencies = [ "thiserror", ] +[[package]] +name = "const_fn" +version = "0.4.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fbdcdcb6d86f71c5e97409ad45898af11cbc995b4ee8112d59095a28d376c935" + [[package]] name = "containerd-shim" version = "0.3.0" @@ -474,7 +502,7 @@ dependencies = [ "serde_json", "signal-hook", "thiserror", - "time", + "time 0.3.23", "windows-sys 0.48.0", ] @@ -500,6 +528,7 @@ dependencies = [ "containerd-shim", "env_logger", "libc", + "libcontainer", "log", "nix 0.26.2", "oci-spec", @@ -927,6 +956,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "discard" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "212d0f5754cb6769937f4501cc0e67f4f4483c8d2c3e1e922ee9edbe4ab4c7c0" + [[package]] name = "either" version = "1.8.1" @@ -1481,6 +1516,18 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "884e2677b40cc8c339eaefcb701c32ef1fd2493d71118dc0ca4b6a736c93bd67" +[[package]] +name = "libbpf-sys" +version = "1.2.1+v1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75adb4021282a72ca63ebbc0e4247750ad74ede68ff062d247691072d709ad8b" +dependencies = [ + "cc", + "nix 0.26.2", + "num_cpus", + "pkg-config", +] + [[package]] name = "libc" version = "0.2.147" @@ -1494,10 +1541,14 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "12f6fef16f505466473eeeee906244e03a437beaf41ccd85c39355b4077890c9" dependencies = [ "dbus", + "errno", "fixedbitset 0.4.2", + "libbpf-sys", + "libc", "nix 0.26.2", "oci-spec", "procfs", + "rbpf", "serde", "thiserror", "tracing", @@ -1993,6 +2044,12 @@ dependencies = [ "version_check", ] +[[package]] +name = "proc-macro-hack" +version = "0.5.20+deprecated" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068" + [[package]] name = "proc-macro2" version = "1.0.65" @@ -2224,6 +2281,18 @@ dependencies = [ "num_cpus", ] +[[package]] +name = "rbpf" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b536dc5c7e3a730d06c578a41df1fbcccd66240a7a9bd5f150a0826291f01c66" +dependencies = [ + "byteorder", + "combine", + "libc", + "time 0.2.27", +] + [[package]] name = "redox_syscall" version = "0.2.16" @@ -2319,6 +2388,15 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" +[[package]] +name = "rustc_version" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a" +dependencies = [ + "semver 0.9.0", +] + [[package]] name = "rustix" version = "0.36.15" @@ -2389,12 +2467,27 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" +[[package]] +name = "semver" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d7eb9ef2c18661902cc47e535f9bc51b78acd254da71d375c2f6720d9a40403" +dependencies = [ + "semver-parser", +] + [[package]] name = "semver" version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b0293b4b29daaf487284529cc2f5675b8e57c61f70167ba415a463651fd6a918" +[[package]] +name = "semver-parser" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" + [[package]] name = "serde" version = "1.0.179" @@ -2451,6 +2544,21 @@ dependencies = [ "syn 2.0.26", ] +[[package]] +name = "sha1" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1da05c97445caa12d05e848c4a4fcbbea29e748ac28f7e80e9b010392063770" +dependencies = [ + "sha1_smol", +] + +[[package]] +name = "sha1_smol" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae1a47186c03a32177042e55dbc5fd5aee900b8e0069a8d70fba96a9375cd012" + [[package]] name = "sha2" version = "0.10.7" @@ -2542,12 +2650,70 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" +[[package]] +name = "standback" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e113fb6f3de07a243d434a56ec6f186dfd51cb08448239fe7bcae73f87ff28ff" +dependencies = [ + "version_check", +] + [[package]] name = "static_assertions" version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" +[[package]] +name = "stdweb" +version = "0.4.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d022496b16281348b52d0e30ae99e01a73d737b2f45d38fed4edf79f9325a1d5" +dependencies = [ + "discard", + "rustc_version", + "stdweb-derive", + "stdweb-internal-macros", + "stdweb-internal-runtime", + "wasm-bindgen", +] + +[[package]] +name = "stdweb-derive" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c87a60a40fccc84bef0652345bbbbbe20a605bf5d0ce81719fc476f5c03b50ef" +dependencies = [ + "proc-macro2", + "quote", + "serde", + "serde_derive", + "syn 1.0.109", +] + +[[package]] +name = "stdweb-internal-macros" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "58fa5ff6ad0d98d1ffa8cb115892b6e69d67799f6763e162a1c9db421dc22e11" +dependencies = [ + "base-x", + "proc-macro2", + "quote", + "serde", + "serde_derive", + "serde_json", + "sha1", + "syn 1.0.109", +] + +[[package]] +name = "stdweb-internal-runtime" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "213701ba3370744dcd1a12960caa4843b3d68b4d1c0a5d575e0d65b2ee9d16c0" + [[package]] name = "strsim" version = "0.10.0" @@ -2651,6 +2817,21 @@ dependencies = [ "syn 2.0.26", ] +[[package]] +name = "time" +version = "0.2.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4752a97f8eebd6854ff91f1c1824cd6160626ac4bd44287f7f4ea2035a02a242" +dependencies = [ + "const_fn", + "libc", + "standback", + "stdweb", + "time-macros 0.1.1", + "version_check", + "winapi", +] + [[package]] name = "time" version = "0.3.23" @@ -2659,7 +2840,7 @@ checksum = "59e399c068f43a5d116fedaf73b203fa4f9c519f17e2b34f63221d3792f81446" dependencies = [ "serde", "time-core", - "time-macros", + "time-macros 0.2.10", ] [[package]] @@ -2668,6 +2849,16 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7300fbefb4dadc1af235a9cef3737cea692a9d97e1b9cbcd4ebdae6f8868e6fb" +[[package]] +name = "time-macros" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "957e9c6e26f12cb6d0dd7fc776bb67a706312e7299aed74c8dd5b17ebb27e2f1" +dependencies = [ + "proc-macro-hack", + "time-macros-impl", +] + [[package]] name = "time-macros" version = "0.2.10" @@ -2677,6 +2868,19 @@ dependencies = [ "time-core", ] +[[package]] +name = "time-macros-impl" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fd3c141a1b43194f3f56a1411225df8646c55781d5f26db825b3d98507eb482f" +dependencies = [ + "proc-macro-hack", + "proc-macro2", + "quote", + "standback", + "syn 1.0.109", +] + [[package]] name = "tinyvec" version = "1.6.0" @@ -3096,7 +3300,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "29e3ac9b780c7dda0cac7a52a5d6d2d6707cc6e3451c9db209b6c758f40d7acb" dependencies = [ "indexmap 1.9.3", - "semver", + "semver 1.0.18", ] [[package]] @@ -3106,7 +3310,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "76c956109dcb41436a39391139d9b6e2d0a5e0b158e1293ef352ec977e5e36c5" dependencies = [ "indexmap 2.0.0", - "semver", + "semver 1.0.18", ] [[package]] @@ -3726,7 +3930,7 @@ dependencies = [ "indexmap 1.9.3", "log", "pulldown-cmark", - "semver", + "semver 1.0.18", "unicode-xid", "url", ] diff --git a/Cargo.toml b/Cargo.toml index 48e2eae8b..cd3883276 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ thiserror = "1.0" libc = "0.2.147" oci-spec = { version = "0.6.1", features = ["runtime"] } sha256 = "1.2.2" -libcontainer = "0.1" +libcontainer = { version = "0.1", default-features = false } [profile.release] panic = "abort" diff --git a/crates/containerd-shim-wasm/Cargo.toml b/crates/containerd-shim-wasm/Cargo.toml index 64b5cb183..0f4cda5f4 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 } [build-dependencies] ttrpc-codegen = { version = "0.4.2", optional = true } @@ -40,6 +41,11 @@ env_logger = { workspace = true } rand = "0.8" [features] -default = [] +default = ["libcontainer/default"] generate_bindings = ["ttrpc-codegen"] -generate_doc = [] \ No newline at end of file +generate_doc = [] +libseccomp = ["libcontainer/libseccomp"] +systemd = ["libcontainer/systemd"] +v2 = ["libcontainer/v2"] +v1 = ["libcontainer/v1"] +cgroupsv2_devices = ["libcontainer/cgroupsv2_devices"] \ No newline at end of file diff --git a/crates/containerd-shim-wasm/src/sandbox/instance.rs b/crates/containerd-shim-wasm/src/sandbox/instance.rs index ce0af303b..b1ff4762e 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance.rs @@ -1,16 +1,26 @@ //! Abstractions for running/managing a wasm/wasi instance. +use std::path::PathBuf; use std::sync::mpsc::Sender; use std::sync::{Arc, Condvar, Mutex}; use std::thread; +use anyhow::Context; use libc::{SIGINT, SIGKILL, SIGTERM}; use chrono::{DateTime, Utc}; +use libcontainer::container::{Container, ContainerStatus}; +use libcontainer::signal::Signal; +use log::error; +use nix::errno::Errno; +use nix::sys::wait::waitid; +use nix::sys::wait::{Id as WaitID, WaitPidFlag, WaitStatus}; + +use crate::sandbox::instance_utils::{get_instance_root, instance_exists}; use super::error::Error; -type ExitCode = (Mutex)>>, Condvar); +type ExitCode = Arc<(Mutex)>>, Condvar)>; /// Generic options builder for creating a wasm instance. /// This is passed to the `Instance::new` method. @@ -142,6 +152,141 @@ pub trait Instance { fn wait(&self, waiter: &Wait) -> Result<(), Error>; } +pub trait YoukiInstance: Instance { + /// Get the exit code of the instance + fn get_exit_code(&self) -> ExitCode; + + /// Get the ID of the instance + fn get_id(&self) -> String; + + /// Get the root directory of the instance + fn get_root_dir(&self) -> Result; + + /// Build the container + fn build_container(&self) -> Result; + + /// 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_youki(&self) -> Result { + let id = self.get_id(); + log::info!("starting instance: {}", id); + + let mut container = self.build_container()?; + let code = self.get_exit_code(); + let pid = container.pid().context("failed to get pid")?; + + 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 { + log::info!("no child process"); + 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) + } + + /// Send a signal to the instance + fn kill_youki(&self, signal: u32) -> Result<(), Error> { + let id = self.get_id(); + let root_dir = self.get_root_dir()?; + log::info!("killing instance: {}", id.clone()); + if signal as i32 != SIGKILL && signal as i32 != SIGINT { + return Err(Error::InvalidArgument( + "only SIGKILL and SIGINT are supported".to_string(), + )); + } + let container_root = get_instance_root(root_dir, id.as_str())?; + let mut container = Container::load(container_root).with_context(|| { + format!("could not load state for container {id}", id = 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())) + } + } + } + + /// Delete any reference to the instance + /// This is called after the instance has exited. + fn delete_youki(&self) -> Result<(), Error> { + let id = self.get_id(); + let root_dir = self.get_root_dir()?; + log::info!("deleting instance: {}", id.clone()); + match instance_exists(&root_dir, id.as_str()) { + Ok(exists) => { + if !exists { + return Ok(()); + } + } + Err(err) => { + error!("could not find the container, skipping cleanup: {}", err); + return Ok(()); + } + } + let container_root = get_instance_root(&root_dir, id.as_str())?; + let container = Container::load(container_root).with_context(|| { + format!( + "could not load state for container {id}", + id = id.clone().as_str() + ) + }); + match container { + Ok(mut container) => container.delete(true).map_err(|err| { + Error::Any(anyhow::anyhow!( + "failed to delete container {}: {}", + id, + err + )) + })?, + Err(err) => { + error!("could not find the container, skipping cleanup: {}", err); + return Ok(()); + } + } + Ok(()) + } + + /// 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 + /// set_up_exit_code_wait() implemented by Wait to set up exit code + /// processing. Note that the "wait" function doesn't block, but + /// it sets up the waiting channel. + fn wait_youki(&self, waiter: &Wait) -> Result<(), Error> { + let id = self.get_id(); + let exit_code = self.get_exit_code(); + log::info!("waiting for instance: {}", id); + let code = exit_code; + waiter.set_up_exit_code_wait(code) + } +} + /// This is used for waiting for the container process to exit and deliver the exit code to the caller. /// Since the shim needs to provide the caller the process exit code, this struct wraps the required /// thread setup to make the shims simpler. @@ -159,7 +304,7 @@ impl Wait { /// code. When the child process exits, the shim will use the ExitCode /// to signal the exit status to the caller. This function returns so that /// the wait() function in the shim implementation API would not block. - pub fn set_up_exit_code_wait(&self, exit_code: Arc) -> Result<(), Error> { + pub fn set_up_exit_code_wait(&self, exit_code: ExitCode) -> Result<(), Error> { let sender = self.tx.clone(); let code = Arc::clone(&exit_code); thread::spawn(move || { @@ -180,7 +325,7 @@ impl Wait { pub struct Nop { /// Since we are faking the container, we need to keep track of the "exit" code/time /// We'll just mark it as exited when kill is called. - exit_code: Arc, + exit_code: ExitCode, } impl Instance for Nop { diff --git a/crates/containerd-shim-wasmedge/Cargo.toml b/crates/containerd-shim-wasmedge/Cargo.toml index a3de71e61..10acc0301 100644 --- a/crates/containerd-shim-wasmedge/Cargo.toml +++ b/crates/containerd-shim-wasmedge/Cargo.toml @@ -18,7 +18,7 @@ serde = { workspace = true } serde_json = { workspace = true } nix = { workspace = true } libc = { workspace = true } -libcontainer = { workspace = true } +libcontainer = { workspace = true, features = ["default"] } [dev-dependencies] tempfile = "3.7" diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index 52ec2da82..387c93e46 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -3,22 +3,15 @@ use std::io::prelude::*; use std::io::ErrorKind; use std::os::unix::io::RawFd; use std::sync::{Arc, Condvar, Mutex}; -use std::thread; use anyhow::Context; -use anyhow::{anyhow, Result}; +use 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::instance::YoukiInstance; +use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio; use containerd_shim_wasm::sandbox::{EngineGetter, Instance, InstanceConfig}; -use libc::{dup2, SIGINT, SIGKILL, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; -use log::{debug, error}; -use nix::errno::Errno; -use nix::sys::signal::Signal as NixSignal; -use nix::sys::wait::{waitid, Id as WaitID, WaitPidFlag, WaitStatus}; +use libc::{dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use nix::unistd::close; use serde::{Deserialize, Serialize}; use wasmedge_sdk::{ @@ -33,8 +26,7 @@ use std::{ }; use libcontainer::container::builder::ContainerBuilder; -use libcontainer::container::{Container, ContainerStatus}; -use libcontainer::signal::Signal; +use libcontainer::container::Container; use libcontainer::syscall::syscall::create_syscall; use crate::executor::WasmEdgeExecutor; @@ -45,11 +37,11 @@ static mut STDERR_FD: Option = None; static DEFAULT_CONTAINER_ROOT_DIR: &str = "/run/containerd/wasmedge"; -type ExitCode = (Mutex)>>, Condvar); +type ExitCode = Arc<(Mutex)>>, Condvar)>; pub struct Wasi { id: String, - exit_code: Arc, + exit_code: ExitCode, stdin: String, stdout: String, @@ -97,155 +89,84 @@ fn determine_rootdir>(bundle: P, namespace: String) -> Result>) -> Self { - let cfg = cfg.unwrap(); // TODO: handle error - let bundle = cfg.get_bundle().unwrap_or_default(); - let namespace = cfg.get_namespace(); - Wasi { - id, - rootdir: determine_rootdir(bundle.as_str(), namespace).unwrap(), - exit_code: Arc::new((Mutex::new(None), Condvar::new())), - stdin: cfg.get_stdin().unwrap_or_default(), - stdout: cfg.get_stdout().unwrap_or_default(), - stderr: cfg.get_stderr().unwrap_or_default(), - bundle, - } +impl YoukiInstance for Wasi { + fn get_exit_code(&self) -> ExitCode { + self.exit_code.clone() } - fn start(&self) -> Result { - debug!("preparing module"); + fn get_id(&self) -> String { + self.id.clone() + } - fs::create_dir_all(&self.rootdir)?; + fn get_root_dir(&self) -> std::result::Result { + Ok(self.rootdir.clone()) + } + fn build_container(&self) -> std::result::Result { + 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 = self.build_container(stdin, stdout, stderr)?; - - let code = self.exit_code.clone(); - let pid = container.pid().unwrap(); + let syscall = create_syscall(); + let err_msg = |err| format!("failed to create container: {}", err); + let container = ContainerBuilder::new(self.id.clone(), syscall.as_ref()) + .with_executor(vec![Box::new(WasmEdgeExecutor { + stdin, + stdout, + stderr, + })]) + .map_err(|err| Error::Others(err_msg(err)))? + .with_root_path(self.rootdir.clone()) + .map_err(|err| Error::Others(err_msg(err)))? + .as_init(&self.bundle) + .with_systemd(false) + .build() + .map_err(|err| Error::Others(err_msg(err)))?; // Close the fds now that they have been passed to the container process // so that we don't leak them. stdin.map(close); stdout.map(close); stderr.map(close); - container - .start() - .map_err(|err| Error::Any(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 { - log::info!("no child process"); - 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) + Ok(container) } +} - fn kill(&self, signal: u32) -> Result<(), Error> { - let signal: Signal = match signal as i32 { - SIGKILL => NixSignal::SIGKILL.into(), - SIGINT => NixSignal::SIGINT.into(), - _ => Err(Error::InvalidArgument( - "only SIGKILL and SIGINT are supported".to_string(), - ))?, - }; - - 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) => { - if container.status() == ContainerStatus::Stopped { - return Err(Error::Others("container not running".into())); - } - Err(Error::Others(e.to_string())) - } +impl Instance for Wasi { + type E = Vm; + fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { + let cfg = cfg.unwrap(); // TODO: handle error + let bundle = cfg.get_bundle().unwrap_or_default(); + let namespace = cfg.get_namespace(); + Wasi { + id, + rootdir: determine_rootdir(bundle.as_str(), namespace).unwrap(), + exit_code: Arc::new((Mutex::new(None), Condvar::new())), + stdin: cfg.get_stdin().unwrap_or_default(), + stdout: cfg.get_stdout().unwrap_or_default(), + stderr: cfg.get_stderr().unwrap_or_default(), + bundle, } } - fn delete(&self) -> Result<(), Error> { - match instance_exists(&self.rootdir, self.id.as_str()) { - Ok(exists) => { - if !exists { - return Ok(()); - } - } - Err(err) => { - error!("could not find the container, skipping cleanup: {}", err); - return Ok(()); - } - } - 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)) - })?, - Err(err) => { - error!("could not find the container, skipping cleanup: {}", err); - return Ok(()); - } - } + fn start(&self) -> std::result::Result { + self.start_youki() + } - Ok(()) + fn kill(&self, signal: u32) -> std::result::Result<(), Error> { + self.kill_youki(signal) } - fn wait(&self, waiter: &Wait) -> Result<(), Error> { - let code = self.exit_code.clone(); - waiter.set_up_exit_code_wait(code) + fn delete(&self) -> std::result::Result<(), Error> { + self.delete_youki() } -} -impl Wasi { - fn build_container( + fn wait( &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) + waiter: &containerd_shim_wasm::sandbox::instance::Wait, + ) -> std::result::Result<(), Error> { + self.wait_youki(waiter) } } @@ -267,6 +188,7 @@ impl EngineGetter for Wasi { .with_config(config) .build() .map_err(anyhow::Error::msg)?; + Ok(vm) } } @@ -280,7 +202,9 @@ mod wasitest { use std::time::Duration; use containerd_shim_wasm::function; + use containerd_shim_wasm::sandbox::instance::Wait; use containerd_shim_wasm::sandbox::testutil::{has_cap_sys_admin, run_test_with_sudo}; + use libc::SIGKILL; use oci_spec::runtime::{ProcessBuilder, RootBuilder, SpecBuilder}; use tempfile::{tempdir, TempDir}; diff --git a/crates/containerd-shim-wasmtime/Cargo.toml b/crates/containerd-shim-wasmtime/Cargo.toml index 24fa3a7b7..4ac147315 100644 --- a/crates/containerd-shim-wasmtime/Cargo.toml +++ b/crates/containerd-shim-wasmtime/Cargo.toml @@ -33,7 +33,7 @@ oci-spec = { workspace = true, features = ["runtime"] } thiserror = { workspace = true } serde_json = { workspace = true } nix = { workspace = true } -libcontainer = { workspace = true } +libcontainer = { workspace = true, features = ["default"]} serde = { workspace = true } libc = { workspace = true } diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index e7d12d1b3..e6b50b9ff 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -1,34 +1,25 @@ use anyhow::Result; -use containerd_shim_wasm::sandbox::instance_utils::{ - get_instance_root, instance_exists, maybe_open_stdio, -}; +use containerd_shim_wasm::sandbox::instance::YoukiInstance; +use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio; use libcontainer::container::builder::ContainerBuilder; -use libcontainer::container::{Container, ContainerStatus}; -use nix::errno::Errno; -use nix::sys::wait::waitid; +use libcontainer::container::Container; 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; 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::{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; -use nix::sys::wait::{Id as WaitID, WaitPidFlag, WaitStatus}; 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)>; @@ -112,145 +103,61 @@ impl Instance for Wasi { rootdir, } } - fn start(&self) -> Result { - log::info!("starting instance: {}", self.id); - let engine: Engine = self.engine.clone(); - - let mut container = self.build_container( - self.stdin.as_str(), - self.stdout.as_str(), - self.stderr.as_str(), - engine, - )?; - - 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 { - log::info!("no child process"); - 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> { - log::info!("killing instance: {}", self.id); - if signal as i32 != SIGKILL && signal as i32 != SIGINT { - return Err(Error::InvalidArgument( - "only SIGKILL and SIGINT are supported".to_string(), - )); - } - 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) { - Ok(_) => Ok(()), - Err(e) => { - if container.status() == ContainerStatus::Stopped { - return Err(Error::Others("container not running".into())); - } - Err(Error::Others(e.to_string())) - } - } + fn start(&self) -> std::result::Result { + self.start_youki() } - fn delete(&self) -> Result<(), Error> { - log::info!("deleting instance: {}", self.id); - match instance_exists(&self.rootdir, self.id.as_str()) { - Ok(exists) => { - if !exists { - return Ok(()); - } - } - Err(err) => { - error!("could not find the container, skipping cleanup: {}", err); - return Ok(()); - } - } - 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 {}: {}", - self.id, - err - )) - })?, - Err(err) => { - error!("could not find the container, skipping cleanup: {}", err); - return Ok(()); - } - } + fn kill(&self, signal: u32) -> std::result::Result<(), Error> { + self.kill_youki(signal) + } - Ok(()) + fn delete(&self) -> std::result::Result<(), Error> { + self.delete_youki() } - 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) + fn wait( + &self, + waiter: &containerd_shim_wasm::sandbox::instance::Wait, + ) -> std::result::Result<(), Error> { + self.wait_youki(waiter) } } -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")?; +impl YoukiInstance for Wasi { + fn get_exit_code(&self) -> ExitCode { + self.exit_code.clone() + } + fn get_id(&self) -> String { + self.id.clone() + } + + fn get_root_dir(&self) -> std::result::Result { + Ok(self.rootdir.clone()) + } + + fn build_container(&self) -> std::result::Result { + let engine = self.engine.clone(); + let syscall = create_syscall(); + let stdin = maybe_open_stdio(&self.stdin).context("could not open stdin")?; + let stdout = maybe_open_stdio(&self.stdout).context("could not open stdout")?; + let stderr = maybe_open_stdio(&self.stderr).context("could not open stderr")?; + let err_msg = |err| format!("failed to create container: {}", err); 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())? + })]) + .map_err(|err| Error::Others(err_msg(err)))? + .with_root_path(self.rootdir.clone()) + .map_err(|err| Error::Others(err_msg(err)))? .as_init(&self.bundle) .with_systemd(false) - .build()?; + .build() + .map_err(|err| Error::Others(err_msg(err)))?; Ok(container) } } @@ -273,6 +180,7 @@ mod wasitest { use containerd_shim_wasm::function; use containerd_shim_wasm::sandbox::instance::Wait; use containerd_shim_wasm::sandbox::testutil::{has_cap_sys_admin, run_test_with_sudo}; + use libc::SIGKILL; use oci_spec::runtime::{ProcessBuilder, RootBuilder, SpecBuilder}; use tempfile::{tempdir, TempDir}; From 355a26788a0034b59418eade82383d994ff0eff5 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 3 Aug 2023 01:36:45 +0000 Subject: [PATCH 02/11] docs: add some comments on the YoukiInstance trait Signed-off-by: jiaxiao zhou --- crates/containerd-shim-wasm/src/sandbox/instance.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/containerd-shim-wasm/src/sandbox/instance.rs b/crates/containerd-shim-wasm/src/sandbox/instance.rs index b1ff4762e..23158a2c3 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance.rs @@ -152,6 +152,10 @@ pub trait Instance { fn wait(&self, waiter: &Wait) -> Result<(), Error>; } +/// YoukiInstance is a trait that gets implemented by a WASI runtime that +/// uses youki's libcontainer library as the container runtime. +/// It provides default implementations for some of the Instance trait methods. +/// The only method that needs to be implemented is `fn new()`. pub trait YoukiInstance: Instance { /// Get the exit code of the instance fn get_exit_code(&self) -> ExitCode; From f415b6299d0bf59ed8e2a09726ad883b085255d5 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 3 Aug 2023 06:15:18 +0000 Subject: [PATCH 03/11] implement Instance for all instances of YoukiInstance Signed-off-by: jiaxiao zhou --- .../src/sandbox/instance.rs | 20 +++++-- .../containerd-shim-wasmedge/src/instance.rs | 57 +++++++------------ .../containerd-shim-wasmtime/src/instance.rs | 29 ++-------- 3 files changed, 39 insertions(+), 67 deletions(-) diff --git a/crates/containerd-shim-wasm/src/sandbox/instance.rs b/crates/containerd-shim-wasm/src/sandbox/instance.rs index 23158a2c3..f89463528 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance.rs @@ -156,7 +156,10 @@ pub trait Instance { /// uses youki's libcontainer library as the container runtime. /// It provides default implementations for some of the Instance trait methods. /// The only method that needs to be implemented is `fn new()`. -pub trait YoukiInstance: Instance { +pub trait YoukiInstance { + type E: Send + Sync + Clone; + fn new_youki(id: String, cfg: Option<&InstanceConfig>) -> Self; + /// Get the exit code of the instance fn get_exit_code(&self) -> ExitCode; @@ -168,11 +171,14 @@ pub trait YoukiInstance: Instance { /// Build the container fn build_container(&self) -> Result; +} +impl Instance for T { + type E = T::E; /// 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_youki(&self) -> Result { + fn start(&self) -> Result { let id = self.get_id(); log::info!("starting instance: {}", id); @@ -210,7 +216,7 @@ pub trait YoukiInstance: Instance { } /// Send a signal to the instance - fn kill_youki(&self, signal: u32) -> Result<(), Error> { + fn kill(&self, signal: u32) -> Result<(), Error> { let id = self.get_id(); let root_dir = self.get_root_dir()?; log::info!("killing instance: {}", id.clone()); @@ -238,7 +244,7 @@ pub trait YoukiInstance: Instance { /// Delete any reference to the instance /// This is called after the instance has exited. - fn delete_youki(&self) -> Result<(), Error> { + fn delete(&self) -> Result<(), Error> { let id = self.get_id(); let root_dir = self.get_root_dir()?; log::info!("deleting instance: {}", id.clone()); @@ -282,13 +288,17 @@ pub trait YoukiInstance: Instance { /// set_up_exit_code_wait() implemented by Wait to set up exit code /// processing. Note that the "wait" function doesn't block, but /// it sets up the waiting channel. - fn wait_youki(&self, waiter: &Wait) -> Result<(), Error> { + fn wait(&self, waiter: &Wait) -> Result<(), Error> { let id = self.get_id(); let exit_code = self.get_exit_code(); log::info!("waiting for instance: {}", id); let code = exit_code; waiter.set_up_exit_code_wait(code) } + + fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { + Self::new_youki(id, cfg) + } } /// This is used for waiting for the container process to exit and deliver the exit code to the caller. diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index 387c93e46..20737e6a7 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -10,7 +10,7 @@ use chrono::{DateTime, Utc}; use containerd_shim_wasm::sandbox::error::Error; use containerd_shim_wasm::sandbox::instance::YoukiInstance; use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio; -use containerd_shim_wasm::sandbox::{EngineGetter, Instance, InstanceConfig}; +use containerd_shim_wasm::sandbox::{EngineGetter, InstanceConfig}; use libc::{dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use nix::unistd::close; use serde::{Deserialize, Serialize}; @@ -90,6 +90,23 @@ fn determine_rootdir>(bundle: P, namespace: String) -> Result>) -> Self { + let cfg = cfg.unwrap(); // TODO: handle error + let bundle = cfg.get_bundle().unwrap_or_default(); + let namespace = cfg.get_namespace(); + Wasi { + id, + rootdir: determine_rootdir(bundle.as_str(), namespace).unwrap(), + exit_code: Arc::new((Mutex::new(None), Condvar::new())), + stdin: cfg.get_stdin().unwrap_or_default(), + stdout: cfg.get_stdout().unwrap_or_default(), + stderr: cfg.get_stderr().unwrap_or_default(), + bundle, + } + } + fn get_exit_code(&self) -> ExitCode { self.exit_code.clone() } @@ -133,43 +150,6 @@ impl YoukiInstance for Wasi { } } -impl Instance for Wasi { - type E = Vm; - fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { - let cfg = cfg.unwrap(); // TODO: handle error - let bundle = cfg.get_bundle().unwrap_or_default(); - let namespace = cfg.get_namespace(); - Wasi { - id, - rootdir: determine_rootdir(bundle.as_str(), namespace).unwrap(), - exit_code: Arc::new((Mutex::new(None), Condvar::new())), - stdin: cfg.get_stdin().unwrap_or_default(), - stdout: cfg.get_stdout().unwrap_or_default(), - stderr: cfg.get_stderr().unwrap_or_default(), - bundle, - } - } - - fn start(&self) -> std::result::Result { - self.start_youki() - } - - fn kill(&self, signal: u32) -> std::result::Result<(), Error> { - self.kill_youki(signal) - } - - fn delete(&self) -> std::result::Result<(), Error> { - self.delete_youki() - } - - fn wait( - &self, - waiter: &containerd_shim_wasm::sandbox::instance::Wait, - ) -> std::result::Result<(), Error> { - self.wait_youki(waiter) - } -} - impl EngineGetter for Wasi { type E = Vm; fn new_engine() -> Result { @@ -204,6 +184,7 @@ mod wasitest { use containerd_shim_wasm::function; use containerd_shim_wasm::sandbox::instance::Wait; use containerd_shim_wasm::sandbox::testutil::{has_cap_sys_admin, run_test_with_sudo}; + use containerd_shim_wasm::sandbox::Instance; use libc::SIGKILL; use oci_spec::runtime::{ProcessBuilder, RootBuilder, SpecBuilder}; use tempfile::{tempdir, TempDir}; diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index e6b50b9ff..471c748cf 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -13,7 +13,7 @@ use std::sync::{Arc, Condvar, Mutex}; use anyhow::Context; use chrono::{DateTime, Utc}; use containerd_shim_wasm::sandbox::error::Error; -use containerd_shim_wasm::sandbox::{EngineGetter, Instance, InstanceConfig}; +use containerd_shim_wasm::sandbox::{EngineGetter, InstanceConfig}; use libc::{dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use libcontainer::syscall::syscall::create_syscall; @@ -83,9 +83,10 @@ fn determine_rootdir>(bundle: P, namespace: String) -> Result>) -> Self { + + fn new_youki(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); @@ -104,27 +105,6 @@ impl Instance for Wasi { } } - fn start(&self) -> std::result::Result { - self.start_youki() - } - - fn kill(&self, signal: u32) -> std::result::Result<(), Error> { - self.kill_youki(signal) - } - - fn delete(&self) -> std::result::Result<(), Error> { - self.delete_youki() - } - - fn wait( - &self, - waiter: &containerd_shim_wasm::sandbox::instance::Wait, - ) -> std::result::Result<(), Error> { - self.wait_youki(waiter) - } -} - -impl YoukiInstance for Wasi { fn get_exit_code(&self) -> ExitCode { self.exit_code.clone() } @@ -180,6 +160,7 @@ mod wasitest { use containerd_shim_wasm::function; use containerd_shim_wasm::sandbox::instance::Wait; use containerd_shim_wasm::sandbox::testutil::{has_cap_sys_admin, run_test_with_sudo}; + use containerd_shim_wasm::sandbox::Instance; use libc::SIGKILL; use oci_spec::runtime::{ProcessBuilder, RootBuilder, SpecBuilder}; use tempfile::{tempdir, TempDir}; From bfb6819663cc7438856aaa1d3eaba6ae29de4bb6 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 3 Aug 2023 22:19:57 +0000 Subject: [PATCH 04/11] docs: add more documentation to the youkiinstance Signed-off-by: jiaxiao zhou --- .../src/sandbox/instance.rs | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/crates/containerd-shim-wasm/src/sandbox/instance.rs b/crates/containerd-shim-wasm/src/sandbox/instance.rs index f89463528..7c29bb6aa 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance.rs @@ -155,9 +155,18 @@ pub trait Instance { /// YoukiInstance is a trait that gets implemented by a WASI runtime that /// uses youki's libcontainer library as the container runtime. /// It provides default implementations for some of the Instance trait methods. -/// The only method that needs to be implemented is `fn new()`. +/// The implementor of this trait is expected to implement the +/// new_youki() +/// get_exit_code() +/// get_id() +/// get_root_dir() +/// build_container() +/// methods. pub trait YoukiInstance { + /// The WASI engine type type E: Send + Sync + Clone; + + /// Create a new instance fn new_youki(id: String, cfg: Option<&InstanceConfig>) -> Self; /// Get the exit code of the instance @@ -173,8 +182,16 @@ pub trait YoukiInstance { fn build_container(&self) -> Result; } +/// Default implementation of the Instance trait for YoukiInstance +/// This implementation uses the libcontainer library to create and start +/// the container. impl Instance for T { type E = T::E; + + fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { + Self::new_youki(id, cfg) + } + /// 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. @@ -295,10 +312,6 @@ impl Instance for T { let code = exit_code; waiter.set_up_exit_code_wait(code) } - - fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { - Self::new_youki(id, cfg) - } } /// This is used for waiting for the container process to exit and deliver the exit code to the caller. From 157b888324b5c6bb811bb37df65f60bc89f3278f Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Fri, 4 Aug 2023 23:35:29 +0000 Subject: [PATCH 05/11] feat: make libcontainer optional in containerd-shim-wasm Signed-off-by: jiaxiao zhou --- crates/containerd-shim-wasm/Cargo.toml | 15 ++++++++------- .../containerd-shim-wasm/src/sandbox/instance.rs | 11 +++++++++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/crates/containerd-shim-wasm/Cargo.toml b/crates/containerd-shim-wasm/Cargo.toml index 0f4cda5f4..4836abf51 100644 --- a/crates/containerd-shim-wasm/Cargo.toml +++ b/crates/containerd-shim-wasm/Cargo.toml @@ -28,7 +28,7 @@ clone3 = "0.2" libc = { workspace = true } caps = "0.5" proc-mounts = "0.3" -libcontainer = { workspace = true } +libcontainer = { workspace = true, optional = true } [build-dependencies] ttrpc-codegen = { version = "0.4.2", optional = true } @@ -41,11 +41,12 @@ env_logger = { workspace = true } rand = "0.8" [features] -default = ["libcontainer/default"] +default = ["libcontainer", "libcontainer/default"] +libcontainer = ["dep:libcontainer"] generate_bindings = ["ttrpc-codegen"] generate_doc = [] -libseccomp = ["libcontainer/libseccomp"] -systemd = ["libcontainer/systemd"] -v2 = ["libcontainer/v2"] -v1 = ["libcontainer/v1"] -cgroupsv2_devices = ["libcontainer/cgroupsv2_devices"] \ No newline at end of file +libseccomp = ["libcontainer", "libcontainer/libseccomp"] +systemd = ["libcontainer", "libcontainer/systemd"] +v2 = ["libcontainer", "libcontainer/v2"] +v1 = ["libcontainer", "libcontainer/v1"] +cgroupsv2_devices = ["libcontainer", "libcontainer/cgroupsv2_devices"] \ No newline at end of file diff --git a/crates/containerd-shim-wasm/src/sandbox/instance.rs b/crates/containerd-shim-wasm/src/sandbox/instance.rs index 7c29bb6aa..97138f96c 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance.rs @@ -9,8 +9,13 @@ use anyhow::Context; use libc::{SIGINT, SIGKILL, SIGTERM}; use chrono::{DateTime, Utc}; -use libcontainer::container::{Container, ContainerStatus}; -use libcontainer::signal::Signal; + +#[cfg(feature = "libcontainer")] +use libcontainer::{ + container::{Container, ContainerStatus}, + signal::Signal, +}; + use log::error; use nix::errno::Errno; use nix::sys::wait::waitid; @@ -162,6 +167,7 @@ pub trait Instance { /// get_root_dir() /// build_container() /// methods. +#[cfg(feature = "libcontainer")] pub trait YoukiInstance { /// The WASI engine type type E: Send + Sync + Clone; @@ -185,6 +191,7 @@ pub trait YoukiInstance { /// Default implementation of the Instance trait for YoukiInstance /// This implementation uses the libcontainer library to create and start /// the container. +#[cfg(feature = "libcontainer")] impl Instance for T { type E = T::E; From 21d8c670017a3a92300f0b6732f5cf829f8d8f95 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Fri, 4 Aug 2023 23:39:50 +0000 Subject: [PATCH 06/11] group imports with conditional compilation Signed-off-by: jiaxiao zhou --- Makefile | 2 ++ .../src/sandbox/instance.rs | 24 +++++++++---------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 16f06da3c..09190db69 100644 --- a/Makefile +++ b/Makefile @@ -18,6 +18,8 @@ KIND_CLUSTER_NAME ?= containerd-wasm build: cargo build -p containerd-shim-wasm --features generate_bindings $(RELEASE_FLAG) cargo build $(RELEASE_FLAG) + # no libcontainer feature on containerd-shim-wasm + cargo build $(RELEASE_FLAG) --no-default-features -p containerd-shim-wasm .PHONY: check check: diff --git a/crates/containerd-shim-wasm/src/sandbox/instance.rs b/crates/containerd-shim-wasm/src/sandbox/instance.rs index 97138f96c..1eba99d01 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance.rs @@ -1,28 +1,28 @@ //! Abstractions for running/managing a wasm/wasi instance. -use std::path::PathBuf; use std::sync::mpsc::Sender; use std::sync::{Arc, Condvar, Mutex}; use std::thread; -use anyhow::Context; use libc::{SIGINT, SIGKILL, SIGTERM}; use chrono::{DateTime, Utc}; #[cfg(feature = "libcontainer")] -use libcontainer::{ - container::{Container, ContainerStatus}, - signal::Signal, +use { + crate::sandbox::instance_utils::{get_instance_root, instance_exists}, + anyhow::Context, + libcontainer::{ + container::{Container, ContainerStatus}, + signal::Signal, + }, + log::error, + nix::errno::Errno, + nix::sys::wait::waitid, + nix::sys::wait::{Id as WaitID, WaitPidFlag, WaitStatus}, + std::path::PathBuf, }; -use log::error; -use nix::errno::Errno; -use nix::sys::wait::waitid; -use nix::sys::wait::{Id as WaitID, WaitPidFlag, WaitStatus}; - -use crate::sandbox::instance_utils::{get_instance_root, instance_exists}; - use super::error::Error; type ExitCode = Arc<(Mutex)>>, Condvar)>; From 2c310e00835626cf2f66ceb840cffeb78062edd4 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Wed, 9 Aug 2023 22:47:50 +0000 Subject: [PATCH 07/11] refactor: apply Jorge's comments Signed-off-by: jiaxiao zhou --- crates/containerd-shim-wasm/Cargo.toml | 4 ++-- crates/containerd-shim-wasm/src/sandbox/instance.rs | 12 ++++++------ crates/containerd-shim-wasmedge/src/instance.rs | 13 ++++++------- crates/containerd-shim-wasmtime/src/instance.rs | 12 +++++------- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/crates/containerd-shim-wasm/Cargo.toml b/crates/containerd-shim-wasm/Cargo.toml index 4836abf51..5f835d536 100644 --- a/crates/containerd-shim-wasm/Cargo.toml +++ b/crates/containerd-shim-wasm/Cargo.toml @@ -47,6 +47,6 @@ generate_bindings = ["ttrpc-codegen"] generate_doc = [] libseccomp = ["libcontainer", "libcontainer/libseccomp"] systemd = ["libcontainer", "libcontainer/systemd"] -v2 = ["libcontainer", "libcontainer/v2"] -v1 = ["libcontainer", "libcontainer/v1"] +cgroupsv2 = ["libcontainer", "libcontainer/v2"] +cgroupsv1 = ["libcontainer", "libcontainer/v1"] cgroupsv2_devices = ["libcontainer", "libcontainer/cgroupsv2_devices"] \ No newline at end of file diff --git a/crates/containerd-shim-wasm/src/sandbox/instance.rs b/crates/containerd-shim-wasm/src/sandbox/instance.rs index 1eba99d01..1ad1018b3 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance.rs @@ -25,7 +25,7 @@ use { use super::error::Error; -type ExitCode = Arc<(Mutex)>>, Condvar)>; +pub type ExitCode = Arc<(Mutex)>>, Condvar)>; /// Generic options builder for creating a wasm instance. /// This is passed to the `Instance::new` method. @@ -161,11 +161,11 @@ pub trait Instance { /// uses youki's libcontainer library as the container runtime. /// It provides default implementations for some of the Instance trait methods. /// The implementor of this trait is expected to implement the -/// new_youki() -/// get_exit_code() -/// get_id() -/// get_root_dir() -/// build_container() +/// * `new_youki()` +/// * `get_exit_code()` +/// * `get_id()` +/// * `get_root_dir()` +/// * `build_container()` /// methods. #[cfg(feature = "libcontainer")] pub trait YoukiInstance { diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index 20737e6a7..15f01ffbb 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -6,9 +6,8 @@ use std::sync::{Arc, Condvar, Mutex}; use anyhow::Context; use anyhow::Result; -use chrono::{DateTime, Utc}; use containerd_shim_wasm::sandbox::error::Error; -use containerd_shim_wasm::sandbox::instance::YoukiInstance; +use containerd_shim_wasm::sandbox::instance::{ExitCode, YoukiInstance}; use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio; use containerd_shim_wasm::sandbox::{EngineGetter, InstanceConfig}; use libc::{dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; @@ -37,7 +36,6 @@ static mut STDERR_FD: Option = None; static DEFAULT_CONTAINER_ROOT_DIR: &str = "/run/containerd/wasmedge"; -type ExitCode = Arc<(Mutex)>>, Condvar)>; pub struct Wasi { id: String, @@ -126,20 +124,20 @@ impl YoukiInstance for Wasi { let stderr = maybe_open_stdio(self.stderr.as_str()).context("could not open stderr")?; let syscall = create_syscall(); - let err_msg = |err| format!("failed to create container: {}", err); + let err_others = |err| Error::Others(format!("failed to create container: {}", err)); let container = ContainerBuilder::new(self.id.clone(), syscall.as_ref()) .with_executor(vec![Box::new(WasmEdgeExecutor { stdin, stdout, stderr, })]) - .map_err(|err| Error::Others(err_msg(err)))? + .map_err(err_others)? .with_root_path(self.rootdir.clone()) - .map_err(|err| Error::Others(err_msg(err)))? + .map_err(err_others)? .as_init(&self.bundle) .with_systemd(false) .build() - .map_err(|err| Error::Others(err_msg(err)))?; + .map_err(err_others)?; // Close the fds now that they have been passed to the container process // so that we don't leak them. stdin.map(close); @@ -181,6 +179,7 @@ mod wasitest { use std::sync::mpsc::channel; use std::time::Duration; + use chrono::{DateTime, Utc}; use containerd_shim_wasm::function; use containerd_shim_wasm::sandbox::instance::Wait; use containerd_shim_wasm::sandbox::testutil::{has_cap_sys_admin, run_test_with_sudo}; diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index 471c748cf..a540bb83d 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_wasm::sandbox::instance::YoukiInstance; +use containerd_shim_wasm::sandbox::instance::{ExitCode, YoukiInstance}; use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio; use libcontainer::container::builder::ContainerBuilder; use libcontainer::container::Container; @@ -11,7 +11,6 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, Condvar, Mutex}; use anyhow::Context; -use chrono::{DateTime, Utc}; use containerd_shim_wasm::sandbox::error::Error; use containerd_shim_wasm::sandbox::{EngineGetter, InstanceConfig}; use libc::{dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; @@ -22,7 +21,6 @@ use wasmtime::Engine; use crate::executor::WasmtimeExecutor; 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; @@ -123,7 +121,7 @@ impl YoukiInstance for Wasi { let stdin = maybe_open_stdio(&self.stdin).context("could not open stdin")?; let stdout = maybe_open_stdio(&self.stdout).context("could not open stdout")?; let stderr = maybe_open_stdio(&self.stderr).context("could not open stderr")?; - let err_msg = |err| format!("failed to create container: {}", err); + let err_others = |err| Error::Others(format!("failed to create container: {}", err)); let container = ContainerBuilder::new(self.id.clone(), syscall.as_ref()) .with_executor(vec![Box::new(WasmtimeExecutor { stdin, @@ -131,13 +129,13 @@ impl YoukiInstance for Wasi { stderr, engine, })]) - .map_err(|err| Error::Others(err_msg(err)))? + .map_err(err_others)? .with_root_path(self.rootdir.clone()) - .map_err(|err| Error::Others(err_msg(err)))? + .map_err(err_others)? .as_init(&self.bundle) .with_systemd(false) .build() - .map_err(|err| Error::Others(err_msg(err)))?; + .map_err(err_others)?; Ok(container) } } From 401d1fd1232fb7af8fecdc81cb6a796305f7e949 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 10 Aug 2023 01:17:14 +0000 Subject: [PATCH 08/11] move youkiinstance to its own module Signed-off-by: jiaxiao zhou --- .../src/sandbox/instance.rs | 179 ----------------- .../containerd-shim-wasm/src/sandbox/mod.rs | 2 + .../src/sandbox/youki_instance.rs | 188 ++++++++++++++++++ .../containerd-shim-wasmedge/src/instance.rs | 3 +- .../containerd-shim-wasmtime/src/instance.rs | 3 +- 5 files changed, 194 insertions(+), 181 deletions(-) create mode 100644 crates/containerd-shim-wasm/src/sandbox/youki_instance.rs diff --git a/crates/containerd-shim-wasm/src/sandbox/instance.rs b/crates/containerd-shim-wasm/src/sandbox/instance.rs index 1ad1018b3..0caf3a461 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance.rs @@ -8,21 +8,6 @@ use libc::{SIGINT, SIGKILL, SIGTERM}; use chrono::{DateTime, Utc}; -#[cfg(feature = "libcontainer")] -use { - crate::sandbox::instance_utils::{get_instance_root, instance_exists}, - anyhow::Context, - libcontainer::{ - container::{Container, ContainerStatus}, - signal::Signal, - }, - log::error, - nix::errno::Errno, - nix::sys::wait::waitid, - nix::sys::wait::{Id as WaitID, WaitPidFlag, WaitStatus}, - std::path::PathBuf, -}; - use super::error::Error; pub type ExitCode = Arc<(Mutex)>>, Condvar)>; @@ -157,170 +142,6 @@ pub trait Instance { fn wait(&self, waiter: &Wait) -> Result<(), Error>; } -/// YoukiInstance is a trait that gets implemented by a WASI runtime that -/// uses youki's libcontainer library as the container runtime. -/// It provides default implementations for some of the Instance trait methods. -/// The implementor of this trait is expected to implement the -/// * `new_youki()` -/// * `get_exit_code()` -/// * `get_id()` -/// * `get_root_dir()` -/// * `build_container()` -/// methods. -#[cfg(feature = "libcontainer")] -pub trait YoukiInstance { - /// The WASI engine type - type E: Send + Sync + Clone; - - /// Create a new instance - fn new_youki(id: String, cfg: Option<&InstanceConfig>) -> Self; - - /// Get the exit code of the instance - fn get_exit_code(&self) -> ExitCode; - - /// Get the ID of the instance - fn get_id(&self) -> String; - - /// Get the root directory of the instance - fn get_root_dir(&self) -> Result; - - /// Build the container - fn build_container(&self) -> Result; -} - -/// Default implementation of the Instance trait for YoukiInstance -/// This implementation uses the libcontainer library to create and start -/// the container. -#[cfg(feature = "libcontainer")] -impl Instance for T { - type E = T::E; - - fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { - Self::new_youki(id, cfg) - } - - /// 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 { - let id = self.get_id(); - log::info!("starting instance: {}", id); - - let mut container = self.build_container()?; - let code = self.get_exit_code(); - let pid = container.pid().context("failed to get pid")?; - - 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 { - log::info!("no child process"); - 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) - } - - /// Send a signal to the instance - fn kill(&self, signal: u32) -> Result<(), Error> { - let id = self.get_id(); - let root_dir = self.get_root_dir()?; - log::info!("killing instance: {}", id.clone()); - if signal as i32 != SIGKILL && signal as i32 != SIGINT { - return Err(Error::InvalidArgument( - "only SIGKILL and SIGINT are supported".to_string(), - )); - } - let container_root = get_instance_root(root_dir, id.as_str())?; - let mut container = Container::load(container_root).with_context(|| { - format!("could not load state for container {id}", id = 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())) - } - } - } - - /// Delete any reference to the instance - /// This is called after the instance has exited. - fn delete(&self) -> Result<(), Error> { - let id = self.get_id(); - let root_dir = self.get_root_dir()?; - log::info!("deleting instance: {}", id.clone()); - match instance_exists(&root_dir, id.as_str()) { - Ok(exists) => { - if !exists { - return Ok(()); - } - } - Err(err) => { - error!("could not find the container, skipping cleanup: {}", err); - return Ok(()); - } - } - let container_root = get_instance_root(&root_dir, id.as_str())?; - let container = Container::load(container_root).with_context(|| { - format!( - "could not load state for container {id}", - id = id.clone().as_str() - ) - }); - match container { - Ok(mut container) => container.delete(true).map_err(|err| { - Error::Any(anyhow::anyhow!( - "failed to delete container {}: {}", - id, - err - )) - })?, - Err(err) => { - error!("could not find the container, skipping cleanup: {}", err); - return Ok(()); - } - } - Ok(()) - } - - /// 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 - /// set_up_exit_code_wait() implemented by Wait to set up exit code - /// processing. Note that the "wait" function doesn't block, but - /// it sets up the waiting channel. - fn wait(&self, waiter: &Wait) -> Result<(), Error> { - let id = self.get_id(); - let exit_code = self.get_exit_code(); - log::info!("waiting for instance: {}", id); - let code = exit_code; - waiter.set_up_exit_code_wait(code) - } -} - /// This is used for waiting for the container process to exit and deliver the exit code to the caller. /// Since the shim needs to provide the caller the process exit code, this struct wraps the required /// thread setup to make the shims simpler. diff --git a/crates/containerd-shim-wasm/src/sandbox/mod.rs b/crates/containerd-shim-wasm/src/sandbox/mod.rs index 603d2c28d..483911d68 100644 --- a/crates/containerd-shim-wasm/src/sandbox/mod.rs +++ b/crates/containerd-shim-wasm/src/sandbox/mod.rs @@ -9,6 +9,8 @@ pub mod instance; pub mod instance_utils; pub mod manager; pub mod shim; +#[cfg(feature = "libcontainer")] +pub mod youki_instance; pub use error::{Error, Result}; pub use instance::{EngineGetter, Instance, InstanceConfig}; diff --git a/crates/containerd-shim-wasm/src/sandbox/youki_instance.rs b/crates/containerd-shim-wasm/src/sandbox/youki_instance.rs new file mode 100644 index 000000000..f4876b140 --- /dev/null +++ b/crates/containerd-shim-wasm/src/sandbox/youki_instance.rs @@ -0,0 +1,188 @@ +//! Abstractions for running/managing a wasm/wasi instance that uses youki's libcontainer library. + +use anyhow::Context; +use chrono::Utc; +use libc::{SIGINT, SIGKILL}; +use libcontainer::{ + container::{Container, ContainerStatus}, + signal::Signal, +}; +use log::error; +use nix::{ + errno::Errno, + sys::wait::{waitid, Id as WaitID, WaitPidFlag, WaitStatus}, +}; + +use crate::sandbox::{ + instance::ExitCode, + instance_utils::{get_instance_root, instance_exists}, +}; + +use crate::sandbox::InstanceConfig; +use std::{path::PathBuf, thread}; + +use super::{error::Error, instance::Wait, Instance}; + +/// YoukiInstance is a trait that gets implemented by a WASI runtime that +/// uses youki's libcontainer library as the container runtime. +/// It provides default implementations for some of the Instance trait methods. +/// The implementor of this trait is expected to implement the +/// * `new_youki()` +/// * `get_exit_code()` +/// * `get_id()` +/// * `get_root_dir()` +/// * `build_container()` +/// methods. +#[cfg(feature = "libcontainer")] +pub trait YoukiInstance { + /// The WASI engine type + type E: Send + Sync + Clone; + + /// Create a new instance + fn new_youki(id: String, cfg: Option<&InstanceConfig>) -> Self; + + /// Get the exit code of the instance + fn get_exit_code(&self) -> ExitCode; + + /// Get the ID of the instance + fn get_id(&self) -> String; + + /// Get the root directory of the instance + fn get_root_dir(&self) -> Result; + + /// Build the container + fn build_container(&self) -> Result; +} + +/// Default implementation of the Instance trait for YoukiInstance +/// This implementation uses the libcontainer library to create and start +/// the container. +#[cfg(feature = "libcontainer")] +impl Instance for T { + type E = T::E; + + fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { + Self::new_youki(id, cfg) + } + + /// 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 { + let id = self.get_id(); + log::info!("starting instance: {}", id); + + let mut container = self.build_container()?; + let code = self.get_exit_code(); + let pid = container.pid().context("failed to get pid")?; + + 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 { + log::info!("no child process"); + 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) + } + + /// Send a signal to the instance + fn kill(&self, signal: u32) -> Result<(), Error> { + let id = self.get_id(); + let root_dir = self.get_root_dir()?; + log::info!("killing instance: {}", id.clone()); + if signal as i32 != SIGKILL && signal as i32 != SIGINT { + return Err(Error::InvalidArgument( + "only SIGKILL and SIGINT are supported".to_string(), + )); + } + let container_root = get_instance_root(root_dir, id.as_str())?; + let mut container = Container::load(container_root).with_context(|| { + format!("could not load state for container {id}", id = 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())) + } + } + } + + /// Delete any reference to the instance + /// This is called after the instance has exited. + fn delete(&self) -> Result<(), Error> { + let id = self.get_id(); + let root_dir = self.get_root_dir()?; + log::info!("deleting instance: {}", id.clone()); + match instance_exists(&root_dir, id.as_str()) { + Ok(exists) => { + if !exists { + return Ok(()); + } + } + Err(err) => { + error!("could not find the container, skipping cleanup: {}", err); + return Ok(()); + } + } + let container_root = get_instance_root(&root_dir, id.as_str())?; + let container = Container::load(container_root).with_context(|| { + format!( + "could not load state for container {id}", + id = id.clone().as_str() + ) + }); + match container { + Ok(mut container) => container.delete(true).map_err(|err| { + Error::Any(anyhow::anyhow!( + "failed to delete container {}: {}", + id, + err + )) + })?, + Err(err) => { + error!("could not find the container, skipping cleanup: {}", err); + return Ok(()); + } + } + Ok(()) + } + + /// 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 + /// set_up_exit_code_wait() implemented by Wait to set up exit code + /// processing. Note that the "wait" function doesn't block, but + /// it sets up the waiting channel. + fn wait(&self, waiter: &Wait) -> Result<(), Error> { + let id = self.get_id(); + let exit_code = self.get_exit_code(); + log::info!("waiting for instance: {}", id); + let code = exit_code; + waiter.set_up_exit_code_wait(code) + } +} diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index 15f01ffbb..4b4ea3e99 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -7,8 +7,9 @@ use std::sync::{Arc, Condvar, Mutex}; use anyhow::Context; use anyhow::Result; use containerd_shim_wasm::sandbox::error::Error; -use containerd_shim_wasm::sandbox::instance::{ExitCode, YoukiInstance}; +use containerd_shim_wasm::sandbox::instance::ExitCode; use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio; +use containerd_shim_wasm::sandbox::youki_instance::YoukiInstance; use containerd_shim_wasm::sandbox::{EngineGetter, InstanceConfig}; use libc::{dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use nix::unistd::close; diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index a540bb83d..c486e11bf 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -1,6 +1,7 @@ use anyhow::Result; -use containerd_shim_wasm::sandbox::instance::{ExitCode, YoukiInstance}; +use containerd_shim_wasm::sandbox::instance::ExitCode; use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio; +use containerd_shim_wasm::sandbox::youki_instance::YoukiInstance; use libcontainer::container::builder::ContainerBuilder; use libcontainer::container::Container; use serde::{Deserialize, Serialize}; From 0c944a4adc16107ed1d22a794466ef2521eadfee Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 10 Aug 2023 02:19:43 +0000 Subject: [PATCH 09/11] make libcontainer an opt in feature Signed-off-by: jiaxiao zhou --- Makefile | 3 +-- crates/containerd-shim-wasm/Cargo.toml | 16 ++++++++-------- crates/containerd-shim-wasmedge/Cargo.toml | 4 ++-- crates/containerd-shim-wasmtime/Cargo.toml | 4 ++-- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index 09190db69..0d5b39346 100644 --- a/Makefile +++ b/Makefile @@ -17,9 +17,8 @@ KIND_CLUSTER_NAME ?= containerd-wasm .PHONY: build build: cargo build -p containerd-shim-wasm --features generate_bindings $(RELEASE_FLAG) + cargo build -p containerd-shim-wasm --features libcontainer $(RELEASE_FLAG) cargo build $(RELEASE_FLAG) - # no libcontainer feature on containerd-shim-wasm - cargo build $(RELEASE_FLAG) --no-default-features -p containerd-shim-wasm .PHONY: check check: diff --git a/crates/containerd-shim-wasm/Cargo.toml b/crates/containerd-shim-wasm/Cargo.toml index 5f835d536..af80d5d0d 100644 --- a/crates/containerd-shim-wasm/Cargo.toml +++ b/crates/containerd-shim-wasm/Cargo.toml @@ -28,7 +28,7 @@ clone3 = "0.2" libc = { workspace = true } caps = "0.5" proc-mounts = "0.3" -libcontainer = { workspace = true, optional = true } +libcontainer = { workspace = true, optional = true, default-features = false } [build-dependencies] ttrpc-codegen = { version = "0.4.2", optional = true } @@ -41,12 +41,12 @@ env_logger = { workspace = true } rand = "0.8" [features] -default = ["libcontainer", "libcontainer/default"] -libcontainer = ["dep:libcontainer"] +default = [] +libcontainer = ["libcontainer/default"] generate_bindings = ["ttrpc-codegen"] generate_doc = [] -libseccomp = ["libcontainer", "libcontainer/libseccomp"] -systemd = ["libcontainer", "libcontainer/systemd"] -cgroupsv2 = ["libcontainer", "libcontainer/v2"] -cgroupsv1 = ["libcontainer", "libcontainer/v1"] -cgroupsv2_devices = ["libcontainer", "libcontainer/cgroupsv2_devices"] \ No newline at end of file +libseccomp = ["libcontainer/libseccomp"] +systemd = ["libcontainer/systemd"] +cgroupsv2 = ["libcontainer/v2"] +cgroupsv1 = ["libcontainer/v1"] +cgroupsv2_devices = ["libcontainer/cgroupsv2_devices"] \ No newline at end of file diff --git a/crates/containerd-shim-wasmedge/Cargo.toml b/crates/containerd-shim-wasmedge/Cargo.toml index 10acc0301..41daaa7cb 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 } +containerd-shim-wasm = { workspace = true, features = ["libcontainer"]} log = { workspace = true } ttrpc = { workspace = true } wasmedge-sdk = { version = "0.10.1", features = [ "standalone", "static" ] } @@ -18,7 +18,7 @@ serde = { workspace = true } serde_json = { workspace = true } nix = { workspace = true } libc = { workspace = true } -libcontainer = { workspace = true, features = ["default"] } +libcontainer = { workspace = true } [dev-dependencies] tempfile = "3.7" diff --git a/crates/containerd-shim-wasmtime/Cargo.toml b/crates/containerd-shim-wasmtime/Cargo.toml index 4ac147315..9da08bd2d 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 } +containerd-shim-wasm = { workspace = true, features = ["libcontainer"]} log = { workspace = true } ttrpc = { workspace = true } @@ -33,7 +33,7 @@ oci-spec = { workspace = true, features = ["runtime"] } thiserror = { workspace = true } serde_json = { workspace = true } nix = { workspace = true } -libcontainer = { workspace = true, features = ["default"]} +libcontainer = { workspace = true } serde = { workspace = true } libc = { workspace = true } From 013247bc5b3bdcab7b08a9ca86eee69f036b68a9 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 10 Aug 2023 22:59:42 +0000 Subject: [PATCH 10/11] refactor: resolve more comments Signed-off-by: jiaxiao zhou --- ...i_instance.rs => libcontainer_instance.rs} | 17 ++++++++--------- .../containerd-shim-wasm/src/sandbox/mod.rs | 4 +++- .../containerd-shim-wasmedge/src/instance.rs | 7 +++---- .../containerd-shim-wasmtime/src/instance.rs | 19 +++++++++++++------ 4 files changed, 27 insertions(+), 20 deletions(-) rename crates/containerd-shim-wasm/src/sandbox/{youki_instance.rs => libcontainer_instance.rs} (95%) diff --git a/crates/containerd-shim-wasm/src/sandbox/youki_instance.rs b/crates/containerd-shim-wasm/src/sandbox/libcontainer_instance.rs similarity index 95% rename from crates/containerd-shim-wasm/src/sandbox/youki_instance.rs rename to crates/containerd-shim-wasm/src/sandbox/libcontainer_instance.rs index f4876b140..d4863ebe5 100644 --- a/crates/containerd-shim-wasm/src/sandbox/youki_instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/libcontainer_instance.rs @@ -23,7 +23,7 @@ use std::{path::PathBuf, thread}; use super::{error::Error, instance::Wait, Instance}; -/// YoukiInstance is a trait that gets implemented by a WASI runtime that +/// LibcontainerInstance is a trait that gets implemented by a WASI runtime that /// uses youki's libcontainer library as the container runtime. /// It provides default implementations for some of the Instance trait methods. /// The implementor of this trait is expected to implement the @@ -33,13 +33,12 @@ use super::{error::Error, instance::Wait, Instance}; /// * `get_root_dir()` /// * `build_container()` /// methods. -#[cfg(feature = "libcontainer")] -pub trait YoukiInstance { +pub trait LibcontainerInstance { /// The WASI engine type type E: Send + Sync + Clone; /// Create a new instance - fn new_youki(id: String, cfg: Option<&InstanceConfig>) -> Self; + fn new_libcontainer(id: String, cfg: Option<&InstanceConfig>) -> Self; /// Get the exit code of the instance fn get_exit_code(&self) -> ExitCode; @@ -57,12 +56,11 @@ pub trait YoukiInstance { /// Default implementation of the Instance trait for YoukiInstance /// This implementation uses the libcontainer library to create and start /// the container. -#[cfg(feature = "libcontainer")] -impl Instance for T { +impl Instance for T { type E = T::E; fn new(id: String, cfg: Option<&InstanceConfig>) -> Self { - Self::new_youki(id, cfg) + Self::new_libcontainer(id, cfg) } /// Start the instance @@ -115,12 +113,13 @@ impl Instance for T { "only SIGKILL and SIGINT are supported".to_string(), )); } + let signal = Signal::try_from(signal as i32) + .map_err(|err| Error::InvalidArgument(format!("invalid signal number: {}", err)))?; let container_root = get_instance_root(root_dir, id.as_str())?; let mut container = Container::load(container_root).with_context(|| { format!("could not load state for container {id}", id = 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) => { diff --git a/crates/containerd-shim-wasm/src/sandbox/mod.rs b/crates/containerd-shim-wasm/src/sandbox/mod.rs index 483911d68..5f683d776 100644 --- a/crates/containerd-shim-wasm/src/sandbox/mod.rs +++ b/crates/containerd-shim-wasm/src/sandbox/mod.rs @@ -7,10 +7,12 @@ pub mod error; // pub mod exec; pub mod instance; pub mod instance_utils; +#[cfg(feature = "libcontainer")] +pub mod libcontainer_instance; pub mod manager; pub mod shim; #[cfg(feature = "libcontainer")] -pub mod youki_instance; +pub use libcontainer_instance::LibcontainerInstance; pub use error::{Error, Result}; pub use instance::{EngineGetter, Instance, InstanceConfig}; diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index 4b4ea3e99..0ba638e56 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -9,8 +9,7 @@ use anyhow::Result; use containerd_shim_wasm::sandbox::error::Error; use containerd_shim_wasm::sandbox::instance::ExitCode; use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio; -use containerd_shim_wasm::sandbox::youki_instance::YoukiInstance; -use containerd_shim_wasm::sandbox::{EngineGetter, InstanceConfig}; +use containerd_shim_wasm::sandbox::{EngineGetter, InstanceConfig, LibcontainerInstance}; use libc::{dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use nix::unistd::close; use serde::{Deserialize, Serialize}; @@ -88,10 +87,10 @@ fn determine_rootdir>(bundle: P, namespace: String) -> Result>) -> Self { + fn new_libcontainer(id: String, cfg: Option<&InstanceConfig>) -> Self { let cfg = cfg.unwrap(); // TODO: handle error let bundle = cfg.get_bundle().unwrap_or_default(); let namespace = cfg.get_namespace(); diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index c486e11bf..ae44c5733 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -1,9 +1,7 @@ use anyhow::Result; -use containerd_shim_wasm::sandbox::instance::ExitCode; -use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio; -use containerd_shim_wasm::sandbox::youki_instance::YoukiInstance; use libcontainer::container::builder::ContainerBuilder; use libcontainer::container::Container; +use nix::unistd::close; use serde::{Deserialize, Serialize}; use std::fs::File; use std::io::{ErrorKind, Read}; @@ -13,7 +11,9 @@ use std::sync::{Arc, Condvar, Mutex}; use anyhow::Context; use containerd_shim_wasm::sandbox::error::Error; -use containerd_shim_wasm::sandbox::{EngineGetter, InstanceConfig}; +use containerd_shim_wasm::sandbox::instance::ExitCode; +use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio; +use containerd_shim_wasm::sandbox::{EngineGetter, InstanceConfig, LibcontainerInstance}; use libc::{dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use libcontainer::syscall::syscall::create_syscall; @@ -82,10 +82,10 @@ fn determine_rootdir>(bundle: P, namespace: String) -> Result>) -> Self { + fn new_libcontainer(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); @@ -137,6 +137,13 @@ impl YoukiInstance for Wasi { .with_systemd(false) .build() .map_err(err_others)?; + + // Close the fds now that they have been passed to the container process + // so that we don't leak them. + stdin.map(close); + stdout.map(close); + stderr.map(close); + Ok(container) } } From 606f1778df1c8639db577ad2aa1d2e323dc95781 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Mon, 14 Aug 2023 17:46:47 +0000 Subject: [PATCH 11/11] fixed a typo in the docs Signed-off-by: jiaxiao zhou --- .../containerd-shim-wasm/src/sandbox/libcontainer_instance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/containerd-shim-wasm/src/sandbox/libcontainer_instance.rs b/crates/containerd-shim-wasm/src/sandbox/libcontainer_instance.rs index d4863ebe5..28dc99cba 100644 --- a/crates/containerd-shim-wasm/src/sandbox/libcontainer_instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/libcontainer_instance.rs @@ -27,7 +27,7 @@ use super::{error::Error, instance::Wait, Instance}; /// uses youki's libcontainer library as the container runtime. /// It provides default implementations for some of the Instance trait methods. /// The implementor of this trait is expected to implement the -/// * `new_youki()` +/// * `new_libcontainer()` /// * `get_exit_code()` /// * `get_id()` /// * `get_root_dir()`