system-reinstall-bootc: fallback image from /usr/lib/os-release#2135
system-reinstall-bootc: fallback image from /usr/lib/os-release#2135hone wants to merge 5 commits intobootc-dev:mainfrom
Conversation
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>
There was a problem hiding this comment.
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.
| 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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
If cloud-init is detected in the target image: 1. Check the host for existing root SSH keys at /root/.ssh/authorized_keys. 2. If host keys exist, automatically configure inheritance via /target/root/.ssh/authorized_keys. 3. If no host keys exist, proceed without manual interaction, trusting cloud-init in the new system to handle key provisioning. If cloud-init is not detected, maintain the existinginteractive behavior to prevent user lock-out. Ref: bootc-dev#1300 Signed-off-by: Terence Lee <hone02@gmail.com>
|
/gemini review |
There was a problem hiding this comment.
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.
Signed-off-by: Terence Lee <hone02@gmail.com>
5bb1c1e to
ee263a3
Compare
Signed-off-by: Terence Lee <hone02@gmail.com>
Signed-off-by: Terence Lee <hone02@gmail.com>
Add a fallback option when arguments are specified for an image. It works in this order:
--imageflagFixes: #1300