Skip to content

Add WASM plugin support #121

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
Merged
mikebrow merged 1 commit into from
Dec 11, 2024
Merged

Add WASM plugin support #121

mikebrow merged 1 commit into from
Dec 11, 2024

Conversation

@saschagrunert
Copy link
Contributor

@saschagrunert saschagrunert commented Nov 6, 2024

Just an inspiration how WASM support could work.

saschagrunert
saschagrunert commented Nov 6, 2024
View reviewed changes
@klihub
Copy link
Member

klihub commented Nov 6, 2024

@saschagrunert Are you able to compile the wasm plugin on your system ? If yes, can you list the tinygo version and other relevant configuration that you are running with ?

@saschagrunert
Copy link
Contributor Author

saschagrunert commented Nov 7, 2024

@saschagrunert Are you able to compile the wasm plugin on your system ? If yes, can you list the tinygo version and other relevant configuration that you are running with ?

Yes it works, using:

> tinygo version
tinygo version 0.34.0 linux/amd64 (using go version go1.23.2 and LLVM version 18.1.8)

I still think knqyf263/go-plugin#67 is a main blocker on this, but there is hope that we can resolve this kind of issue soon.

@saschagrunert saschagrunert mentioned this pull request Nov 21, 2024
Merged
@saschagrunert saschagrunert force-pushed the wasm-support branch 6 times, most recently from 5d2afdc to a058129 Compare November 21, 2024 13:31
@saschagrunert
Copy link
Contributor Author

saschagrunert commented Nov 21, 2024

Requires knqyf263/go-plugin#69

@saschagrunert saschagrunert force-pushed the wasm-support branch 12 times, most recently from c655cfc to 72d834f Compare November 22, 2024 10:33
@saschagrunert saschagrunert marked this pull request as ready for review November 22, 2024 10:33
@saschagrunert saschagrunert force-pushed the wasm-support branch 2 times, most recently from 3e4c1d7 to 50e60c0 Compare November 22, 2024 10:47
@saschagrunert
Copy link
Contributor Author

saschagrunert commented Nov 22, 2024

@klihub this is ready for review now.

klihub
klihub reviewed Nov 22, 2024
View reviewed changes
Copy link
Member

@klihub klihub left a comment

Choose a reason for hiding this comment

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

@saschagrunert This looks really good and I'd like to start playing around with it more. Thank you for taking the time and effort with this !

I have a small suggestion in the review comments which would allow folks to get the wasm plugin built using the tinygo docker image if their distro does not offer a new enough tinygo toolchain natively.

@askervin
Copy link
Contributor

askervin commented Nov 25, 2024

@saschagrunert, looks very nice! Welcome Wasm plugins.

@saschagrunert saschagrunert force-pushed the wasm-support branch 5 times, most recently from 7ba58db to dd57194 Compare November 26, 2024 09:58
@klihub klihub requested a review from askervin November 26, 2024 11:37
askervin
askervin approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@askervin askervin left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you, @saschagrunert!

@klihub klihub self-requested a review November 26, 2024 14:02
@saschagrunert
Copy link
Contributor Author

saschagrunert commented Dec 3, 2024

@mikebrow are your around to give this a review?

mikebrow
mikebrow reviewed Dec 5, 2024
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

Nice piece of work.. see comments :-)

@saschagrunert
Add WASM plugin support
112fc36 
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@saschagrunert
Copy link
Contributor Author

saschagrunert commented Dec 6, 2024

Updated the PR, thank you!

@klihub klihub requested a review from mikebrow December 11, 2024 08:26
mikebrow
mikebrow approved these changes Dec 11, 2024
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM

@mikebrow mikebrow merged commit d138684 into containerd:main Dec 11, 2024
8 checks passed
@mikebrow
Copy link
Member

mikebrow commented Dec 11, 2024

TODO: note .. #121 (comment) need a new struct to formally support multiple plugin types

@saschagrunert saschagrunert deleted the wasm-support branch December 12, 2024 07:46
saschagrunert added a commit to saschagrunert/nri that referenced this pull request Dec 12, 2024
@saschagrunert
Add dedicated plugin type struct
b9766d5 
Follow-up on: containerd#121 (comment)

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@saschagrunert saschagrunert mentioned this pull request Dec 12, 2024
Merged
@saschagrunert
Copy link
Contributor Author

saschagrunert commented Dec 12, 2024

TODO: note .. #121 (comment) need a new struct to formally support multiple plugin types

Yes: #125

saschagrunert added a commit to saschagrunert/nri that referenced this pull request Dec 12, 2024
@saschagrunert
Add dedicated plugin type struct
b29b0bd 
Follow-up on: containerd#121 (comment)

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
saschagrunert added a commit to saschagrunert/nri that referenced this pull request Dec 12, 2024
@saschagrunert
Add dedicated plugin type struct
47b39c4 
Follow-up on: containerd#121 (comment)

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@dmcgowan dmcgowan added the area/nri Node Resource Interface (NRI) label Sep 11, 2025
@dmcgowan dmcgowan mentioned this pull request Sep 11, 2025
Merged
@y-ykcir y-ykcir mentioned this pull request Oct 10, 2025
Open
@dmcgowan dmcgowan mentioned this pull request Oct 26, 2025
Merged
@dmcgowan dmcgowan mentioned this pull request Nov 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@klihub klihub klihub approved these changes

@mikebrow mikebrow mikebrow approved these changes

@fuweid fuweid Awaiting requested review from fuweid

+1 more reviewer

@askervin askervin askervin approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/nri Node Resource Interface (NRI) impact/changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@saschagrunert @klihub @askervin @haircommander @mikebrow @dmcgowan