From c809ea3be35e249806fb00fff9100bf199e4b6a3 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Thu, 17 Aug 2023 23:31:42 +0000 Subject: [PATCH] refactor: moved determine_rootdir to containerd-shim-wasm Signed-off-by: jiaxiao zhou --- Cargo.lock | 2 - .../src/sandbox/instance_utils.rs | 87 ++++++++++++++++- crates/containerd-shim-wasmedge/Cargo.toml | 1 - .../containerd-shim-wasmedge/src/instance.rs | 93 +++---------------- crates/containerd-shim-wasmtime/Cargo.toml | 1 - .../containerd-shim-wasmtime/src/instance.rs | 57 +++--------- 6 files changed, 113 insertions(+), 128 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 95e5fc77e..bd3b2f8e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -578,7 +578,6 @@ dependencies = [ "nix 0.26.2", "oci-spec", "pretty_assertions", - "serde", "serde_json", "serial_test", "tempfile", @@ -605,7 +604,6 @@ dependencies = [ "nix 0.26.2", "oci-spec", "pretty_assertions", - "serde", "serde_json", "tempfile", "thiserror", diff --git a/crates/containerd-shim-wasm/src/sandbox/instance_utils.rs b/crates/containerd-shim-wasm/src/sandbox/instance_utils.rs index 187a1c900..b90ed14ea 100644 --- a/crates/containerd-shim-wasm/src/sandbox/instance_utils.rs +++ b/crates/containerd-shim-wasm/src/sandbox/instance_utils.rs @@ -1,9 +1,10 @@ //! Common utilities for the containerd shims. use crate::sandbox::error::Error; use anyhow::{bail, Context, Result}; +use serde::{Deserialize, Serialize}; use std::{ fs::{self, File, OpenOptions}, - io::ErrorKind, + io::{ErrorKind, Read}, path::{Path, PathBuf}, }; @@ -45,6 +46,38 @@ pub fn maybe_open_stdio(path: &str) -> Result, Error> { } } +#[derive(Serialize, Deserialize)] +struct Options { + root: Option, +} + +pub fn determine_rootdir>( + bundle: P, + namespace: &str, + container_root_dir: P, +) -> Result { + log::info!( + "determining rootdir for bundle: {}", + bundle.as_ref().display() + ); + let mut file = match File::open(bundle.as_ref().join("options.json")) { + Ok(f) => f, + Err(err) => match err.kind() { + ErrorKind::NotFound => return Ok(container_root_dir.as_ref().join(namespace)), + _ => return Err(err.into()), + }, + }; + let mut data = String::new(); + file.read_to_string(&mut data)?; + let options: Options = serde_json::from_str(&data)?; + let path = options + .root + .unwrap_or(container_root_dir.as_ref().to_path_buf()) + .join(namespace); + log::info!("youki root path is: {}", path.display()); + Ok(path) +} + fn construct_instance_root>(root_path: P, container_id: &str) -> Result { let root_path = fs::canonicalize(&root_path).with_context(|| { format!( @@ -58,11 +91,13 @@ fn construct_instance_root>(root_path: P, container_id: &str) -> #[cfg(test)] mod tests { - use std::fs::File; - - use tempfile::tempdir; + use std::{ + fs::{File, OpenOptions}, + io::Write, + }; use super::*; + use tempfile::tempdir; #[test] fn test_maybe_open_stdio() -> Result<(), Error> { @@ -79,4 +114,48 @@ mod tests { assert!(f.is_some()); Ok(()) } + + #[test] + fn test_determine_rootdir_with_options_file() -> Result<(), Error> { + let namespace = "test_namespace"; + let dir = tempdir()?; + let rootdir = dir.path().join("runwasi"); + let opts = Options { + root: Some(rootdir.clone()), + }; + let opts_file = OpenOptions::new() + .read(true) + .create(true) + .truncate(true) + .write(true) + .open(dir.path().join("options.json"))?; + write!(&opts_file, "{}", serde_json::to_string(&opts)?)?; + let root = determine_rootdir( + dir.path(), + namespace.into(), + &PathBuf::from("/run/containerd/runtime"), + )?; + assert_eq!(root, rootdir.join(namespace)); + Ok(()) + } + + #[test] + fn test_determine_rootdir_without_options_file() -> Result<(), Error> { + let dir = tempdir()?; + let namespace = "test_namespace"; + let root = determine_rootdir( + dir.path(), + namespace.into(), + &PathBuf::from("/run/containerd/runtime"), + )?; + assert!(root.is_absolute()); + assert_eq!( + root, + PathBuf::from("/run/containerd/runtime").join(namespace) + ); + Ok(()) + } } + +#[cfg(test)] +mod rootdirtest {} diff --git a/crates/containerd-shim-wasmedge/Cargo.toml b/crates/containerd-shim-wasmedge/Cargo.toml index 70c8896a2..6e90cc087 100644 --- a/crates/containerd-shim-wasmedge/Cargo.toml +++ b/crates/containerd-shim-wasmedge/Cargo.toml @@ -15,7 +15,6 @@ anyhow = { workspace = true } cap-std = { workspace = true } oci-spec = { workspace = true, features = ["runtime"] } thiserror = { workspace = true } -serde = { workspace = true } serde_json = { workspace = true } nix = { workspace = true } libc = { workspace = true } diff --git a/crates/containerd-shim-wasmedge/src/instance.rs b/crates/containerd-shim-wasmedge/src/instance.rs index e81798656..d5da42a5c 100644 --- a/crates/containerd-shim-wasmedge/src/instance.rs +++ b/crates/containerd-shim-wasmedge/src/instance.rs @@ -4,20 +4,14 @@ use containerd_shim_wasm::libcontainer_instance::LibcontainerInstance; use containerd_shim_wasm::libcontainer_instance::LinuxContainerExecutor; use containerd_shim_wasm::sandbox::error::Error; use containerd_shim_wasm::sandbox::instance::ExitCode; +use containerd_shim_wasm::sandbox::instance_utils::determine_rootdir; use containerd_shim_wasm::sandbox::instance_utils::maybe_open_stdio; use containerd_shim_wasm::sandbox::InstanceConfig; use nix::unistd::close; -use serde::{Deserialize, Serialize}; -use std::fs::File; -use std::io::prelude::*; -use std::io::ErrorKind; use std::os::fd::IntoRawFd; use std::sync::{Arc, Condvar, Mutex}; -use std::{ - fs, - path::{Path, PathBuf}, -}; +use std::{fs, path::PathBuf}; use libcontainer::container::builder::ContainerBuilder; use libcontainer::container::Container; @@ -40,30 +34,6 @@ pub struct Wasi { rootdir: PathBuf, } -#[derive(Serialize, Deserialize)] -struct Options { - root: Option, -} - -fn determine_rootdir>(bundle: P, namespace: String) -> Result { - let mut file = match File::open(bundle.as_ref().join("options.json")) { - Ok(f) => f, - Err(err) => match err.kind() { - ErrorKind::NotFound => { - return Ok(<&str as Into>::into(DEFAULT_CONTAINER_ROOT_DIR).join(namespace)) - } - _ => return Err(err.into()), - }, - }; - let mut data = String::new(); - file.read_to_string(&mut data)?; - let options: Options = serde_json::from_str(&data)?; - Ok(options - .root - .unwrap_or(PathBuf::from(DEFAULT_CONTAINER_ROOT_DIR)) - .join(namespace)) -} - impl LibcontainerInstance for Wasi { type Engine = (); @@ -73,7 +43,12 @@ impl LibcontainerInstance for Wasi { let namespace = cfg.get_namespace(); Wasi { id, - rootdir: determine_rootdir(bundle.as_str(), namespace).unwrap(), + rootdir: determine_rootdir( + bundle.as_str(), + namespace.as_str(), + DEFAULT_CONTAINER_ROOT_DIR, + ) + .unwrap(), exit_code: Arc::new((Mutex::new(None), Condvar::new())), stdin: cfg.get_stdin().unwrap_or_default(), stdout: cfg.get_stdout().unwrap_or_default(), @@ -134,7 +109,9 @@ impl LibcontainerInstance for Wasi { #[cfg(test)] mod wasitest { use std::borrow::Cow; + use std::collections::HashMap; use std::fs::{create_dir, read_to_string, File, OpenOptions}; + use std::io::Write; use std::os::unix::io::RawFd; use std::os::unix::prelude::OpenOptionsExt; use std::sync::mpsc::channel; @@ -215,16 +192,17 @@ mod wasitest { create_dir(dir.path().join("rootfs"))?; let rootdir = dir.path().join("runwasi"); create_dir(&rootdir)?; - let opts = Options { - root: Some(rootdir), - }; + let rootdir = PathBuf::from("/path/to/root"); + let mut opts = HashMap::new(); + opts.insert("root", rootdir); + let serialized = serde_json::to_string(&opts)?; let opts_file = OpenOptions::new() .read(true) .create(true) .truncate(true) .write(true) .open(dir.path().join("options.json"))?; - write!(&opts_file, "{}", serde_json::to_string(&opts)?)?; + write!(&opts_file, "{}", serialized)?; let wasm_path = dir.path().join("rootfs/hello.wasm"); let mut f = OpenOptions::new() @@ -335,44 +313,3 @@ mod wasitest { Ok(()) } } - -#[cfg(test)] -mod rootdirtest { - use std::fs::OpenOptions; - - use super::*; - use tempfile::tempdir; - - #[test] - fn test_determine_rootdir_with_options_file() -> Result<(), Error> { - let namespace = "test_namespace"; - let dir = tempdir()?; - let rootdir = dir.path().join("runwasi"); - let opts = Options { - root: Some(rootdir.clone()), - }; - let opts_file = OpenOptions::new() - .read(true) - .create(true) - .truncate(true) - .write(true) - .open(dir.path().join("options.json"))?; - write!(&opts_file, "{}", serde_json::to_string(&opts)?)?; - let root = determine_rootdir(dir.path(), namespace.into())?; - assert_eq!(root, rootdir.join(namespace)); - Ok(()) - } - - #[test] - fn test_determine_rootdir_without_options_file() -> Result<(), Error> { - let dir = tempdir()?; - let namespace = "test_namespace"; - let root = determine_rootdir(dir.path(), namespace.into())?; - assert!(root.is_absolute()); - assert_eq!( - root, - PathBuf::from(DEFAULT_CONTAINER_ROOT_DIR).join(namespace) - ); - Ok(()) - } -} diff --git a/crates/containerd-shim-wasmtime/Cargo.toml b/crates/containerd-shim-wasmtime/Cargo.toml index ed4749e6c..a773e4aa4 100644 --- a/crates/containerd-shim-wasmtime/Cargo.toml +++ b/crates/containerd-shim-wasmtime/Cargo.toml @@ -35,7 +35,6 @@ serde_json = { workspace = true } nix = { workspace = true } libcontainer = { workspace = true } dbus = { version = "*", optional = true } -serde = { workspace = true } libc = { workspace = true } [dev-dependencies] diff --git a/crates/containerd-shim-wasmtime/src/instance.rs b/crates/containerd-shim-wasmtime/src/instance.rs index 0f4f0b045..1a7eb719e 100644 --- a/crates/containerd-shim-wasmtime/src/instance.rs +++ b/crates/containerd-shim-wasmtime/src/instance.rs @@ -1,18 +1,14 @@ -use anyhow::Result; 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}; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::sync::{Arc, Condvar, Mutex}; use anyhow::Context; use containerd_shim_wasm::libcontainer_instance::{LibcontainerInstance, LinuxContainerExecutor}; 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::instance_utils::{determine_rootdir, maybe_open_stdio}; use containerd_shim_wasm::sandbox::InstanceConfig; use libcontainer::syscall::syscall::create_syscall; use std::os::fd::IntoRawFd; @@ -32,36 +28,6 @@ pub struct Wasi { id: String, } -#[derive(Serialize, Deserialize)] -struct Options { - root: Option, -} - -fn determine_rootdir>(bundle: P, namespace: String) -> Result { - log::info!( - "determining rootdir for bundle: {}", - bundle.as_ref().display() - ); - let mut file = match File::open(bundle.as_ref().join("options.json")) { - Ok(f) => f, - Err(err) => match err.kind() { - ErrorKind::NotFound => { - return Ok(<&str as Into>::into(DEFAULT_CONTAINER_ROOT_DIR).join(namespace)) - } - _ => return Err(err.into()), - }, - }; - let mut data = String::new(); - file.read_to_string(&mut data)?; - let options: Options = serde_json::from_str(&data)?; - let path = options - .root - .unwrap_or(PathBuf::from(DEFAULT_CONTAINER_ROOT_DIR)) - .join(namespace); - log::info!("youki root path is: {}", path.display()); - Ok(path) -} - impl LibcontainerInstance for Wasi { type Engine = wasmtime::Engine; @@ -71,7 +37,12 @@ impl LibcontainerInstance for Wasi { log::info!("creating new instance: {}", id); let cfg = cfg.unwrap(); let bundle = cfg.get_bundle().unwrap_or_default(); - let rootdir = determine_rootdir(bundle.as_str(), cfg.get_namespace()).unwrap(); + let rootdir = determine_rootdir( + bundle.as_str(), + &cfg.get_namespace(), + DEFAULT_CONTAINER_ROOT_DIR, + ) + .unwrap(); Wasi { id, exit_code: Arc::new((Mutex::new(None), Condvar::new())), @@ -136,6 +107,7 @@ impl LibcontainerInstance for Wasi { #[cfg(test)] mod wasitest { use std::borrow::Cow; + use std::collections::HashMap; use std::fs::{create_dir, read_to_string, File, OpenOptions}; use std::io::prelude::*; use std::os::fd::RawFd; @@ -208,7 +180,7 @@ mod wasitest { } #[test] - fn test_delete_after_create() -> Result<()> { + fn test_delete_after_create() -> anyhow::Result<()> { let cfg = InstanceConfig::new( Default::default(), "test_namespace".into(), @@ -281,16 +253,17 @@ mod wasitest { create_dir(dir.path().join("rootfs"))?; let rootdir = dir.path().join("runwasi"); create_dir(&rootdir)?; - let opts = Options { - root: Some(rootdir), - }; + let rootdir = PathBuf::from("/path/to/root"); + let mut opts = HashMap::new(); + opts.insert("root", rootdir); + let serialized = serde_json::to_string(&opts)?; let opts_file = OpenOptions::new() .read(true) .create(true) .truncate(true) .write(true) .open(dir.path().join("options.json"))?; - write!(&opts_file, "{}", serde_json::to_string(&opts)?)?; + write!(&opts_file, "{}", serialized)?; let wasm_path = dir.path().join("rootfs/hello.wat"); let mut f = OpenOptions::new()