set up containers-storage in supermin cache #4286
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors build-with-buildah to set up and use a containers-storage cache within the supermin VM. This is a significant improvement as it unifies the build logic for both direct and supermin paths, where buildah now consistently builds to containers-storage, and skopeo handles the export to an OCI archive. The changes in src/supermin-init-prelude.sh to set up the cache via a symlink are well-commented and seem correct. The related adjustments to cache size and the unmount command in src/cmdlib.sh are also appropriate.
I have two main concerns regarding src/cmd-build-with-buildah: one about argument quoting in the generated script, which could lead to build failures, and another about a logic change for mounting the overrides directory that might break some use cases. Please see the detailed comments.
dustymabe
force-pushed
the
dusty-cache-containers-storage
branch
from
e949a94 to
b9dbc5c
Compare
| fi | ||
| cat <<EOF > "${tempdir}/build-with-buildah-script.sh" | ||
| set -euxo pipefail | ||
| env -C ${tempdir}/src TMPDIR=$(realpath cache) buildah $@ |
There was a problem hiding this comment.
I usually wouldn't worry about spaces the way Gemini did, but there's an easy way I think to do this which is to use \"\$@\" here and then just pass the arguments when you call $cmd below.
This will make the copy super lightweight if it's on a filesystem that supports reflinks.
In the container native build flow where we are building from quay.io/fedora/fedora-bootc:xx it's better for the local developer use case if we don't need to pull the container from the registry on each iteration. Let's use containers-storage from the cache and also use `buildah build --layers=true` in all cases. This also bumps the default runvm cache size to account for the extra usage associated with the containers storage.
We now have persistent containers storage in the supermin VM with the cache. Let's unify the direct and non-direct paths in the code. This does mean we need to skopeo copy to the ociarchive inside the supermin VM (because we don't have access to that containers storage outside of it). Alternatively we could have tried to run the `cosa import` inside the supermin VM as well, but the use of a bare-user repository over a virtiofs share made that option not work.
This means that if the process gets interrupted the next invocation will clean up the temporary directory before continuing. I noticed that locally I had a bunch of these directories laying around eating up a decent amount of space.
This backports osbuild/osbuild#2222 which will allow us to drop at least one skopeo copy to help speed things up.
dustymabe
force-pushed
the
dusty-cache-containers-storage
branch
from
b9dbc5c to
e174809
Compare
|
The new upload here saves a I'll wait until that PR merges to merge this, but encourage reviews at this point. |
In the case where we built the container inside the supermin VM then the container is available in the container-storage inside the supermin VM already so let's use the org.osbuild.containers-storage input type here for that.
dustymabe
force-pushed
the
dusty-cache-containers-storage
branch
from
e174809 to
5436124
Compare
2 participants
and use it for
cosa build-with-buildah.See individual commit messages.