-
Notifications
You must be signed in to change notification settings - Fork 189
install: Enable installing to multi device parents #1911
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
base: main
Are you sure you want to change the base?
Changes from all commits
1c9e4a7
5e921d5
6601a67
d72b344
eb5d5f4
b7d4d61
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -529,8 +529,18 @@ pub(crate) fn setup_composefs_bls_boot( | |
| ComposefsCmdline::build(&id_hex, state.composefs_options.allow_missing_verity); | ||
| cmdline_options.extend(&Cmdline::from(&composefs_cmdline.to_string())); | ||
|
|
||
| // Locate ESP partition device | ||
| let esp_part = root_setup.device_info.find_partition_of_esp()?; | ||
| // Locate ESP partition device by walking up to the root disk(s) | ||
| let esp_part = root_setup | ||
| .device_info | ||
| .find_colocated_esps()? | ||
| .and_then(|mut v| { | ||
| if v.is_empty() { | ||
| None | ||
| } else { | ||
| Some(v.remove(0)) | ||
| } | ||
| }) | ||
| .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; | ||
|
|
||
| ( | ||
| root_setup.physical_root_path.clone(), | ||
|
|
@@ -567,10 +577,18 @@ pub(crate) fn setup_composefs_bls_boot( | |
| .context("Failed to create 'composefs=' parameter")?; | ||
| cmdline.add_or_modify(¶m); | ||
|
|
||
| // Locate ESP partition device | ||
| let root_dev = | ||
| bootc_blockdev::list_dev_by_dir(&storage.physical_root)?.require_single_root()?; | ||
| let esp_dev = root_dev.find_partition_of_esp()?; | ||
| // Locate ESP partition device by walking up to the root disk(s) | ||
| let root_dev = bootc_blockdev::list_dev_by_dir(&storage.physical_root)?; | ||
| let esp_dev = root_dev | ||
| .find_colocated_esps()? | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May as well have a |
||
| .and_then(|mut v| { | ||
| if v.is_empty() { | ||
| None | ||
| } else { | ||
| Some(v.remove(0)) | ||
| } | ||
| }) | ||
| .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; | ||
|
|
||
| ( | ||
| Utf8PathBuf::from("/sysroot"), | ||
|
|
@@ -687,7 +705,14 @@ pub(crate) fn setup_composefs_bls_boot( | |
| options: Some(cmdline_refs), | ||
| }); | ||
|
|
||
| match find_vmlinuz_initrd_duplicates(&boot_digest)? { | ||
| // Only check for shared boot binaries during upgrades. During a | ||
| // fresh install the target has no existing entries, and the host's | ||
| // /sysroot/state/deploy would incorrectly match. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait a minute this looks like a bad unrelated bug - we shouldn't be opening up the absolute path |
||
| match if is_upgrade { | ||
| find_vmlinuz_initrd_duplicates(&boot_digest)? | ||
| } else { | ||
| None | ||
| } { | ||
| Some(shared_entries) => { | ||
| // Multiple deployments could be using the same kernel + initrd, but there | ||
| // would be only one available | ||
|
|
@@ -1103,7 +1128,18 @@ pub(crate) fn setup_composefs_uki_boot( | |
| BootSetupType::Setup((root_setup, state, postfetch, ..)) => { | ||
| state.require_no_kargs_for_uki()?; | ||
|
|
||
| let esp_part = root_setup.device_info.find_partition_of_esp()?; | ||
| // Locate ESP partition device by walking up to the root disk(s) | ||
| let esp_part = root_setup | ||
| .device_info | ||
| .find_colocated_esps()? | ||
| .and_then(|mut v| { | ||
| if v.is_empty() { | ||
| None | ||
| } else { | ||
| Some(v.remove(0)) | ||
| } | ||
| }) | ||
| .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; | ||
|
|
||
| ( | ||
| root_setup.physical_root_path.clone(), | ||
|
|
@@ -1118,10 +1154,18 @@ pub(crate) fn setup_composefs_uki_boot( | |
| let sysroot = Utf8PathBuf::from("/sysroot"); // Still needed for root_path | ||
| let bootloader = host.require_composefs_booted()?.bootloader.clone(); | ||
|
|
||
| // Locate ESP partition device | ||
| let root_dev = | ||
| bootc_blockdev::list_dev_by_dir(&storage.physical_root)?.require_single_root()?; | ||
| let esp_dev = root_dev.find_partition_of_esp()?; | ||
| // Locate ESP partition device by walking up to the root disk(s) | ||
| let root_dev = bootc_blockdev::list_dev_by_dir(&storage.physical_root)?; | ||
| let esp_dev = root_dev | ||
| .find_colocated_esps()? | ||
| .and_then(|mut v| { | ||
| if v.is_empty() { | ||
| None | ||
| } else { | ||
| Some(v.remove(0)) | ||
| } | ||
| }) | ||
| .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; | ||
|
|
||
| ( | ||
| sysroot, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,13 @@ const BOOTUPD_UPDATES: &str = "usr/lib/bootupd/updates"; | |
| // from: https://github.com/systemd/systemd/blob/26b2085d54ebbfca8637362eafcb4a8e3faf832f/man/systemd-boot.xml#L392 | ||
| const SYSTEMD_KEY_DIR: &str = "loader/keys"; | ||
|
|
||
| /// Mount ESP part at /boot/efi | ||
| /// Mount the first ESP found among backing devices at /boot/efi. | ||
| /// | ||
| /// This is used by the install-alongside path to clean stale bootloader | ||
| /// files before reinstallation. On multi-device setups only the first | ||
| /// ESP is mounted and cleaned; stale files on additional ESPs are left | ||
| /// in place (bootupd will overwrite them during installation). | ||
| // TODO: clean all ESPs on multi-device setups | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm yeah maybe should move this logic into bootupd |
||
| pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) -> Result<()> { | ||
| let efi_path = Utf8Path::new("boot").join(crate::bootloader::EFI_DIR); | ||
| let Some(esp_fd) = root | ||
|
|
@@ -45,11 +51,14 @@ pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) | |
| root | ||
| }; | ||
|
|
||
| let dev = bootc_blockdev::list_dev_by_dir(physical_root)?.require_single_root()?; | ||
| if let Some(esp_dev) = dev.find_partition_of_type(bootc_blockdev::ESP) { | ||
| let esp_path = esp_dev.path(); | ||
| bootc_mount::mount(&esp_path, &root_path.join(&efi_path))?; | ||
| tracing::debug!("Mounted {esp_path} at /boot/efi"); | ||
| let roots = bootc_blockdev::list_dev_by_dir(physical_root)?.find_all_roots()?; | ||
| for dev in &roots { | ||
| if let Some(esp_dev) = dev.find_partition_of_type(bootc_blockdev::ESP) { | ||
| let esp_path = esp_dev.path(); | ||
| bootc_mount::mount(&esp_path, &root_path.join(&efi_path))?; | ||
| tracing::debug!("Mounted {esp_path} at /boot/efi"); | ||
| return Ok(()); | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
@@ -67,6 +76,48 @@ pub(crate) fn supports_bootupd(root: &Dir) -> Result<bool> { | |
| Ok(r) | ||
| } | ||
|
|
||
| /// Check whether the target bootupd supports `--filesystem`. | ||
| /// | ||
| /// Runs `bootupctl backend install --help` and looks for `--filesystem` in the | ||
| /// output. When `deployment_path` is set the command runs inside a bwrap | ||
| /// container so we probe the binary from the target image. | ||
| fn bootupd_supports_filesystem(rootfs: &Utf8Path, deployment_path: Option<&str>) -> Result<bool> { | ||
| let help_args = ["bootupctl", "backend", "install", "--help"]; | ||
| let output = if let Some(deploy) = deployment_path { | ||
| let target_root = rootfs.join(deploy); | ||
| BwrapCmd::new(&target_root) | ||
| .setenv( | ||
| "PATH", | ||
| "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's use a shared |
||
| ) | ||
| .run_get_string(help_args)? | ||
| } else { | ||
| Command::new("bootupctl") | ||
| .args(&help_args[1..]) | ||
| .log_debug() | ||
| .run_get_string()? | ||
| }; | ||
|
|
||
| let use_filesystem = output.contains("--filesystem"); | ||
|
|
||
| if use_filesystem { | ||
| tracing::debug!("bootupd supports --filesystem"); | ||
| } else { | ||
| tracing::debug!("bootupd does not support --filesystem, falling back to --device"); | ||
| } | ||
|
|
||
| Ok(use_filesystem) | ||
| } | ||
|
|
||
| /// Install the bootloader via bootupd. | ||
| /// | ||
| /// When the target bootupd supports `--filesystem` we pass it pointing at a | ||
| /// block-backed mount so that bootupd can resolve the backing device(s) itself | ||
| /// via `lsblk`. In the bwrap path we bind-mount the physical root at | ||
| /// `/sysroot` to give `lsblk` a real block-backed path. | ||
| /// | ||
| /// For older bootupd versions that lack `--filesystem` we fall back to the | ||
| /// legacy `--device <device_path> <rootfs>` invocation. | ||
| #[context("Installing bootloader")] | ||
| pub(crate) fn install_via_bootupd( | ||
| device: &bootc_blockdev::Device, | ||
|
|
@@ -91,8 +142,6 @@ pub(crate) fn install_via_bootupd( | |
|
|
||
| println!("Installing bootloader via bootupd"); | ||
|
|
||
| let device_path = device.path(); | ||
|
|
||
| // Build the bootupctl arguments | ||
| let mut bootupd_args: Vec<&str> = vec!["backend", "install"]; | ||
| if configopts.bootupd_skip_boot_uuid { | ||
|
|
@@ -107,26 +156,55 @@ pub(crate) fn install_via_bootupd( | |
| if let Some(ref opts) = bootupd_opts { | ||
| bootupd_args.extend(opts.iter().copied()); | ||
| } | ||
| bootupd_args.extend(["--device", &device_path, rootfs_mount]); | ||
|
|
||
| // When the target bootupd lacks --filesystem support, fall back to the | ||
| // legacy --device flag. For --device we need the whole-disk device path | ||
| // (e.g. /dev/vda), not a partition (e.g. /dev/vda3), so resolve the | ||
| // parent via require_single_root(). (Older bootupd doesn't support | ||
| // multiple backing devices anyway.) | ||
| // Computed before building bootupd_args so the String lives long enough. | ||
| let root_device_path = if bootupd_supports_filesystem(rootfs, deployment_path) | ||
| .context("Probing bootupd --filesystem support")? | ||
| { | ||
| None | ||
| } else { | ||
| Some(device.require_single_root()?.path()) | ||
| }; | ||
| if let Some(ref dev) = root_device_path { | ||
| tracing::debug!("bootupd does not support --filesystem, falling back to --device {dev}"); | ||
| bootupd_args.extend(["--device", dev]); | ||
| bootupd_args.push(rootfs_mount); | ||
| } else { | ||
| tracing::debug!("bootupd supports --filesystem"); | ||
| bootupd_args.extend(["--filesystem", rootfs_mount]); | ||
| bootupd_args.push(rootfs_mount); | ||
| } | ||
|
|
||
| // Run inside a bwrap container. It takes care of mounting and creating | ||
| // the necessary API filesystems in the target deployment and acts as | ||
| // a nicer `chroot`. | ||
| if let Some(deploy) = deployment_path { | ||
| let target_root = rootfs.join(deploy); | ||
| let boot_path = rootfs.join("boot"); | ||
| let rootfs_path = rootfs.to_path_buf(); | ||
|
|
||
| tracing::debug!("Running bootupctl via bwrap in {}", target_root); | ||
|
|
||
| // Prepend "bootupctl" to the args for bwrap | ||
| let mut bwrap_args = vec!["bootupctl"]; | ||
| bwrap_args.extend(bootupd_args); | ||
|
|
||
| let cmd = BwrapCmd::new(&target_root) | ||
| let mut cmd = BwrapCmd::new(&target_root) | ||
| // Bind mount /boot from the physical target root so bootupctl can find | ||
| // the boot partition and install the bootloader there | ||
| .bind(&boot_path, &"/boot"); | ||
|
|
||
| // Only bind mount the physical root at /sysroot when using --filesystem; | ||
| // bootupd needs it to resolve backing block devices via lsblk. | ||
| if root_device_path.is_none() { | ||
| cmd = cmd.bind(&rootfs_path, &"/sysroot"); | ||
| } | ||
|
|
||
| // The $PATH in the bwrap env is not complete enough for some images | ||
| // so we inject a reasonnable default. | ||
| // This is causing bootupctl and/or sfdisk binaries | ||
|
|
@@ -145,6 +223,11 @@ pub(crate) fn install_via_bootupd( | |
| } | ||
| } | ||
|
|
||
| /// Install systemd-boot to the first ESP found among backing devices. | ||
| /// | ||
| /// On multi-device setups only the first ESP is installed to; additional | ||
| /// ESPs on other backing devices are left untouched. | ||
| // TODO: install to all ESPs on multi-device setups | ||
| #[context("Installing bootloader")] | ||
| pub(crate) fn install_systemd_boot( | ||
| device: &bootc_blockdev::Device, | ||
|
|
@@ -153,8 +236,10 @@ pub(crate) fn install_systemd_boot( | |
| _deployment_path: Option<&str>, | ||
| autoenroll: Option<SecurebootKeys>, | ||
| ) -> Result<()> { | ||
| let esp_part = device | ||
| .find_partition_of_type(discoverable_partition_specification::ESP) | ||
| let roots = device.find_all_roots()?; | ||
| let esp_part = roots | ||
| .iter() | ||
| .find_map(|root| root.find_partition_of_type(discoverable_partition_specification::ESP)) | ||
| .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; | ||
|
|
||
| let esp_mount = mount_esp(&esp_part.path()).context("Mounting ESP")?; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,6 +69,44 @@ resize_rootfs: false | |
| CLOUDEOF | ||
| fi | ||
|
|
||
| # Temporary: update bootupd from @CoreOS/continuous copr until | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit tricky as we probably also want to CI test the case without the new bootupd too... I know, another matrix entry in theory... I guess it'll be tested in post submits by the workflow test. |
||
| # base images include a version supporting --filesystem | ||
| . /usr/lib/os-release | ||
| case $ID in | ||
| fedora) copr_distro="fedora" ;; | ||
| *) copr_distro="centos-stream" ;; | ||
| esac | ||
| # Update bootc from rhcontainerbot copr; the new bootupd | ||
| # requires a newer bootc than what ships in some base images. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But wait we're building this as part of our CI here right? |
||
| cat >/etc/yum.repos.d/rhcontainerbot-bootc.repo <<REPOEOF | ||
| [copr:copr.fedorainfracloud.org:rhcontainerbot:bootc] | ||
| name=Copr repo for bootc owned by rhcontainerbot | ||
| baseurl=https://download.copr.fedorainfracloud.org/results/rhcontainerbot/bootc/${copr_distro}-\$releasever-\$basearch/ | ||
| type=rpm-md | ||
| skip_if_unavailable=True | ||
| gpgcheck=1 | ||
| gpgkey=https://download.copr.fedorainfracloud.org/results/rhcontainerbot/bootc/pubkey.gpg | ||
| repo_gpgcheck=0 | ||
| enabled=1 | ||
| enabled_metadata=1 | ||
| REPOEOF | ||
| dnf -y update bootc | ||
| rm -f /etc/yum.repos.d/rhcontainerbot-bootc.repo | ||
| cat >/etc/yum.repos.d/coreos-continuous.repo <<REPOEOF | ||
| [copr:copr.fedorainfracloud.org:group_CoreOS:continuous] | ||
| name=Copr repo for continuous owned by @CoreOS | ||
| baseurl=https://download.copr.fedorainfracloud.org/results/@CoreOS/continuous/${copr_distro}-\$releasever-\$basearch/ | ||
| type=rpm-md | ||
| skip_if_unavailable=True | ||
| gpgcheck=1 | ||
| gpgkey=https://download.copr.fedorainfracloud.org/results/@CoreOS/continuous/pubkey.gpg | ||
| repo_gpgcheck=0 | ||
| enabled=1 | ||
| enabled_metadata=1 | ||
| REPOEOF | ||
| dnf -y install bootupd-0.2.32.41.gb788553 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this the main thing we need? |
||
| rm -f /etc/yum.repos.d/coreos-continuous.repo | ||
|
|
||
| dnf clean all | ||
| # Stock extra cleaning of logs and caches in general (mostly dnf) | ||
| rm /var/log/* /var/cache /var/lib/{dnf,rpm-state,rhsm} -rf | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This construction looks strange, isn't it the same as just
find_colocated_esps()?.first()?