feat(provider): add wasmer provider #677
Conversation
Signed-off-by: Benjamin Coenen <[email protected]>
|
Hey @bnjjj! Thanks for working on this one.
In general we've started splitting out all of the providers into their own projects. For example the wasmCloud provider now lives at https://github.com/wasmCloud/krustlet-wasmcloud-provider and the wasm3 provider is over at https://github.com/bacongobbler/krustlet-wasm3. Same for the CRI provider: https://github.com/kflansburg/krustlet-cri Would you be okay with putting this work in its own repository? Thanks! |
|
Yes why not, if it's the way to do that I'm okay. What I suggest is to let this PR as a draft and open for now. I will continue my work and maybe when it's done you will be able to give me a little review/overview before closing this PR and put everything in my own repository. What do you think ? |
|
That sounds like a great idea @bnjjj! |
|
WHOA! This is so cool! It'll be great to have a Wasmer provider! |
|
If you have time to review feel free, I can't implement for now the http experimental part and the interrupt part as some features/documentation are missing on Wasmer. So I plan to put this code into his own repository after your review and then document all current limitations. I will then implement missing features on Wasmer to have all the features for krustlet. |
| use wasmer_wasi::{VirtualFile, WasiFsError}; | ||
|
|
||
| /// For piping stdio. Stores all output / input in a byte-vector. | ||
| pub struct FilePipe { |
There was a problem hiding this comment.
Out of curiosity, is there a reason not to use the newtype pattern? i.e.
pub struct FilePipe(std::fs::File);
There was a problem hiding this comment.
No I will move to this pattern you're right. Thanks
| fn try_from(value: File) -> Result<Self, Self::Error> { | ||
| Ok(Self { | ||
| file: value | ||
| .try_into_std() |
There was a problem hiding this comment.
So this will immediately attempt to convert a File to a StdFile without allowing async write operations to complete IIRC. Is there a technical reason why we can't implement From instead of TryFrom to allow a cleaner migration?
https://docs.rs/tokio/1.12.0/tokio/fs/struct.File.html#method.try_into_std
| @@ -66,6 +68,8 @@ pub struct Config { | |||
| /// device plugins lives. This is also where device plugins | |||
| /// should host their services. | |||
| pub device_plugins_dir: PathBuf, | |||
| /// Wasm runtime to use to execute your wasm | |||
| pub provider: Provider, | |||
There was a problem hiding this comment.
So generally speaking we've avoided adding internal implementation details like a Provider type directly into the kubelet crate. The goal of this crate is to implement the kubelet API without leaking any implementation details of Krustlet. That way clients can use the kubelet crate to interact with the Kubernetes API and impersonate a kubelet any way they choose.
Since you're just introducing this type to check a string value.... This seems more appropriate in the provider code itself.
| "wasmtime" => Ok(Self::WasmTime), | ||
| "wasmer" => Ok(Self::Wasmer), | ||
| other => Err(anyhow::anyhow!( | ||
| "unknown provider '{:?}', please choose between 'wasmer' and 'wasmtime'", |
There was a problem hiding this comment.
There are other providers out there than just these two by the way. For example:
https://github.com/bacongobbler/krustlet-wasm3
https://github.com/kflansburg/krustlet-cri
https://github.com/wasmCloud/krustlet-wasmcloud-provider
https://github.com/deislabs/krustlet-wagi-provider
https://github.com/bacongobbler/krustlet-wasmdome
https://github.com/soenkeliebau/stackable_krustlet
These are just some of the examples I could find through Github's search engine.
I don't think we should be in the business of listing "known providers" in the crate anyways.
| @@ -17,7 +17,7 @@ use super::running::Running; | |||
| use super::terminated::Terminated; | |||
| use super::ContainerState; | |||
|
|
|||
| pub const MAX_CONNCURRENT_REQUESTS_ANNOTATION_KEY: &str = | |||
| pub const MAX_CONCURRENT_REQUESTS_ANNOTATION_KEY: &str = | |||
2569: refactor(api): add Send + Sync trait when creating an instance r=Amanieu a=bnjjj # Description I'm currently implementing wasmer inside Krustlet (krustlet/krustlet#677). Then I had to force the thread safety of trait object used to create a wasmer instance. I know it could be a breaking change and maybe I miss some backgrounds about this. So feel free to let me know what do you think and how we could have a better implementation if needed. I'm open to any suggestions. Co-authored-by: Benjamin Coenen <[email protected]>
Signed-off-by: Benjamin Coenen <[email protected]>
Here is the draft to implement wasmer as a runtime to execute wasm in krustlet. It's working with demos, excepting the demo with postman as I didn't already implemented the experimental http methods.
I have few questions regarding my implementation. I added a "provider" crate but it seems all the files are almost the same except the method
spawn_wasmtimeandspawn_wasmer. Do you think it's a good idea to keep these 2 separated crates or just have only one cratewasi_runtimeand putspawn_wasmermethods and so one inside ?Todo: