Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd-osbuild: Add support for LiveISO #3861

Closed
wants to merge 6 commits into from

Conversation

ravanelli
Copy link
Member

  • Add support for OSBUILD via COSA_USE_LIVEISO env var for now

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach looks sane overall! Want to fold the commits in #3847 into this one?

src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
@ravanelli ravanelli marked this pull request as ready for review October 1, 2024 15:17
@ravanelli ravanelli force-pushed the pr/cosa_live branch 2 times, most recently from 954c139 to 543fdf6 Compare October 1, 2024 15:22
@ravanelli ravanelli requested a review from jlebon October 8, 2024 19:34
Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a review here but I'm not sure we should really merge this until we get a better idea of what the actual stages will look like (i.e. the manifests and the inputs will change).

src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two of the commits have a leading space in their titles.

The last commit is missing a blank line between the title and the body, which messes up git log --oneline.

Agree we should wait until we finalize the stage before adding the pipeline here. Though if you'd like, we can at least get the prep commits in for now in a separate PR.

src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
src/cmd-buildextend-live Outdated Show resolved Hide resolved
options:
filename: efiboot.img
size:
mpp-format-string: '{live_efiboot_img_size_mb * 1024 * 1024}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mpp-format-string: '{live_efiboot_img_size_mb * 1024 * 1024}'
mpp-format-string: '{16 * 1024 * 1024}'

We discussed this last week and decided to just hardcode it to 16 here for now and add a comment about it and mention we can parameterize it if we ever need to adjust it in the future.

Comment on lines 45 to 47
# XXX: we should rename this pipeline now that we don't use it as
# a buildroot
- name:build
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if #3946 merges we'll need to update this to use the new name.

Suggested change
# XXX: we should rename this pipeline now that we don't use it as
# a buildroot
- name:build
# XXX: we should rename this pipeline now that we don't use it as
# a buildroot
- name:build

src/cmd-osbuild Outdated
@@ -242,6 +246,7 @@ main() {
"cmd-buildextend-qemu") platforms=(qemu);;
"cmd-buildextend-qemu-secex") platforms=(qemu-secex);;
"cmd-buildextend-secex") platforms=(qemu-secex);;
"cmd-buildextend-live") platforms=(live-iso);;
Copy link
Member

@dustymabe dustymabe Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'd need a corresponding entry in the Perform postprocessing case statement in this file to handle the multiple artifacts that are being generated (i.e. not just the ISO should be getting generated now)

src/runvm-osbuild Outdated Show resolved Hide resolved
@dustymabe
Copy link
Member

Two largish change suggestions:

  1. We consider the "platform" here to be live and not live-iso.

This means platform.live-iso.ipp.yaml -> platform.live.ipp.yaml and that generates the ISO/PXEkernel/initrd/rootfs.

  1. we rename the stage we are proposing to OSBuild to be stages/org.osbuild.coreos.live OR platform.live-artifacts.

# This file defines the pipeline for building the live ISO.
version: '2'
pipelines:
- name: live-iso
Copy link
Member

@dustymabe dustymabe Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we are copying in the metal.raw and metal4k.raw images here when we copy out of this pipeline it will include those metal.raw and metal4k.raw images which isn't what we want.

what we'll have to do is have one stage for creating the live artifacts and another stage where we copy only the created artifacts, which then gets copied out.

That is... unless we figure out how to do the loopback device stuff inside the stage. If we can do the loopback stuff inside the stage code then we can use an input: from a previous pipeline instead and then set up the loopback device from within the stage code.

I guess another option would be to teach the devices/org.osbuild.loopback code to somehow work on inputs.. but I'm not sure if it can be done easily or if it would be accepted.

@ravanelli ravanelli changed the title buildextend-live: Add support OSBUILD cmd-osbuild: Add support for LiveISO Nov 19, 2024
@ravanelli ravanelli force-pushed the pr/cosa_live branch 2 times, most recently from 9e1cf94 to 8a0b479 Compare November 20, 2024 00:43
dustymabe and others added 3 commits November 21, 2024 12:32
The `build` pipeline was renamed to `deployed-tree` in 3a91ec2
and this should have been updated in that commit as well.
We run osbuild inside of supermin, so for building the live ISO we'll
need all the tools it needs in there. This includes `genisoimage` and
`syslinux-nonlinux`.

We don't currently name `squashfs-tools` as a toplevel dependency. It's
currently pulled in by libguestfs, but we need it in supermin too, so
explicitly list it there.
 - Introduce necessary variables for platform.live-iso.ipp.yaml.
 - Add the platform.live-iso.ipp manifest to support LiveISO creation.
 - Adapt CoreOS platform manifests

Signed-off-by: Renata Ravanelli <[email protected]>
ravanelli and others added 3 commits November 21, 2024 12:32
 - Introduce LiveISO creation via cmd-osbuild.

Signed-off-by: Renata Ravanelli <[email protected]>
 - Add checkpoints for metal and metal4k pipelines due LiveISO builds;
 - Bump cache size from 14G to 20G.

Signed-off-by: Renata Ravanelli <[email protected]>
@jlebon
Copy link
Member

jlebon commented Nov 21, 2024

Ran this through the differ #3968 on FCOS x86_64:

  • We're missing everything in /live in the live ISO. This can be obtained from the container image under /usr/share/coreos-assembler/live (see cmdlib: bake src config's live/ directory in compose #3846).
  • The osmet filenames in the live rootfs changed from e.g. fedora-coreos-41.20241121.dev.0-1-metal.x86_64.raw.osmet to just metal.osmet and similarly for metal4k. I don't think this breaks anything and we could do without, though having the build ID in the name is a nice cross-check.
  • /boot on the squashfs is missing everything under /boot/efi. We need to mount the ESP onto /boot/efi when building the squashfs like we do in coreos_gf_run_mount.

We should repeat the exercise on other arches.

@dustymabe
Copy link
Member

successor in #3976

@ravanelli ravanelli closed this Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants