Use youki's libcontainer APIs to implement wasmtime shim. #142

Mossaka · 2023-06-13T23:52:09Z

Implement Executor trait for wasmtime executor.
Use libcontainer's lifecycle APIs to implement runwasi's Instance
trait.

TODO

refactor out common functions from both wasmedge and wasmtime shims
figure out how to pass the "test_delete_after_create" test
figure out how to pass the "test_wasi" test
add more tests

Since youki now uses thiserror instead of anyhow, this PR also updates the containerd-shim-wasmedge to properly map the error. Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

- Implement Executor trait for wasmtime executor. - Use libcontainer's lifecycle APIs to implement runwasi's Instance trait. Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

utam0k · 2023-06-14T11:48:55Z

@Mossaka Do you want me to review this PR?

Mossaka · 2023-06-14T23:53:08Z

@Mossaka Do you want me to review this PR?

I would appreciate your feedback once this PR is ready to review :)

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

crates/containerd-shim-wasmtime/src/executor.rs

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

The purpose of this crate is to provide common helper functionalities to shim implementors that use youki's libcontainer crate. Notice that the "containerd-shim-wasm" crate does not depend on libcontainer. This commit refactors shared implementation from the wasmtime and wasmedge shims to the newly created common crate. Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Mossaka · 2023-06-21T18:23:33Z

This PR is ready to review for now. The test_wasi test is still failing with the following message:

Error: Any(failed to receive. "waiting for init ready". BrokenChannel

Caused by:
    channel connection broken)

jsturtevant

Looks great, I am looking with an eye towards how this would work when Windows is added and I think this actually makes a few things easier. The awkward part might be re-using the WasmtimeExecutor directly vs via a youki Container since Windows isn't supported in youki but I think it will work.

crates/common/Cargo.toml

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

crates/containerd-shim-wasmtime/src/executor.rs

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

utam0k · 2023-06-22T12:51:44Z

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

+                Ok(_) => 0,
+                Err(e) => {
+                    if e == Errno::ECHILD {
+                        0


I assume this code came from my code, but it has to plant the log in this case.

Could you elaborate more about "plant the log"? Do you want me to log a message when Errno::ECHILD was returned?

when Errno::ECHILD was returned?

Yes 👍

I have added a log messaging saying "no child process" when Errno::ECHILD was returned in both the shims.

I am not entirely sure when people see this log message, that actions they can do to mitigate. Or is this an issue at all in the first place? I would appreciate if anyone can help me to understand it.

crates/containerd-shim-wasmtime/src/executor.rs

Co-authored-by: Toru Komatsu <k0ma@utam0k.jp> Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Mossaka · 2023-07-05T17:07:08Z

All the tests are passing 🥳

crates/containerd-shim-wasmtime/src/executor.rs

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

danbugs

LGTM, Joe – just some small comments here and there.

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

crates/containerd-shim-wasmtime/src/executor.rs

…hims Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Co-authored-by: Dan Chiarlone <danilochiarlone@gmail.com> Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

…nto wasmtime-youki

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

arschles

LGTM

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Mossaka · 2023-07-07T18:08:33Z

Could you please take another look? 🙏 @utam0k , @ipuustin , @cpuguy83

crates/containerd-shim-wasmtime/src/executor.rs

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

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

utam0k

Looks good to me, but This PR introduces a big change, so I recommend getting one more approval.

Mossaka · 2023-07-10T20:44:05Z

Thanks everyone for reviewing!! 🥂🙌

Mossaka added 4 commits June 12, 2023 22:46

Update youki to the latest main branch and oci-spec to 0.6.1.

407b883

Since youki now uses thiserror instead of anyhow, this PR also updates the containerd-shim-wasmedge to properly map the error. Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Merge remote-tracking branch 'upstream/main' into oci-0.6.1

8ca83f2

Removed some uses of unsafe in wasmedge shim

c555e3f

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Use youki's libcontainer APIs to implement wasmtime shim.

fc1bb2e

- Implement Executor trait for wasmtime executor. - Use libcontainer's lifecycle APIs to implement runwasi's Instance trait. Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Mossaka requested review from cpuguy83 and ipuustin June 13, 2023 23:52

Mossaka marked this pull request as draft June 13, 2023 23:53

Mossaka linked an issue Jun 13, 2023 that may be closed by this pull request

Use youki libcontainer crate for Wasmtime shim #110

Closed

Fix some clippy warnings

83ed1a2

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Merge remote-tracking branch 'upstream/main' into wasmtime-youki

d8e4c1a

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

ipuustin mentioned this pull request Jun 16, 2023

Handle args for modules with out extra first argument in prep for oci articfacts #146

Closed

thangchung mentioned this pull request Jun 20, 2023

Discussion about multiple runtimes on a Kubernetes pod? deislabs/containerd-wasm-shims#105

Closed

Mossaka commented Jun 20, 2023

View reviewed changes

crates/containerd-shim-wasmtime/src/executor.rs Show resolved Hide resolved

Mossaka commented Jun 20, 2023

View reviewed changes

crates/containerd-shim-wasmtime/src/executor.rs Show resolved Hide resolved

Mossaka linked an issue Jun 20, 2023 that may be closed by this pull request

child process gets killed by SIGKILL after using cgroup v1 API #40

Closed

Mossaka mentioned this pull request Jun 20, 2023

child process gets killed by SIGKILL after using cgroup v1 API #40

Closed

ipuustin mentioned this pull request Jun 21, 2023

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

Closed

Mossaka added 2 commits June 21, 2023 17:56

Passed the test_delete_after_create test

d3b7f69

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Merge remote-tracking branch 'upstream/main' into wasmtime-youki

d998250

Mossaka marked this pull request as ready for review June 21, 2023 17:57

Mossaka requested a review from utam0k June 21, 2023 18:23

Mossaka mentioned this pull request Jun 21, 2023

What paths to expose to the runtime #154

Open

jsturtevant reviewed Jun 21, 2023

View reviewed changes

utam0k reviewed Jun 22, 2023

View reviewed changes

crates/containerd-shim-wasmtime/src/executor.rs Outdated Show resolved Hide resolved

Apply suggestions from code review

47ce17d

Co-authored-by: Toru Komatsu <k0ma@utam0k.jp> Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

Mossaka added 4 commits June 29, 2023 22:16

Fixed clippy issues

4b106b2

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Reset stdio at the end of the tests

51182c6

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Need rollback: commenting out the stdio thing

7ca42a1

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Use inherit_stdio()

8d5c858

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Mossaka force-pushed the wasmtime-youki branch from 8531643 to 8d5c858 Compare June 29, 2023 23:19

Mossaka added 4 commits June 30, 2023 00:05

Setup CI build env for wasmtimie e2e

65177c3

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Add inpsection step to e2e-wasmtime

bb1e940

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Add deps to kind nodes that run k8s tests

e120093

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Merge remote-tracking branch 'upstream/main' into wasmtime-youki

76fa9cb

arschles suggested changes Jul 5, 2023

View reviewed changes

danbugs reviewed Jul 5, 2023

View reviewed changes

crates/containerd-shim-wasm/src/sandbox/instance_utils.rs Show resolved Hide resolved

crates/containerd-shim-wasmtime/src/executor.rs Outdated Show resolved Hide resolved

Mossaka and others added 4 commits July 6, 2023 21:53

Handle Result types from dup and dup2 in both wasmtime and wasmedge s…

f7472fb

…hims Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Small refactoring

d807601

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Update crates/containerd-shim-wasmtime/src/executor.rs

3f31397

Co-authored-by: Dan Chiarlone <danilochiarlone@gmail.com> Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>

Merge branch 'wasmtime-youki' of https://github.com/Mossaka/runwasi i…

2c64834

…nto wasmtime-youki

Mossaka requested review from danbugs and arschles July 6, 2023 22:23

Add a log to when Errno::ECHILD was returned

729c3b4

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

arschles approved these changes Jul 6, 2023

View reviewed changes

Merge remote-tracking branch 'upstream/main' into wasmtime-youki

a28fad9

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

utam0k reviewed Jul 8, 2023

View reviewed changes

crates/containerd-shim-wasmtime/src/executor.rs Outdated Show resolved Hide resolved

crates/containerd-shim-wasmtime/src/executor.rs Outdated Show resolved Hide resolved

crates/containerd-shim-wasm/src/sandbox/instance_utils.rs Show resolved Hide resolved

Mossaka added 2 commits July 9, 2023 19:55

Resolve comments

aaaea73

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

Rustfmt

dfc680d

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>

utam0k approved these changes Jul 10, 2023

View reviewed changes

cpuguy83 approved these changes Jul 10, 2023

View reviewed changes

cpuguy83 merged commit 0a5a758 into containerd:main Jul 10, 2023

Mossaka deleted the wasmtime-youki branch July 10, 2023 20:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use youki's libcontainer APIs to implement wasmtime shim. #142

Use youki's libcontainer APIs to implement wasmtime shim. #142

Mossaka commented Jun 13, 2023 •

edited

Loading

utam0k commented Jun 14, 2023

Mossaka commented Jun 14, 2023

Mossaka commented Jun 21, 2023

jsturtevant left a comment

utam0k Jun 22, 2023

Mossaka Jun 23, 2023

utam0k Jun 24, 2023

Mossaka Jul 6, 2023

Mossaka Jul 6, 2023

Mossaka commented Jul 5, 2023

danbugs left a comment

arschles left a comment

Mossaka commented Jul 7, 2023

utam0k left a comment

Mossaka commented Jul 10, 2023

Use youki's libcontainer APIs to implement wasmtime shim. #142

Use youki's libcontainer APIs to implement wasmtime shim. #142

Conversation

Mossaka commented Jun 13, 2023 • edited Loading

TODO

utam0k commented Jun 14, 2023

Mossaka commented Jun 14, 2023

Mossaka commented Jun 21, 2023

jsturtevant left a comment

Choose a reason for hiding this comment

utam0k Jun 22, 2023

Choose a reason for hiding this comment

Mossaka Jun 23, 2023

Choose a reason for hiding this comment

utam0k Jun 24, 2023

Choose a reason for hiding this comment

Mossaka Jul 6, 2023

Choose a reason for hiding this comment

Mossaka Jul 6, 2023

Choose a reason for hiding this comment

Mossaka commented Jul 5, 2023

danbugs left a comment

Choose a reason for hiding this comment

arschles left a comment

Choose a reason for hiding this comment

Mossaka commented Jul 7, 2023

utam0k left a comment

Choose a reason for hiding this comment

Mossaka commented Jul 10, 2023

Mossaka commented Jun 13, 2023 •

edited

Loading