diff --git a/crates/containerd-shim-wasm/CHANGELOG.md b/crates/containerd-shim-wasm/CHANGELOG.md index 9419c8705..332d79a60 100644 --- a/crates/containerd-shim-wasm/CHANGELOG.md +++ b/crates/containerd-shim-wasm/CHANGELOG.md @@ -4,6 +4,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] +### Added +- Added test for signal handling issue [#755](https://github.com/containerd/runwasi/issues/755) ([#756](https://github.com/containerd/runwasi/pull/756)) + ## [v0.8.0] — 2024-12-04 ### Added diff --git a/crates/containerd-shim-wasm/Cargo.toml b/crates/containerd-shim-wasm/Cargo.toml index c9700f2d5..6f453cf27 100644 --- a/crates/containerd-shim-wasm/Cargo.toml +++ b/crates/containerd-shim-wasm/Cargo.toml @@ -78,6 +78,7 @@ windows-sys = { workspace = true, features = [ ] } [dev-dependencies] +tokio = { workspace = true, features = ["signal"] } containerd-shim-wasm-test-modules = { workspace = true } env_logger = { workspace = true } tempfile = { workspace = true } diff --git a/crates/containerd-shim-wasm/src/lib.rs b/crates/containerd-shim-wasm/src/lib.rs index fcf7e8cac..b46b95211 100644 --- a/crates/containerd-shim-wasm/src/lib.rs +++ b/crates/containerd-shim-wasm/src/lib.rs @@ -11,6 +11,12 @@ pub mod sandbox; pub(crate) mod sys; #[cfg(any(test, feature = "testing"))] +/// Utilities for writing shims tests. +/// You can use this to test your runwasi based shim. pub mod testing; +#[cfg(test)] +/// Tests for runwasi's containerd-shim-wasm. +mod test; + pub use containerd_shim::Config; diff --git a/crates/containerd-shim-wasm/src/test/mod.rs b/crates/containerd-shim-wasm/src/test/mod.rs new file mode 100644 index 000000000..c10475fb2 --- /dev/null +++ b/crates/containerd-shim-wasm/src/test/mod.rs @@ -0,0 +1 @@ +mod signals; diff --git a/crates/containerd-shim-wasm/src/test/signals.rs b/crates/containerd-shim-wasm/src/test/signals.rs new file mode 100644 index 000000000..6a223a5fa --- /dev/null +++ b/crates/containerd-shim-wasm/src/test/signals.rs @@ -0,0 +1,157 @@ +//! This module includes a test for sending signals to containers when +//! the shim is managing two or more containers. +//! +//! See https://github.com/containerd/runwasi/issues/755 for context. +//! +//! This test is currently broken for the reasons explained in #755. +//! Running the test will result in a failure: +//! ``` +//! cargo test -p containerd-shim-wasm -- test::signals::test_handling_signals --exact --show-output --nocapture --ignored +//! ``` +//! +//! This is because the current implementation breaks `tokio::signal`. +//! You can verify this by using `libc::signal` instead, and the test will succeed +//! ``` +//! USE_LIBC=1 cargo test -p containerd-shim-wasm -- test::signals::test_handling_signals --exact --show-output --nocapture --ignored +//! ``` +//! +//! Once #755 is fixed we can remove the libc based implementation and +//! remove the ignore attribute from the test. + +use std::future::pending; +use std::sync::mpsc::channel; +use std::sync::{Arc, LazyLock}; +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::sandbox::Stdio; +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" + } + + fn run_wasi(&self, ctx: &impl RuntimeContext, stdio: Stdio) -> Result { + stdio.redirect()?; + let name = ctx.entrypoint().func; + tokio::runtime::Builder::new_current_thread() + .enable_all() + .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; + println!("{name}> received signal, bye!"); + }; + let task = async { + sleep(Duration::from_millis(10)).await; + eprintln!("{name}> ready"); + pending().await + }; + tokio::select! { + _ = signal => {} + _ = task => {} + }; + Ok(0) + }) + } +} + +type SomeInstance = Instance; + +struct KillGuard(Arc>); +impl Drop for KillGuard { + fn drop(&mut self) { + let _ = self.0.kill(); + } +} + +#[test] +#[ignore = "this currently fails due to tokio's global state"] +fn test_handling_signals() -> Result<()> { + // use a thread scope to ensure we join all threads at the end + std::thread::scope(|s| -> Result<()> { + let mut containers = vec![]; + + for i in 0..20 { + let container = WasiTest::::builder()? + .with_name(format!("test-{i}")) + .with_start_fn(format!("test-{i}")) + .with_stdout("/proc/self/fd/1")? + .with_wasm(HELLO_WORLD)? + .build()?; + containers.push(Arc::new(container)); + } + + let guard: Vec<_> = containers.iter().cloned().map(KillGuard).collect(); + + for container in containers.iter() { + container.start()?; + } + + let (tx, rx) = channel(); + + for (i, container) in containers.iter().cloned().enumerate() { + let tx = tx.clone(); + s.spawn(move || -> anyhow::Result<()> { + println!("shim> waiting for container {i}"); + let (code, ..) = container.wait(Duration::from_secs(10000))?; + println!("shim> container test-{i} exited with code {code}"); + tx.send(i)?; + Ok(()) + }); + } + + 'outer: for (i, container) in containers.iter().enumerate() { + for _ in 0..100 { + let stderr = container.read_stderr()?.unwrap_or_default(); + if stderr.contains("ready") { + continue 'outer; + } + sleep(Duration::from_millis(1)); + } + bail!("timeout waiting for container test-{i}"); + } + + println!("shim> all containers ready"); + + for (i, container) in containers.iter().enumerate() { + println!("shim> sending ctrl-c to container test-{i}"); + let _ = container.ctrl_c()?; + let id = rx.recv_timeout(Duration::from_secs(5))?; + println!("shim> received exit from container test-{id} (expected test-{i})"); + assert_eq!(id, i); + } + + drop(guard); + + Ok(()) + }) +} diff --git a/crates/containerd-shim-wasm/src/testing.rs b/crates/containerd-shim-wasm/src/testing.rs index 5379828fa..9fd2831ea 100644 --- a/crates/containerd-shim-wasm/src/testing.rs +++ b/crates/containerd-shim-wasm/src/testing.rs @@ -4,6 +4,11 @@ use std::collections::HashMap; use std::fs::{self, create_dir, read, read_to_string, write, File}; use std::marker::PhantomData; use std::ops::Add; +#[cfg(unix)] +use std::os::unix::fs::symlink; +#[cfg(windows)] +use std::os::windows::fs::symlink_file as symlink; +use std::path::Path; use std::time::Duration; use anyhow::{bail, Result}; @@ -76,6 +81,11 @@ where Ok(builder) } + pub fn with_name(mut self, name: impl Into) -> Self { + self.container_name = name.into(); + self + } + pub fn with_host_network(mut self) -> Self { // Removing the `network` namespace results in the binding to the host's socket. // This allows for direct communication with the host's networking interface. @@ -113,6 +123,32 @@ where Ok(self) } + pub fn with_stdout(self, stdout: impl AsRef) -> Result { + let stdout = fs::canonicalize(stdout.as_ref())?; + + let dir = self.tempdir.path(); + + log::info!("setting wasi test stdout to {:?}", stdout); + + std::fs::remove_file(dir.join("stdout"))?; + symlink(stdout, dir.join("stdout"))?; + + Ok(self) + } + + pub fn with_stderr(self, stderr: impl AsRef) -> Result { + let stderr = fs::canonicalize(stderr.as_ref())?; + + let dir = self.tempdir.path(); + + log::info!("setting wasi test stderr to {:?}", stderr); + + std::fs::remove_file(dir.join("stderr"))?; + symlink(stderr, dir.join("stderr"))?; + + Ok(self) + } + pub fn as_oci_image( mut self, image_name: Option, @@ -229,9 +265,13 @@ where Ok(self) } - pub fn wait(&self, timeout: Duration) -> Result<(u32, String, String)> { - let dir = self.tempdir.path(); + pub fn kill(&self) -> Result<&Self> { + log::info!("sending SIGKILL"); + self.instance.kill(SIGKILL as u32)?; + Ok(self) + } + pub fn wait(&self, timeout: Duration) -> Result<(u32, String, String)> { log::info!("waiting wasi test"); let (status, _) = match self.instance.wait_timeout(timeout) { Some(res) => res, @@ -241,8 +281,8 @@ where } }; - let stdout = read_to_string(dir.join("stdout"))?; - let stderr = read_to_string(dir.join("stderr"))?; + let stdout = self.read_stdout()?.unwrap_or_default(); + let stderr = self.read_stderr()?.unwrap_or_default(); self.instance.delete()?; @@ -250,6 +290,26 @@ where Ok((status, stdout, stderr)) } + + pub fn root(&self) -> &Path { + self.tempdir.path() + } + + pub fn read_stdout(&self) -> Result> { + let path = self.tempdir.path().join("stdout"); + if path.is_symlink() { + return Ok(None); + } + Ok(Some(read_to_string(path)?)) + } + + pub fn read_stderr(&self) -> Result> { + let path = self.tempdir.path().join("stderr"); + if path.is_symlink() { + return Ok(None); + } + Ok(Some(read_to_string(path)?)) + } } pub mod oci_helpers {