Repository navigation
install: Default to the composefs backend for composefs-only images - #23
cgwalters-bot wants to merge 5 commits into
Conversation
d8a3d26 to
1d68266
Compare
cgwalters-bot
left a comment
There was a problem hiding this comment.
Review guide for head 1d68266739: where to look closely and what is safe to skim. It is advice from the bot's reviewer and doesn't replace reading the diff; the review app walks it.
Adds a composefs-backend key to the [install] config so a BLS (non-UKI) image can select the composefs backend itself, as bootc-image-builder and Anaconda never pass --composefs-backend. The core is small: one resolver (CLI flag wins, then config, UKI forces true and rejects an explicit false) and moving the --allow-missing-verity/--uki-addon checks from clap to after resolution. The risk is in reach rather than code: prepare_install is shared by to-disk, to-filesystem and to-existing-root, so an image carrying the key now changes the backend on all three, and the new key is part of print-configuration output that external tools parse. Tests cover the resolver table and a to-disk install inspected on disk, but nothing boots the result or exercises the moved flag checks.
Hotspots
- risky · api —
crates/lib/src/install.rs:1749-1753(38b648a): prepare_install is shared by to-disk, to-filesystem and to-existing-root, so an image with composefs-backend = true now selects composefs for to-existing-root on an ostree host with no flag at all. Per #26 that path empties /boot and the ESP before failing; land #26 first or have this refuse there. - look-closely · logic —
crates/lib/src/install.rs:1615-1630(38b648a): Precedence is one-way: the CLI can only force composefs on, so once an image ships composefs-backend = true there is no flag to install it with ostree (only a later drop-in with false). Check that is acceptable for bib/Anaconda users; the PR body defers an enum. - note · error-handling —
crates/lib/src/install.rs:1754-1763(38b648a): With the clap requires= gone (lines 412, 419), misuse is caught only here. All three install subcommands reach prepare_install, so nothing is silently ignored, but the error now comes after the root, pidns/userns and container checks instead of at argument parsing. - note · api —
crates/lib/src/install/config.rs:135-137(38b648a): InstallConfiguration has deny_unknown_fields, so an image whose bootc predates this key fails the whole install on parsing the drop-in rather than ignoring it; and with no skip_serializing_if, print-configuration now emits "composefs-backend": null for every image. Both are probably fine, but they are external contract changes. - look-closely · test-gap —
tmt/tests/booted/test-52-install-composefs-config.nu:40-57(1d68266): The test only inspects the installed disk's layout (composefs/, one state/deploy entry, no ostree/deploy); it never boots it, so an unbootable composefs install from a BLS image would still pass. If to-disk or the mount fails, the 15G disk and the derived image are left behind, since cleanup only runs after the try block. - note · test-gap —
crates/lib/src/install.rs:3098-3123(38b648a): The unit table covers the resolver fully, but nothing tests the moved checks: --allow-missing-verity or --uki-addon on an ostree install should still error, and should be accepted when only the config key selects composefs, which is the behavior the commit message promises.
Safe to skim
docs/src/man/bootc-install-to-disk.8.md: generated option docs from the clap doc commentsdocs/src/man/bootc-install-to-existing-root.8.md: generated option docs from the clap doc commentsdocs/src/man/bootc-install-to-filesystem.8.md: generated option docs from the clap doc commentstmt/plans/integration.fmf: generated plan entry from the test's headertmt/tests/tests.fmf: generated test entry from the test's headercrates/lib/src/install/config.rs:997-1044: table-driven parse, merge and serialization test for the new keycrates/lib/src/install.rs:402-405: doc comment onlydocs/src/experimental-composefs.md: prose documenting the key; matches the codedocs/src/man/bootc-install-config.5.md: man page entry and example for the key; matches the code
cgwalters
left a comment
There was a problem hiding this comment.
Hold up let's align with the other change we did to the linter, but even more strongly:
If the image has a compsoefs setup-root.toml and does not have an ostree one, then it's composefs backend by default.
1d68266 to
250a1a3
Compare
The baseimage-root lint insists on an /ostree -> sysroot/ostree symlink, and baseimage-composefs warns unless ostree's prepare-root.conf enables composefs. Both are meaningless for images that are only ever deployed with the composefs backend, and just force them to carry ostree cruft. Use the presence of /usr/lib/composefs/setup-root-conf.toml (even if empty) as the signal that an image is composefs-native, mirroring how prepare-root.conf signals ostree. If /ostree is present anyway it is still validated, and /sysroot is still required since both backends mount the physical root there. Closes: bootc-dev#2256 Generated-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
"printconfig --all" writes /run/bootc/install/10-test.toml and deletes it again, while "install config" runs `bootc install print-configuration` in parallel. When the drop-in vanishes between load_config()'s directory scan and its read, the latter fails with "Loading configuration: No such file or directory". This flakes the container integration tests on random PRs and variants. Merge the two into one test that checks both outputs with the drop-in in place. Generated-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
Prep for bootc install using the same signal to pick its default backend. Generated-by: AI
250a1a3 to
14c84f1
Compare
|
Reworked: Tested on a devspace with rhel-bootc-examples' composefs example: installed without the flag, it booted composefs; the unchanged image booted ostree. Note that image-builder#4, which reads the removed key, would need to check these files instead. Generated-by: https://github.com/cgwalters/#llms |
cgwalters-bot
left a comment
There was a problem hiding this comment.
Review guide for head 14c84f16e8: where to look closely and what is safe to skim. It is advice from the bot's reviewer and doesn't replace reading the diff; the review app walks it.
Stacked on bootc#2500 (first two commits, identical to its head; skip them). Commit 1 moves is_composefs_native into bootc_composefs/image.rs, where it is the one function the lints and install share. Commit 2 makes install default to composefs when the image has setup-root-conf.toml and no prepare-root.conf in /usr/lib/ostree or /etc/ostree. It also makes prepare-root.conf optional on the composefs path; the ostree path is unchanged. Commit 3 adds tmt test 52 (50 and 51 are taken by the composefs-stabilization branch). Main risk: detection only runs when install uses the running container as its source (target_rootfs is Some). osbuild's install-to-filesystem stage passes --source-imgref, so bootc-image-builder and image-builder get no auto-detection, and neither does Anaconda. The docs, the commit message, the PR body and the test comment all say they do. The PR body's image-builder consequence paragraph is wrong for the same reason: image-builder has to apply the rule itself, as it already does for UnifiedKernel.
Hotspots
- risky · logic —
crates/lib/src/install.rs:1715-1736(34fd5b3): Detection only runs when target_rootfs is Some, i.e. no --source-imgref. osbuild (bootc-image-builder, image-builder) and Anaconda pass --source-imgref, so they never get the new default, contrary to the commit message and docs. - look-closely · api —
docs/src/bootc-experimental-composefs.7.md:279-283(34fd5b3): Says the default also applies when install is run by bootc-image-builder. It does not: osbuild passes --source-imgref. Either drop the claim or say that such tools must check the two files themselves. - note · logic —
crates/lib/src/bootc_composefs/image.rs:23-30(34fd5b3): The rule is implemented faithfully, with /etc/ostree checked as well as /usr/lib/ostree. load_config_from_root parses the keyfile only to test that it exists, so a malformed prepare-root.conf now fails detection. That failure also happened before, so nothing regresses. - note · api —
crates/lib/src/install.rs:1737-1746(34fd5b3): The clap requires= becomes a runtime check. bcvk still has requires=composefs_backend on its own --allow-missing-verity, so bcvk users of a composefs-only image must still pass --composefs-backend to use it. That is a follow-up for bcvk, not a blocker here. - look-closely · logic —
crates/lib/src/install.rs:1856-1873(34fd5b3): prepare-root.conf becomes optional only when composefs_backend is set. The else branch is the old require_config_from_root, and prepareroot_config is read only in the ostree install path, so ostree behaviour is unchanged. - note · test-gap —
tmt/tests/booted/test-52-install-composefs-native.nu:10-14(14c84f1): The comment says this works as bootc-image-builder does, but BIB uses --source-imgref, which this test does not exercise. The both-files case (ostree default) is covered only by the unit test.
Safe to skim
crates/lib/src/lints.rs: bootc-dev#2500 commit plus a mechanical move of the helpercrates/tests-integration/src/container.rs: bootc-dev#2500 commit, identical to its upstream headdocs/src/bootc-compatible-images.7.md: bootc-dev#2500 commitdocs/src/man/bootc-install-to-disk.8.md: generated from the clap doc commentsdocs/src/man/bootc-install-to-existing-root.8.md: generated from the clap doc commentsdocs/src/man/bootc-install-to-filesystem.8.md: generated from the clap doc commentstmt/plans/integration.fmf: generated plantmt/tests/tests.fmf: generated test entry
Today only a UKI selects the composefs backend automatically. An image built for composefs with a traditional kernel and initramfs (BLS) is installed with ostree unless every caller passes --composefs-backend, which bootc-image-builder and Anaconda don't. The lints already treat /usr/lib/composefs/setup-root-conf.toml as the marker of a composefs-native image. Use the same marker here: when an image ships it and has no ostree prepare-root.conf, it can't be installed with ostree anyway, so default to composefs. An image with both is still installed with ostree by default, since it may be meant for either backend: bootc's own composefs CI images, for example, add setup-root-conf.toml to a stock base image and pass the flag. Both files are read from the root bootc runs in, as the install configuration and prepare-root.conf already were, including with --source-imgref: bootc-image-builder runs bootc from the image it installs, so it gets the same default. Such an image also failed with --composefs-backend, since install required prepare-root.conf regardless of the backend; only the ostree backend reads it, so it's optional for composefs now. --allow-missing-verity and --uki-addon used to require --composefs-backend at the clap level, which would reject them for an image selecting the backend by itself, so check them after the backend is decided. Generated-by: AI
Build a derived image that adds an empty setup-root-conf.toml and drops ostree's prepare-root.conf, and check that `bootc install to-disk` run from it installs the composefs backend without the flag, both as a self-install and with --source-imgref as bootc-image-builder runs it. An image that keeps prepare-root.conf must still get ostree. On the ostree variant, nothing else selects composefs. Generated-by: AI
14c84f1 to
ff09bd0
Compare
|
Folded into bootc#2500 per review Generated-by: https://github.com/cgwalters/#llms |
Today only a UKI selects the composefs backend at install time. An image built for composefs with a regular kernel and initramfs (BLS) is installed with ostree unless every caller passes
--composefs-backend, which bootc-image-builder and Anaconda don't.This uses the marker the lints already use (bootc-dev#2500): if the image ships
/usr/lib/composefs/setup-root-conf.tomland has no ostreeprepare-root.conf(in/usr/lib/ostreeor/etc/ostree),bootc installdefaults to the composefs backend. Such an image can't be installed with ostree anyway. An image that has both still defaults to ostree, since it may be meant for either backend. bootc's own composefs CI images are in that case: they addsetup-root-conf.tomlto the stock base image and pass the flag, so CI doesn't change. The detection lives in one function next to the composefs code, which the lints and install both use. Both files are read from the root bootc runs in, as the install configuration andprepare-root.confalready were, including with--source-imgref: bootc-image-builder runs bootc from the image it installs, so it gets the same default.Installing such an image also failed with
--composefs-backenduntil now: install requiredprepare-root.confwhatever the backend, but only the ostree backend reads it.--allow-missing-verityand--uki-addonused to require--composefs-backendin clap; they're now checked after the backend is chosen, so they also work when the image selects it.bootloader=noneon a composefs-only image gets an error naming why composefs was selected.An earlier version of this PR added a
composefs-backendinstall-config key instead. Auto-detection replaces it, so the key is gone: the image's files are the signal, and a separate key could contradict them.Consequence for bootc-image-builder/image-builder: osbuild's
org.osbuild.bootc.install-to-filesystemstage runsbootc installinside the image, so a composefs-only image now gets composefs there. But osbuild/images only expects composefs for UKIs (UnifiedKernelinpkg/manifest/raw_bootc.go), and on the non-UKI path it then mountsostree.deploymentfor customizations, which fails. It would need to apply the same rule (the file checks above) to the image.Depends on bootc-dev#2500, whose two commits are included here until it merges.
Testing, on a 16-core RHEL 10 devspace:
cargo test -p bootc-lib: 278 passed, including the newtest_defaults_to_composefs_backend(composefs-only, ostree-only in/usr/liband/etc, both, neither) and the existing lint testsjust validate: passedjust test-tmt install-composefs-native(ostree variant, where nothing else selects composefs): passed. It installs, withbootc install to-diskand no flag, a derived image with an emptysetup-root-conf.tomland noprepare-root.confboth as a self-install and with--source-imgref(composefs expected), and one that keepsprepare-root.conf(ostree expected)composefs/example (from composefs: Build on CentOS Stream 10 with bootc from git, test through image-builder rhel-bootc-examples#4, CentOS Stream 10, bootc RPMs built from this branch), installed and booted withbcvk libvirt runwithout--composefs-backend: withsetup-root-conf.tomladded andprepare-root.confremoved it booted on composefs; the unchanged image (which hasprepare-root.conf) booted on ostreeRelated: bootc-dev#2256
Generated-by: https://github.com/cgwalters/#llms
Review draft in cgwalters-forge, not upstream yet. This section is removed when the PR is opened upstream.
bootc-dev/bootc, basemainPVTI_lADOE9oHIs4BlJLczg9kEdkTo review:
/promoteon a line of its own, to open it upstream, ready for review. Either covers only the commits pushed so far.Signed-off-by: Colin Walters <walters@verbum.org>to the commits lacking it (the bot's and yours; anyone else's only if you ask), with you as committer./draftline (in the same comment or before) to open it upstream as a draft (/readyundoes that).