Use youki libcontainer crate for all shims

[jprendes]

The work in #78 enabled youki's libcontainer for the WasmEdge shim.
The Issue #110 intents to repeat the same work for the wasmtime shim.
During #78, @rumpl 's comment mentioned that we could enable youki's libcontainer in a lower level in runwasi, so that all shims can benefit from it.
Any thoughts?

[jprendes]

I'd be happy to work on this

[ipuustin]

I did try to use libcontainer functions for adding support for OCI devices and seccomp. The seccomp code is at https://github.com/ipuustin/runwasi/commits/seccomp and the devices code is here: https://github.com/ipuustin/runwasi/commits/oci-devices. My finding was that libcontainer can very well be used on this level, but it is a lot of effort and will bring a lot of duplicated code to runwasi. I mean, the seccomp code I needed to add in the seccomp branch alone is hundreds of lines of code, a lot of it unsafe.

However, what would make sense IMHO would be to go through the libcontainer initialization code and figure out what exactly is needed for running Wasm/WASI modules securely. For example, do we really need a PID namespace if the Wasm module will not even have the concept of a process? Or does it make sense to have the Masked Paths OCI feature, if we anyway control precisely which files are available? If the result of this study is that the optimal Wasm initialization path is a lot different from traditional container initialization, we might want to contribute to libcontainer a different way for loading the container, or maybe just do it using the lower-level primitives as you suggest.

I'm myself curious to what we could do in runwasi shared mode using just threads for running the Wasm VMs -- would we get anywhere close to a working "container"?

[rumpl]

@ipuustin looking briefly at your code for seccomp I don't understand completely why you need to duplicate the code for each shim, can't we add this one level up the stack, in containerd-shim-wasm?

[ipuustin]

Most of the seccomp code is in containerd-shim-wasm (see ipuustin@78c3c4d). It's just being called from the WasmEdge and Wasmtime shims. The reason there needs to be some code in shims is that the open FD for seccomp notification needs to be passed over socket to main process and then to the external notifier process.

[cpuguy83]

However, what would make sense IMHO would be to go through the libcontainer initialization code and figure out what exactly is needed for running Wasm/WASI modules securely

The main thing here is we are using linux containerization as an extra boundary, plus it also helps wasm play nicely with other linux containers/processes.

I'm myself curious to what we could do in runwasi shared mode using just threads for running the Wasm VMs -- would we get anywhere close to a working "container"?

Primarily we can't use memory cgroups with threads.
There may be some other things (such as pid namespaces) that just don't make sense in a threaded mode as well.

[Mossaka]

After #64 is finished, I will spend some time on this task - so assigning it to myself. Happy to discuss how we can approach this with you @jprendes

[Mossaka]

I have implemented the integration with libcontainer on both the wasmtime shim and the spin shim (see this PR). I've seen a few places where there are a lot of duplicate code blocks that could be moved into the containerd-shim-wasm crate, thus requiring an explicit dependency on libcontainer.

Implement the libcontainer::workload::Executor trait

This is provided by libcontainer to implement any container runtime as an executor. For example, in the spin shim, I had to implement exec() function to get the Wasm module path and then invoke wasmtime API to call this module. I believe this is handled similarly in the WasmEdge shim. In the spin shim, I built spin triggers and then run the spin trigger with spin manifestation file found on the rootfs.

This trait is REQUIRED to be implemented by any shim author. Thus I believe it's reasonable to expose this trait in the containerd_shim_wasm::sandbox::Instance trait to inform it how to build a youki Container instance.

kill, delete and wait all should have a default implementation

The code for Instance::kill / delete / wait are all the same. They invoke the respective Container APIs given self.id, self.exit_code and self.rootdir. I think we can provide them default implementation in the Instance trait itself.

start has the same procedure

The Instance::start() function has a fixed sequence of steps to start a container. It first builds a container by providing it its std I/O, and Wasm runtime. Then it calls container.start() to start the container. Finally it spawns a thread to handle the exit of the child process, and notify the condition variable which is waiting in the set_up_exit_code_wait() function.

I believe this entire thread code could be abstract away and be provided as a default implementation.

What do y'all think?

[jprendes]

Tha ks for the analysis, it's very useful. I'm all in for moving as much as possible into containerd-shim-wasm and simplify writing new shims.

Do you know if there is any incompatibility between the way libcontainer does things and the daemon based shim?