Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
add containerd-wasm-shims to enable Wasm / WASI workloads #1037
add containerd-wasm-shims to enable Wasm / WASI workloads #1037
Changes from all commits
97f0502
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't garmented both runtime would be installed for this check so this could fail if only one is installed. I am not sure this is a big deal at this point for the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unfortunate that the containerd wasm shims are packaged together into one release artifact. When you download one, you get both.
I think in the future, I'd like to change the shims to be packaged individually and change the variables / tests in image-builder to allow for individual installation and verification.
I've also opened an issue to add
--version
support for the shims. I didn't like the error test. The test does the job of verifying the shim is installed, on the path, and responding in a reasonable way, but I would much rather see something closer to the containerd version verification, so we can ensure we have the specific version installed that we expect.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's acceptable to assume both shims for now, and to fix this when their packaging changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think this wouldn't verify both are there are present only one of them (similar to comment above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are 100% correct. I was having a hard time dealing with
.Vars.containerd_wasm_shims_runtime
as it's a comma delimited string ("slight, spin"
). It's much easier to deal with it in Python in Ansible using a string contains.Do you have a suggestion of how to handle it better in Goss?