UKI Cleanup#2200
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the UKI (Unified Kernel Image) build process to support passing explicit kernel and initramfs paths via CLI arguments, reducing reliance on auto-discovery within the rootfs. Key changes include updating the seal-uki and finalize-uki scripts to use named arguments, modifying Dockerfile stages to extract and clean up kernel components, and extending the Rust library and CLI to handle the new parameters. Review feedback identified a potential path resolution bug in the Rust file existence checks, a filename mismatch in the upgrade test Dockerfile, and suggested improvements for error handling and validation in the seal-uki script.
cb42d78 to
8a3ed64
Compare
66dc0e3 to
3a9dc2b
Compare
3a9dc2b to
8e581d4
Compare
8e581d4 to
503b07a
Compare
| kver=$(bootc container inspect --rootfs /run/base-penultimate-src --json | jq -r '.kernel.version') | ||
|
|
||
| rm -v "/usr/lib/modules/$kver/vmlinuz" | ||
| rm -v "/usr/lib/modules/$kver/initramfs.img" | ||
| fi |
There was a problem hiding this comment.
Since this one would end up being closer to user-facing, I would prefer to streamline this more.
I think what would work best is to have our "base" build process be a stage that generates a single intermediate image with /rootfs and /kernel or so. This could be part of bootc-base-imagectl, or we could have it be part of something more like bootc container split-root-and-uki?
There was a problem hiding this comment.
sgtm. By the "base" build process, are you referring to the Dockerfile (in bootc) or is it about something else?
There was a problem hiding this comment.
The pattern we apply to this should generalize to https://github.com/cgwalters/rhel-bootc-examples/tree/sealed/sealing as well etc.
There was a problem hiding this comment.
Again - I may be missing something that's not possible - let me know if you want me to try - but I would really like to not have these rm -v type invocations be a required thing to use this feature.
I think my above comment would work.
There was a problem hiding this comment.
or we could have it be part of something more like bootc container split-root-and-uki?
If I understand this correctly, this would simply call /usr/libexec/bootc-base-imagectl build-rootfs --manifest=standard /target-rootfs, then move /target-rootfs/usr/lib/modules/$kver/{vmlinuz,initramfs.img} to /kernel/$kver?
There was a problem hiding this comment.
This get a bit complicated as the building of the rootfs and splitting the kernel needs to happen as the very first step, but we generate our custom initramfs much later in the build, just before the seal-uki step. We could chroot into the new root (created by bootc-base-imagectl) and install our rpms and other packages.
There was a problem hiding this comment.
This get a bit complicated as the building of the rootfs and splitting the kernel needs to happen as the very first step, but we generate our custom initramfs much later in the build,
Yes...but we can and should rework the build here so the initramfs is only generated once. I'll look at a prep PR for that.
There was a problem hiding this comment.
Now we generate the target-rootfs with --no-initramfs param, then install our dependencies by chrooting and then generate the initramfs. After that we split the rootfs and kernel
|
I think we should have the kernel and initrd as required arguments and thus only support the case where they are not in the rootfs anymore as I don't see a use case where someone would want a sealed image with both a UKI and split out kernel and initrd. Edit: That would break the current |
|
Hmm. I kind of lean towards not breaking it at least right away, it seems really easy to continue to support what we have now too. We could mark it deprecated though. |
503b07a to
8a5c7ae
Compare
|
Failures are transient & volatile related. Maybe from #2201? |
8a5c7ae to
cc8a2d8
Compare
f2c3424 to
667a0fd
Compare
Don't know if clap has a way to do this, if it does I couldn't find it. We just print our custom warning if |
4d6bed6 to
20e2fed
Compare
b9b57cb to
45f9bd2
Compare
|
How about splitting out the "prep" changes from this PR ? |
Opened #2240 |
45f9bd2 to
7a9ee13
Compare
3e9a91e to
135915e
Compare
Now that we can pass kernel and initrd paths to `bootc ukify`, rework our UKI Dockerfile to remove kernel + initrd from the final layer and only keep the UKI This still will not *remove* the kernel + initrd from the tarball but have whiteout instead See bootc-dev#2027 (comment) Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
vmlinuz and intrd should not be present in UKI images; add test for the same Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
So we can just use bootc to extract the `.linux` and `.initrd` sections from the UKI and not have to use objcopy Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
This command is equivalent to
`mv /target-root/usr/lib/modules/$kver/{vmlinuz,initramfs.img} /out/$kver`
We could just use `mv`, but having an actual bootc cmd is cleaner
Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
866acdc to
c41bdc8
Compare
|
Converting to draft. Still needs a bit more work |
c41bdc8 to
7710290
Compare
Since we do not want kernel + initrd in the final UKI dockerfile, we now build the initrd inside the `target-rootfs` generated by `bootc-base-imagectl` by chrooting into it and installing the required packages. After that's done we split the rootfs and vmlinuz + initrd into /target-rootfs and /kernel/$kver respectively Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Now since we need to build our initramfs before the `fetch` build stage, we need packages built first as we need `bootc` and `bootc-initramfs-setup` binaries Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
045567b to
f5c48ad
Compare
To avoid complexity with setting up networking inside `/target-rootfs` for when we chroot and install stuff, now we make sure to only have operations that do not require networking inside the chrooted env Separate out `downgrade-kernel.sh` script and accept a rootfs parameter so we can run this from outside target-rootfs but still operate on the target-rootfs Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
f5c48ad to
e2b3c1d
Compare
ukify: Allow passing custom kernel, initramfs
While building a sealed UKI image we'd want to remove the original
kernel + initramfs from the final image and have only the final UKI
present. This was not possible before as
bootc container ukifyexpected kernel + initramfs to be present in
usr/lib/modulesofcontainer root
Fixes: #2185
dockerfile/uki: Rework to remove kernel + initrd
Now that we can pass kernel and initrd paths to
bootc ukify, reworkour UKI Dockerfile to remove kernel + initrd from the final layer
and only keep the UKI
This still will not remove the kernel + initrd from the tarball but
have whiteout instead
See #2027 (comment)
test/integration: Test vmlinuz non-existence with UKI
vmlinuz and intrd should not be present in UKI images; add test for the
same