feat: Improve Containerfile / enable workflow usage of containerized buildah (tag: v1)

[m0gg]

These are the first bits of my suggestions to improve the buildprocess via Containerfile.
Before going any further, i'd like to hear some feedback whether these kind of refactorings are welcome or not.

[m2Giles]

Thank you for this. One thing is that we don't build with a new enough version of podman to take advantage of heredoc. It's extremely sad and unfortunate.

It looks like most of the changes are taking advantage of dockers latest syntax and things like caching.

[m0gg]

It's extremely sad and unfortunate.

I absolutely agree.

looks like most of the changes are taking advantage of dockers latest syntax and things like caching.

It does run as expected on recent podman though.

May I ask for the reason for such an "old" version of podman?

[castrojo]

May I ask for the reason for such an "old" version of podman?

The ubuntu builders are 22.04, we're waiting for 24.04 builders to go GA so we can get newer podman versions, which should be sometime soon.

[m0gg]

May I ask for the reason for such an "old" version of podman?

The ubuntu builders are 22.04, we're waiting for 24.04 builders to go GA so we can get newer podman versions, which should be sometime soon.

Sigh ...

In that case, i could continue to test this stuff with gitea and ubuntu-latest runners.

While familiarising myself with the ublue projects, I got the urge to refactor some parts of the build system/scripts.
Are you open to review/comment changes requiring the newer builders?

[castrojo]

We need newer builders for zstd:chunked compression anyway, so welcome to the queue! We did opt in for a while into 24.04 but we had some builder failures so we rolled back to 22.04, but it's worth investigating again.

Open an issue on what you'd like to refactor and slap an enhancement label on it. Then we can have the other maintainers take a look and work towards consensus. Looking forward to seeing what you have in mind!

[castrojo]

Looks like the version in 24.04 is also too old for heredoc support, we should perhaps consider running with the podman OBS repo so we're always on the newest version, I think someone's proposed that before?

[m0gg]

Looks like the version in 24.04 is also too old for heredoc support

Not really too old - more like: broken. I don't know what they did to that poor thing, but it told me it's version 1.33.7.

runner@6562b73dd4be:/$ buildah --version
buildah version 1.33.7 (image-spec 1.1.0-rc.5, runtime-spec 1.1.0)
runner@6562b73dd4be:/$ buildah build -f- <<EOC
> FROM alpine
> RUN <<EOF
> echo I am groot
> EOF
> EOC
STEP 1/4: FROM alpine
Resolved "alpine" as an alias (/etc/containers/registries.conf.d/shortnames.conf)
Trying to pull docker.io/library/alpine:latest...
Getting image source signatures
Copying blob ec99f8b99825 done   |
Copying config a606584aa9 done   |
Writing manifest to image destination
STEP 2/4: RUN <<EOF
STEP 3/4: echo I am groot
ERRO[0002] +(UNHANDLED LOGLEVEL) &imagebuilder.Step{Env:[]string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}, Command:"echo", Args:[]string{""}, Flags:[]string{}, Attrs:map[string]bool(nil), Message:"ECHO ", Original:"echo I am groot"}
Error: building at STEP "ECHO ": Build error: Unknown instruction: "ECHO" &imagebuilder.Step{Env:[]string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}, Command:"echo", Args:[]string{""}, Flags:[]string{}, Attrs:map[string]bool(nil), Message:"ECHO ", Original:"echo I am groot"}

Support for here-documents was added in 1.33.0:

$ podman -r run --rm -i --privileged quay.io/buildah/stable:v1.33.2 buildah build -f- <<EOC
FROM alpine
RUN <<EOF
echo I am groot
EOF
EOC
STEP 1/2: FROM alpine
Resolved "alpine" as an alias (/etc/containers/registries.conf.d/000-shortnames.conf)
Trying to pull docker.io/library/alpine:latest...
Getting image source signatures
Copying blob sha256:ec99f8b99825a742d50fb3ce173d291378a46ab54b8ef7dd75e5654e2a296e99
Copying config sha256:a606584aa9aa875552092ec9e1d62cb98d486f51f389609914039aabd9414687
Writing manifest to image destination
STEP 2/2: RUN <<EOF
time="2024-07-12T20:22:43Z" level=warning msg="Path \"/run/secrets/etc-pki-entitlement\" from \"/etc/containers/mounts.conf\" doesn't exist, skipping"
I am groot
COMMIT
Getting image source signatures
Copying blob sha256:94e5f06ff8e3d4441dc3cd8b090ff38dc911bfa8ebdb0dc28395bc98f82f983f
Copying blob sha256:47f646e0d5136a366531b0238f6d2d6af4db40b315bcd4c96b37e14fd0c59c38
Copying config sha256:6e7b5d99878d8e949c40600eb4ca47dc37e6ff3be2900dcba73049e25425de44
Writing manifest to image destination
--> 6e7b5d99878d
6e7b5d99878d8e949c40600eb4ca47dc37e6ff3be2900dcba73049e25425de44

[m0gg]

While it might seem a little hacky, this worked well when i tested it with gitea actions and the 22.04 builder: 5ca8240

[m2Giles]

That's hacky, but we do have to workaround the limitations of the builders. Letting this start to run workflows

[bigpod98]

not exactly all that up to speed on heredocs but what does a build gain with using heredocs

[m0gg]

Now it successfully runs on GH. Maybe someone would like to trigger the workflow again?

[m0gg]

not exactly all that up to speed on heredocs but what does a build gain with using heredocs

Ultimately, it's preference. Heredocs can be written like normal scripts do: multiple lines, set -eu, etc., etc.
They are a lot more maintainable than sequences of && \.

But this isn't just about heredocs.

[m2Giles]

I noticed that for the mount commands there is a difference between how docker and podman handle these. Docker appears to not like nested mounts. Also docker gets angry with it called context since that's a reserved word.

While we use podman for building, keeping compatibility with docker is a plus.

Overall I really like what this is doing and especially considering that we are copying in the kernel now, reducing the size of the image by removing those COPY layers is great.

[m0gg]

I noticed that for the mount commands there is a difference between how docker and podman handle these. Docker appears to not like nested mounts. Also docker gets angry with it called context since that's a reserved word.

While we use podman for building, keeping compatibility with docker is a plus.

Overall I really like what this is doing and especially considering that we are copying in the kernel now, reducing the size of the image by removing those COPY layers is great.

Interesting. A quick just build did not yield any complaints or warnings on my rootless docker 25.0.2 (using buildx). @m2Giles what version are you using?

edit:
I did already remove the nested mounts because the github runner didn't like aswell.

[m2Giles]

docker buildx is v0.16
docker is 27.0.3

Wouldn't be surprised if it's some weird ordering

[m0gg]

This latest version runs without complaints on recent docker/buildx and podman. I've already put in too many commits which are may be considered unrelated or personal preference, so please comment away. I could split out some of them if desired.

[m2Giles]

I like this a lot and gives us a good blueprint for improving containerfiles in the rest of the organization.

[bsherman]

I'm requesting a few small changes. My reasons are either to maintain readability of the code or to avoid changes which I don't see as needed.

Happy to discuss.

Thanks!

[m2Giles]

One thing I'm also noticing in the build logs is that dracut no longer has access to /dev/kmsg and/or /dev/logger so dracuts output are hidden.

The other change that is catching my eye is the change from a for loop to a while loop in github installer script. I know read is preferable when working with arrays, but just trying to understand why the change was made there.

The xargs change is more readable now, bracket bash is something shellcheck indicates to use at times, but I think this is more readable.

Again, I do think the vast majority of this is an excellent improvement, and just would like some clarification on style changes or rationale for using different techniques.