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

Wasm shims detection #8751

Merged
merged 3 commits into from
Nov 13, 2023
Merged

Wasm shims detection #8751

merged 3 commits into from
Nov 13, 2023

Conversation

flavio
Copy link
Contributor

@flavio flavio commented Oct 31, 2023

Proposed Changes

As developers start to experiment building WebAssembly applications, they desire to deploy these workloads on top of Kubernetes, using the same paradigm of "traditional" applications.
The containerd/runwasi project provides a way to integrate these workloads into Kubernetes.

The WebAssembly workloads require dedicated containerd runtimes to be installed and configured on the node. This PR provides a way to automatically discover the already installed runtimes and properly configure containerd to consume them.

The PR doesn't focus on shipping the pre-built runtimes as part of the k3s installation. I think this should be done by another dedicated PR.

Types of Changes

This PR introduces a new feature. It does that by leveraging the same mechanism used to discover the nvidia runtimes. As you will see, the existing nvidia discovery code has been refactored. Now we have a generic discovery mechanism that can be reused to discover any kind of containerd runtime.

This can be useful not only to add the WebAssembly runtimes, but to discover and configure other runtimes such as gVisor, Kata, ...

Verification

On a node where k3s agent is about to be installed create the following files:

  • /usr/bin/containerd-shim-lunatic-v1
  • /usr/sbin/containerd-shim-spin-v1
  • /opt/kwasm/bin/containerd-shim-lunatic-v1
  • /usr/bin/nvidia-container-runtime
  • /usr/bin/nvidia-container-runtime-experimental

All these files can be created with a touch command, they must have the executable permission set.

Install the k3s agent, check the containerd config.toml. Each runtime should have a dedicated section inside of the file.

Testing

The new code is covered by dedicated unit tests.

Linked Issues

This PR partially fixes #8700 . This PR doesn't handle the registration of the RuntimeClass resource for each of the runtimes discovered.

User-Facing Change

automatic discovery of WebAssembly runtimes

Further Comments

This PR is somehow related with #6872 . I've discussed with @brandon an alternative approach to his orignal PR. The idea is to focus on the discovery phase of the WebAssembly runtimes. We can expect the user has somehow installed them on the nodes. We will create another PR later on that changes the build scripts of k3s to ensure the most relevant runtimes are pre-installed with k3s.

@flavio flavio requested a review from a team as a code owner October 31, 2023 16:41
@flavio
Copy link
Contributor Author

flavio commented Oct 31, 2023

CC @vitorsavian

pkg/agent/containerd/config_linux.go Outdated Show resolved Hide resolved
pkg/agent/containerd/config_linux.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (6aef26e) 46.34% compared to head (57db827) 50.44%.
Report is 9 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8751      +/-   ##
==========================================
+ Coverage   46.34%   50.44%   +4.09%     
==========================================
  Files         148      150       +2     
  Lines       15664    15727      +63     
==========================================
+ Hits         7260     7933     +673     
+ Misses       7248     6538     -710     
- Partials     1156     1256     +100     
Flag Coverage Δ
e2etests 47.17% <83.60%> (?)
inttests 43.94% <84.61%> (+0.12%) ⬆️
unittests 18.32% <89.74%> (+0.43%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
pkg/agent/containerd/config_linux.go 57.69% <100.00%> (+2.28%) ⬆️
pkg/agent/containerd/nvidia.go 100.00% <100.00%> (+6.97%) ⬆️
pkg/agent/containerd/runwasi.go 100.00% <100.00%> (ø)
pkg/agent/containerd/runtimes.go 88.88% <88.88%> (ø)

... and 43 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@flavio
Copy link
Contributor Author

flavio commented Nov 2, 2023

@brandond thanks for the feedback, I've applied the changes you proposed and squashed them into the last commit

pkg/agent/containerd/nvidia.go Outdated Show resolved Hide resolved
pkg/agent/containerd/runtimes.go Outdated Show resolved Hide resolved
pkg/agent/containerd/runtimes_test.go Outdated Show resolved Hide resolved
pkg/agent/containerd/runwasi_test.go Outdated Show resolved Hide resolved
pkg/agent/containerd/runwasi_test.go Outdated Show resolved Hide resolved
pkg/agent/containerd/runwasi_test.go Outdated Show resolved Hide resolved
@flavio
Copy link
Contributor Author

flavio commented Nov 7, 2023

@dereknola I've applied your suggestions and performed the changes you requested. Can you take another look please?

Before merging the PR I'll clean the commit history, I'm talking about the last 2 commits which I left around to simplify the review process

@dereknola
Copy link
Member

Drone failed because of linter

pkg/agent/containerd/runtimes.go:25: File is not `gofmt`-ed with `-s` (gofmt)

Create a generic helper function that finds extra containerd runtimes.
The code was originally inside of the nvidia container discovery file.

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Discover the containerd shims based on runwasi that are already
available on the node.

The runtimes could have been installed either by a package manager or by
the kwasm operator.

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
The containerd configuration on a Linux system now handles the nvidia
and the WebAssembly runtimes.

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
@flavio
Copy link
Contributor Author

flavio commented Nov 8, 2023

I've fixed the gofmt issue and cleaned up the commit history

@vitorsavian vitorsavian merged commit ba5fcf1 into k3s-io:master Nov 13, 2023
15 checks passed
@flavio flavio deleted the wasm-shims-detection branch November 14, 2023 07:28
@flavio
Copy link
Contributor Author

flavio commented Nov 14, 2023

Nice! I'll put aside some time to update the docs

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.

Make alternative container runtimes easier to use
4 participants