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.

Sign up for GitHub

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

Jump to bottom

WASI support #202

Merged
merged 11 commits into from
Apr 30, 2024
Merged

WASI support #202

merged 11 commits into from
Apr 30, 2024

Conversation

No9
Copy link
Contributor

@No9 No9 commented Jan 5, 2023 •
edited
Loading

Hi All,
Opening this draft to solicit feedback and gauge interest.
I've been building out a kn func project for WASI using WASMEdge
https://github.com/uirlis/func-rust-wasi/tree/main/cloudevents

As part of that work I have put together a first pass at getting cloudevents to compile for WASI which is this PR.

The majority of the work is redefining the features in the Cargo.toml to bring in hyper-wasi when the OS is wasi

However I had to patch rust-claim in the test suite as the autocfg doesn't play well with a hyper-wasi so this placeholder is used
https://github.com/cloudevents/sdk-rust/compare/main...No9:wasi-support?expand=1#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542R74.
Which points to this patch
svartalf/rust-claim@master...No9:rust-claim:ignore-autocfg-wasi
No longer an issue as latest active claims repo has been added.

To build and run this PR you will need:
WASMEdge installed - https://wasmedge.org/book/en/quick_start/install.html
WASI Tools for Rust - cargo install cargo-wasi
Set the was env - export CARGO_TARGET_WASM32_WASI_RUNNER=wasmedge

Currently this PR should pass testing with with the following command that enables the features used in the sample kn project

cargo test --features "http-binding hyper hyper_wasi" --target=wasm32-wasi
... 
test result: ok. 57 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

running 2 tests
test test_html_root_url ... ignored
test test_readme_deps ... ignored

test result: ok. 0 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out

And the main contribution command cargo test --all-features --all also passes

But the main contribution command with wasm32-wasi fails as expected with numerous errors.

cargo build --all-features --all --target=wasm32-wasi

If this is of interest and we can work out what to do about claim I'm happy working on the CI pieces to finalise.
Also understand if this is using projects that are too nascent and you want to keep it down stream for a while.

This was referenced Jan 5, 2023
Closed
Closed
@juntao
Copy link

juntao commented Feb 9, 2023

This is very cool. Thanks!

With "Wasm containers" supported in containerd and Docker Desktop etc, developers can build very small container apps to respond to events on demand -- image size for a Wasm app is often 1-2 MB as opposed to 20+MB for LXC; and Wasm starts up much faster than LXC apps.

I would like to understand if there is anything we can do on the hyper_wasi side to help with the issue with the autocfg / claim crates? Thanks.

@No9
Copy link
Contributor Author

No9 commented Feb 9, 2023

@juntao Thanks for the feedback. As you mention the main item is the autocfg changes as it was just a naive removal without to much consideration as I wanted to gauge interest here. I don't think there is anything that can be done in hyper-wasi as the problem is around target detection.
There will also be a modification to the build system here to test the wasm path.
@jcrossley3 @markmandel If we can resolve the dependency issues and build item would you know if there is interest in landing this?
cc/ @font

@juntao
Copy link

juntao commented Feb 9, 2023

Thanks @No9

I looked into the autocfg issue. It seems that it can be fixed with a patch section in cargo.toml file to force the autocfg dependency in hyper_wasi / tokio_wasi at 1.0 (the published crates have it on 1.1).

@No9
Copy link
Contributor Author

No9 commented Feb 9, 2023 •
edited
Loading

@juntao Great news! Do you want to PR it in and we can monitor? claim is used quite widely so it would be great to get it in.
Happy to do it but it won't be until next week.
[Edit]
Actually it looks like claim has died svartalf/rust-claim#12 and has been picked up here
https://github.com/mattwilkinsonn/rust-claims with the 1.1 patch.
I'll validate it and add it to this PR.

@No9
fix: use supported claims
1d26236 
Signed-off-by: Anthony Whalley <[email protected]>
@No9
Copy link
Contributor Author

No9 commented Feb 9, 2023

@juntao That worked out and I've updated the PR.
Once we get the workflows enabled I'll add the build config

@No9 No9 marked this pull request as ready for review February 9, 2023 18:28
@juntao
Copy link

juntao commented Feb 9, 2023

Very cool. Thank you!

I look forward to building and deploying cloudevent functions in Wasm containers!

@No9
Copy link
Contributor Author

No9 commented Mar 17, 2024

@jcrossley3 Now that wasm support has been added to OpenShift in developer preview with Knative support my understanding is that this is based on wasmedge.
https://www.redhat.com/en/blog/webassembly-wasm-and-openshift-a-powerful-duo-for-modern-applications

is there anychance we can get this PR to land?
cc @gauravsingh85 @mrunalp @sohankunkerkar

@jcrossley3
Copy link
Contributor

@No9 Can you rebase through the conflicts, please?

Lazzaretti
Lazzaretti reviewed Apr 2, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@No9
fix: update ticks
345face 
Signed-off-by: Anton Whalley <[email protected]>
@No9
Copy link
Contributor Author

No9 commented Apr 10, 2024

@Lazzaretti @jcrossley3 I've rebased and merged the changes
Clicking the accept changes in the GH UI broke the DCO so I reset and applied the tick changes by hand.
The workflow will need to be updated to run the tests as only the WASI reqwest system is currently tested.
Would you like that as part of this PR or should I open a separate one?

@Lazzaretti
Copy link
Member

@No9 Thanks for the update! I would prefer to have the tests including CI in the same PR :)

@No9
Copy link
Contributor Author

No9 commented Apr 11, 2024

@Lazzaretti updated the PR to support wasm32-wasi target in the build system.

Notes:
It installs wasmedge to run the tests as per these instructions here:
https://wasmedge.org/docs/start/install/#install-for-all-users

nit: hyper had slipped out of the conditional in Cargo.toml during a merge so that has been fixed.

Lazzaretti
Lazzaretti requested changes Apr 19, 2024
View reviewed changes
.github/workflows/rust_tests.yml Outdated Show resolved Hide resolved
.github/workflows/rust_tests.yml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@No9 No9 requested a review from Lazzaretti April 30, 2024 14:59
@Lazzaretti
fix: remove formatting of cargo file
811b44e 
Signed-off-by: Fabrizio Lazzaretti <[email protected]>
@Lazzaretti Lazzaretti merged commit 13c36fd into cloudevents:main Apr 30, 2024
10 checks passed
@Lazzaretti
Copy link
Member

@No9 Thanks a lot for your contribution 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lazzaretti Lazzaretti Lazzaretti approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@No9 @juntao @jcrossley3 @Lazzaretti