USHIFT-6812: Optimize bootc image building for upgrade tests#6540
Draft
vanhalenar wants to merge 10 commits intoopenshift:mainfrom
Draft
USHIFT-6812: Optimize bootc image building for upgrade tests#6540vanhalenar wants to merge 10 commits intoopenshift:mainfrom
vanhalenar wants to merge 10 commits intoopenshift:mainfrom
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
Contributor
|
Skipping CI for Draft Pull Request. |
WalkthroughThis PR restructures bootc image build infrastructure to support per-OS blueprint paths and multi-layer builds. Changes include updating CI phase script to build layer1-base and layer4-release for both el9 and el10, refactoring the Python build utility to accept multiple layer directories, and adding el10 layer5-upgrade blueprint files that reference el9 content. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vanhalenar The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Chores
Tests