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

Import static grub configuration #536

Merged
merged 1 commit into from
Sep 21, 2023
Merged

Conversation

cgwalters
Copy link
Member

Pull this from coreos-assembler; prep for moving ownership of it into bootupd.

In the short term, this will allow install tools to copy it manually.

Comment on lines 61 to 63
if [ -f $prefix/platform.cfg ]; then
source $prefix/platform.cfg
fi
Copy link
Member Author

Choose a reason for hiding this comment

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

@cgwalters
Copy link
Member Author

There's zero risk to this, it's just shipping some files in a subpackage.

The larger set of work is adding support for importing this into being "owned" by bootupd. But that can come after centralizing the static config into this package, where it can be reused by both osbuild and anaconda style flows.

@jlebon
Copy link
Member

jlebon commented Sep 21, 2023

Hmm, if we're going to move the GRUB configs, I think they belong better in e.g. https://github.com/coreos/fedora-coreos-config instead. There's a lot of integration points between the two and other distros that want bootupd likely will have their own GRUB config. (I guess we could set up some inheritance here with source, though not sure it's worth the complexity.)

For the update case, we could easily ship an overlay somewhere under e.g. /usr/lib/grubcfgs/ and bootupd could learn to look there.

@cgwalters
Copy link
Member Author

Hmm, if we're going to move the GRUB configs, I think they belong better in e.g. https://github.com/coreos/fedora-coreos-config instead.

The main problem is that doesn't come as an RPM today, and that causes friction with tooling currently oriented towards that as primary input sources (e.g. rpm-ostree compose image and Image Builder namely). It's not that RPMs are amazing or anything, but from the pungi side (which sagano is aiming to integrate with) adding more git repositories complicates the picture and current "hermetic" nature of things.

@jlebon
Copy link
Member

jlebon commented Sep 21, 2023

We're already going to have to solve the RPM problem for a lot of overlay files we have in https://github.com/coreos/fedora-coreos-config so I'd just glob it as part of that effort. If the goal of bootupd is distro-independence, then IMO it doesn't make sense to have a GRUB config with opinionated things like bootuuid, Ignition firstboot stuff, our RAID1 setup, user dropins, etc...

@cgwalters
Copy link
Member Author

Today, Image Builder has a separate copy of a grub config it uses, see https://github.com/osbuild/osbuild/blob/e48360e01c84ef702b6e5d34a26cedddc55a60c0/stages/org.osbuild.grub2.legacy#L204

And it'd be a giant effort to somehow wedge github.com/coreos/fedora-coreos-config into that. And as far as an RPM package place, given we already have bootupd and the plan is to move ownership there, it really seems simpler to move it here for now.

Other distributions can just ignore it (it's not even installed by default!). Other ostree/container-native distributions don't have the problem with RPM-obsessed build systems 😄

@cgwalters
Copy link
Member Author

OK I dropped out the Ignition glue from this PR, so what we can do is use this even more minimal static version for Sagano work, and rework the version that lives in FCOS to write its additions into platform.cfg instead.

it doesn't make sense to have a GRUB config with opinionated things like bootuuid, Ignition firstboot stuff, our RAID1 setup, user dropins, etc...

bootuuid is used by bootc install today.

Ignition firstboot: dropped!

RAID1: eh...it aligns with also supporting mirrored EFI which we also want in bootupd.

user dropins: Also something desired for anyone using a static grub config I think.

IOW I don't want to block this demo I'm trying to put together of installing a Sagano container w/Anaconda (and bootupd) on refactoring the FCOS structure entirely by packaging fedora-coreos-config. And even if we did that, the converse is true that Sagano grub.cfg shouldn't have Ignition, so we still need two versions.

Hence I think this model of changing FCOS to inherit from this version should work out well.

@cgwalters
Copy link
Member Author

Incidentally also, this project is still currently hardcoded to work with RPM #468 which is a far larger distro-independence problem, and one that will be hard to get away from. There's just going to be some ugly distro stuff in this project probably forever.

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.

Yeah, that looks better. A bit apprehensive about setting up more inheritance chains here for this but I'm cool also evaluating the full impact when we get to this in FCOS land if this is considered demo/WIP material.

Again, what I think we should aim for is to have GRUB logic located closer to the code that leverages it. E.g. the thing that writes the bootuuids is called by f-c-c. A lot, but not all, of password and Ignition integration live in f-c-c. Once we get to the packaging hurdle, these things would go together.

Having the update mechanism live in bootupd sounds great, and then f-c-c can leverage that by placing its file in some predefined directory or something?

contrib/packaging/bootupd.spec Outdated Show resolved Hide resolved
Pull this from coreos-assembler; prep for moving ownership
of it into bootupd.  The big change though is the addition of
sourcing a `platform.cfg` which is where we'll put the stuff
that currently lives in `platforms.yaml` in FCOS as well as
the Ignition glue.

This glue can be used as is by Sagano, where the base images don't
need the Ignition glue or platforms.yaml.
@cgwalters
Copy link
Member Author

E.g. the thing that writes the bootuuids is called by f-c-c.

Only in a "golden image" case. The Anaconda and bootc paths want to ensure that the system is set up properly with uuids from the start.

(And actually, I want to change Anaconda to use bootc install-to-filesystem which would then mean it's only bootc)

@cgwalters
Copy link
Member Author

/override continuous-integration/jenkins/pr-merge

@openshift-ci
Copy link

openshift-ci bot commented Sep 21, 2023

@cgwalters: Overrode contexts on behalf of cgwalters: continuous-integration/jenkins/pr-merge

In response to this:

/override continuous-integration/jenkins/pr-merge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@cgwalters cgwalters merged commit affee1f into coreos:main Sep 21, 2023
6 checks passed
@cgwalters
Copy link
Member Author

Also thanks for the review and debate here! It's a tricky topic we're navigating and I think while the debate felt contentious at first I actually think the result ended up working well! Or at least it hopefully will 😄

@cgwalters
Copy link
Member Author

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.

2 participants