Merge pull request #788 from jprendes/use-youki-stdio

Use youki's with_stdxx functions

Cargo.toml

@@ -29,7 +29,6 @@ containerd-shim = "0.7.4"
 containerd-shim-wasm = { path = "crates/containerd-shim-wasm", version = "0.8.0" }
 containerd-shim-wasm-test-modules = { path = "crates/containerd-shim-wasm-test-modules", version = "0.4.0"}
 oci-tar-builder = { path = "crates/oci-tar-builder", version = "0.4.0" }
-crossbeam = { version = "0.8.4", default-features = false }
 env_logger = "0.11"
 libc = "0.2.169"
 libcontainer = { git = "https://github.com/youki-dev/youki.git", default-features = false }

crates/containerd-shim-wamr/src/instance.rs

@@ -1,5 +1,5 @@
 use anyhow::{Context, Result};
-use containerd_shim_wasm::container::{Engine, Entrypoint, Instance, RuntimeContext, Stdio};
+use containerd_shim_wasm::container::{Engine, Entrypoint, Instance, RuntimeContext};
 use wamr_rust_sdk::function::Function;
 use wamr_rust_sdk::instance::Instance as WamrInst;
 use wamr_rust_sdk::module::Module;
@@ -36,7 +36,7 @@ impl Engine for WamrEngine {
         "wamr"
     }
 
-    fn run_wasi(&self, ctx: &impl RuntimeContext, stdio: Stdio) -> Result<i32> {
+    fn run_wasi(&self, ctx: &impl RuntimeContext) -> Result<i32> {
         let args = ctx.args();
         let envs = ctx.envs();
         let Entrypoint {
@@ -73,9 +73,6 @@ impl Engine for WamrEngine {
         let instance = WamrInst::new(&self.runtime, &module, 1024 * 64)
             .context("Failed to create instance")?;
 
-        log::info!("redirect stdio");
-        stdio.redirect()?;
-
         log::info!("Running {func:?}");
         let function =
             Function::find_export_func(&instance, &func).context("Failed to find function")?;

crates/containerd-shim-wasm/CHANGELOG.md

@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 
 ### Changed
 - Reuse and synchronise access to `Container` object instead of reloading form disk ([#763](https://github.com/containerd/runwasi/pull/763))
+- Remove custom stdio redicrection: The `run_wasi` method doesn't receive the `Stdio` object anymore, and redirection is done before the method is called ([#788](https://github.com/containerd/runwasi/pull/788))
 
 ### Removed
 - Removed `containerd_shim_wasm::sandbox::instance_utils::get_instance_root` and `containerd_shim_wasm::sandbox::instance_utils::instance_exists` functions ([#763](https://github.com/containerd/runwasi/pull/763))

crates/containerd-shim-wasm/Cargo.toml

@@ -17,7 +17,6 @@ chrono = { workspace = true }
 containerd-shim = { workspace = true }
 containerd-shim-wasm-test-modules = { workspace = true, optional = true }
 oci-tar-builder = { workspace = true, optional = true }
-crossbeam = { workspace = true }
 env_logger = { workspace = true, optional = true }
 git-version = { version = "0.3.9" }
 libc = { workspace = true }

crates/containerd-shim-wasm/src/container/engine.rs

@@ -6,14 +6,13 @@ use anyhow::{bail, Context, Result};
 use super::Source;
 use crate::container::{PathResolve, RuntimeContext};
 use crate::sandbox::oci::WasmLayer;
-use crate::sandbox::Stdio;
 
 pub trait Engine: Clone + Send + Sync + 'static {
     /// The name to use for this engine
     fn name() -> &'static str;
 
     /// Run a WebAssembly container
-    fn run_wasi(&self, ctx: &impl RuntimeContext, stdio: Stdio) -> Result<i32>;
+    fn run_wasi(&self, ctx: &impl RuntimeContext) -> Result<i32>;
 
     /// Check that the runtime can run the container.
     /// This checks runs after the container creation and before the container starts.

crates/containerd-shim-wasm/src/container/mod.rs

@@ -22,7 +22,6 @@ pub use instance::Instance;
 pub use path::PathResolve;
 pub use wasm::WasmBinaryType;
 
-pub use crate::sandbox::stdio::Stdio;
 use crate::sys::container::instance;
 
 #[cfg(test)]

crates/containerd-shim-wasm/src/container/tests.rs

@@ -1,6 +1,6 @@
 use anyhow::bail;
 
-use crate::container::{Engine, RuntimeContext, Stdio};
+use crate::container::{Engine, RuntimeContext};
 use crate::sys::container::instance::Instance;
 use crate::testing::WasiTest;
 
@@ -14,7 +14,7 @@ impl Engine for EngineFailingValidation {
     fn can_handle(&self, _ctx: &impl RuntimeContext) -> anyhow::Result<()> {
         bail!("can't handle");
     }
-    fn run_wasi(&self, _ctx: &impl RuntimeContext, _stdio: Stdio) -> anyhow::Result<i32> {
+    fn run_wasi(&self, _ctx: &impl RuntimeContext) -> anyhow::Result<i32> {
         Ok(0)
     }
 }

crates/containerd-shim-wasm/src/sandbox/containerd/client.rs

@@ -602,7 +602,6 @@ mod tests {
 
     use super::*;
     use crate::container::RuntimeContext;
-    use crate::sandbox::Stdio;
     use crate::testing::oci_helpers::ImageContent;
     use crate::testing::{oci_helpers, TEST_NAMESPACE};
 
@@ -992,11 +991,7 @@ mod tests {
             "fake"
         }
 
-        fn run_wasi(
-            &self,
-            _ctx: &impl RuntimeContext,
-            _stdio: Stdio,
-        ) -> std::result::Result<i32, anyhow::Error> {
+        fn run_wasi(&self, _ctx: &impl RuntimeContext) -> std::result::Result<i32, anyhow::Error> {
             panic!("not implemented")
         }
 

crates/containerd-shim-wasm/src/sandbox/mod.rs

@@ -5,13 +5,11 @@ pub mod error;
 pub mod instance;
 pub mod instance_utils;
 pub mod shim;
-pub mod stdio;
 pub mod sync;
 
 pub use error::{Error, Result};
 pub use instance::{Instance, InstanceConfig};
 pub use shim::Cli as ShimCli;
-pub use stdio::Stdio;
 
 pub(crate) mod containerd;
 pub(crate) mod oci;

crates/containerd-shim-wasm/src/sys/unix/container/executor.rs

@@ -14,7 +14,7 @@ use libcontainer::workload::{
 use oci_spec::image::Platform;
 use oci_spec::runtime::Spec;
 
-use crate::container::{Engine, PathResolve, RuntimeContext, Source, Stdio, WasiContext};
+use crate::container::{Engine, PathResolve, RuntimeContext, Source, WasiContext};
 use crate::sandbox::oci::WasmLayer;
 
 #[derive(Clone)]
@@ -27,7 +27,6 @@ enum InnerExecutor {
 #[derive(Clone)]
 pub(crate) struct Executor<E: Engine> {
     engine: E,
-    stdio: Stdio,
     inner: OnceCell<InnerExecutor>,
     wasm_layers: Vec<WasmLayer>,
     platform: Platform,
@@ -51,12 +50,11 @@ impl<E: Engine> LibcontainerExecutor for Executor<E> {
             InnerExecutor::CantHandle => Err(LibcontainerExecutorError::CantHandle(E::name())),
             InnerExecutor::Linux => {
                 log::info!("executing linux container");
-                self.stdio.take().redirect().unwrap();
                 DefaultExecutor {}.exec(spec)
             }
             InnerExecutor::Wasm => {
                 log::info!("calling start function");
-                match self.engine.run_wasi(&self.ctx(spec), self.stdio.take()) {
+                match self.engine.run_wasi(&self.ctx(spec)) {
                     Ok(code) => std::process::exit(code),
                     Err(err) => {
                         log::info!("error running start function: {err}");
@@ -84,10 +82,9 @@ impl<E: Engine> LibcontainerExecutor for Executor<E> {
 }
 
 impl<E: Engine> Executor<E> {
-    pub fn new(engine: E, stdio: Stdio, wasm_layers: Vec<WasmLayer>, platform: Platform) -> Self {
+    pub fn new(engine: E, wasm_layers: Vec<WasmLayer>, platform: Platform) -> Self {
         Self {
             engine,
-            stdio,
             inner: Default::default(),
             wasm_layers,
             platform,

crates/containerd-shim-wasm/src/sys/unix/container/instance.rs

@@ -20,9 +20,10 @@ use crate::sandbox::async_utils::AmbientRuntime as _;
 use crate::sandbox::instance_utils::determine_rootdir;
 use crate::sandbox::sync::WaitableCell;
 use crate::sandbox::{
-    containerd, Error as SandboxError, Instance as SandboxInstance, InstanceConfig, Stdio,
+    containerd, Error as SandboxError, Instance as SandboxInstance, InstanceConfig,
 };
 use crate::sys::container::executor::Executor;
+use crate::sys::stdio::open;
 
 static DEFAULT_CONTAINER_ROOT_DIR: &str = "/run/containerd";
 
@@ -44,7 +45,6 @@ impl<E: Engine> SandboxInstance for Instance<E> {
         let namespace = cfg.get_namespace();
         let rootdir = Path::new(DEFAULT_CONTAINER_ROOT_DIR).join(E::name());
         let rootdir = determine_rootdir(&bundle, &namespace, rootdir)?;
-        let stdio = Stdio::init_from_cfg(cfg)?;
 
         // 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()?
@@ -55,12 +55,21 @@ impl<E: Engine> SandboxInstance for Instance<E> {
                 (vec![], Platform::default())
             });
 
-        let container = ContainerBuilder::new(id.clone(), SyscallType::Linux)
-            .with_executor(Executor::new(engine, stdio, modules, platform))
-            .with_root_path(rootdir.clone())?
-            .as_init(&bundle)
-            .with_systemd(false)
-            .build()?;
+        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()?;
 
         Ok(Self {
             id,