-
Notifications
You must be signed in to change notification settings - Fork 172
Composefs test parity with ostree #1913
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
835e4e2
99199d3
c354606
ec86999
6af9e75
b9dc8c7
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 |
|---|---|---|
|
|
@@ -157,7 +157,7 @@ jobs: | |
| matrix: | ||
| # No fedora-44 due to https://bugzilla.redhat.com/show_bug.cgi?id=2429501 | ||
| test_os: [fedora-43, centos-9, centos-10] | ||
| variant: [ostree, composefs-sealeduki-sdboot] | ||
| variant: [ostree, composefs-sealeduki-sdboot, composefs-sdboot, composefs-grub] | ||
| exclude: | ||
| # centos-9 UKI is experimental/broken (https://github.com/bootc-dev/bootc/issues/1812) | ||
| - test_os: centos-9 | ||
|
|
@@ -178,7 +178,18 @@ jobs: | |
| run: | | ||
| BASE=$(just pullspec-for-os base ${{ matrix.test_os }}) | ||
| echo "BOOTC_base=${BASE}" >> $GITHUB_ENV | ||
| echo "BOOTC_variant=${{ matrix.variant }}" >> $GITHUB_ENV | ||
|
|
||
| case "${{ matrix.variant }}" in | ||
| composefs-grub|composefs-sdboot) | ||
|
Comment on lines
+182
to
+183
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. I'd prefer we do this kind of thing in Justfile so it's easier to do locally. |
||
| echo "BOOTC_variant=composefs" >> $GITHUB_ENV | ||
| ;; | ||
|
|
||
| *) | ||
| echo "BOOTC_variant=${{ matrix.variant }}" >> $GITHUB_ENV | ||
| ;; | ||
| esac | ||
|
|
||
|
|
||
|
|
||
| if [ "${{ matrix.variant }}" = "composefs-sealeduki-sdboot" ]; then | ||
| BUILDROOTBASE=$(just pullspec-for-os buildroot-base ${{ matrix.test_os }}) | ||
|
|
@@ -207,11 +218,12 @@ jobs: | |
|
|
||
| - name: Run TMT integration tests | ||
| run: | | ||
| if [ "${{ matrix.variant }}" = "composefs-sealeduki-sdboot" ]; then | ||
| just test-composefs | ||
|
Comment on lines
-210
to
-211
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. I think at this point it'd be cleaner to have a mechainism to skip some tests (tmt already has metadata) so it's always |
||
| if [[ "${{ matrix.variant }}" = composefs* ]]; then | ||
| just "test-${{ matrix.variant }}" | ||
| else | ||
| just test-tmt integration | ||
| fi | ||
|
|
||
| just clean-local-images | ||
|
|
||
| - name: Archive TMT logs | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,9 +105,32 @@ test-container: build build-units | |
|
|
||
| # Build and test sealed composefs images | ||
| [group('core')] | ||
| test-composefs: | ||
| test-composefs-sealeduki-sdboot: | ||
| just variant=composefs-sealeduki-sdboot test-tmt readonly local-upgrade-reboot | ||
|
|
||
| [group('core')] | ||
| test-composefs bootloader: | ||
| just variant=composefs test-tmt --composefs-backend --bootloader {{bootloader}} \ | ||
|
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. Per above I think we can attach "supports composefs" as metadata to tests instead |
||
| readonly \ | ||
| bib-build \ | ||
| download-only \ | ||
| image-pushpull-upgrade \ | ||
| image-upgrade-reboot \ | ||
| install-outside-container \ | ||
| install-to-filesystem-var-mount \ | ||
| soft-reboot \ | ||
| usroverlay | ||
|
|
||
| # Build and test composefs images booted using Type1 boot entries and systemd-boot as the bootloader | ||
| [group('core')] | ||
| test-composefs-sdboot: | ||
| just test-composefs systemd | ||
|
|
||
| # Build and test composefs images booted using Type1 boot entries and grub as the bootloader | ||
| [group('core')] | ||
| test-composefs-grub: | ||
| just test-composefs grub | ||
|
|
||
| # Run cargo fmt and clippy checks in container | ||
| [group('core')] | ||
| validate: | ||
|
|
@@ -220,6 +243,7 @@ clean-local-images: | |
| podman image prune -f | ||
| podman rmi {{fedora-coreos}} -f | ||
|
|
||
|
|
||
| # Build packages (RPM) into target/packages/ | ||
| [group('maintenance')] | ||
| package: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| #!/bin/bash | ||
| # Install unsigned systemd-boot RPM (already downloaded by tools stage) | ||
| set -xeuo pipefail | ||
|
|
||
| # Uninstall bootupd if present (we're switching to sd-boot managed differently) | ||
| if rpm -q bootupd &>/dev/null; then | ||
| rpm -e bootupd | ||
| rm -vrf /usr/lib/bootupd/updates | ||
| fi | ||
|
|
||
| # Install the unsigned systemd-boot RPM that was downloaded by the tools stage | ||
| # The RPM is available in /run/sdboot-signed/out (copied from tools stage) | ||
| rpm -Uvh /run/sdboot-signed/out/*.rpm |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -109,13 +109,14 @@ pub(crate) fn install_via_bootupd( | |
| .run_inherited_with_cmd_context() | ||
| } | ||
|
|
||
| #[context("Installing bootloader")] | ||
| #[context("Installing systemd boot")] | ||
| pub(crate) fn install_systemd_boot( | ||
| device: &PartitionTable, | ||
| _rootfs: &Utf8Path, | ||
| _configopts: &crate::install::InstallConfigOpts, | ||
| _deployment_path: Option<&str>, | ||
| autoenroll: Option<SecurebootKeys>, | ||
| mounted_erofs: &Dir, | ||
| ) -> Result<()> { | ||
| let esp_part = device | ||
| .find_partition_of_type(discoverable_partition_specification::ESP) | ||
|
|
@@ -131,6 +132,73 @@ pub(crate) fn install_systemd_boot( | |
| .log_debug() | ||
| .run_inherited_with_cmd_context()?; | ||
|
|
||
| // Check for systemd-boot binaries in the EFI/systemd directory | ||
| // systemd v258 won't copy the binary if an EFI booted system is not detected | ||
|
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. I think we really want The logic here feels too manual. (I mean if we had to I'd argue we just fake it out by creating |
||
| let systemd_dir = esp_mount.fd.open_dir_optional("EFI/systemd")?; | ||
|
|
||
| let systemd_boot_found = if let Some(dir) = systemd_dir { | ||
| let mut found = false; | ||
| for entry in dir.entries()? { | ||
| let entry = entry?; | ||
| let name = entry.file_name(); | ||
|
|
||
| if let Some(name_str) = name.to_str() { | ||
| if name_str.starts_with("systemd-boot") && name_str.ends_with(".efi") { | ||
| found = true; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| found | ||
| } else { | ||
| false | ||
| }; | ||
|
|
||
| if !systemd_boot_found { | ||
| println!("Copying systemd-boot binary manually"); | ||
|
|
||
| // Find the systemd-boot binary in the source directory | ||
| let boot_dir = mounted_erofs.open_dir("usr/lib/systemd/boot/efi")?; | ||
| let mut systemd_boot_binary = None; | ||
|
|
||
| for entry in boot_dir.entries()? { | ||
| let entry = entry?; | ||
| let name = entry.file_name(); | ||
| if let Some(name_str) = name.to_str() { | ||
| if name_str.starts_with("systemd-boot") && name_str.ends_with(".efi") { | ||
| systemd_boot_binary = Some(name_str.to_string()); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let binary_name = systemd_boot_binary | ||
| .ok_or_else(|| anyhow::anyhow!("No systemd-boot binary found in source"))?; | ||
|
|
||
| let src_path = format!("usr/lib/systemd/boot/efi/{}", binary_name); | ||
| let systemd_dest_path = format!("EFI/systemd/{}", binary_name); | ||
|
|
||
| // Determine the appropriate BOOT binary name based on architecture | ||
| let boot_binary_name = if binary_name.contains("x64") { | ||
| "BOOTX64.EFI" | ||
| } else if binary_name.contains("ia32") { | ||
| "BOOTIA32.EFI" | ||
| } else if binary_name.contains("aa64") { | ||
| "BOOTAA64.EFI" | ||
| } else { | ||
| "BOOTX64.EFI" // Default fallback | ||
| }; | ||
|
|
||
| mounted_erofs.copy(&src_path, &esp_mount.fd, &systemd_dest_path)?; | ||
|
|
||
| mounted_erofs.copy( | ||
| &src_path, | ||
| &esp_mount.fd, | ||
| &format!("EFI/BOOT/{}", boot_binary_name), | ||
| )?; | ||
| } | ||
|
|
||
| if let Some(SecurebootKeys { dir, keys }) = autoenroll { | ||
| let path = esp_path.join(SYSTEMD_KEY_DIR); | ||
| create_dir_all(&path)?; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,8 @@ const ENV_BOOTC_UPGRADE_IMAGE: &str = "BOOTC_upgrade_image"; | |
| // Distro identifiers | ||
| const DISTRO_CENTOS_9: &str = "centos-9"; | ||
|
|
||
| const COMPOSEFS_KERNEL_ARGS: [&str; 1] = ["--karg=enforcing=0"]; | ||
|
|
||
| // Import the argument types from xtask.rs | ||
| use crate::{RunTmtArgs, TmtProvisionArgs}; | ||
|
|
||
|
|
@@ -430,6 +432,17 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { | |
| opts.push("--filesystem=xfs".to_string()); | ||
| } | ||
| } | ||
|
|
||
| if args.composefs_backend { | ||
| opts.push("--filesystem=ext4".into()); | ||
|
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. I think this shouldn't be hardcoded here...actually "filesystem choice" is a whole new dimension to the matrix in general. We absolutely should be testing (and support!) e.g. "insecure" (fsverity disabled) composefs-rs storage too. I think we don't even have a mechanism to specify that, but we should. I guess it could be part of the install config? |
||
| opts.push("--composefs-backend".into()); | ||
| opts.extend(COMPOSEFS_KERNEL_ARGS.map(|x| x.into())); | ||
| } | ||
|
|
||
| if let Some(b) = &args.bootloader { | ||
| opts.push(format!("--bootloader={b}")); | ||
| } | ||
|
|
||
| opts | ||
| }; | ||
|
|
||
|
|
||
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.
Exciting!