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

Build live PXE and ISO images #738

Merged
merged 7 commits into from
Sep 16, 2019
Merged

Build live PXE and ISO images #738

merged 7 commits into from
Sep 16, 2019

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented Sep 5, 2019

Add buildextend command to build live PXE and ISO images, with a squashfs of the root filesystem appended to the initrd. Pursuant to coreos/fedora-coreos-tracker#203, we'd eventually drop the separate installer images.

Part of coreos/fedora-coreos-config#155.

Allow buildextend-installer to be called as "buildextend-live", and
adjust the artifact names and meta.json keys accordingly.
src/cmd-buildextend-installer Outdated Show resolved Hide resolved
src/cmd-buildextend-installer Show resolved Hide resolved
src/gf-mksquashfs Outdated Show resolved Hide resolved
@bgilbert
Copy link
Contributor Author

bgilbert commented Sep 7, 2019

Updated.

@cgwalters
Copy link
Member

Random background thought; given how entangled cosa and the "base" fedora-coreos-config git repos are, I wonder if it'd be cleaner to actually merge the true basics of the config into cosa.

Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

LGTM!

@tuan-hoang1
Copy link
Contributor

GTG @dustymabe ?

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.

here are some initial comments. Will probably have more tomorrow when I test the built artifacts

# Work in a tmpdir on the destination so that we don't inherit some MCS labeling
# from the /tmp dir in the container. This also ensures that the final move is a
# pure `rename()`.
# See also: https://github.com/coreos/coreos-assembler/issues/292
Copy link
Member

Choose a reason for hiding this comment

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

can we add a link to #394 as I think that gives us better information now. Whatever changes we make here we should also make in gf-platformid

coreos_gf_run_mount "${src}" --ro

# coreos_gf_run_mount helpfully mounted filesystems other than root.
# We only want root.
Copy link
Member

Choose a reason for hiding this comment

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

maybe it would be simpler just to replace this all with:

coreos_gf_run  "${src}" --ro
root=$(coreos_gf findfs-label root)
coreos_gf mount "${root}" /        

@@ -31,32 +31,38 @@ if not args.build:

print(f"Targeting build: {args.build}")

# Hacky mode switch, until we can drop support for the installer images
Copy link
Member

Choose a reason for hiding this comment

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

definitely took me a minute looking at the code sideways :) - any idea how long before we can drop the other images?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For FCOS, hopefully soon. For RHCOS it's more of a policy question.

src/gf-mksquashfs Show resolved Hide resolved
In particular, the caller might want to pass --ro.

Also clean up some overly indirect code.
Use guestfish to generate a zstd-compressed squashfs of the root
filesystem, then pack it into a cpio archive and append it to the
initramfs.
The live image needs different bootloader messages and kargs than the
installer.
@bgilbert
Copy link
Contributor Author

Updated.

@dustymabe
Copy link
Member

We need to update cmd-compress to handle the new types:

$ git diff
diff --git a/src/cmd-compress b/src/cmd-compress
index 7cb7a4d..63262d5 100755
--- a/src/cmd-compress
+++ b/src/cmd-compress
@@ -41,7 +41,7 @@ else:
 print(f"Targeting build: {build}")
 
 # Don't compress certain images
-imgs_to_skip = ["iso", "vmware", "initramfs", "kernel"]
+imgs_to_skip = ["live-iso", "live-kernel", "live-initramfs", "iso", "vmware", "initramfs", "kernel"]
 
 
 def get_cpu_param(param):

coreos_gf_run "${src}" --ro

root=$(coreos_gf findfs-label root)
coreos_gf mount "${root}" /
Copy link
Member

Choose a reason for hiding this comment

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

I know /boot/ won't be used but wondering if we should include it for completeness? Is that was is causing this message:

[core@localhost ~]$ ostree admin status 
error: Unexpected state: /run/ostree-booted found and in / sysroot but not in a booted deployment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't include the kernel and initramfs. We could include everything else, if that's the right way to proceed. Filed as ostreedev/ostree#1921, and let's not block this PR for it.

We already skip the corresponding installer artifacts.
@bgilbert
Copy link
Contributor Author

Good catch on cmd-compress. Updated.

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.

LGTM

@bgilbert bgilbert merged commit 1c84755 into coreos:master Sep 16, 2019
@bgilbert bgilbert deleted the live branch September 16, 2019 19:58
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.

4 participants