Skip to content

Commit 9a8bf1b

Browse files
committed
install: Detect bootloader from target image instead of host
Fixes a regression where bootupd detection was happening before the container was deployed, causing bootc to incorrectly check the host system instead of the target container image. This led to false negatives when the container had bootupd but the host didn't. The fix moves bootloader detection into a new PostFetchState that's created after the deployment is available, ensuring we check the actual target filesystem. Fixes: #1778 Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent 9142b88 commit 9a8bf1b

File tree

4 files changed

+91
-39
lines changed

4 files changed

+91
-39
lines changed

crates/lib/src/bootc_composefs/boot.rs

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ use rustix::{mount::MountFlags, path::Arg};
3333
use schemars::JsonSchema;
3434
use serde::{Deserialize, Serialize};
3535

36-
use crate::bootc_composefs::status::get_sorted_grub_uki_boot_entries;
3736
use crate::composefs_consts::{TYPE1_ENT_PATH, TYPE1_ENT_PATH_STAGED};
3837
use crate::parsers::bls_config::{BLSConfig, BLSConfigType};
3938
use crate::parsers::grub_menuconfig::MenuEntry;
@@ -46,6 +45,7 @@ use crate::{
4645
bootc_composefs::state::{get_booted_bls, write_composefs_state},
4746
bootloader::esp_in,
4847
};
48+
use crate::{bootc_composefs::status::get_sorted_grub_uki_boot_entries, install::PostFetchState};
4949
use crate::{
5050
composefs_consts::{
5151
BOOT_LOADER_ENTRIES, COMPOSEFS_CMDLINE, ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_DIGEST,
@@ -77,7 +77,14 @@ pub(crate) const SYSTEMD_UKI_DIR: &str = "EFI/Linux/bootc";
7777

7878
pub(crate) enum BootSetupType<'a> {
7979
/// For initial setup, i.e. install to-disk
80-
Setup((&'a RootSetup, &'a State, &'a ComposefsFilesystem)),
80+
Setup(
81+
(
82+
&'a RootSetup,
83+
&'a State,
84+
&'a PostFetchState,
85+
&'a ComposefsFilesystem,
86+
),
87+
),
8188
/// For `bootc upgrade`
8289
Upgrade((&'a Storage, &'a ComposefsFilesystem, &'a Host)),
8390
}
@@ -378,7 +385,7 @@ pub(crate) fn setup_composefs_bls_boot(
378385
let id_hex = id.to_hex();
379386

380387
let (root_path, esp_device, cmdline_refs, fs, bootloader) = match setup_type {
381-
BootSetupType::Setup((root_setup, state, fs)) => {
388+
BootSetupType::Setup((root_setup, state, postfetch, fs)) => {
382389
// root_setup.kargs has [root=UUID=<UUID>, "rw"]
383390
let mut cmdline_options = Cmdline::new();
384391

@@ -400,7 +407,7 @@ pub(crate) fn setup_composefs_bls_boot(
400407
esp_part.node.clone(),
401408
cmdline_options,
402409
fs,
403-
state.detected_bootloader.clone(),
410+
postfetch.detected_bootloader.clone(),
404411
)
405412
}
406413

@@ -854,15 +861,15 @@ pub(crate) fn setup_composefs_uki_boot(
854861
entries: Vec<ComposefsBootEntry<Sha512HashValue>>,
855862
) -> Result<()> {
856863
let (root_path, esp_device, bootloader, is_insecure_from_opts, uki_addons) = match setup_type {
857-
BootSetupType::Setup((root_setup, state, ..)) => {
864+
BootSetupType::Setup((root_setup, state, postfetch, ..)) => {
858865
state.require_no_kargs_for_uki()?;
859866

860867
let esp_part = esp_in(&root_setup.device_info)?;
861868

862869
(
863870
root_setup.physical_root_path.clone(),
864871
esp_part.node.clone(),
865-
state.detected_bootloader.clone(),
872+
postfetch.detected_bootloader.clone(),
866873
state.composefs_options.insecure,
867874
state.composefs_options.uki_addon.as_ref(),
868875
)
@@ -964,6 +971,18 @@ pub(crate) fn setup_composefs_boot(
964971
state: &State,
965972
image_id: &str,
966973
) -> Result<()> {
974+
let repo = open_composefs_repo(&root_setup.physical_root)?;
975+
let mut fs = create_composefs_filesystem(&repo, image_id, None)?;
976+
let entries = fs.transform_for_boot(&repo)?;
977+
let id = fs.commit_image(&repo, None)?;
978+
let mounted_fs = Dir::reopen_dir(
979+
&repo
980+
.mount(&id.to_hex())
981+
.context("Failed to mount composefs image")?,
982+
)?;
983+
984+
let postfetch = PostFetchState::new(state, &mounted_fs)?;
985+
967986
let boot_uuid = root_setup
968987
.get_boot_uuid()?
969988
.or(root_setup.rootfs_uuid.as_deref())
@@ -972,7 +991,7 @@ pub(crate) fn setup_composefs_boot(
972991
if cfg!(target_arch = "s390x") {
973992
// TODO: Integrate s390x support into install_via_bootupd
974993
crate::bootloader::install_via_zipl(&root_setup.device_info, boot_uuid)?;
975-
} else if state.detected_bootloader == Bootloader::Grub {
994+
} else if postfetch.detected_bootloader == Bootloader::Grub {
976995
crate::bootloader::install_via_bootupd(
977996
&root_setup.device_info,
978997
&root_setup.physical_root_path,
@@ -988,13 +1007,6 @@ pub(crate) fn setup_composefs_boot(
9881007
)?;
9891008
}
9901009

991-
let repo = open_composefs_repo(&root_setup.physical_root)?;
992-
993-
let mut fs = create_composefs_filesystem(&repo, image_id, None)?;
994-
995-
let entries = fs.transform_for_boot(&repo)?;
996-
let id = fs.commit_image(&repo, None)?;
997-
9981010
let Some(entry) = entries.iter().next() else {
9991011
anyhow::bail!("No boot entries!");
10001012
};
@@ -1005,7 +1017,7 @@ pub(crate) fn setup_composefs_boot(
10051017
match boot_type {
10061018
BootType::Bls => {
10071019
let digest = setup_composefs_bls_boot(
1008-
BootSetupType::Setup((&root_setup, &state, &fs)),
1020+
BootSetupType::Setup((&root_setup, &state, &postfetch, &fs)),
10091021
repo,
10101022
&id,
10111023
entry,
@@ -1014,7 +1026,7 @@ pub(crate) fn setup_composefs_boot(
10141026
boot_digest = Some(digest);
10151027
}
10161028
BootType::Uki => setup_composefs_uki_boot(
1017-
BootSetupType::Setup((&root_setup, &state, &fs)),
1029+
BootSetupType::Setup((&root_setup, &state, &postfetch, &fs)),
10181030
repo,
10191031
&id,
10201032
entries,

crates/lib/src/bootloader.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::process::Command;
33
use anyhow::{anyhow, bail, Context, Result};
44
use bootc_utils::CommandRunExt;
55
use camino::Utf8Path;
6+
use cap_std_ext::cap_std::fs::Dir;
67
use fn_error_context::context;
78

89
use bootc_blockdev::{Partition, PartitionTable};
@@ -28,15 +29,12 @@ pub(crate) fn esp_in(device: &PartitionTable) -> Result<&Partition> {
2829
/// Determine if the invoking environment contains bootupd, and if there are bootupd-based
2930
/// updates in the target root.
3031
#[context("Querying for bootupd")]
31-
#[allow(dead_code)]
32-
pub(crate) fn supports_bootupd(deployment_path: Option<&str>) -> Result<bool> {
32+
pub(crate) fn supports_bootupd(root: &Dir) -> Result<bool> {
3333
if !utils::have_executable("bootupctl")? {
3434
tracing::trace!("No bootupctl binary found");
3535
return Ok(false);
3636
};
37-
let deployment_path = Utf8Path::new(deployment_path.unwrap_or("/"));
38-
let updates = deployment_path.join(BOOTUPD_UPDATES);
39-
let r = updates.try_exists()?;
37+
let r = root.try_exists(BOOTUPD_UPDATES)?;
4038
tracing::trace!("bootupd updates: {r}");
4139
Ok(r)
4240
}

crates/lib/src/install.rs

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,11 @@ pub(crate) struct State {
476476

477477
// If Some, then --composefs_native is passed
478478
pub(crate) composefs_options: InstallComposefsOpts,
479+
}
479480

481+
// Shared read-only global state
482+
#[derive(Debug)]
483+
pub(crate) struct PostFetchState {
480484
/// Detected bootloader type for the target system
481485
pub(crate) detected_bootloader: crate::spec::Bootloader,
482486
}
@@ -1453,21 +1457,6 @@ async fn prepare_install(
14531457
.map(|p| std::fs::read_to_string(p).with_context(|| format!("Reading {p}")))
14541458
.transpose()?;
14551459

1456-
// Determine bootloader type for the target system
1457-
// Priority: user-specified > bootupd availability > systemd-boot fallback
1458-
let detected_bootloader = {
1459-
if let Some(bootloader) = composefs_options.bootloader.clone() {
1460-
bootloader
1461-
} else {
1462-
if crate::bootloader::supports_bootupd(None)? {
1463-
crate::spec::Bootloader::Grub
1464-
} else {
1465-
crate::spec::Bootloader::Systemd
1466-
}
1467-
}
1468-
};
1469-
println!("Bootloader: {detected_bootloader}");
1470-
14711460
// Create our global (read-only) state which gets wrapped in an Arc
14721461
// so we can pass it to worker threads too. Right now this just
14731462
// combines our command line options along with some bind mounts from the host.
@@ -1483,13 +1472,35 @@ async fn prepare_install(
14831472
tempdir,
14841473
host_is_container,
14851474
composefs_required,
1486-
detected_bootloader,
14871475
composefs_options,
14881476
});
14891477

14901478
Ok(state)
14911479
}
14921480

1481+
impl PostFetchState {
1482+
pub(crate) fn new(state: &State, d: &Dir) -> Result<Self> {
1483+
// Determine bootloader type for the target system
1484+
// Priority: user-specified > bootupd availability > systemd-boot fallback
1485+
let detected_bootloader = {
1486+
if let Some(bootloader) = state.composefs_options.bootloader.clone() {
1487+
bootloader
1488+
} else {
1489+
if crate::bootloader::supports_bootupd(d)? {
1490+
crate::spec::Bootloader::Grub
1491+
} else {
1492+
crate::spec::Bootloader::Systemd
1493+
}
1494+
}
1495+
};
1496+
println!("Bootloader: {detected_bootloader}");
1497+
let r = Self {
1498+
detected_bootloader,
1499+
};
1500+
Ok(r)
1501+
}
1502+
}
1503+
14931504
/// Given a baseline root filesystem with an ostree sysroot initialized:
14941505
/// - install the container to that root
14951506
/// - install the bootloader
@@ -1513,11 +1524,17 @@ async fn install_with_sysroot(
15131524

15141525
let deployment_path = ostree.deployment_dirpath(&deployment);
15151526

1527+
let deployment_dir = rootfs
1528+
.physical_root
1529+
.open_dir(&deployment_path)
1530+
.context("Opening deployment dir")?;
1531+
let postfetch = PostFetchState::new(state, &deployment_dir)?;
1532+
15161533
if cfg!(target_arch = "s390x") {
15171534
// TODO: Integrate s390x support into install_via_bootupd
15181535
crate::bootloader::install_via_zipl(&rootfs.device_info, boot_uuid)?;
15191536
} else {
1520-
match state.detected_bootloader {
1537+
match postfetch.detected_bootloader {
15211538
Bootloader::Grub => {
15221539
crate::bootloader::install_via_bootupd(
15231540
&rootfs.device_info,
@@ -1660,6 +1677,7 @@ async fn install_to_filesystem_impl(
16601677

16611678
let (id, verity) = initialize_composefs_repository(state, rootfs).await?;
16621679
tracing::info!("id: {}, verity: {}", hex::encode(id), verity.to_hex());
1680+
16631681
setup_composefs_boot(rootfs, state, &hex::encode(id))?;
16641682
} else {
16651683
ostree_install(state, rootfs, cleanup).await?;
Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,38 @@
11
use std assert
22
use tap.nu
33

4+
# In this test we install a generic image mainly because it keeps
5+
# this test in theory independent of starting from a bootc host,
6+
# but also because it's useful to test "skew" between the bootc binary
7+
# doing the install and the target image.
8+
let target_image = "docker://quay.io/centos-bootc/centos-bootc:stream10"
9+
410
# setup filesystem
511
mkdir /var/mnt
6-
truncate -s 100M disk.img
12+
truncate -s 10G disk.img
713
mkfs.ext4 disk.img
814
mount -o loop disk.img /var/mnt
915

1016
# attempt to install to filesystem without specifying a source-imgref
1117
let result = bootc install to-filesystem /var/mnt e>| find "--source-imgref must be defined"
1218
assert not equal $result null
19+
umount /var/mnt
20+
21+
# Mask off the bootupd state to reproduce https://github.com/bootc-dev/bootc/issues/1778
22+
# Also it turns out that installation outside of containers dies due to `error: Multiple commit objects found`
23+
# so we mask off /sysroot/ostree
24+
# And using systemd-run here breaks our install_t so we disable SELinux enforcement
25+
setenforce 0
26+
systemd-run -p MountFlags=slave -qdPG -- /bin/sh -c $"
27+
set -xeuo pipefail
28+
if test -d /sysroot/ostree; then mount --bind /usr/share/empty /sysroot/ostree; fi
29+
mkdir -p /tmp/ovl/{upper,work}
30+
mount -t overlay -olowerdir=/usr,workdir=/tmp/ovl/work,upperdir=/tmp/ovl/upper overlay /usr
31+
# Note we do keep the other bootupd state
32+
rm -vrf /usr/lib/bootupd/updates
33+
# Another bootc install bug, we should not look at this in outside-of-container flows
34+
rm -vrf /usr/lib/bootc/bound-images.d
35+
bootc install to-disk --disable-selinux --via-loopback --filesystem xfs --source-imgref ($target_image) ./disk.img
36+
"
1337

1438
tap ok

0 commit comments

Comments
 (0)