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.

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

Enable WasmEdge roofs support and start writing OCI tests for Wasmedge and Wasmtime #111

Closed
wants to merge 3 commits into from

Conversation

ipuustin
Copy link
Contributor

@ipuustin ipuustin commented May 4, 2023

Now that we have support for OCI runtime spec for Wasmedge shim, we should try to do two things:

  1. find out which OCI spec features are actually reasonable to test for runwasi shims
  2. create a set of Wasm modules which exercise those OCI spec features so that we can test them

Those OCI spec features which don't really map to Wasm modules we can try to test on the runtime level, checking for example which capabilities the runtime process has?

This PR adds an external directory for storing/building the Wasm modules and an integration test directory for Wasmedge shim for trying out the OCI spec features. I added some seccomp and device file tests for seeing how this would work in practice. Support for Wasmtime is commented out because the OCI support (#142) hasn't been merged yet, but could be useful in testing the PR.

So, I think the first question is: which OCI features are meaningful and should be tested for Wasm/WASI containers? For example it could be argued that the Wasm modules should never see the device files, but the Wasm runtimes should, so that they could expose the features through the WASI APIs. The second question is that is this approach of having the test Wasm modules in-tree the right way forward? The problem is that it appears to be difficult or impossible to run cargo build for two architectures at the same time (making the tests depend on the Wasm module build).

@ipuustin ipuustin changed the title RFC: Start writing OCI tests for Wasmedge Start writing OCI tests for Wasmedge May 12, 2023
@ipuustin ipuustin changed the title Start writing OCI tests for Wasmedge Enable WasmEdge roofs support and start writing OCI tests for Wasmedge Jun 21, 2023
@ipuustin ipuustin marked this pull request as ready for review June 21, 2023 08:07
@ipuustin ipuustin requested review from utam0k and cpuguy83 June 21, 2023 08:08
Copy link
Member

@Mossaka Mossaka left a comment

Choose a reason for hiding this comment

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

Thanks for contributing more tests for OCI!

I quickly skimmed through the test cases and noticed that the larger part of them are not Wasmedge specific - which means the same set of tests can be port to test the wasmtime shim as well.

After #142 is merged upstream, I am happy to take this task of making these tests available across shims.

Makefile Outdated Show resolved Hide resolved
@ipuustin
Copy link
Contributor Author

Thanks for contributing more tests for OCI!

I quickly skimmed through the test cases and noticed that the larger part of them are not Wasmedge specific - which means the same set of tests can be port to test the wasmtime shim as well.

After #142 is merged upstream, I am happy to take this task of making these tests available across shims.

Thanks! In fact the Wasmtime support is already present in the tests but commented out, because the OCI support is not there yet. If you like, you can try rebasing #142 on top of this PR, removing the comments (marked with TODO:) and running the tests.

Copy link
Contributor

@jsturtevant jsturtevant left a comment

Choose a reason for hiding this comment

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

This is a great, It feels like you're on the verge of creating a framework for testing different module specs. I've made one suggestion to leverage our Instance trait and invert the logic a bit that would make it so we could easily re-use the tests across different runtimes. Let me know if that makes sense

.set_bundle(dir.path().to_str().unwrap().to_string())
.set_stdout(dir.path().join("stdout").to_str().unwrap().to_string());

let wasi = Arc::new(WasmEdgeWasi::new("test".to_string(), Some(cfg)));
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this returns the trait Instance? If so we could invert this. Create the config and the instance outside the module and make this function generic run_test_with_spec and pass in the module.

This would make the tests written below to not be specific calls to each runtime let (output, retval) = common::run_wasmedge_test(&spec, &bytes)?; making it more of a framework.

@@ -62,6 +62,10 @@ jobs:
args: --all --verbose
- name: Validate docs
run: ./scripts/validate-docs.sh
- name: Setup rust-wasm target and compile test modules
run: |
rustup target add wasm32-wasi
Copy link
Contributor

Choose a reason for hiding this comment

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

if you put this in the make target it makes, it will just skip if already present and makes it so some one runs that step they don't miss it

@Mossaka
Copy link
Member

Mossaka commented Jul 19, 2023

Is this PR ready for review? @ipuustin

@ipuustin
Copy link
Contributor Author

ipuustin commented Aug 1, 2023

Is this PR ready for review? @ipuustin

It will be shortly -- I'll still add the Wasmtime OCI tests, because the support should be there now with the libcontainer integration.

@ipuustin ipuustin marked this pull request as draft August 1, 2023 08:04
@ipuustin ipuustin changed the title Enable WasmEdge roofs support and start writing OCI tests for Wasmedge Enable WasmEdge roofs support and start writing OCI tests for Wasmedge and Wasmtime Aug 1, 2023
The first such moduels are a test to enumerate default devices in the
container and a hello world application, which also gets the current
working directory. This can be used to test seccomp filter.

Signed-off-by: Ismo Puustinen <ismo.puustinen@intel.com>
Signed-off-by: Ismo Puustinen <ismo.puustinen@intel.com>
Enable WasmEdge modules to access the container filesystem. This
includes:

  * all the files on the container image
  * /proc filesystem
  * /sys filesysten
  * parts of /dev filesystem

Signed-off-by: Ismo Puustinen <ismo.puustinen@intel.com>
@ipuustin ipuustin marked this pull request as ready for review August 2, 2023 11:06
@ipuustin
Copy link
Contributor Author

ipuustin commented Aug 2, 2023

One missing part is that we now need to have a direct dependency to Wasmtime and WasmEdge crates to have access to the engine/Vm object. If we go this way, we probably should have Wasmtime and WasmEdge dependency in the workspace.

@jsturtevant
Copy link
Contributor

#306 built upon this work and is now merged. Thanks @ipuustin for the initial work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants