buildextend-installer: refactoring for separate rootfs image #1612
Conversation
|
All the easy patches here look right. I read the big initramfs refactoring...and it looks right at a high level. I think /approve |
|
/hold |
It's wired up, but: #1613 |
Every file we're writing (except the flag file in /etc) is already compressed.
The kernel and initrd don't need to be executable.
Accumulate additional initrd contents into temporary directories and then cpio them all up at once. This ensures we don't have a bunch of little cpios concatenated together, which is nice for debugging and will be required for the separate PXE rootfs.
RHCOS supports osmet now.
In practice it's required for all release images, and we've forgotten
about this before:
coreos/fedora-coreos-pipeline#250
This increases build time for local development, but improves the
consistency of build artifacts and reduces code complexity a bit.
| if img_metal4k_obj is not None: | ||
| if is_live: | ||
| if img_metal_obj is None or img_metal4k_obj is None: | ||
| raise Exception("Live image generation requires `metal` and `metal4k` images") |
There was a problem hiding this comment.
Not a blocker, but I think we should have a switch like --allow-missing-metal4k to still allow skipping this for the local dev case. Iterating on the live ISO is super painful, and while the majority of the time is the mksquashfs step, having to recreate a metal4k image we don't care about each time accumulates to a non-trivial amount of time.
There was a problem hiding this comment.
<jlebon> i'd say we can try getting it in as is, and if someone (perhaps me) gets annoyed enough
in the future to add that option, i wouldn't oppose :)
👍 Let's skip for now. If we want to re-add the option, I agree that --allow-missing-... is more palatable, since the caller knows they're asking for that, and it doesn't matter as much if it breaks.
|
Let's wait until #1613 is in to make sure CI exercises the 4k path too. |
|
Rebased on #1613. /unhold |
|
/lgtm |
|
yay - we're able to |
|
/hold briefly |
| else: | ||
| if img_metal_obj is None: | ||
| raise Exception("ISO image generation requires `metal` image") |
There was a problem hiding this comment.
If we're building a legacy installer (that doesn't include the rootfs) do we really require the metal image?
There was a problem hiding this comment.
ahh, seems like we get the kernel arguments and such from the img_metal later on so I guess it is required.
|
/unhold /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bgilbert, cgwalters, dustymabe, jlebon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Split refactoring patches out of #1608 and add a couple more cleanups.