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

feat: Move WASM to components.json #4969

Open
wants to merge 49 commits into
base: master
Choose a base branch
from
Open

Conversation

AlisonB319
Copy link
Collaborator

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Requirements:

Special notes for your reviewer:

Release note:

none

@@ -193,9 +193,6 @@ downloadCNI() {
retrycmd_get_tarball 120 5 "$downloadDir/${cniTgzTmp}" ${CNI_PLUGINS_URL} || exit $ERR_CNI_DOWNLOAD_TIMEOUT
}

downloadContainerdWasmShims
echo " - containerd-wasm-shims ${CONTAINERD_WASM_VERSIONS}" >> ${VHD_LOGS_FILEPATH}

echo "VHD will be built with containerd as the container runtime"
updateAptWithMicrosoftPkg
capture_benchmark "create_containerd_service_directory_download_shims_configure_runtime_and_network"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: would you mind removing the download_shims part out of the benchmark title and replacing it with and? The rest looks good to me!

},
{
"renovateTag": "<DO_NOT_UPDATE>",
"latestVersion": "0.8.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

we also want 0.15.1 now as well right?

# Install, download, update wasm must all be run from the same function call
# in order to ensure WASMSHIMPIDS persists correctly since in bash a new
# function call from install-dependnecies will create a new shell process.
installingContainerdWasmShims(){
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: installContainerdWasmShims?

}

updateContainerdWasmShimsPermissions() {
containerd_wasm_filepath=${1}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: let's use local when defining local vars in general

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.

4 participants