generic/bootc: allow disk.yaml to provide root filesystem type

[joelcapitao]

When a container includes a disk.yaml with a partition table, its
root filesystem type always takes priority over both
--bootc-default-fs CLI argument and bootc install print-configuration.

The GetDiskYamlRootFs method is added to osinfo.Info to allow reuse
of the partition table root filesystem detection logic from any
package.

Closes: #2380

Assisted-by: OpenCode (Claude Opus 4)

[supakeen]

Thanks, this is how we discussed it 🙂, bootc install print-configuration 'wins' otherwise we can still use disk.yaml's information.

[jbtrystram]

Thanks, this is how we discussed it 🙂, bootc install print-configuration 'wins' otherwise we can still use disk.yaml's information.

TBH I find it a bit weird UX-wise. Why would the bootc config override the image-builder specific config when image-builder is used as the tool to create the disk image ?

I thought it would be the other way around : diskl.yaml always take precedence over Bootc config for the image-builder path. And that make sense, because all other partitions are defined in that disk.yaml. so why split the source of truth?

So you could override the roots through a bootc drop-in, but only the rootfs? Also, did someone even test that it work ? :)
IMHO sourcing disk.yaml and writing a log saying partition table found in the container, using it . Otherwise we default to bootc install to-disk

[supakeen]

Thanks, this is how we discussed it 🙂, bootc install print-configuration 'wins' otherwise we can still use disk.yaml's information.

TBH I find it a bit weird UX-wise. Why would the bootc config override the image-builder specific config when image-builder is used as the tool to create the disk image ?

I have the idea (but I could be convinced otherwise) that if a bootc source of information is available we should prefer to use it; it's what other tooling uses as well while the disk.yaml (and iso.yaml) is only used by image-builder.

I thought it would be the other way around : diskl.yaml always take precedence over Bootc config for the image-builder path. And that make sense, because all other partitions are defined in that disk.yaml. so why split the source of truth?

See above, though I agree it's not ideal especially when deriving containers that already contain a bootc installation configuration but no disk.yaml.

Let's tag some others who might have ideas about this: @achilleas-k / @ondrejbudai / @cgwalters / @alexlarsson. How do you all feel about the ordering of configuration here?

So you could override the roots through a bootc drop-in, but only the rootfs? Also, did someone even test that it work ? :) IMHO sourcing disk.yaml and writing a log saying partition table found in the container, using it . Otherwise we default to bootc install to-disk

I'm planning to document the behavior, we can definitely add a log output or message in image-builder that we're using the embedded partition table instead of the default one but it's probably something better done on that side instead of the images side. I recently added image-builder bootc inspect to print the information we distill from a container. Perhaps we can add a message there; or add a lint subcommand that would warn about potentially confusing configuration sources?

[joelcapitao]

IMHO sourcing disk.yaml and writing a log saying partition table found in the container, using it . Otherwise we default to bootc install to-disk

I was planning to handle this in the image-builder repo with the below table as a target from the discussion we had:

Scenario	disk.yaml root fs	bootc config	--bootc-default-fs	Result
1	ext4	empty	not provided	✅ ext4 (from disk.yaml)
2	ext4	xfs	not provided	❌ Error: conflicting root filesystem type
3	ext4	empty	xfs	❌ Error: cannot use --bootc-default-fs
4	empty	xfs	not provided	✅ xfs (from bootc config)
5	empty	empty	xfs	✅ xfs (from CLI flag)
6	empty	empty	not provided	❌ Error: missing DefaultRootFs

IMHO it makes more sense to handle all the exceptions in the image-builder repo which is the one exposed to the user. But I might be wrong from design/architecture perspective.
I agree this PR alone is confusing as it does not fulfill the scenario 2

[jbtrystram]

Scenario disk.yaml root fs bootc config --bootc-default-fs Result
1 ext4 empty not provided ✅ ext4 (from disk.yaml)
2 ext4 xfs not provided ❌ Error: conflicting root filesystem type
3 ext4 empty xfs ❌ Error: cannot use --bootc-default-fs
4 empty xfs not provided ✅ xfs (from bootc config)
5 empty empty xfs ✅ xfs (from CLI flag)
6 empty empty not provided ❌ Error: missing DefaultRootFs

Here in scenarios 4 to 6, why not simply rely on bootc install to-disk to use bootc defaults ?

[alexlarsson]

From the automotive side we don't really have any specific requirements here other than we need to be able to make it respect the disk.yaml as the only source of truth somehow. We don't expect the user to run image-builder manually, or have custom bootc configs.

[cgwalters]

I thought it would be the other way around : diskl.yaml always take precedence over Bootc config for the image-builder path. And that make sense, because all other partitions are defined in that disk.yaml. so why split the source of truth?

I think it could be just a warning, but not fatal if disk.yaml has conflicts with the root filesystem type of the bootc config.

What would also help here is to have more unified tool to apply configs from inside the container, so one could see these things at container build time and not disk image build time.

Perhaps we could extend bootc container lint with plugins, though it'd need some nontrivial design.

[joelcapitao]

Perhaps we could extend bootc container lint with plugins, though it'd need some nontrivial design.

That's interesting. We could add this linter plugin in bootc codebase (not externally), and enable it with a envvar e.g: BOOTC_LINT_IMAGEBUILDER during the container image build if one plans to build the disk images from it with osbuild/imabe-builder-cli. Also, we would need to add it in the osbuild stage, as the CLI tool can override data e.g: --bootc-default-fs of i-b-c.

[supakeen]

Perhaps we could extend bootc container lint with plugins, though it'd need some nontrivial design.

That's interesting. We could add this linter plugin in bootc codebase (not externally), and enable it with a envvar e.g: BOOTC_LINT_IMAGEBUILDER during the container image build if one plans to build the disk images from it with osbuild/imabe-builder-cli. Also, we would need to add it in the osbuild stage, as the CLI tool can override data e.g: --bootc-default-fs of i-b-c.

Going off topic here but there has been thought to have image-builder bootc prepare --for=$imagetype (probably not what that command will look like) to prepare (at container build time) a given container so people don't have to copy/paste dracut regeneration and that sort of stuff. If that's the case having image-builder bootc lint (to at build time find any problems) would also work and we'd only need to determine when to run it from bootc lint.

[achilleas-k]

I think it could be just a warning, but not fatal if disk.yaml has conflicts with the root filesystem type of the bootc config.

I'm also generally in favour of warnings for this kind of thing instead of erroring out. I can imagine it's quite annoying to be experimenting with different configurations, or deriving containers and adding to them, and needing to modify or delete files in the container to get things to work.

In fact, it might not even be a warning level message but an informational one. Something like:

Found bootc root filesystem config (xfs)
Found /path/to/disk.yaml (<short description of disk layout?>)
Using disk.yaml

and

Found bootc root filesystem config (xfs)
Found /path/to/disk.yaml (<short description of disk layout?>)
--bootc-default-fs provided (ext4)
Using disk.yaml with ext4

[achilleas-k]

This is good to merge as is, I think. We can document the override logic separately.

[supakeen]

@jbtrystram / @joelcapitao has either of you managed to bring this up at a bootc community meeting and if not do we want to hold off on this until we've done so?

I think the last one was cancelled with Flock/devconf going on.

[supakeen]

At the bootc community meeting we talked this through and I changed my opinion a bit. I think if disk.yaml is provided it should always win, as its specific to the tool.

This is the same approach as when any other tooling is used (systemd-repart, kickstarts, etc).

@joelcapitao could you change this PR to work in that way or shall I since I initially steered you in the wrong direction? 🙂

[joelcapitao]

@supakeen I'm back from some time off. I'll soon amend the commit accordingly to the new agreement.

[joelcapitao]

My last commit i.e: d385cfc was tested it with joelcapitao/bib-fcos-experimentation#45
https://konflux-ui.apps.kflux-fedora-01.84db.p1.openshiftapps.com/ns/coreos-tenant/applications/image-builder-poc/pipelineruns/rawhide-x86-metal-on-pull-request-fzgh6

+ sudo podman run --rm --privileged --pull=never -v /var/lib/containers/storage:/var/lib/containers/storage -v /home/builder/konflux-workspace/metal/workspaces/source:/repo:ro -v /home/builder/konflux-workspace/metal/output:/output quay.io/jcapitao/image-builder:latest build raw --bootc-build-ref quay.io/bootc-devel/fedora-bootc-rawhide-standard --bootc-ref quay.io/konflux-fedora/coreos-tenant/fedora-coreos-rawhide:9a07698dfa8d9e40f0490a91951a9474ead66398 --blueprint /repo/blueprints/generated/metal-x86_64.toml --output-dir /output --output-name fedora-coreos-rawhide --with-buildlog
Manifest generation step
Building manifest for bootc-based-raw
Using disk.yaml root filesystem (xfs), ignoring bootc config (xfs)
Image building step
starting -Pipeline source org.osbuild.containers-storage: f40da0d32ab7d58aeaaba32c6d3cff931f8599aa65444e37ff36acced3740ea1

[supakeen]

Thanks @joelcapitao, I've self-requested review and re-requested @achilleas-k as well 🙂

[supakeen]

Thanks.