From 0556b79a6fff37625cb226a271f802fcf60de0da Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Wed, 18 Dec 2024 14:35:40 +0000 Subject: [PATCH 1/4] Use a zygote process Signed-off-by: Jorge Prendes --- Cargo.lock | 46 +++++++++++++ crates/containerd-shim-wasm/Cargo.toml | 6 ++ .../containerd-shim-wasm/src/sandbox/cli.rs | 3 + .../src/sandbox/instance.rs | 3 +- .../containerd-shim-wasm/src/sandbox/oci.rs | 4 +- .../src/sys/unix/container/instance.rs | 65 ++++++++++++------- .../containerd-shim-wasm/src/test/signals.rs | 34 +++------- 7 files changed, 110 insertions(+), 51 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8e8ac9120..1f55ccd8a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -926,6 +926,7 @@ dependencies = [ "protobuf 3.2.0", "rand", "serde", + "serde_bytes", "serde_json", "sha256", "temp-env", @@ -940,6 +941,7 @@ dependencies = [ "wasmparser 0.220.0", "wat", "windows-sys 0.59.0", + "zygote", ] [[package]] @@ -4514,6 +4516,28 @@ dependencies = [ "syn 2.0.87", ] +[[package]] +name = "rmp" +version = "0.8.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "228ed7c16fa39782c3b3468e974aec2795e9089153cd08ee2e9aefb3613334c4" +dependencies = [ + "byteorder", + "num-traits", + "paste", +] + +[[package]] +name = "rmp-serde" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "52e599a477cf9840e92f2cde9a7189e67b42c57532749bf90aea6ec10facd4db" +dependencies = [ + "byteorder", + "rmp", + "serde", +] + [[package]] name = "rust-criu" version = "0.4.0" @@ -4837,6 +4861,15 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "serde_bytes" +version = "0.11.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "387cc504cb06bb40a96c8e04e951fe01854cf6bc921053c954e4a606d9675c6a" +dependencies = [ + "serde", +] + [[package]] name = "serde_derive" version = "1.0.217" @@ -7742,3 +7775,16 @@ dependencies = [ "cc", "pkg-config", ] + +[[package]] +name = "zygote" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b96e95c06f0156974c3cbdac79e5454938c1d301fca4224f5e17f8020cf011a0" +dependencies = [ + "libc", + "nix 0.29.0", + "rmp-serde", + "serde", + "thiserror 2.0.11", +] diff --git a/crates/containerd-shim-wasm/Cargo.toml b/crates/containerd-shim-wasm/Cargo.toml index 746765a3f..8c409974b 100644 --- a/crates/containerd-shim-wasm/Cargo.toml +++ b/crates/containerd-shim-wasm/Cargo.toml @@ -33,6 +33,7 @@ futures = { version = "0.3.30" } wasmparser = { version = "0.220.0" } tokio-stream = { version = "0.1" } sha256 = { workspace = true } +serde_bytes = "0.11" # tracing # note: it's important to keep the version of tracing in sync with tracing-subscriber @@ -63,6 +64,7 @@ tracing-opentelemetry = { version = "0.24", optional = true } [target.'cfg(unix)'.dependencies] +zygote = { version = "0.1.1" } caps = "0.5" # this must match the version pulled by libcontainer dbus = { version = "0", features = ["vendored"] } @@ -108,3 +110,7 @@ opentelemetry = [ "dep:tracing-opentelemetry", ] tracing = ["dep:tracing", "dep:tracing-subscriber"] + +[package.metadata.cargo-machete] +# used as part of a derive macro +ignored = ["serde_bytes"] \ No newline at end of file diff --git a/crates/containerd-shim-wasm/src/sandbox/cli.rs b/crates/containerd-shim-wasm/src/sandbox/cli.rs index b12a91cd9..63a41a989 100644 --- a/crates/containerd-shim-wasm/src/sandbox/cli.rs +++ b/crates/containerd-shim-wasm/src/sandbox/cli.rs @@ -47,6 +47,9 @@ pub fn shim_main<'a, I>( I: 'static + Instance + Sync + Send, I::Engine: Default, { + #[cfg(unix)] + zygote::Zygote::init(); + #[cfg(feature = "opentelemetry")] if otel_traces_enabled() { // opentelemetry uses tokio, so we need to initialize a runtime diff --git a/crates/containerd-shim-wasm/src/sandbox/instance.rs b/crates/containerd-shim-wasm/src/sandbox/instance.rs index efb596041..0db5c5c7d 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance.rs @@ -4,12 +4,13 @@ use std::path::{Path, PathBuf}; use std::time::Duration; use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; use super::error::Error; /// Generic options builder for creating a wasm instance. /// This is passed to the `Instance::new` method. -#[derive(Clone)] +#[derive(Clone, Serialize, Deserialize)] pub struct InstanceConfig { /// Optional stdin named pipe path. stdin: PathBuf, diff --git a/crates/containerd-shim-wasm/src/sandbox/oci.rs b/crates/containerd-shim-wasm/src/sandbox/oci.rs index 0d3478007..8817b16cc 100644 --- a/crates/containerd-shim-wasm/src/sandbox/oci.rs +++ b/crates/containerd-shim-wasm/src/sandbox/oci.rs @@ -8,12 +8,14 @@ use std::process; use anyhow::Context; use oci_spec::image::Descriptor; +use serde::{Deserialize, Serialize}; use super::error::Result; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct WasmLayer { pub config: Descriptor, + #[serde(with = "serde_bytes")] pub layer: Vec, } diff --git a/crates/containerd-shim-wasm/src/sys/unix/container/instance.rs b/crates/containerd-shim-wasm/src/sys/unix/container/instance.rs index 6255a52f7..8694f1444 100644 --- a/crates/containerd-shim-wasm/src/sys/unix/container/instance.rs +++ b/crates/containerd-shim-wasm/src/sys/unix/container/instance.rs @@ -14,6 +14,7 @@ use nix::errno::Errno; use nix::sys::wait::{waitid, Id as WaitID, WaitPidFlag, WaitStatus}; use nix::unistd::Pid; use oci_spec::image::Platform; +use zygote::{WireError, Zygote}; use crate::container::Engine; use crate::sandbox::async_utils::AmbientRuntime as _; @@ -25,7 +26,7 @@ use crate::sandbox::{ use crate::sys::container::executor::Executor; use crate::sys::stdio::open; -static DEFAULT_CONTAINER_ROOT_DIR: &str = "/run/containerd"; +const DEFAULT_CONTAINER_ROOT_DIR: &str = "/run/containerd"; pub struct Instance { exit_code: WaitableCell<(u32, DateTime)>, @@ -39,36 +40,52 @@ impl SandboxInstance for Instance { #[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))] fn new(id: String, cfg: &InstanceConfig) -> Result { - let engine = Self::Engine::default(); - let bundle = cfg.get_bundle().to_path_buf(); - let namespace = cfg.get_namespace(); - let rootdir = Path::new(DEFAULT_CONTAINER_ROOT_DIR).join(E::name()); - let rootdir = determine_rootdir(&bundle, &namespace, rootdir)?; - // check if container is OCI image with wasm layers and attempt to read the module - let (modules, platform) = containerd::Client::connect(cfg.get_containerd_address().as_str(), &namespace).block_on()? - .load_modules(&id, &engine) + let (modules, platform) = containerd::Client::connect(cfg.get_containerd_address(), &cfg.get_namespace()).block_on()? + .load_modules(&id, &E::default()) .block_on() .unwrap_or_else(|e| { log::warn!("Error obtaining wasm layers for container {id}. Will attempt to use files inside container image. Error: {e}"); (vec![], Platform::default()) }); - let mut builder = ContainerBuilder::new(id.clone(), SyscallType::Linux) - .with_executor(Executor::new(engine, modules, platform)) - .with_root_path(rootdir.clone())?; - - if let Ok(f) = open(cfg.get_stdin()) { - builder = builder.with_stdin(f); - } - if let Ok(f) = open(cfg.get_stdout()) { - builder = builder.with_stdout(f); - } - if let Ok(f) = open(cfg.get_stderr()) { - builder = builder.with_stderr(f); - } - - let container = builder.as_init(&bundle).with_systemd(false).build()?; + let (root, state) = Zygote::global() + .run( + |(id, cfg, modules, platform)| -> Result<_, WireError> { + let namespace = cfg.get_namespace(); + + let bundle = cfg.get_bundle().to_path_buf(); + let rootdir = Path::new(DEFAULT_CONTAINER_ROOT_DIR).join(E::name()); + let rootdir = determine_rootdir(&bundle, &namespace, rootdir)?; + let engine = E::default(); + + let mut builder = ContainerBuilder::new(id.clone(), SyscallType::Linux) + .with_executor(Executor::new(engine, modules, platform)) + .with_root_path(rootdir.clone())?; + + if let Ok(f) = open(cfg.get_stdin()) { + builder = builder.with_stdin(f); + } + if let Ok(f) = open(cfg.get_stdout()) { + builder = builder.with_stdout(f); + } + if let Ok(f) = open(cfg.get_stderr()) { + builder = builder.with_stderr(f); + } + + let Container { root, state } = builder + .as_init(&bundle) + .as_sibling(true) + .with_systemd(false) + .build()?; + + // Container is not serializable, but its parts are + Ok((root, state)) + }, + (id.clone(), cfg.clone(), modules, platform), + ) + .map_err(|e| SandboxError::Others(e.to_string()))?; + let container = Container { root, state }; Ok(Self { id, diff --git a/crates/containerd-shim-wasm/src/test/signals.rs b/crates/containerd-shim-wasm/src/test/signals.rs index 54798592a..87debbe52 100644 --- a/crates/containerd-shim-wasm/src/test/signals.rs +++ b/crates/containerd-shim-wasm/src/test/signals.rs @@ -19,14 +19,14 @@ //! remove the ignore attribute from the test. use std::future::pending; +use std::io::{stderr, Write as _}; use std::sync::mpsc::channel; -use std::sync::{Arc, LazyLock}; +use std::sync::Arc; use std::thread::sleep; use std::time::Duration; use anyhow::{bail, Result}; use containerd_shim_wasm_test_modules::HELLO_WORLD; -use tokio::sync::Notify; use crate::container::{Engine, Instance, RuntimeContext}; use crate::testing::WasiTest; @@ -34,21 +34,6 @@ use crate::testing::WasiTest; #[derive(Clone, Default)] pub struct SomeEngine; -async fn ctrl_c(use_libc: bool) { - static CANCELLATION: LazyLock = LazyLock::new(|| Notify::new()); - - fn on_ctr_c(_: libc::c_int) { - CANCELLATION.notify_waiters(); - } - - if use_libc { - unsafe { libc::signal(libc::SIGINT, on_ctr_c as _) }; - CANCELLATION.notified().await; - } else { - let _ = tokio::signal::ctrl_c().await; - } -} - impl Engine for SomeEngine { fn name() -> &'static str { "some-engine" @@ -61,16 +46,16 @@ impl Engine for SomeEngine { .build()? .block_on(async move { use tokio::time::sleep; - let use_libc = std::env::var("USE_LIBC").unwrap_or_default(); - let use_libc = !use_libc.is_empty() && use_libc != "0"; let signal = async { println!("{name}> waiting for signal!"); - ctrl_c(use_libc).await; + let _ = tokio::signal::ctrl_c().await; println!("{name}> received signal, bye!"); }; let task = async { sleep(Duration::from_millis(10)).await; - eprintln!("{name}> ready"); + // use writeln to avoid output capturing from the + // testing framework + let _ = writeln!(stderr(), "{name}> ready"); pending().await }; tokio::select! { @@ -92,8 +77,9 @@ impl Drop for KillGuard { } #[test] -#[ignore = "this currently fails due to tokio's global state"] fn test_handling_signals() -> Result<()> { + zygote::Zygote::global(); + // use a thread scope to ensure we join all threads at the end std::thread::scope(|s| -> Result<()> { let mut containers = vec![]; @@ -108,7 +94,7 @@ fn test_handling_signals() -> Result<()> { containers.push(Arc::new(container)); } - let guard: Vec<_> = containers.iter().cloned().map(KillGuard).collect(); + let _guard: Vec<_> = containers.iter().cloned().map(KillGuard).collect(); for container in containers.iter() { container.start()?; @@ -148,8 +134,6 @@ fn test_handling_signals() -> Result<()> { assert_eq!(id, i); } - drop(guard); - Ok(()) }) } From fd13cd11a8bb896d026518f1ae419386d904bb05 Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Fri, 20 Dec 2024 16:15:44 +0000 Subject: [PATCH 2/4] initialize zygote in tests Signed-off-by: Jorge Prendes --- crates/containerd-shim-wasm/src/testing.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/containerd-shim-wasm/src/testing.rs b/crates/containerd-shim-wasm/src/testing.rs index 25dda1fb5..78bf02d4c 100644 --- a/crates/containerd-shim-wasm/src/testing.rs +++ b/crates/containerd-shim-wasm/src/testing.rs @@ -48,6 +48,8 @@ where WasiInstance::Engine: Default + Send + Sync + Clone, { pub fn new() -> Result { + zygote::Zygote::init(); + // start logging // to enable logging run `export RUST_LOG=trace` and append cargo command with // --show-output before running test From 4e1112668c1bcb00970c0a853743baba91d07353 Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Fri, 27 Dec 2024 17:36:51 +0000 Subject: [PATCH 3/4] fix CI failure in signals test Signed-off-by: Jorge Prendes --- crates/containerd-shim-wasm/src/test/signals.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/containerd-shim-wasm/src/test/signals.rs b/crates/containerd-shim-wasm/src/test/signals.rs index 87debbe52..2423ca088 100644 --- a/crates/containerd-shim-wasm/src/test/signals.rs +++ b/crates/containerd-shim-wasm/src/test/signals.rs @@ -18,6 +18,7 @@ //! Once #755 is fixed we can remove the libc based implementation and //! remove the ignore attribute from the test. +use std::fs::canonicalize; use std::future::pending; use std::io::{stderr, Write as _}; use std::sync::mpsc::channel; @@ -85,12 +86,18 @@ fn test_handling_signals() -> Result<()> { let mut containers = vec![]; for i in 0..20 { - let container = WasiTest::::builder()? + let builder = WasiTest::::builder()? .with_name(format!("test-{i}")) .with_start_fn(format!("test-{i}")) - .with_stdout("/proc/self/fd/1")? - .with_wasm(HELLO_WORLD)? - .build()?; + .with_wasm(HELLO_WORLD)?; + + // In CI /proc/self/fd/1 doesn't seem to be available + let builder = match canonicalize("/proc/self/fd/1") { + Ok(stdout) => builder.with_stdout(stdout)?, + _ => builder, + }; + + let container = builder.build()?; containers.push(Arc::new(container)); } From b9d7303e23a8a910b5a125a975e119639d0c1917 Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Fri, 17 Jan 2025 17:13:17 +0000 Subject: [PATCH 4/4] bump zygote Signed-off-by: Jorge Prendes --- Cargo.lock | 6 +++--- crates/containerd-shim-wasm/Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1f55ccd8a..e1d45845c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3972,7 +3972,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "81bddcdb20abf9501610992b6759a4c888aef7d1a7247ef75e2404275ac24af1" dependencies = [ "anyhow", - "itertools 0.10.5", + "itertools 0.12.1", "proc-macro2", "quote", "syn 2.0.87", @@ -7778,9 +7778,9 @@ dependencies = [ [[package]] name = "zygote" -version = "0.1.1" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b96e95c06f0156974c3cbdac79e5454938c1d301fca4224f5e17f8020cf011a0" +checksum = "8b78cf6140893151497b58da3c52d4e4dda37be26273b4acefe92a6f294d7cb6" dependencies = [ "libc", "nix 0.29.0", diff --git a/crates/containerd-shim-wasm/Cargo.toml b/crates/containerd-shim-wasm/Cargo.toml index 8c409974b..4f6dda883 100644 --- a/crates/containerd-shim-wasm/Cargo.toml +++ b/crates/containerd-shim-wasm/Cargo.toml @@ -64,7 +64,7 @@ tracing-opentelemetry = { version = "0.24", optional = true } [target.'cfg(unix)'.dependencies] -zygote = { version = "0.1.1" } +zygote = { version = "0.1.2" } caps = "0.5" # this must match the version pulled by libcontainer dbus = { version = "0", features = ["vendored"] }