NO-ISSUE: use reqpart in devenv kickstart for RHEL 10 BIOS+GPT compatibility#7048
NO-ISSUE: use reqpart in devenv kickstart for RHEL 10 BIOS+GPT compatibility#7048copejon wants to merge 1 commit into
Conversation
|
@copejon: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughDevelopment environment provisioning now uses Anaconda-managed boot partitions and recognizes RHEL 10 DVD ISO filenames. ChangesDevelopment environment provisioning
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: copejon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/devenv-builder/manage-vm.sh (1)
64-67: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMissing fallback for major version
10.The script handles
8)and9)to provide default minor versions, but there is no10)case. IfMICROSHIFT_RHEL_VERSIONis set to exactly10, the script will fail. Consider adding a10)case with an appropriate default minor version.♻️ Proposed fix
+ 10) + echo "rhel-10.0-$(uname -m)-dvd.iso" + ;; 10.*) echo "rhel-${rhel_version}-$(uname -m)-dvd.iso" ;;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/devenv-builder/manage-vm.sh` around lines 64 - 67, Update the RHEL version case logic in manage-vm.sh to add a 10) branch alongside the existing 8) and 9) branches. For an exact MICROSHIFT_RHEL_VERSION of 10, assign the appropriate default minor version before constructing the ISO filename, while preserving the existing handling for other versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/devenv-builder/manage-vm.sh`:
- Around line 64-67: Update the RHEL version case logic in manage-vm.sh to add a
10) branch alongside the existing 8) and 9) branches. For an exact
MICROSHIFT_RHEL_VERSION of 10, assign the appropriate default minor version
before constructing the ISO filename, while preserving the existing handling for
other versions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ad4811a3-a19f-4846-817b-662780420468
📒 Files selected for processing (2)
scripts/devenv-builder/config/kickstart.ks.templatescripts/devenv-builder/manage-vm.sh
0d4726d to
fbee40f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/devenv-builder/manage-vm.sh`:
- Around line 64-69: Update the version option usage text in the help output to
include RHEL 10 examples, changing the major-version example to 10 and the
minor-version example to 10.2 while preserving the existing supported-version
guidance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 76e565ad-49b9-4769-8fa2-5b15bb6d4ab3
📒 Files selected for processing (2)
scripts/devenv-builder/config/kickstart.ks.templatescripts/devenv-builder/manage-vm.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/devenv-builder/config/kickstart.ks.template
| 10) | ||
| echo "rhel-10.2-$(uname -m)-dvd.iso" | ||
| ;; | ||
| 10.*) | ||
| echo "rhel-${rhel_version}-$(uname -m)-dvd.iso" | ||
| ;; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document RHEL 10 in the -v help text.
These branches add RHEL 10 support, but help still says the major-version example is "9" and only shows "8.7" for a minor version. Update the corresponding usage text to mention 10/10.2, so users discover the new supported version.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/devenv-builder/manage-vm.sh` around lines 64 - 69, Update the version
option usage text in the help output to include RHEL 10 examples, changing the
major-version example to 10 and the minor-version example to 10.2 while
preserving the existing supported-version guidance.
|
@copejon: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/close |
|
@copejon: Closed this PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
RHEL 10 anaconda defaults to GPT disk labels even in BIOS mode, which
requires a biosboot partition that the hard-coded EFI partition layout
did not provide. Replace the explicit boot partition directives with
reqpart --add-boot, matching the pattern used by all other kickstart
templates in the repo. This lets anaconda auto-create the correct
platform partition for any firmware type.
Also add RHEL 10 support to get_base_isofile in manage-vm.sh.
Summary by CodeRabbit