packaging: cache: Fix caching kernels which rely on extra modules #8987
Conversation
tools/packaging/kata-deploy/local-build/kata-deploy-binaries.sh
Outdated
Show resolved
Hide resolved
tools/packaging/kata-deploy/local-build/kata-deploy-binaries.sh
Outdated
Show resolved
Hide resolved
Right now this is just being added but not used yet. The idea is to use this to both cache and later on untar the kernel modules needed for some of the kernel targets we have (specifically looking at the confidential one). Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
This allows us to add a map, in the format of: `"tarball1_name:tarball1_path tarball2_name:tarball2_path ..."` With this we have a base to start doing a better job when caching extra artefacts, like kernel modules. Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Let's start doing this for the confidential kernels (and also for SEV, till it gets removed). Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
This will save us a lot of time, as right now the CI is rebuilding the kernel for absolutely no reason. Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Till now we didn't have a logic to consume the kernel modules cached tarball. Let's make sure those are consumed as it'll save us a reasonable amount of build time. Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Just to make sure we won't use cached components. Fixes: kata-containers#6415 Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
fidencio
force-pushed
the
topic/fix-cache-for-confidential-kernel
branch
from
7d62724
to
5d2906c
Compare
|
/test |
There was a problem hiding this comment.
Hi @fidencio, I haven't tested the changes... So my LGTM is from the bash PoV. Take it with a grant of salt.
Just left two small suggestions for you.
| kernel*-confidential|kernel-sev) | ||
| local modules_final_tarball_path="${workdir}/kata-static-${build_target}-modules.tar.xz" | ||
| if [ ! -f "${modules_final_tarball_path}" ]; then | ||
| local modules_dir=$(get_kernel_modules_dir ${kernel_version} ${kernel_kata_config_version}) |
There was a problem hiding this comment.
A small suggestion is to squash this commit with the first one that you are introducing the function.
| ${build_target}-builder-image-version \ | ||
| {build_target}-sha256sum | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
Another suggestion would be to try removing this duplication. Something like this (if possible):
additional_arg=""
case ${build_target} in
kernel*-confidential|kernel-sev)
additional_arg="kata-static-${build_target}-modules.tar.xz"
;;
esac
sudo oras push \
${ARTEFACT_REGISTRY}/kata-containers/cached-artefacts/${build_target}:latest-${TARGET_BRANCH}-$(uname -m) \
${final_tarball_name} \
${additional_arg:+$additional_arg} \
${build_target}-version \
${build_target}-builder-image-version \
${build_target}-sha256sumI haven´t tested this code, so not sure if will work as it is.
There was a problem hiding this comment.
I'll do this in a follow-up, just to make sure things work first with the "dumber" version.
| @@ -141,6 +172,9 @@ install_cached_tarball_component() { | |||
| local current_image_version="${3}" | |||
| local component_tarball_name="${4}" | |||
| local component_tarball_path="${5}" | |||
| # extra_tarballs must be in the following format: | |||
There was a problem hiding this comment.
Looks like you future-proofed this format here? I saw one case where extra_tarballs gets set in your new changes (line 380), and it just specifies one tarball (not a space-separated list). Seems reasonable to allow for more in the future, but thought it worth asking.
There was a problem hiding this comment.
I future proofed this as I can see NVIDIA wanting to also cache their deb files generated from the kernel build.
This logic has been broken forever, let's try to fix it here.
Please, see each commit message for more details and, please, do the best review possible on this one.