USHIFT-6812: Optimize bootc image building for upgrade tests#6540
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@vanhalenar: This pull request references USHIFT-6812 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. 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. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (7)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe CI build script is updated to use the el10 layer5-upgrade blueprint layer for upgrade testing instead of el9 layers. Eight reference files are added under the el10 layer5-upgrade directory, each pointing to corresponding el9 base layer files. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/bin/pyutils/build_bootc_images.py (1)
603-608: De-duplicate repeated layer dirs before processing.If the same
-lis passed more than once, the same groups are rebuilt multiple times. A small normalization step avoids accidental duplicate work.♻️ Proposed fix
- args.layer_dir = [os.path.abspath(d) for d in args.layer_dir] + args.layer_dir = list(dict.fromkeys(os.path.abspath(d) for d in args.layer_dir))Also applies to: 679-684
🤖 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 603 - 608, The code currently converts args.layer_dir to absolute paths and then iterates them, but does not remove duplicates so repeated -l entries cause redundant rebuilds; before validating and processing, normalize args.layer_dir by deduplicating while preserving order (e.g., replace args.layer_dir with a list of unique absolute paths derived from args.layer_dir) and then continue with the existing isdir check and downstream processing (apply the same deduplication step in the other identical block that handles args.layer_dir later around the second occurrence).test/bin/ci_phase_iso_build.sh (1)
140-140: Inconsistent path quoting.Other paths in this function use double quotes (e.g., lines 135, 136, 144, 147), but this one doesn't. While it works since it's a literal path, consider quoting for consistency.
Suggested fix
- $(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/el10/layer5-upgrade + $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/el10/layer5-upgrade"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/bin/ci_phase_iso_build.sh` at line 140, The call to launch build_bootc_images.sh uses an unquoted path argument ($(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/el10/layer5-upgrade) which is inconsistent with other invocations; update the invocation to quote the path argument (for example -l "./image-blueprints-bootc/el10/layer5-upgrade") so it matches the quoting style used on surrounding lines and prevents word-splitting if the path ever contains spaces.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/bin/ci_phase_iso_build.sh`:
- Line 140: The call to launch build_bootc_images.sh uses an unquoted path
argument ($(dry_run) bash -x ./bin/build_bootc_images.sh -l
./image-blueprints-bootc/el10/layer5-upgrade) which is inconsistent with other
invocations; update the invocation to quote the path argument (for example -l
"./image-blueprints-bootc/el10/layer5-upgrade") so it matches the quoting style
used on surrounding lines and prevents word-splitting if the path ever contains
spaces.
In `@test/bin/pyutils/build_bootc_images.py`:
- Around line 603-608: The code currently converts args.layer_dir to absolute
paths and then iterates them, but does not remove duplicates so repeated -l
entries cause redundant rebuilds; before validating and processing, normalize
args.layer_dir by deduplicating while preserving order (e.g., replace
args.layer_dir with a list of unique absolute paths derived from args.layer_dir)
and then continue with the existing isdir check and downstream processing (apply
the same deduplication step in the other identical block that handles
args.layer_dir later around the second occurrence).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: cbe2fa57-d081-4dba-9491-d52ac2bb15fe
📒 Files selected for processing (36)
test/bin/ci_phase_iso_build.shtest/bin/pyutils/build_bootc_images.pytest/image-blueprints-bootc/el10/layer1-base/group1/rhel102-test-agent.containerfiletest/image-blueprints-bootc/el10/layer1-base/group2/rhel102-bootc-crel-isolated.containerfiletest/image-blueprints-bootc/el10/layer1-base/group2/rhel102-bootc-crel-optionals.containerfiletest/image-blueprints-bootc/el10/layer1-base/group2/rhel102-bootc-crel.containerfiletest/image-blueprints-bootc/el10/layer1-base/group2/rhel102-bootc.image-bootctest/image-blueprints-bootc/el10/layer4-release/group1/rhel102-bootc-brew-lrel-optional.containerfiletest/image-blueprints-bootc/el10/layer4-release/group1/rhel102-bootc-brew-nightly-with-optional.containerfiletest/image-blueprints-bootc/el10/layer4-release/group1/rhel102-bootc-brew.containerfiletest/image-blueprints-bootc/el10/layer4-release/group2/rhel102-bootc-brew-lrel-fips.containerfiletest/image-blueprints-bootc/el10/layer4-release/group2/rhel102-bootc-brew-lrel-optional.image-bootctest/image-blueprints-bootc/el10/layer4-release/group2/rhel102-bootc-brew-lrel-tuned.containerfiletest/image-blueprints-bootc/el10/layer5-upgrade/group1/rhel96-bootc.image-bootctest/image-blueprints-bootc/el10/layer5-upgrade/group1/rhel96-test-agent.containerfiletest/image-blueprints-bootc/el10/layer5-upgrade/group1/rhel98-bootc.image-bootctest/image-blueprints-bootc/el10/layer5-upgrade/group1/rhel98-test-agent.containerfiletest/image-blueprints-bootc/el10/layer5-upgrade/group2/rhel96-bootc-prel.containerfiletest/image-blueprints-bootc/el10/layer5-upgrade/group2/rhel96-bootc-yminus2.containerfiletest/image-blueprints-bootc/el10/layer5-upgrade/group2/rhel98-bootc-source.containerfiletest/image-blueprints-bootc/el9/layer1-base/group1/rhel96-test-agent.containerfiletest/image-blueprints-bootc/el9/layer1-base/group1/rhel98-test-agent.containerfiletest/image-blueprints-bootc/el9/layer1-base/group2/rhel96-bootc-prel.containerfiletest/image-blueprints-bootc/el9/layer1-base/group2/rhel96-bootc-yminus2.containerfiletest/image-blueprints-bootc/el9/layer1-base/group2/rhel96-bootc.image-bootctest/image-blueprints-bootc/el9/layer1-base/group2/rhel98-bootc-crel-isolated.containerfiletest/image-blueprints-bootc/el9/layer1-base/group2/rhel98-bootc-crel-optionals.containerfiletest/image-blueprints-bootc/el9/layer1-base/group2/rhel98-bootc-crel.containerfiletest/image-blueprints-bootc/el9/layer1-base/group2/rhel98-bootc.image-bootctest/image-blueprints-bootc/el9/layer4-release/group1/rhel96-bootc-brew-y1-with-optional.containerfiletest/image-blueprints-bootc/el9/layer4-release/group1/rhel96-bootc-brew-y2-with-optional.containerfiletest/image-blueprints-bootc/el9/layer4-release/group1/rhel98-bootc-brew-lrel-optional.containerfiletest/image-blueprints-bootc/el9/layer4-release/group1/rhel98-bootc-brew-nightly-with-optional.containerfiletest/image-blueprints-bootc/el9/layer4-release/group2/rhel98-bootc-brew-lrel-fips.containerfiletest/image-blueprints-bootc/el9/layer4-release/group2/rhel98-bootc-brew-lrel-optional.image-bootctest/image-blueprints-bootc/el9/layer4-release/group2/rhel98-bootc-brew-lrel-tuned.containerfile
vanhalenar
force-pushed
the
upgrade-test-symlinks
branch
from
7ba71ee to
2dd919b
Compare
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: agullon, vanhalenar 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 |
openshift-ci
Bot
added
the
approved
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/verified by ci |
openshift-ci-robot
added
the
verified
|
@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. |
|
/retest |
|
@vanhalenar: 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. |
4 participants
Contains changes from this PR, should be merged after.
This PR introduces a new el10 bootc image layer called
layer5-upgrade, which contains symbolic links to el9 images required for upgrade scenarios. This allows us to not build all el9 images in el10 jobs, but only the required ones, thus saving time.Summary by CodeRabbit