adaptation: allow compiling out WASM support altogether. #253

klihub · 2025-12-11T08:44:35Z

This PR introduces the "nri_no_wasm" build tag which now allows disabling WASM support altogether at compile time. This can be helpful in resource constrained or other environments, where WASM support is not needed and the related binary size increase penalty is considered difficult to justify.

thaJeztah · 2025-12-11T09:30:58Z

pkg/adaptation/adaptation.go

 		socketPath:  DefaultSocketPath,
 		syncLock:    sync.RWMutex{},
-		wasmService: wasmPlugins,
+		wasmService: wasmService,


I was originally considering if this could be a functional option (WithWASMService); that could potentially allow the caller to either pass, or not pass the option.

Compile-time could still be an option for that if a stub WithWASMService was created (that would either return an error, or nil).

pkg/adaptation/wasm-disabled.go

thaJeztah · 2025-12-11T09:39:27Z

Oh, thanks! @robmry PTAL 🤗

The thing I was struggling with was that various of the WASM types are also deeply engrained in the API definition, but not sure if that caused the binary size to grow as well.

I started looking if we could somehow get that dependency out of the API definition, but didn't get very far; just a quick "let's see if this could work", but there were a lot of places and additional definitions; I pushed what I had here, but it very likely won't even compile;

[DNM] try defining interfaces locally #254

edit: wrong link

klihub · 2025-12-11T11:35:59Z

Oh, thanks! @robmry PTAL 🤗

The thing I was struggling with was that various of the WASM types are also deeply engrained in the API definition, but not sure if that caused the binary size to grow as well.

Yeah, I'm unable to tell. But I tried the effect of this and compiling containerd main/HEAD now with BUILDTAGS=nri_nowasm results on x86_64 in a 45272408 binary size vs. 48184952 when compiling with WASM support enabled. A size reduction of 2912544 bytes.

thaJeztah

thanks!

I think this change looks good; AFAICS, it doesn't paint us in a corner on considering some of the other options (functional option to add the plugin and looking further at what can be done for the API definitions).

I left one nit for the naming of the files, but not really blocking, just a suggestion.

LGTM

thaJeztah · 2025-12-11T12:00:19Z

pkg/adaptation/wasm-disabled.go

I think using underscores instead of hyphens is more common in Go; perhaps adaptation_wasm.go and adaptation_nowasm.go could work?

I think that calling it adaptation_wasm.go would not work. It would break the builds, because wasm is a valid GOARCH, so it would be excluded from x86_64 and arm builds.

Oh! good call, forgot that wasm was a builtin... 😢

klihub · 2025-12-11T12:37:39Z

thanks!

I think this change looks good; AFAICS, it doesn't paint us in a corner on considering some of the other options (functional option to add the plugin and looking further at what can be done for the API definitions).

Yes, I also think we should explore your 'inject WASM support using a functional option' further, and see if we can sort everything out to get it working, and then take a look at what that would mean in terms of maintenance wrt. expected WASM runtime API stability/etc.

robmry · 2025-12-11T12:41:33Z

Yeah, I'm unable to tell. But I tried the effect of this and compiling containerd main/HEAD now with BUILDTAGS=nri_nowasm results on x86_64 in a 45272408 binary size vs. 48184952 when compiling with WASM support enabled. A size reduction of 2912544 bytes.

Great - thank you! I tried it on a dockerd build and it saves about the same.

mikebrow · 2025-12-11T17:22:21Z

thanks!
I think this change looks good; AFAICS, it doesn't paint us in a corner on considering some of the other options (functional option to add the plugin and looking further at what can be done for the API definitions).

Yes, I also think we should explore your 'inject WASM support using a functional option' further, and see if we can sort everything out to get it working, and then take a look at what that would mean in terms of maintenance wrt. expected WASM runtime API stability/etc.

nod.. I wonder if functional would allow for variants per runtime type?

klihub · 2025-12-11T17:39:56Z

@saschagrunert Would you guys be okay with enabling WASM support behind a build flag and have it disabled by default (so the other way around than proposed in this PR) ?

pkg/adaptation/wasm-disabled.go

AkihiroSuda

The build tag seems inconsistent with existing ones

https://github.com/containerd/nri/pull/253/changes#r2612981454

klihub · 2025-12-12T08:08:38Z

thanks!
I think this change looks good; AFAICS, it doesn't paint us in a corner on considering some of the other options (functional option to add the plugin and looking further at what can be done for the API definitions).

Yes, I also think we should explore your 'inject WASM support using a functional option' further, and see if we can sort everything out to get it working, and then take a look at what that would mean in terms of maintenance wrt. expected WASM runtime API stability/etc.

nod.. I wonder if functional would allow for variants per runtime type?

If you mean plugging in other WASM implementations than wazero, then moving to a functional option to set up WASM support would make this theoretically/from a pure mechanics PoV possible. However, since our WASM support uses knqyf263/go-plugin to generate all the necessary bindings and other boilerplate to interact with WASM-compiled code from our protobuf definitions, and since go-plugin uses wazero, this would not really be possible in practice at the moment.

saschagrunert · 2025-12-12T09:16:44Z

@saschagrunert Would you guys be okay with enabling WASM support behind a build flag and have it disabled by default (so the other way around than proposed in this PR) ?

Should be fine, yes.

AkihiroSuda

Thanks

klihub · 2025-12-16T08:17:53Z

@mikebrow @dmcgowan Could you guys chime in if we are fine with the current build tag semantics, IOW keeping WASM support on by default for backward compatibility and turning it off using the no_wasm tag ? If we are fine with it, I'll merge this as is, then tag a v0.11.0 release.

marquiz

Looks good to me.

That said, if we think that disabling wasm support by default is desirable and a good thing, I think now would be a good time to do it (before we get to NRI v1.0). Put a big fat note in the release notes.

mikebrow

LGTM

klihub · 2025-12-16T16:17:37Z

Looks good to me.

That said, if we think that disabling wasm support by default is desirable and a good thing, I think now would be a good time to do it (before we get to NRI v1.0). Put a big fat note in the release notes.

I think disabling WASM makes the most sense for those who consume and compile containerd themselves from sources and really count bytes, for instance k3s. I'd be fine with keeping it enabled by default.

Allow WASM support to be disabled at compile time using the "nri_no_wasm" build tag. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>

klihub · 2025-12-16T19:19:16Z

@AkihiroSuda Updated the build tag according to the latest preference/review comment. Now I'd need one re-approval to be able to merge this.

klihub requested a review from thaJeztah December 11, 2025 08:44

klihub mentioned this pull request Dec 11, 2025

SIGILL: illegal instruction on arm64 debian 11 for containerd 2.2.0 (github.com/tetratelabs/wazero/internal/platform.getisar0()) containerd/containerd#12665

Open

thaJeztah reviewed Dec 11, 2025

View reviewed changes

pkg/adaptation/wasm-disabled.go Outdated Show resolved Hide resolved

thaJeztah mentioned this pull request Dec 11, 2025

[DNM] try defining interfaces locally #254

Draft

klihub force-pushed the devel/nowasm-build-flag branch 2 times, most recently from 9b9b452 to b5c71f1 Compare December 11, 2025 11:34

thaJeztah approved these changes Dec 11, 2025

View reviewed changes

klihub requested a review from mikebrow December 11, 2025 13:10

austinvazquez approved these changes Dec 11, 2025

View reviewed changes

AkihiroSuda reviewed Dec 12, 2025

View reviewed changes

pkg/adaptation/wasm-disabled.go Outdated Show resolved Hide resolved

AkihiroSuda requested changes Dec 12, 2025

View reviewed changes

AkihiroSuda mentioned this pull request Dec 12, 2025

[release/2.2] Prepare release notes for v2.2.1 containerd/containerd#12677

Merged

klihub force-pushed the devel/nowasm-build-flag branch from b5c71f1 to e10f3bf Compare December 12, 2025 13:57

klihub requested a review from AkihiroSuda December 12, 2025 13:58

AkihiroSuda approved these changes Dec 12, 2025

View reviewed changes

AkihiroSuda requested a review from dmcgowan December 16, 2025 05:55

marquiz approved these changes Dec 16, 2025

View reviewed changes

mikebrow approved these changes Dec 16, 2025

View reviewed changes

adaptation: allow compiling out WASM support altogether.

ab88fe6

Allow WASM support to be disabled at compile time using the "nri_no_wasm" build tag. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>

klihub force-pushed the devel/nowasm-build-flag branch from e10f3bf to ab88fe6 Compare December 16, 2025 19:16

klihub requested a review from AkihiroSuda December 16, 2025 19:18

AkihiroSuda approved these changes Dec 17, 2025

View reviewed changes

AkihiroSuda merged commit 1078130 into containerd:main Dec 17, 2025
16 checks passed

klihub deleted the devel/nowasm-build-flag branch December 17, 2025 07:38

y-ykcir mentioned this pull request Dec 18, 2025

pr-tracker y-ykcir/containerd-release-tracker#1

Open

adaptation: allow compiling out WASM support altogether. #253

adaptation: allow compiling out WASM support altogether. #253

Uh oh!

Conversation

klihub commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thaJeztah commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

klihub commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

klihub Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

klihub commented Dec 11, 2025

Uh oh!

robmry commented Dec 11, 2025

Uh oh!

mikebrow commented Dec 11, 2025

Uh oh!

klihub commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

klihub commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saschagrunert commented Dec 12, 2025

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

klihub commented Dec 16, 2025

Uh oh!

marquiz left a comment

Choose a reason for hiding this comment

Uh oh!

mikebrow left a comment

Choose a reason for hiding this comment

Uh oh!

klihub commented Dec 16, 2025

Uh oh!

klihub commented Dec 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

klihub commented Dec 11, 2025 •

edited

Loading

thaJeztah commented Dec 11, 2025 •

edited

Loading

klihub commented Dec 11, 2025 •

edited

Loading

klihub Dec 11, 2025 •

edited

Loading

klihub commented Dec 11, 2025 •

edited

Loading

klihub commented Dec 12, 2025 •

edited

Loading