OCPBUGS-83693: RHEL 10 RPM-based installation testing in CI#6587
Conversation
|
Skipping CI for Draft Pull Request. |
|
@ggiguash: This pull request references Jira Issue OCPBUGS-83693, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
WalkthroughAdds support for building Changes
Sequence Diagram(s)sequenceDiagram
participant Script as build_bootc_images.py
participant Blueprint as Blueprint Template
participant Podman as podman
participant Container as IBC_IMAGE Container
participant FS as Filesystem
Script->>Blueprint: Load & template installer blueprint
Script->>Podman: podman run --rm IBC_IMAGE (build)
Podman->>Container: Start build container
Container->>FS: Produce image-installer ISO & logs
Container-->>Podman: Exit
Podman-->>Script: Stream/capture logs
Script->>FS: chown + rename produced ISO (if not dry_run)
Script->>Podman: Stop any builder containers (cleanup)
sequenceDiagram
participant Test as rpm-install.sh
participant VM as Kickstart VM
participant Repo as Repo Config
participant SubMgr as subscription-manager
participant Robot as Robot Suite
Test->>Test: export SKIP_GREENBOOT, disable randomization
Test->>VM: Create & launch VM (kickstart)
VM-->>Test: VM ready
Test->>Repo: Configure MicroShift mirror & RHOCP repos (numeric/HTTP/beta)
Test->>SubMgr: Register host (if needed)
Test->>Robot: Run install.robot (SOURCE_REPO_URL, TARGET_VERSION)
Test->>Robot: Run remove.robot
Test->>Robot: Run upgrade-successful.robot
Robot-->>Test: Results
Test->>VM: Remove VM
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@ggiguash: This pull request references Jira Issue OCPBUGS-83693, which is invalid:
Comment 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. |
|
/jira refresh |
|
@ggiguash: This pull request references Jira Issue OCPBUGS-83693, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
|
/test ? |
|
/test e2e-aws-tests-bootc-arm-el10 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/bin/pyutils/build_bootc_images.py (1)
457-525: Heavy structural overlap withprocess_image_bootc.
process_image_installerreuses the should_skip helper, file-prep, podman-run scaffolding, junit/log handling, and chown+rename tail almost verbatim. Extracting a small_run_iso_builder(...)(or factoringshould_skipand the post-process steps) would avoid the next builder type drifting from this one. Optional, doesn't need to block the PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/bin/pyutils/build_bootc_images.py` around lines 457 - 525, process_image_installer mostly duplicates logic from process_image_bootc (should_skip, file prep, podman run scaffolding, junit/log handling, and chown+rename tail); refactor by extracting the shared behavior into a small helper (e.g., _run_iso_builder or shared helpers for should_skip, run-and-record, and post_process_iso) and replace the duplicated blocks in process_image_installer and process_image_bootc with calls to that helper; ensure the helper accepts parameters used above (groupdir/installerfile or their derived ii_path/ii_outdir/ii_outfile/ii_logfile/ii_targetiso, the image name IBC_IMAGE, the build target like "image-installer", and dry_run) and preserves existing junit/logging semantics (common.record_junit, common.retry_on_exception, exception handling, sed post-processing, chown and iso rename) so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/bin/pyutils/build_bootc_images.py`:
- Around line 521-524: The glob.glob call that builds iso_candidates (in
build_bootc_images.py) can return filesystem-dependent ordering so using
iso_candidates[0] is non-deterministic; update the logic that finds the ISO
under ii_outdir to either assert there's exactly one match or deterministically
choose the newest/desired file (e.g., sort iso_candidates or pick by
os.path.getmtime) before calling os.rename to ii_targetiso, and keep references
to iso_candidates, ii_outdir and ii_targetiso when making the change.
In `@test/scenarios-bootc/el10/presubmits/el102-src`@rpm-install.sh:
- Around line 54-67: Replace the hardcoded "rhel-9" strings used for repo naming
in the EL10 scenario: update the subscription-manager enable command string
"rhocp-${major}.${rhocp}-for-rhel-9-$(uname -m)-rpms", the ocp_repo_name
assignment "rhocp-${major}.${minor}-for-rhel-9-mirrorbeta-rpms", and the repo
stanza name/value ("Beta rhocp RPMs for RHEL 9") to use "rhel-10" / "RHEL 10"
(matching the existing fast-datapath-for-rhel-10 pattern) so the commands and
generated mirrorbeta repo file target RHEL 10 instead of RHEL 9.
---
Nitpick comments:
In `@test/bin/pyutils/build_bootc_images.py`:
- Around line 457-525: process_image_installer mostly duplicates logic from
process_image_bootc (should_skip, file prep, podman run scaffolding, junit/log
handling, and chown+rename tail); refactor by extracting the shared behavior
into a small helper (e.g., _run_iso_builder or shared helpers for should_skip,
run-and-record, and post_process_iso) and replace the duplicated blocks in
process_image_installer and process_image_bootc with calls to that helper;
ensure the helper accepts parameters used above (groupdir/installerfile or their
derived ii_path/ii_outdir/ii_outfile/ii_logfile/ii_targetiso, the image name
IBC_IMAGE, the build target like "image-installer", and dry_run) and preserves
existing junit/logging semantics (common.record_junit,
common.retry_on_exception, exception handling, sed post-processing, chown and
iso rename) so behavior remains identical.
🪄 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: b975de6b-f7ff-41c8-a73b-47e22e50d12f
📒 Files selected for processing (3)
test/bin/pyutils/build_bootc_images.pytest/image-blueprints-bootc/el10/layer1-base/group2/rhel102-installer.image-installertest/scenarios-bootc/el10/presubmits/el102-src@rpm-install.sh
|
@ggiguash: This pull request references Jira Issue OCPBUGS-83693, which is valid. 3 validation(s) were run on this bug
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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/scenarios-bootc/el9/presubmits/el98-src@rpm-install.sh (2)
16-72: Hoistconfigure_microshift_mirror/configure_rhocp_repointo a shared helper.These two helpers are duplicated almost verbatim between this file and
test/scenarios-bootc/el10/presubmits/el102-src@rpm-install.sh, differing only in the hardcodedrhel-9vsrhel-10strings (lines 55, 57, 62, 109). With more EL variants on the way, this is a prime spot for drift — a fix in one will inevitably miss the other.Consider moving them to a sourced helper (e.g., under
test/bin/) and parameterizing the RHEL major (local -r rhel_major=${1:-9}or derived fromMAJOR_VERSION). The presubmit scripts would then just source it and call.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/scenarios-bootc/el9/presubmits/el98-src`@rpm-install.sh around lines 16 - 72, The two nearly identical functions configure_microshift_mirror and configure_rhocp_repo are duplicated across EL9 and EL10 presubmit scripts; extract them into a shared sourced helper (e.g., test/bin/rpm_repos.sh) that accepts a rhel_major parameter (or derives it from MAJOR_VERSION) and parameterize the hardcoded "rhel-9"/"rhel-10" strings; then replace the local definitions in el9/presubmits/el98-src@rpm-install.sh and el10/presubmits/el102-src@rpm-install.sh with a source of the helper and calls to configure_microshift_mirror and configure_rhocp_repo passing the repo/rhocp args and rhel_major.
74-88: Naming nit:rhel98-installervs blueprint pinned torhel-9.7.Per
test/image-blueprints-bootc/el9/layer1-base/group2/rhel98-installer.image-installer, the blueprint is currently pinned torhel-9.7with a TODO to switch to 9.8 once RPM repos land. The98in the artifact/scenario name will be misleading until that swap happens — leaving a quick comment here pointing at the blueprint TODO would save the next reader a grep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/scenarios-bootc/el9/presubmits/el98-src`@rpm-install.sh around lines 74 - 88, The scenario_create_vms function uses the artifact name "rhel98-installer" which is misleading because the underlying image blueprint (test/image-blueprints-bootc/el9/layer1-base/group2/rhel98-installer.image-installer) is currently pinned to rhel-9.7; add a short comment above the launch_vm rhel98-installer call clarifying that the "98" name is historical and linking/mentioning the blueprint TODO to switch to 9.8 so future readers aren’t confused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/scenarios-bootc/el9/presubmits/el98-src`@rpm-install.sh:
- Around line 30-41: The temp file created as tmp_file is removed only locally
(rm -f "${tmp_file}") but the copy placed on the VM by copy_file_to_vm is left
behind; after run_command_on_vm host1 "sudo cp ${tmp_file}
/etc/yum.repos.d/microshift-mirror-rpms.repo" add a remote cleanup using
run_command_on_vm host1 "sudo rm -f ${tmp_file}" (or equivalent) so the
temporary file is removed from the VM as well; ensure you use the same tmp_file
variable and place the remote rm before the local rm to avoid race conditions.
---
Nitpick comments:
In `@test/scenarios-bootc/el9/presubmits/el98-src`@rpm-install.sh:
- Around line 16-72: The two nearly identical functions
configure_microshift_mirror and configure_rhocp_repo are duplicated across EL9
and EL10 presubmit scripts; extract them into a shared sourced helper (e.g.,
test/bin/rpm_repos.sh) that accepts a rhel_major parameter (or derives it from
MAJOR_VERSION) and parameterize the hardcoded "rhel-9"/"rhel-10" strings; then
replace the local definitions in el9/presubmits/el98-src@rpm-install.sh and
el10/presubmits/el102-src@rpm-install.sh with a source of the helper and calls
to configure_microshift_mirror and configure_rhocp_repo passing the repo/rhocp
args and rhel_major.
- Around line 74-88: The scenario_create_vms function uses the artifact name
"rhel98-installer" which is misleading because the underlying image blueprint
(test/image-blueprints-bootc/el9/layer1-base/group2/rhel98-installer.image-installer)
is currently pinned to rhel-9.7; add a short comment above the launch_vm
rhel98-installer call clarifying that the "98" name is historical and
linking/mentioning the blueprint TODO to switch to 9.8 so future readers aren’t
confused.
🪄 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: 5e7cf87d-5f81-4122-bab1-b127624d90bd
📒 Files selected for processing (2)
test/image-blueprints-bootc/el9/layer1-base/group2/rhel98-installer.image-installertest/scenarios-bootc/el9/presubmits/el98-src@rpm-install.sh
✅ Files skipped from review due to trivial changes (1)
- test/image-blueprints-bootc/el9/layer1-base/group2/rhel98-installer.image-installer
|
/verified by ci |
|
@ggiguash: This PR has been marked as verified by 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: copejon, ggiguash 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 |
|
/cherry-pick release-4.22 |
|
@ggiguash: once the present PR merges, I will cherry-pick it on top of 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. |
|
@ggiguash: 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. |
|
@ggiguash: Jira Issue OCPBUGS-83693: All pull requests linked via external trackers have merged: All linked pull requests have the 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. |
|
@ggiguash: new pull request created: #6593 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. |
Summary by CodeRabbit
New Features
Chores