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

feat: add a new shim for Wasm Workers Server #88

Merged
merged 15 commits into from
May 12, 2023

Conversation

Angelmmiguel
Copy link
Contributor

Hello 👋

My name is Angel and I'm part of the Wasm Labs team. I'm a maintainer of the Wasm Workers Server project, an OSS project to develop and run serverless applications on top of WebAssembly.

I created a new shim that adds support for this kind of workloads. It allows you to run Wasm Workers Server apps the same way you can now with Spin and Slight. Here you have several example apps.

The code is pretty similar to the slight and spin shims. It was an easy integration thanks to all the features provided by runwasi and the previous work on those shims. Thank you for all your effort on this! 😄

I also updated the documentation and scripts to reflect the new shim.

Summary

  • Introduce a new containerd-shim-wws-v1 shim
  • Update build and testing scripts to cover it
  • Test the changes in the CI
  • Update documentation to information about the new shim

Please, feel free to suggest any change or requirement on the PR.

Thank you!

@Angelmmiguel
Copy link
Contributor Author

@microsoft-github-policy-service agree company="VMware, Inc"

@squillace
Copy link
Contributor

I'm really looking forward to this.

@Mossaka
Copy link
Member

Mossaka commented Apr 14, 2023

@Angelmmiguel thanks for your contribution! I am really excited for this. I will be travelling to attend KubeCon EU next week and will be back on April 23rd. Will review your PR by then.

@Angelmmiguel
Copy link
Contributor Author

Angelmmiguel commented Apr 15, 2023

@Angelmmiguel thanks for your contribution! I am really excited for this. I will be travelling to attend KubeCon EU next week and will be back on April 23rd. Will review your PR by then.

That’s great @Mossaka! I’ll be in KubeCon too 😄

@Mossaka
Copy link
Member

Mossaka commented Apr 26, 2023

Will try to run this locally on my machine today and review the PR! Thanks for waiting

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.

Mostly LGTM! I will need to run commands locally to test everything still works, and run CI to make sures it still works. Awesome work @Angelmmiguel !

I will ask @devigned to take another look.

containerd-shim-wws-v1/Dockerfile Outdated Show resolved Hide resolved
Copy link
Member

@devigned devigned left a comment

Choose a reason for hiding this comment

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

Fantastic work, @Angelmmiguel! I have a couple questions.

containerd-shim-wws-v1/src/main.rs Outdated Show resolved Hide resolved
containerd-shim-wws-v1/src/main.rs Outdated Show resolved Hide resolved
deployments/k3d/Makefile Outdated Show resolved Hide resolved
deployments/k3d/Makefile Outdated Show resolved Hide resolved
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.

I've tried make up and make test locally and it looks like all the images were able to built correctly and loaded correctly to k3d, but I was not able to get the response from wws shim, unlike the other shims.

image

I believe the issue was that the lable was not correctly set up
image

The pod had an error message in its events saying

Warning  FailedScheduling  16s   default-scheduler  0/2 nodes are available: 2 node(s) didn't match Pod's node affinity/selector. preemption: 0/2 nodes are available: 2 Preemption is not helpful for scheduling.

@Mossaka
Copy link
Member

Mossaka commented Apr 28, 2023

Heyy I got the result!! That's awesome

image

The issue was described above: when I did make up, the wws-enabled=true label was not applied to the agent node. Once I added this label, the pod started normally and I was able to get this result.

@squillace
Copy link
Contributor

whooohoooooo

@Angelmmiguel
Copy link
Contributor Author

@Mossaka @devigned thank you for taking a look at the PR and for the complete testing! Agree with all your comments. I will fix the different issues and update the PR :)

@Angelmmiguel
Copy link
Contributor Author

@Mossaka @devigned thank you for the thorough review! I already submitted some changes regarding using the 3000 port and the missing Makefile tasks and deps.

The pending changes are:

  • Expose workers logs in the pod via stderr
  • Ensure the make up command works correctly with the wws-shim

I'm having some issues regarding my environment (Docker Desktop on MacOS M1) when building the different shims. I get an error when compiling the openssl rust dependency as it tries to use the musl-ranlib binary instead of the x86_64-linux-musl-gcc-ranlib provided by the Cross base image. That's the reason I had to add all the environment variables to the Dockerfile for building wws. For now, I'm focusing on the wws-shim, but I may open a different issue and PR in the future to ensure the compilation works in the different environments.

Thank you!

@squillace
Copy link
Contributor

If you wanna chat with @rumpl about Docker, he knows ALL :-)

@Angelmmiguel
Copy link
Contributor Author

If you wanna chat with @rumpl about Docker, he knows ALL :-)

That would be really great! I think this case is a mix of Docker and Rust + Cross. It should a simple configuration thing. If you have some time @rumpl, it would be great to check it :)

Thank you for the introduction @squillace!

@Angelmmiguel
Copy link
Contributor Author

@Mossaka @devigned the PR is ready for a second review when you have some time! 🎉

@@ -1,50 +1,59 @@
IMAGE_NAME ?= k3swithshims
CLUSTER_NAME ?= k3s-default
PLATFORM ?= linux/amd64
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found this configuration option useful when working with MacOS M1. Now, I can configure the image to linux/arm64 when working with k3d. However, I can move this to a separate PR if it's more convenient

Copy link
Member

@devigned devigned left a comment

Choose a reason for hiding this comment

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

lgtm!

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.

lgtm! Thanks for your contribution @Angelmmiguel

@Mossaka Mossaka merged commit 88aa1c1 into deislabs:main May 12, 2023
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