system-reinstall-bootc: fallback image from /usr/lib/os-release by hone · Pull Request #2135 · bootc-dev/bootc

hone · 2026-04-09T16:52:53Z

Add a fallback option when arguments are specified for an image. It works in this order:

BOOTC_REINSTALL_CONFIG env var to a YAML file
--image flag
reads /etc/os-release for BOOTC_IMAGE
reads /usr/lib/os-release for BOOTC_IMAGE

gemini-code-assist

Code Review

This pull request introduces a fallback mechanism to retrieve the bootc image from os-release files when not specified via CLI or environment variables. The changes include a new parsing module, refactored argument handling, and updated CI scripts. Feedback suggests parsing CLI arguments earlier to support standard flags like --help, filtering empty strings during os-release lookups for better fallback reliability, and consolidating redundant CI steps.

gemini-code-assist · 2026-04-09T16:57:06Z

+    order: 97
    script:
      - mkdir -p bootc && cp /var/share/test-artifacts/*.src.rpm bootc
      - cd bootc && rpm2cpio *.src.rpm | cpio -idmv && rm -f *-vendor.tar.zstd && zstd -d *.tar.zstd && tar -xvf *.tar -C . --strip-components=1 && ls -al
      - pwd && ls -al && cd bootc/hack && ./provision-packit.sh
    when: running_env != image_mode
+  - how: shell
+    order: 98
+    script:
+      - echo 'BOOTC_IMAGE=localhost/bootc' | tee -a /usr/lib/os-release
+      - pwd && ls -al && cd bootc/hack && ./provision-packit.sh
+    when: running_env != image_mode


These two shell steps are redundant and inefficient. Both steps perform the same setup and run provision-packit.sh, which includes a time-consuming podman build operation. Since provision-packit.sh is already designed to handle both the explicit argument and the os-release fallback, you should combine these into a single step to avoid rebuilding the image and repeating the setup in CI.

order: 97 script: - mkdir -p bootc && cp /var/share/test-artifacts/*.src.rpm bootc - cd bootc && rpm2cpio *.src.rpm | cpio -idmv && rm -f *-vendor.tar.zstd && zstd -d *.tar.zstd && tar -xvf *.tar -C . --strip-components=1 && ls -al - pwd && ls -al && cd bootc/hack && ./provision-packit.sh - echo 'BOOTC_IMAGE=localhost/bootc' | tee -a /usr/lib/os-release - ./provision-packit.sh when: running_env != image_mode

I thought it was easier to just build the image twice than try to do a cleanup step for testing the BOOTC_IMAGE path on reinstall.

hone · 2026-04-09T21:12:40Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces the ability to automatically detect the bootc image from os-release files (/etc/os-release or /usr/lib/os-release) if not provided via the CLI. It also adds logic to automatically inherit host SSH keys when cloud-init is detected in the target image. Key changes include a new os_release module for parsing, updates to the CLI argument handling, and integration test adjustments. Feedback highlights a critical path error where a container-internal path is used as a host path for SSH key inheritance, potential inefficiencies and failures in the updated test plan, and an opportunity to refactor the image resolution logic for better maintainability.

cgwalters

Thanks so much for working on this!

cgwalters · 2026-04-12T16:48:27Z

+            continue;
+        }
+
+        if let Some((key, value)) = line.split_once('=') {


Style nit, also could be let/else here and avoid further nesting

cgwalters · 2026-04-12T21:01:41Z

+                if value.starts_with('"') && value.ends_with('"') && value.len() >= 2 {
+                    let unquoted = &value[1..value.len() - 1];
+                    let processed = unquoted
+                        .replace(r#"\""#, "\"")
+                        .replace(r#"\\"#, "\\")
+                        .replace(r#"\$"#, "$")
+                        .replace(r#"\`"#, "`");


I think we can use shlex to parse this, it's already in the depchain

cgwalters · 2026-04-12T21:03:15Z

+                .iter()
+                .find_map(|path| {
+                    os_release::get_bootc_image_from_file(path)
+                        .ok()


Generally avoid "error swallowing" like this. There's two different cases:

The file doens't exist - this is normal, and we have https://docs.rs/cap-std-ext/latest/cap_std_ext/dirext/trait.CapStdExtDirExt.html#tymethod.open_optional for this reason

All other errors (permission denied, failure to parse the file, etc.). I think for this we'd want at least to use

bootc/crates/utils/src/result_ext.rs

Line 4 in b1fe5d6

fn log_err_or_else<F>(self, default: F) -> T

)

cgwalters · 2026-04-12T21:04:18Z

 }

+#[context("image_has_cloud_init")]
+pub(crate) fn image_has_cloud_init(image: &str) -> Result<bool> {


In bootc-dev/bcvk#245 we're proposing to standardize labels on images that have Ignition, it could make sense to do the same for cloud-init.

Separate effort from this PR.

Is there already an issue for that?

hone · 2026-04-13T17:49:56Z

How concerned should I be about these packit failures?

jeckersb · 2026-04-14T00:07:49Z

How concerned should I be about these packit failures?

The s390x build ones at least can be ignored for now, there's a known issue with those.

jmarrero · 2026-04-21T18:42:07Z

Thank you again for this. This needs another run of: cargo fmt which CI is complaining about and commits should be squashed. We tend to keep commits functionally grouped, if it's addressing reviews just add it to the main commit that touched that file.

hone · 2026-04-23T05:27:13Z

Thank you again for this. This needs another run of: cargo fmt which CI is complaining about and commits should be squashed. We tend to keep commits functionally grouped, if it's addressing reviews just add it to the main commit that touched that file.

@jmarrero happy to do that. Before I squash, should I split out the ssh authorized keys pieces or keep it in this PR?

jmarrero · 2026-04-23T15:47:20Z

Thank you again for this. This needs another run of: cargo fmt which CI is complaining about and commits should be squashed. We tend to keep commits functionally grouped, if it's addressing reviews just add it to the main commit that touched that file.

@jmarrero happy to do that. Before I squash, should I split out the ssh authorized keys pieces or keep it in this PR?

Yes, I would move the ssh stuff to it's own PR where we will have some additional discussions on it. The background here is that we are not 100% sure yet if we can assume the target image has cloud-init and we might want to support checking for ignition or/and cloud init on the distributions we maintain. There is some uncertainty of assuming this still. When the issue was created I think were more certain about cloud-init vs ignition.

Add a fallback option when arguments are specified for an image. It works in this order: 1. BOOTC_REINSTALL_CONFIG env var to a YAML file 2. `--image` flag 3. reads /etc/os-release for BOOTC_IMAGE 4. reads /usr/lib/os-release for BOOTC_IMAGE Fixes: bootc-dev#1300 Signed-off-by: Terence Lee <hone02@gmail.com>

Remove duplicate cloud-init from hack/packages.txt Signed-off-by: Terence Lee <hone02@gmail.com>

hone · 2026-04-23T23:30:16Z

Yes, I would move the ssh stuff to it's own PR where we will have some additional discussions on it. The background here is that we are not 100% sure yet if we can assume the target image has cloud-init and we might want to support checking for ignition or/and cloud init on the distributions we maintain. There is some uncertainty of assuming this still. When the issue was created I think were more certain about cloud-init vs ignition.

Ok, I've squased the PR. I didn't squash 8a9b9d4 b/c it's not totally related to my PR, but something I needed to do to get CI to pass.

I've also moved the ssh authorized_keys + cloud-init commits into #2163 .

jmarrero

lgtm

github-actions Bot added the area/system-reinstall-bootc Issues related to system-reinstall-botoc label Apr 9, 2026

hone marked this pull request as draft April 9, 2026 16:53

bootc-bot Bot requested a review from jmarrero April 9, 2026 16:53

gemini-code-assist Bot reviewed Apr 9, 2026

View reviewed changes

Comment thread crates/system-reinstall-bootc/src/main.rs Outdated

Comment thread tmt/plans/integration.fmf

Comment thread crates/system-reinstall-bootc/src/main.rs Outdated

hone force-pushed the default-image-os-release branch from 5bb1c1e to ee263a3 Compare April 9, 2026 22:43

hone marked this pull request as ready for review April 10, 2026 06:06

hone force-pushed the default-image-os-release branch from a168777 to a21dcba Compare April 10, 2026 16:49

cgwalters requested changes Apr 12, 2026

View reviewed changes

hone requested a review from cgwalters April 14, 2026 04:20

hone force-pushed the default-image-os-release branch from 9bb1f27 to c8cc3f2 Compare April 18, 2026 01:13

hone force-pushed the default-image-os-release branch 2 times, most recently from c2827c9 to 8a9b9d4 Compare April 23, 2026 23:15

hone added 2 commits April 23, 2026 18:29

Use --allowerase on cloud-init for centos package drift

5bc8f73

Remove duplicate cloud-init from hack/packages.txt Signed-off-by: Terence Lee <hone02@gmail.com>

hone force-pushed the default-image-os-release branch from 8a9b9d4 to 5bc8f73 Compare April 23, 2026 23:30

jmarrero approved these changes Apr 24, 2026

View reviewed changes

Conversation

hone commented Apr 9, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

hone Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

hone commented Apr 9, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

cgwalters Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

cgwalters Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

cgwalters Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

cgwalters Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

hone Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hone commented Apr 13, 2026

Uh oh!

jeckersb commented Apr 14, 2026

Uh oh!

jmarrero commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hone commented Apr 23, 2026

Uh oh!

jmarrero commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hone commented Apr 23, 2026

Uh oh!

jmarrero left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jmarrero commented Apr 21, 2026 •

edited

Loading

jmarrero commented Apr 23, 2026 •

edited

Loading