From bf84bccf531e090c32fa8cdfc6d4f68a6a245c92 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Fri, 23 Jun 2023 18:36:57 +0000 Subject: [PATCH] Happy medium (?) for getting containerd path Signed-off-by: James Sturtevant --- .../benches/webassembly-benchmarks.rs | 4 +- .../src/sandbox/containerd.rs | 11 +++-- .../src/sandbox/instance.rs | 11 +++-- .../containerd-shim-wasm/src/sandbox/shim.rs | 31 ++------------ .../containerd-shim-wasmedge/src/executor.rs | 24 +++++++---- .../containerd-shim-wasmedge/src/instance.rs | 13 ++++-- .../containerd-shim-wasmtime/src/instance.rs | 42 ++++++++----------- 7 files changed, 62 insertions(+), 74 deletions(-) diff --git a/benches/containerd-shim-benchmarks/benches/webassembly-benchmarks.rs b/benches/containerd-shim-benchmarks/benches/webassembly-benchmarks.rs index 80f71613c..7079b442e 100644 --- a/benches/containerd-shim-benchmarks/benches/webassembly-benchmarks.rs +++ b/benches/containerd-shim-benchmarks/benches/webassembly-benchmarks.rs @@ -108,7 +108,7 @@ fn run_wasmtime_test_with_spec( spec.save(dir.path().join("config.json"))?; - let mut cfg = InstanceConfig::new(WasmtimeWasi::new_engine()?, "test_namespace".into(), None); + let mut cfg = InstanceConfig::new(WasmtimeWasi::new_engine()?, "test_namespace".into()); let cfg = cfg .set_bundle(dir.path().to_str().unwrap().to_string()) .set_stdout(dir.path().join("stdout").to_str().unwrap().to_string()); @@ -169,7 +169,7 @@ fn run_wasmedge_test_with_spec( spec.save(dir.path().join("config.json"))?; - let mut cfg = InstanceConfig::new(WasmEdgeWasi::new_engine()?, "test_namespace".into(), None); + let mut cfg = InstanceConfig::new(WasmEdgeWasi::new_engine()?, "test_namespace".into()); let cfg = cfg .set_bundle(dir.path().to_str().unwrap().to_string()) .set_stdout(dir.path().join("stdout").to_str().unwrap().to_string()); diff --git a/crates/containerd-shim-wasm/src/sandbox/containerd.rs b/crates/containerd-shim-wasm/src/sandbox/containerd.rs index fe3324089..3abb76348 100644 --- a/crates/containerd-shim-wasm/src/sandbox/containerd.rs +++ b/crates/containerd-shim-wasm/src/sandbox/containerd.rs @@ -9,8 +9,8 @@ use client::{ with_namespace, }; use containerd_client as client; -use tonic::Request; use std::ffi::OsStr; +use tonic::Request; pub struct SyncContentClient { inner: ContentClient, @@ -69,10 +69,9 @@ impl SyncContentClient { } } - -// this is from https://github.com/containerd/rust-extensions/blob/main/crates/shim/src/error.rs -// as it not exported -// TODO export upstream and remove this +// this is from https://github.com/containerd/rust-extensions/blob/main/crates/shim/src/error.rs +// as it not exported +// TODO export upstream and remove this #[derive(Debug, Default)] pub struct Flags { /// Enable debug output in logs. @@ -121,4 +120,4 @@ pub fn parse>(args: &[S]) -> Result { } Ok(flags) -} \ 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 3e76b77aa..d3ba7a758 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance.rs @@ -2,12 +2,13 @@ use std::sync::mpsc::Sender; use std::sync::{Arc, Condvar, Mutex}; -use std::thread; +use std::{env, thread}; use libc::{SIGINT, SIGKILL, SIGTERM}; use chrono::{DateTime, Utc}; +use super::containerd; use super::error::Error; type ExitCode = (Mutex)>>, Condvar); @@ -40,8 +41,12 @@ impl InstanceConfig where E: Send + Sync + Clone, { - pub fn new(engine: E, namespace: String, containerd_address: Option) -> Self { - // todo read containerd address + pub fn new(engine: E, namespace: String) -> Self { + let os_args: Vec<_> = env::args_os().collect(); + let containerd_address = match containerd::parse(&os_args[1..]) { + Ok(flags) => Some(flags.address), + _ => None, + }; Self { engine, namespace, diff --git a/crates/containerd-shim-wasm/src/sandbox/shim.rs b/crates/containerd-shim-wasm/src/sandbox/shim.rs index c962c4e4f..796cfbeb1 100644 --- a/crates/containerd-shim-wasm/src/sandbox/shim.rs +++ b/crates/containerd-shim-wasm/src/sandbox/shim.rs @@ -3,7 +3,7 @@ //! the container/sandbox. use std::collections::HashMap; -use std::env::{current_dir, self}; +use std::env::current_dir; use std::fs::{self, File}; use std::fs::{canonicalize, create_dir_all, OpenOptions}; use std::ops::Not; @@ -14,7 +14,7 @@ use std::sync::{Arc, Condvar, Mutex, RwLock}; use std::thread; use super::instance::{EngineGetter, Instance, InstanceConfig, Nop, Wait}; -use super::{oci, Error, SandboxService, containerd}; +use super::{oci, Error, SandboxService}; use chrono::{DateTime, Utc}; use containerd_shim::{ self as shim, api, @@ -339,7 +339,6 @@ where events: Arc>, exit: Arc, namespace: String, - containerd_address: Option } #[cfg(test)] @@ -425,7 +424,6 @@ mod localtests { tx, Arc::new(ExitSignal::default()), "test_namespace".into(), - Some("/pipe/to/containerd".into()), )); let mut _wrapped = LocalWithDescrutor::new(local.clone()); @@ -456,7 +454,6 @@ mod localtests { etx, exit_signal, "test_namespace".into(), - Some("/pipe/to/containerd".into()), )); let mut _wrapped = LocalWithDescrutor::new(local.clone()); @@ -625,7 +622,6 @@ mod localtests { etx, exit_signal, "test_namespace".into(), - Some("/pipe/to/containerd".into()), )); let mut _wrapped = LocalWithDescrutor::new(local.clone()); @@ -739,7 +735,6 @@ where tx: Sender<(String, Box)>, exit: Arc, namespace: String, - containerd_address: Option, ) -> Self where T: Instance + Sync + Send, @@ -752,12 +747,11 @@ where events: Arc::new(Mutex::new(tx)), exit, namespace, - containerd_address, } } fn new_base(&self, id: String) -> InstanceData { - let cfg = InstanceConfig::new(self.engine.clone(), self.namespace.clone(), self.containerd_address.clone()); + let cfg = InstanceConfig::new(self.engine.clone(), self.namespace.clone()); InstanceData { instance: None, base: Some(Nop::new(id, None)), @@ -948,7 +942,7 @@ where } let engine = self.engine.clone(); - let mut builder = InstanceConfig::new(engine, self.namespace.clone(), self.containerd_address.clone()); + let mut builder = InstanceConfig::new(engine, self.namespace.clone()); builder .set_stdin(req.get_stdin().into()) .set_stdout(req.get_stdout().into()) @@ -1207,13 +1201,6 @@ where { type Instance = T; fn new(namespace: String, _id: String, engine: E, publisher: RemotePublisher) -> Self { - - let os_args: Vec<_> = env::args_os().collect(); - let containerd_address = match containerd::parse(&os_args[1..]) { - Ok(flags) => Some(flags.address), - _ => None - }; - let (tx, rx) = channel::<(String, Box)>(); forward_events(namespace.clone(), publisher, rx); Local::::new( @@ -1221,7 +1208,6 @@ where tx.clone(), Arc::new(ExitSignal::default()), namespace, - containerd_address, ) } } @@ -1358,7 +1344,6 @@ where { pub engine: E, namespace: String, - containerd_address: Option, phantom: std::marker::PhantomData, exit: Arc, _id: String, @@ -1373,17 +1358,10 @@ where fn new(_runtime_id: &str, id: &str, namespace: &str, _config: &mut shim::Config) -> Self { // Ideally this function passes in either the containerd address or the flags from the cli - let os_args: Vec<_> = env::args_os().collect(); - let containerd_address = match containerd::parse(&os_args[1..]) { - Ok(flags) => Some(flags.address), - _ => None - }; - Cli { engine: I::new_engine().unwrap(), phantom: std::marker::PhantomData, namespace: namespace.to_string(), - containerd_address: containerd_address, exit: Arc::new(ExitSignal::default()), _id: id.to_string(), } @@ -1507,7 +1485,6 @@ where tx.clone(), self.exit.clone(), self.namespace.clone(), - self.containerd_address.clone(), ) } diff --git a/crates/containerd-shim-wasmedge/src/executor.rs b/crates/containerd-shim-wasmedge/src/executor.rs index f6ad23a70..ef8afe02b 100644 --- a/crates/containerd-shim-wasmedge/src/executor.rs +++ b/crates/containerd-shim-wasmedge/src/executor.rs @@ -6,7 +6,7 @@ use oci_spec::runtime::Spec; use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use libcontainer::workload::{Executor, ExecutorError}; use log::debug; -use std::{os::unix::io::RawFd, env}; +use std::os::unix::io::RawFd; use wasmedge_sdk::{ config::{CommonConfigOptions, ConfigBuilder, HostRegistrationConfigOptions}, params, VmBuilder, @@ -18,6 +18,8 @@ pub struct WasmEdgeExecutor { pub stdin: Option, pub stdout: Option, pub stderr: Option, + pub namespace: String, + pub containerd_address: Option, } impl Executor for WasmEdgeExecutor { @@ -65,14 +67,20 @@ impl Executor for WasmEdgeExecutor { let vm = match oci::get_oci_artifact(spec) { Some(oci_module) => { debug!("loading module from annotations"); - let os_args: Vec<_> = env::args_os().collect(); - let flags = containerd::parse(&os_args[1..]).map_err(|err| ExecutorError::Execution(err.into()))?; - let mut ctrd_client = containerd::SyncContentClient::connect( - flags.address, - ) - .map_err(|err| ExecutorError::Execution(err.into()))?; + let containerd_address = match &self.containerd_address { + Some(addr) => addr.clone(), + None => return Err(ExecutorError::Execution( + anyhow::Error::msg( + "no containerd address provided, cannot load module from containerd", + ) + .into(), + )), + }; + + let mut ctrd_client = containerd::SyncContentClient::connect(containerd_address) + .map_err(|err| ExecutorError::Execution(err.into()))?; let module = ctrd_client - .read_content(oci_module, &flags.namespace) + .read_content(oci_module, &self.namespace) .map_err(|err| ExecutorError::Execution(err.into()))?; vm.register_module_from_bytes("main", module) diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index 52dbe5f5c..305c6ee42 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -53,6 +53,9 @@ pub struct Wasi { bundle: String, rootdir: PathBuf, + + namespace: String, + containerd_address: Option, } fn construct_container_root>(root_path: P, container_id: &str) -> Result { @@ -207,12 +210,14 @@ impl Instance for Wasi { let namespace = cfg.get_namespace(); Wasi { id, - rootdir: determine_rootdir(bundle.as_str(), namespace).unwrap(), + rootdir: determine_rootdir(bundle.as_str(), namespace.clone()).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, + namespace, + containerd_address: cfg.get_containerd_address(), } } @@ -328,6 +333,8 @@ impl Wasi { stdin, stdout, stderr, + namespace: self.namespace.clone(), + containerd_address: self.containerd_address.clone(), })])? .with_root_path(self.rootdir.clone())? .as_init(&self.bundle) @@ -438,7 +445,7 @@ mod wasitest { spec.save(dir.path().join("config.json"))?; - let mut cfg = InstanceConfig::new(Wasi::new_engine()?, "test_namespace".into(), Some("/test/pipe".into())); + let mut cfg = InstanceConfig::new(Wasi::new_engine()?, "test_namespace".into()); let cfg = cfg .set_bundle(dir.path().to_str().unwrap().to_string()) .set_stdout(dir.path().join("stdout").to_str().unwrap().to_string()); @@ -474,7 +481,7 @@ mod wasitest { let vm = VmBuilder::new().with_config(config).build().unwrap(); let i = Wasi::new( "".to_string(), - Some(&InstanceConfig::new(vm, "test_namespace".into(), Some("".into()))), + Some(&InstanceConfig::new(vm, "test_namespace".into())), ); i.delete().unwrap(); } diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index e2a17548e..0601253f7 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -1,7 +1,7 @@ use std::fs::OpenOptions; use std::path::Path; use std::sync::{Arc, Condvar, Mutex}; -use std::{thread}; +use std::thread; use anyhow::Context; use chrono::{DateTime, Utc}; @@ -86,47 +86,44 @@ fn load_spec(bundle: String) -> Result { impl Wasi { fn prepare_module(&self, spec: &oci::Spec) -> Result<(WasiCtx, Module, String), WasmtimeError> { - let stdin_path = self.stdin.clone(); let stdout_path = self.stdout.clone(); let stderr_path = self.stderr.clone(); - let engine = self.engine.clone(); - debug!("opening rootfs"); let rootfs = oci_wasmtime::get_rootfs(spec)?; let args = oci::get_args(spec); let env = oci_wasmtime::env_to_wasi(spec); - + debug!("setting up wasi"); let mut wasi_builder = WasiCtxBuilder::new() .args(args)? .envs(env.as_slice())? .preopened_dir(rootfs, "/")?; - + debug!("opening stdin"); let stdin = maybe_open_stdio(&stdin_path).context("could not open stdin")?; if let Some(sin) = stdin { wasi_builder = wasi_builder.stdin(Box::new(sin)); } - + debug!("opening stdout"); let stdout = maybe_open_stdio(&stdout_path).context("could not open stdout")?; if let Some(sout) = stdout { wasi_builder = wasi_builder.stdout(Box::new(sout)); } - + debug!("opening stderr"); let stderr = maybe_open_stdio(&stderr_path).context("could not open stderr")?; if let Some(serr) = stderr { wasi_builder = wasi_builder.stderr(Box::new(serr)); } - + debug!("building wasi context"); let wctx = wasi_builder.build(); debug!("wasi context ready"); - + let (module_name, method) = oci::get_module(spec); let module_name = match module_name { Some(m) => m, @@ -136,7 +133,7 @@ impl Wasi { ))) } }; - + if let Some(oci_module) = oci::get_oci_artifact(spec) { debug!("loading module from annotations"); @@ -151,12 +148,13 @@ impl Wasi { let mut ctrd_client = containerd::SyncContentClient::connect(containerd_address)?; let module = ctrd_client.read_content(oci_module, &self.namespace)?; - let module = Module::from_binary(&engine, &module) - .map_err(|err| Error::Others(format!("could not load module from file: {}", err)))?; - + let module = Module::from_binary(&engine, &module).map_err(|err| { + Error::Others(format!("could not load module from file: {}", err)) + })?; + return Ok((wctx, module, method)); } - + debug!( "loading module from file in container {} with method {}", module_name, method @@ -164,12 +162,11 @@ impl Wasi { let mod_path = oci::get_root(spec).join(module_name); let module = Module::from_file(&engine, mod_path) .map_err(|err| Error::Others(format!("could not load module from file: {}", err)))?; - + Ok((wctx, module, method)) } } - impl Instance for Wasi { type E = wasmtime::Engine; fn new(_id: String, cfg: Option<&InstanceConfig>) -> Self { @@ -197,9 +194,9 @@ impl Instance for Wasi { .map_err(|err| Error::Others(format!("error adding to linker: {}", err)))?; debug!("preparing module"); - - let m = self.prepare_module(&spec) + let m = self + .prepare_module(&spec) .map_err(|e| Error::Others(format!("error setting up module: {}", e)))?; let mut store = Store::new(&engine, m.0); @@ -263,8 +260,6 @@ impl Instance for Wasi { } } - - fn kill(&self, signal: u32) -> Result<(), Error> { if signal != SIGKILL as u32 { return Err(Error::InvalidArgument( @@ -298,8 +293,6 @@ impl Instance for Wasi { } } - - #[cfg(test)] mod wasitest { use std::fs::{create_dir, read_to_string, File}; @@ -352,7 +345,6 @@ mod wasitest { Some(&InstanceConfig::new( Engine::default(), "test_namespace".into(), - Some("/pipe/to/containerd".into()), )), ); i.delete().unwrap(); @@ -403,7 +395,7 @@ mod wasitest { } fn run_module(dir: tempfile::TempDir) -> Result<(), Error> { - let mut cfg = InstanceConfig::new(Engine::default(), "test_namespace".into(), Some("/pipe/to/containerd".into())); + let mut cfg = InstanceConfig::new(Engine::default(), "test_namespace".into()); let cfg = cfg .set_bundle(dir.path().to_str().unwrap().to_string()) .set_stdout(dir.path().join("stdout").to_str().unwrap().to_string());