rpm-ostree icon indicating copy to clipboard operation
rpm-ostree copied to clipboard

kernel-install wrap should include any dracut arguments being set in the manifest.

Open jmarrero opened this issue 2 years ago • 7 comments

Opening this issue based on the discussion on #3364 https://github.com/coreos/rpm-ostree/pull/3689#discussion_r908856156 We want to support any dracut arguments being specified in the manifest/tree file when running kernel-install even when wrapped.

jmarrero avatar Jun 28 '22 19:06 jmarrero

Moving this comment here:

I think we want to use the same logic in the sysroot upgrader, i.e.: check for initramfs args specified,

So this is an interesting issue. Today on the host/client side, we have access to the ostree commit object directly - everything works using that as a base.

But here in the container flow, we would need to load the commit object, and...that doesn't yet have a precedent. It would make total sense to do so - we have the commit object in /ostree/repo/objects/00/abcde.commit etc, but actually as far as I know we don't know which commit we're using - that's in the container image metadata.

We could read /usr/share/rpm-ostree/treefile.json but so far we also haven't been processing that client side. This gets into https://github.com/coreos/rpm-ostree/issues/2326

I would lean actually that we should instead of having initramfs-args we should have from the start been writing config files into /etc/dracut.conf.d - that's the way it also works the same way on traditional yum systems too, etc. So to flesh this out a bit, we'd add /etc/dracut.conf.d/ignition.conf in fedora-coreos-config instead of initramfs-args.

Another way to say this is: dracut already has a declarative way to specify its configuration via config files - and crucially that same mechanism should work the same in rpm-ostree compose tree and the container override flow. So let's use that instead of carrying forward our own custom declarative config format. (We shouldn't drop support for initramfs-args in rpm-ostree today, but we can just stop using it for FCOS etc.)

cgwalters avatar Jul 01 '22 15:07 cgwalters

But here in the container flow, we would need to load the commit object, and...that doesn't yet have a precedent. It would make total sense to do so - we have the commit object in /ostree/repo/objects/00/abcde.commit etc, but actually as far as I know we don't know which commit we're using - that's in the container image metadata.

Hmm, independent of this, I think it might be worth adding a ref like base in that repo to make it easier to access. Should we file an issue for that in https://github.com/ostreedev/ostree-rs-ext ?

Another way to say this is: dracut already has a declarative way to specify its configuration via config files - and crucially that same mechanism should work the same in rpm-ostree compose tree and the container override flow. So let's use that instead of carrying forward our own custom declarative config format. (We shouldn't drop support for initramfs-args in rpm-ostree today, but we can just stop using it for FCOS etc.)

This sounds reasonable to me, and makes a lot of sense long-term. Maybe we should then deprecate initramfs-args? I'd rather drop support for it eventually if it's not fully plumbed through in all the paths. Hmm, it'd be nice if we could auto-convert it to /etc/dracut.conf.d dropins, but that doesn't seem straightforward.

Short-term though, WDYT on just continuing to read from the commit metadata? I don't think we're talking about a lot of code. (Though we'd have to solve the ref issue mentioned above, which I think would be good to do regardless.)

jlebon avatar Jul 07 '22 21:07 jlebon

This [dracut.conf.d] sounds reasonable to me, and makes a lot of sense long-term.

Long term can be now :smile: - PR in https://github.com/coreos/fedora-coreos-config/pull/1828

Maybe we should then deprecate initramfs-args?

Yeah, no urgency to remove it, but indeed we should deprecate.

Hmm, independent of this, I think it might be worth adding a ref like base in that repo to make it easier to access. Should we file an issue for that in https://github.com/ostreedev/ostree-rs-ext ?

I think I'd vote for an API to "fetch me the single commit object in a repo" but that said...

Short-term though, WDYT on just continuing to read from the commit metadata? I don't think we're talking about a lot of code.

Not using initramfs-args anymore is also quite easy I think, so...why not do that?

cgwalters avatar Jul 07 '22 21:07 cgwalters

Short-term though, WDYT on just continuing to read from the commit metadata? I don't think we're talking about a lot of code.

Not using initramfs-args anymore is also quite easy I think, so...why not do that?

Because other potential users of CoreOS layering (or maybe I should just say "layering"? not sure how much we care about such users right now, but the tech is pretty CoreOS-agnostic) outside FCOS could equally hit this. :)

jlebon avatar Jul 07 '22 21:07 jlebon

For sure, though there aren't many of these things. It looks like there is no usage of initramfs-args in workstation:

walters@toolbox /v/s/w/s/p/workstation-ostree-config (main) [1]> git rev-parse HEAD
082755c17ff0568a37ce59cdcffd81036bfe21ab
walters@toolbox /v/s/w/s/p/workstation-ostree-config (main)> rg initramfs-args
walters@toolbox /v/s/w/s/p/workstation-ostree-config (main) [1]> 

There is in fedora-iot/ostree but that's something equally easily fixed.

cgwalters avatar Jul 08 '22 01:07 cgwalters

I agree with moving now to dracut.conf.d and moving away from initramfs-args. If dracut already provides us a way of doing this, I don't see why we should duplicate the effort.

jmarrero avatar Jul 08 '22 19:07 jmarrero

Opened https://github.com/coreos/rpm-ostree/pull/3834.

jlebon avatar Jul 08 '22 21:07 jlebon