stages: add `coreos.live-artifacts.mono` stage #1947

dustymabe · 2024-11-27T21:51:16Z

This adds a new org.osbuild.coreos.live-artifacts.mono stage to build CoreOS Live ISO/PXE artifacts. The code is heavily based on the cmd-buildextend-live script from coreos-assembler [1], but a lot of things had to be adapted:

the stage is provided the deployed oscontainer tree, metal, and
metal4k images as inputs
we use chroot instead of supermin to execute some commands in the
context of the target oscontainer
a bunch of calls that were wrapped by libguestfs for us (e.g.
mkfs.vfat, mksquashfs), we now have to call ourselves; to retain
maximum compatibility, we ensured that we still effectively use the
same args that libguestfs passed

And various other minor adjustments.

Of course, this is not really in line with the OSBuild philosophy of having smaller-scoped stages. We have labeled this with a .mono suffix to denote it is monolithic, similar to the existing org.osbuild.bootiso.mono stage today.

Eventually we may be able to break this stage down if we find it worth the effort. Alternatively the need for it may go away as we align more with Image Mode.

[1] https://github.com/coreos/coreos-assembler/blob/43a9c80e1f548269d71d6d586f0d5754c60f6144/src/cmd-buildextend-live

Co-authored-by: Dusty Mabe [email protected]
Co-authored-by: Renata Ravanelli [email protected]

dustymabe · 2024-11-27T21:52:30Z

marking as draft since this requires a few other PRs to merge:

dustymabe · 2024-12-04T20:38:38Z

marking as draft since this requires a few other PRs to merge:

osbuild/remoteloop: add more loop device options #1944

util/chroot: Add support for custom directory bind mounts #1917

util/osrelease.py: improve quote stripping #1927

All three have been merged now. Rebased on top of latest main and lifting draft status on this.

dustymabe · 2024-12-05T18:07:07Z

added a commit so that the org.osbuild.container-deploy will work with the fedora buildroot

dustymabe · 2024-12-06T17:10:46Z

Not really sure if the CI failures here are related to this PR.. At least Schutzbot passed!

ravanelli

LGTM, only minor typos.

stages/org.osbuild.coreos.live-artifacts.mono.meta.json

stages/org.osbuild.coreos.live-artifacts.mono

mvo5

I only managed 1/4 so far, there is a lot going on, I wanted to share my initial comments. I would love to write more tests for this but I am conscious of time - when ideally would you like to see this merged (ASAP is a fine answer, really only trying to get some more context here).

stages/org.osbuild.coreos.live-artifacts.mono

dustymabe · 2024-12-09T14:56:50Z

I only managed 1/4 so far

No worries. Thanks for posting initial review!

there is a lot going on

Yep. Sorry for that. A lot of this is historical.

I wanted to share my initial comments. I would love to write more tests for this but I am conscious of time

Thank you for helping us do that.

when ideally would you like to see this merged (ASAP is a fine answer, really only trying to get some more context here).

Ideally this week or early next week. I'll try to be very responsive in code review to ensure you don't spend any time waiting on us. If there are places where video chat and screenshare would be beneficial then let's not hesitate to do that.

test/data/manifests/fedora-build-v2.ipp.yaml

mvo5

Some more small comments/ideas/questions

stages/org.osbuild.coreos.live-artifacts.mono

achilleas-k · 2024-12-12T12:16:02Z

As part of the review, to make sense of everything going on in main(), I've been extracting blocks into functions. I initially did it as a way to make it easier for me to read and understand, but now I'd like to merge my changes here to make the whole thing more readable generally and hopefully more testable (unit tests on each function).

It's still a WIP, I'm still making sense of some things and I suspect some of the functions I made can be further split into smaller logical units.

Here's the branch: https://github.com/achilleas-k/osbuild/tree/coreos-iso/function-split

achilleas-k · 2024-12-12T12:24:09Z

If we decide we want this split to be merged here (and I would prefer we did), we would ask you (@dustymabe) to test the final stage in your build pipeline to verify it. How does that sound?

dustymabe · 2024-12-12T14:35:21Z

we would ask you (@dustymabe) to test the final stage in your build pipeline to verify it. How does that sound?

absolutely. I'll look at your changes now. If that's the direction we want to go we can test them out easily.

achilleas-k · 2024-12-12T18:06:57Z

I'm done with the first pass over main() and split it into multiple smaller functions. I'm quite happy with the size of each function now, but I'd still like to make another pass over and see if I can disentangle some of the side effects from each. I did the current splitting based almost entirely on line sequences, so there might be some functions that do a lot more than their name, arguments, and return values suggest. Tomorrow I hope to do another pass and trace everything and get a better sense of all the side effects from each of these functions and make the separation into functions more logical.

dustymabe

@achilleas-k - seriously thank you for going through and trying to improve things. I'm adding some comments here.

I haven't run any tests on this because it's still changing, but can do so once we get closer.

stages/org.osbuild.coreos.live-artifacts.mono

achilleas-k · 2024-12-13T10:20:54Z

Thanks for the review, Dusty. These are great comments. Exactly what I was hoping for. Gonna go through them now.

achilleas-k · 2024-12-13T14:53:35Z

I addressed most of the comments and pushed now, thanks again @dustymabe.
One thing that's missing is the part about genisoimage() becomine gen_live_artifacts() (#1947 (comment)). I wanted to get the smaller bits out first before diving into that.

The org.osbuild.container-deploy stage uses podman. Including it in the build here will allow that stage to be used with this pipeline as the buildroot. Include a workaround here for what I consider to be a bug [1] in that `podman` will create `/etc/containers/networks` on first run if it doesn't exist. That dir should just be created by an RPM. If we don't include this workaround then the stage will fail when `podman` attempts the `mkdir` because `/etc/containers` is mounted in from the buildroot readonly. [1] containers/common#2265

There have been a lot of changes to the CoreOS definitions in [1]. Let's update the test manifest here to more closely match what is running in the field there. [1] https://github.com/coreos/coreos-assembler/tree/dcd60cfe01f1d42e8d5f63a77db8172f2f1101ae/src/osbuild-manifests

This adds a new `org.osbuild.coreos.live-artifacts.mono` stage to build CoreOS Live ISO/PXE artifacts. The code is heavily based on the `cmd-buildextend-live` script from coreos-assembler [1], but a lot of things had to be adapted: - the stage is provided the deployed oscontainer tree, metal, and metal4k images as inputs - we use chroot instead of supermin to execute some commands in the context of the target oscontainer - a bunch of calls that were wrapped by libguestfs for us (e.g. mkfs.vfat, mksquashfs), we now have to call ourselves; to retain maximum compatibility, we ensured that we still effectively use the same args that libguestfs passed And various other minor adjustments. Of course, this is not really in line with the OSBuild philosophy of having smaller-scoped stages. We have labeled this with a .mono suffix to denote it is monolithic, similar to the existing `org.osbuild.bootiso.mono` stage today. Eventually we may be able to break this stage down if we find it worth the effort. Alternatively the need for it may go away as we align more with Image Mode. [1] https://github.com/coreos/coreos-assembler/blob/43a9c80e1f548269d71d6d586f0d5754c60f6144/src/cmd-buildextend-live Co-authored-by: Dusty Mabe <[email protected]> Co-authored-by: Renata Ravanelli <[email protected]>

This improves the readability and maintainability of the code to have it split into smaller pieces. Co-Authored-by: Dusty Mabe <[email protected]>

This commit adds some unit tests around the coreos live-artifcats mono stage. - test/coreos_live_artifacts: add test for align_initrd() - test/coreos_live_artifacts: add test for extend_initramfs() - This actually tests the mkinitrd_pipe() function, which extend_initramfs() calls after opening the file. - test/coreos_live_artifacts: add test for make_stream_hash() - test/coreos_live_artifacts: add test for make_efi_bootfile() Co-authored-by: Achilleas Koutsou <[email protected]>

The rename to get file names + suffix with < 8 characters was done in [1] when our initramfs was named initramfs.img. It was subsequently renamed to initrd.img in [2] and the rename of the initramfs was dropped but the rename of the kernel was never dropped. Since vmlinuz is already < 8 characters let's just drop the rename here too. [1] coreos/coreos-assembler@6040091 [2] coreos/coreos-assembler@6f533ef

The initramfs.img in Fedora and RHEL CoreOS images is already 644 so this isn't needed.

Rework rename of vendor directory to not use dfd APIs. This was requested in code review. Also added comments since I now understand it better.

Seems more appropriate in copy_configs_and_init_kargs_json() since this is where files were originally copied in.

jlebon

LGTM too. There's... a lot of changes happening in the follow-up commits, and I didn't check all the moves deeply. But overall, the refactoring looks quite good and I'm happy to see tests as well. Nice work all and thank you!

stages/org.osbuild.coreos.live-artifacts.mono

mvo5

Thank you!

stages/test/test_coreos_live_artifacts_mono.py

All patches are upstream and released so we can drop these now. Notably this includes initial full upstream support for building live artifacts using OSBuild [1]. [1] osbuild/osbuild#1947

All patches are upstream and released so we can drop these now. Notably this includes initial full upstream support for building live artifacts using OSBuild [1]. [1] osbuild/osbuild#1947 (cherry picked from commit 4f64339)

This includes support for generating live ISO/PXE artifacts from the derived container image, which can be required for initial installs. This leverages a new stage of osbuild that was recently released upstream with osbuild/osbuild#1947 included.

dustymabe marked this pull request as draft November 27, 2024 21:51

achilleas-k requested review from achilleas-k, mvo5 and ondrejbudai November 28, 2024 14:00

dustymabe mentioned this pull request Dec 2, 2024

osbuild/remoteloop: add more loop device options #1944

Merged

dustymabe force-pushed the dusty-live-artifacts branch from 13cdb52 to 6a6e9fd Compare December 4, 2024 20:38

dustymabe marked this pull request as ready for review December 4, 2024 20:38

dustymabe force-pushed the dusty-live-artifacts branch from 6a6e9fd to 6e546c9 Compare December 5, 2024 18:06

dustymabe requested review from jlebon and ravanelli as code owners December 5, 2024 18:06

dustymabe mentioned this pull request Dec 6, 2024

spec: create/own /etc/containers/networks containers/common#2265

Merged

ravanelli reviewed Dec 6, 2024

View reviewed changes

stages/org.osbuild.coreos.live-artifacts.mono.meta.json Outdated Show resolved Hide resolved

stages/org.osbuild.coreos.live-artifacts.mono Outdated Show resolved Hide resolved

mvo5 reviewed Dec 9, 2024

View reviewed changes

achilleas-k reviewed Dec 11, 2024

View reviewed changes

test/data/manifests/fedora-build-v2.ipp.yaml Show resolved Hide resolved

mvo5 reviewed Dec 12, 2024

View reviewed changes

dustymabe commented Dec 12, 2024

View reviewed changes

dustymabe commented Dec 13, 2024

View reviewed changes

stages/org.osbuild.coreos.live-artifacts.mono Outdated Show resolved Hide resolved

achilleas-k removed the request for review from ondrejbudai December 13, 2024 13:25

achilleas-k force-pushed the dusty-live-artifacts branch from f31fcf8 to b39faa0 Compare December 13, 2024 14:52

dustymabe and others added 9 commits December 18, 2024 13:43

stages/coreos.live-artifacts: refactor into more functions

cb27833

This improves the readability and maintainability of the code to have it split into smaller pieces. Co-Authored-by: Dusty Mabe <[email protected]>

stages/coreos.live-artifacts: stop chmod initrd.img

76219a7

The initramfs.img in Fedora and RHEL CoreOS images is already 644 so this isn't needed.

stages/coreos.live-artifacts: drop usage of dir fd

b87b2bc

Rework rename of vendor directory to not use dfd APIs. This was requested in code review. Also added comments since I now understand it better.

stages/coreos.live-artifacts: move rename of vendor dir

4c54b06

Seems more appropriate in copy_configs_and_init_kargs_json() since this is where files were originally copied in.

achilleas-k force-pushed the dusty-live-artifacts branch from e35db92 to 4c54b06 Compare December 18, 2024 12:45

achilleas-k approved these changes Dec 18, 2024

View reviewed changes

jlebon approved these changes Dec 18, 2024

View reviewed changes

stages/org.osbuild.coreos.live-artifacts.mono Show resolved Hide resolved

stages/org.osbuild.coreos.live-artifacts.mono Show resolved Hide resolved

dustymabe enabled auto-merge (rebase) December 18, 2024 14:14

dustymabe disabled auto-merge December 18, 2024 14:14

mvo5 approved these changes Dec 18, 2024

View reviewed changes

stages/test/test_coreos_live_artifacts_mono.py Show resolved Hide resolved

stages/test/test_coreos_live_artifacts_mono.py Show resolved Hide resolved

dustymabe enabled auto-merge (rebase) December 18, 2024 14:41

dustymabe merged commit cd19587 into osbuild:main Dec 18, 2024
45 checks passed

dustymabe mentioned this pull request Dec 19, 2024

osbuild: drop patchset given OSBuild v137 release coreos/coreos-assembler#3997

Merged

dustymabe mentioned this pull request Jan 16, 2025

support building live artifacts coreos/custom-coreos-disk-images#14

Merged

stages: add coreos.live-artifacts.mono stage #1947

stages: add coreos.live-artifacts.mono stage #1947

Uh oh!

Conversation

dustymabe commented Nov 27, 2024

Uh oh!

dustymabe commented Nov 27, 2024

Uh oh!

dustymabe commented Dec 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dustymabe commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dustymabe commented Dec 6, 2024

Uh oh!

ravanelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mvo5 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dustymabe commented Dec 9, 2024

Uh oh!

Uh oh!

mvo5 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

achilleas-k commented Dec 12, 2024

Uh oh!

achilleas-k commented Dec 12, 2024

Uh oh!

dustymabe commented Dec 12, 2024

Uh oh!

achilleas-k commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dustymabe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

achilleas-k commented Dec 13, 2024

Uh oh!

achilleas-k commented Dec 13, 2024

Uh oh!

jlebon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mvo5 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

stages: add `coreos.live-artifacts.mono` stage #1947

stages: add `coreos.live-artifacts.mono` stage #1947

dustymabe commented Dec 4, 2024 •

edited

Loading

dustymabe commented Dec 5, 2024 •

edited

Loading

achilleas-k commented Dec 12, 2024 •

edited

Loading