Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor entrypoint logic to one location #219

Merged
merged 2 commits into from
Aug 14, 2023
Merged

Refactor entrypoint logic to one location #219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f26fb2e
Refactor to put logic in one place
jsturtevant Jul 21, 2023
7d70f40
Add comment on the new function
jsturtevant Aug 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 152 additions & 0 deletions crates/containerd-shim-wasm/src/sandbox/oci.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,36 @@ pub fn get_args(spec: &Spec) -> &[String] {
}
}

// get_module returns the module name and exported function name to be called
// from the arguments on the runtime spec process field. The first argument will
// be the module name and the default function name is "_start".
//
// If there is a '#' in the argument it will split the string
// returning the first part as the module name and the second part as the function
// name.
//
// example: "module.wasm#function" will return (Some("module.wasm"), "function")
//
// If there are no arguments then it will return (None, "_start")
pub fn get_module(spec: &Spec) -> (Option<String>, String) {
jsturtevant marked this conversation as resolved.
Show resolved Hide resolved
let args = get_args(spec);

if !args.is_empty() {
let start = args[0].clone();
let mut iterator = start.split('#');
let mut cmd = iterator.next().unwrap().to_string();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess #_start is invalid module name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a little bit of a discussion in #102 where this was originally introduced.

My understanding is that this is really a runwasi specific seperator and could be anything (:: was suggested at one point). I scanned through https://github.com/WebAssembly/WASI but couldn't find anything that said # is valid or not

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that if this is a runwasi-specific thing, we could actually write down the semantics in how this should work and especially which entrypoint strings are valid and which not. Then we could validate the input string based on that. The benefit would be that other projects could use the same notation and semantics, and we could document the notation also for the users.

For example, should the following strings work (or not):

  1. foo#bar
  2. #bar
  3. foo#bar#bar
  4. foo
  5. #foo#bar
  6. foo#
  7. foo#bar; -- %

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a good idea, are you ok with opening an issue to track this? This particular PR isn't introducing anything new, just moving some code around so we don't have the same logic in several different places.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, added the issue.


let stripped = cmd.strip_prefix(std::path::MAIN_SEPARATOR);
if let Some(strpd) = stripped {
cmd = strpd.to_string();
}
let method = iterator.next().unwrap_or("_start");
return (Some(cmd), method.to_string());
}

(None, "_start".to_string())
}

pub fn spec_from_file<P: AsRef<Path>>(path: P) -> Result<Spec> {
let file = File::open(path)?;
let cfg: Spec = json::from_reader(file)?;
Expand Down Expand Up @@ -112,3 +142,125 @@ pub fn setup_prestart_hooks(hooks: &Option<oci_spec::runtime::Hooks>) -> Result<
}
Ok(())
}

#[cfg(test)]
mod oci_tests {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see more tests were added!

use super::*;
use oci_spec::runtime::{ProcessBuilder, RootBuilder, SpecBuilder};

#[test]
fn test_get_args() -> Result<()> {
let spec = SpecBuilder::default()
.root(RootBuilder::default().path("rootfs").build()?)
.process(
ProcessBuilder::default()
.cwd("/")
.args(vec!["hello.wat".to_string()])
.build()?,
)
.build()?;

let args = get_args(&spec);
assert_eq!(args.len(), 1);
assert_eq!(args[0], "hello.wat");

Ok(())
}

#[test]
fn test_get_args_return_empty() -> Result<()> {
let spec = SpecBuilder::default()
.root(RootBuilder::default().path("rootfs").build()?)
.process(ProcessBuilder::default().cwd("/").args(vec![]).build()?)
.build()?;

let args = get_args(&spec);
assert_eq!(args.len(), 0);

Ok(())
}

#[test]
fn test_get_args_returns_all() -> Result<()> {
let spec = SpecBuilder::default()
.root(RootBuilder::default().path("rootfs").build()?)
.process(
ProcessBuilder::default()
.cwd("/")
.args(vec![
"hello.wat".to_string(),
"echo".to_string(),
"hello".to_string(),
])
.build()?,
)
.build()?;

let args = get_args(&spec);
assert_eq!(args.len(), 3);
assert_eq!(args[0], "hello.wat");
assert_eq!(args[1], "echo");
assert_eq!(args[2], "hello");

Ok(())
}

#[test]
fn test_get_module_returns_none_when_not_present() -> Result<()> {
let spec = SpecBuilder::default()
.root(RootBuilder::default().path("rootfs").build()?)
.process(ProcessBuilder::default().cwd("/").args(vec![]).build()?)
.build()?;

let (module, _) = get_module(&spec);
assert_eq!(module, None);

Ok(())
}

#[test]
fn test_get_module_returns_function() -> Result<()> {
let spec = SpecBuilder::default()
.root(RootBuilder::default().path("rootfs").build()?)
.process(
ProcessBuilder::default()
.cwd("/")
.args(vec![
"hello.wat#foo".to_string(),
"echo".to_string(),
"hello".to_string(),
])
.build()?,
)
.build()?;

let (module, function) = get_module(&spec);
assert_eq!(module, Some("hello.wat".to_string()));
assert_eq!(function, "foo");

Ok(())
}

#[test]
fn test_get_module_returns_start() -> Result<()> {
let spec = SpecBuilder::default()
.root(RootBuilder::default().path("rootfs").build()?)
.process(
ProcessBuilder::default()
.cwd("/")
.args(vec![
"hello.wat".to_string(),
"echo".to_string(),
"hello".to_string(),
])
.build()?,
)
.build()?;

let (module, function) = get_module(&spec);
assert_eq!(module, Some("hello.wat".to_string()));
assert_eq!(function, "_start");

Ok(())
}
}
17 changes: 11 additions & 6 deletions crates/containerd-shim-wasmedge/src/executor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,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;

use wasmedge_sdk::{
Expand Down Expand Up @@ -34,7 +35,9 @@ impl Executor for WasmEdgeExecutor {

// TODO: How to get exit code?
// This was relatively straight forward in go, but wasi and wasmtime are totally separate things in rust
match vm.run_func(Some("main"), "_start", params!()) {
let (module_name, method) = oci::get_module(spec);
debug!("running {:?} with method {}", module_name, method);
match vm.run_func(Some("main"), method, params!()) {
Ok(_) => std::process::exit(0),
Err(_) => std::process::exit(137),
};
Expand All @@ -51,10 +54,6 @@ impl Executor for WasmEdgeExecutor {

impl WasmEdgeExecutor {
fn prepare(&self, args: &[String], spec: &Spec) -> anyhow::Result<wasmedge_sdk::Vm> {
let mut cmd = args[0].clone();
if let Some(stripped) = args[0].strip_prefix(std::path::MAIN_SEPARATOR) {
cmd = stripped.to_string();
}
let envs = env_to_wasi(spec);
let config = ConfigBuilder::new(CommonConfigOptions::default())
.with_host_registration_config(HostRegistrationConfigOptions::default().wasi(true))
Expand All @@ -73,8 +72,14 @@ impl WasmEdgeExecutor {
Some(envs.iter().map(|s| s as &str).collect()),
None,
);

let (module_name, _) = oci::get_module(spec);
let module_name = match module_name {
Some(m) => m,
None => return Err(anyhow::Error::msg("no module provided cannot load module")),
};
let vm = vm
.register_module_from_file("main", cmd)
.register_module_from_file("main", module_name)
.map_err(|err| ExecutorError::Execution(err))?;
if let Some(stdin) = self.stdin {
dup(STDIN_FILENO)?;
Expand Down
8 changes: 4 additions & 4 deletions crates/containerd-shim-wasmedge/src/instance.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,9 +429,9 @@ mod wasitest {

let dir = tempdir()?;
let path = dir.path();
let wasmbytes = wat2wasm(WASI_HELLO_WAT).unwrap();
let wasm_bytes = wat2wasm(WASI_HELLO_WAT).unwrap();

let res = run_wasi_test(&dir, wasmbytes)?;
let res = run_wasi_test(&dir, wasm_bytes)?;

assert_eq!(res.0, 0);

Expand All @@ -451,9 +451,9 @@ mod wasitest {
}

let dir = tempdir()?;
let wasmbytes = wat2wasm(WASI_RETURN_ERROR).unwrap();
let wasm_bytes = wat2wasm(WASI_RETURN_ERROR).unwrap();

let res = run_wasi_test(&dir, wasmbytes)?;
let res = run_wasi_test(&dir, wasm_bytes)?;

// Expect error code from the run.
assert_eq!(res.0, 137);
Expand Down
28 changes: 13 additions & 15 deletions crates/containerd-shim-wasmtime/src/executor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use nix::unistd::{dup, dup2};
use std::{fs::OpenOptions, os::fd::RawFd};

use anyhow::{anyhow, Context, Result};
use anyhow::{anyhow, Result};
use containerd_shim_wasm::sandbox::oci;
use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};
use libcontainer::workload::{Executor, ExecutorError};
Expand Down Expand Up @@ -82,20 +82,18 @@ impl WasmtimeExecutor {
let wctx = wasi_builder.build();

log::info!("wasi context ready");
let mut iterator = args
.first()
.context("args must have at least one argument.")?
.split('#');
let mut cmd = iterator.next().unwrap().to_string();
let stripped = cmd.strip_prefix(std::path::MAIN_SEPARATOR);
if let Some(strpd) = stripped {
cmd = strpd.to_string();
}
let method = iterator.next().unwrap_or("_start");
let mod_path = cmd;
let (module_name, method) = oci::get_module(spec);
let module_name = match module_name {
Some(m) => m,
None => {
return Err(anyhow::format_err!(
"no module provided, cannot load module from file within container"
))
}
};

log::info!("loading module from file");
let module = Module::from_file(&self.engine, mod_path)?;
log::info!("loading module from file {} ", module_name);
let module = Module::from_file(&self.engine, module_name)?;
let mut linker = Linker::new(&self.engine);

wasmtime_wasi::add_to_linker(&mut linker, |s| s)?;
Expand All @@ -106,7 +104,7 @@ impl WasmtimeExecutor {

log::info!("getting start function");
let start_func = instance
.get_func(&mut store, method)
.get_func(&mut store, &method)
.ok_or_else(|| anyhow!("module does not have a WASI start function".to_string()))?;
Ok((store, start_func))
}
Expand Down
Loading
Loading