USHIFT-6547: Add RHEL 10 CI lane#6330
Draft
vanhalenar wants to merge 2 commits intoopenshift:mainfrom
Draft
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@vanhalenar: This pull request references USHIFT-6547 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 "4.22.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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughCI ISO build script made OS-aware (el9/el10), now pre-builds templates and runs per-OS loops to build bootc layers (layer1..layer4) and upstream variants; added el10 periodic/release images and scenario updates for rhel102 artifacts; build-cache script now pins an S3 bucket name. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Job
participant Script as ci_phase_iso_build.sh
participant Templates as Template Builder
participant Builder as bootc Image Builder
participant Registry as Image Registry
CI->>Script: trigger job
Script->>Script: derive OS targets from CI_JOB_NAME (el9, el10) or choose multi-OS
Script->>Templates: build templates (pre-build)
loop per OS (el9, el10)
Script->>Builder: build layer1-base (per-OS)
Builder->>Registry: push base image
Script->>Builder: build layer2-presubmit (per-OS)
Builder->>Registry: push presubmit image
alt periodic requested
Script->>Builder: build layer3-periodic (per-OS)
Builder->>Registry: push periodic image
end
alt release requested
Script->>Builder: build layer4-release (per-OS)
Builder->>Registry: push release image
end
alt upstream requested
Script->>Builder: build upstream (per-OS or multi-OS)
Builder->>Registry: push upstream image
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
|
@vanhalenar: This pull request references USHIFT-6547 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 "4.22.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. |
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 |
|
@vanhalenar: This pull request references USHIFT-6547 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 "4.22.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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/bin/ci_phase_iso_build.sh (1)
156-164: This non-CI bootc fallback is dead from the current dispatch.
run_bootc_image_build()is only reached through theelseat Lines 229-234, andCOMPOSER_CLI_BUILDSis only flipped tofalsewhenCI_JOB_NAMEalready exists at Lines 27-29. So this branch never runs, and local executions still will not exercise the full el9/el10 bootc matrix. Either remove it or route bootc mode off an explicit flag instead ofCI_JOB_NAME.🤖 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` around lines 156 - 164, The fallback full el9/el10 bootc matrix in the else branch (the loop calling build_bootc_images.sh) is dead because run_bootc_image_build() is only reached via the CI_JOB_NAME-based flow and COMPOSER_CLI_BUILDS is toggled when CI_JOB_NAME exists; either remove this unreachable branch or make bootc mode explicit: change the control from relying on CI_JOB_NAME/COMPOSER_CLI_BUILDS to an explicit flag (e.g., BOOTC_LOCAL_MATRIX or RUN_BOOTC_FULL) checked in run_bootc_image_build(), and update the code that sets COMPOSER_CLI_BUILDS/bootc logic so local runs can opt into the full el9/el10 matrix using that flag instead of CI_JOB_NAME.
🤖 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/ci_phase_iso_build.sh`:
- Around line 121-149: update_build_cache currently warms only the legacy flat
bootc paths, so -update_cache misses artifacts for the new layout consumed by
run_bootc_image_build; modify update_build_cache to also warm the new
./image-blueprints-bootc/templates plus the OS-specific layer trees (e.g.,
./image-blueprints-bootc/el9/{layer1-base,layer2-presubmit,layer3-periodic,layer4-release}
and
./image-blueprints-bootc/el10/{layer1-base,layer2-presubmit,layer3-periodic,layer4-release}),
ensuring el10 release artifacts are included for the CI scenarios that call
run_bootc_image_build.
---
Nitpick comments:
In `@test/bin/ci_phase_iso_build.sh`:
- Around line 156-164: The fallback full el9/el10 bootc matrix in the else
branch (the loop calling build_bootc_images.sh) is dead because
run_bootc_image_build() is only reached via the CI_JOB_NAME-based flow and
COMPOSER_CLI_BUILDS is toggled when CI_JOB_NAME exists; either remove this
unreachable branch or make bootc mode explicit: change the control from relying
on CI_JOB_NAME/COMPOSER_CLI_BUILDS to an explicit flag (e.g., BOOTC_LOCAL_MATRIX
or RUN_BOOTC_FULL) checked in run_bootc_image_build(), and update the code that
sets COMPOSER_CLI_BUILDS/bootc logic so local runs can opt into the full
el9/el10 matrix using that flag instead of CI_JOB_NAME.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 53f37e74-ecd8-4cc9-87ee-118198facc52
📒 Files selected for processing (132)
test/bin/ci_phase_iso_build.shtest/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/layer2-presubmit/group1/rhel102-bootc-source.containerfiletest/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-optionals.containerfiletest/image-blueprints-bootc/el10/layer3-periodic/group1/rhel102-bootc-source-isolated.containerfiletest/image-blueprints-bootc/el10/layer3-periodic/group2/rhel102-bootc-source-isolated.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.containerfiletest/image-blueprints-bootc/el10/layer4-release/group2/rhel102-bootc-brew-lrel-optional.image-bootctest/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/layer2-presubmit/group1/rhel98-bootc-source-base.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group1/rhel98-bootc-source.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-aux.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-fake-next-minor.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-fips.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-optionals.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group1/rhel98-bootc-source-ai-model-serving.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group1/rhel98-bootc-source-isolated.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group2/rhel98-bootc-source-ai-model-serving.image-bootctest/image-blueprints-bootc/el9/layer3-periodic/group2/rhel98-bootc-source-gitops.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group2/rhel98-bootc-source-isolated.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.containerfiletest/image-blueprints-bootc/templates/ai-model-serving-test-data.sh.templatetest/image-blueprints-bootc/templates/build-serialsim.sh.templatetest/image-blueprints-bootc/templates/microshift-copy-images.conf.templatetest/image-blueprints-bootc/templates/microshift-copy-images.sh.templatetest/image-blueprints-bootc/templates/microshift-ovsdb-ownership.conf.templatetest/image-blueprints-bootc/templates/rpm-repo-config.sh.templatetest/image-blueprints-bootc/upstream/group1/cos10-test-agent.containerfiletest/image-blueprints-bootc/upstream/group1/cos9-test-agent.containerfiletest/image-blueprints-bootc/upstream/group2/centos10-bootc.image-bootctest/image-blueprints-bootc/upstream/group2/centos9-bootc.image-bootctest/image-blueprints-bootc/upstream/group2/cos10-bootc-source.containerfiletest/image-blueprints-bootc/upstream/group2/cos9-bootc-source.containerfiletest/image-blueprints-bootc/upstream/group3/cos10-bootc-source-isolated.containerfiletest/image-blueprints-bootc/upstream/group3/cos10-bootc-source-optionals.containerfiletest/image-blueprints-bootc/upstream/group3/cos9-bootc-source-isolated.containerfiletest/image-blueprints-bootc/upstream/group3/cos9-bootc-source-optionals.containerfiletest/scenarios-bootc/el10/periodics/el102-src@optional.shtest/scenarios-bootc/el10/periodics/el102-src@standard-suite1.shtest/scenarios-bootc/el10/periodics/el102-src@standard-suite2.shtest/scenarios-bootc/el10/periodics/el96-prel@el102-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el10/periodics/el96-yminus2@el102-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el10/periodics/el98-src@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/presubmits/el96-prel@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/presubmits/el96-yminus2@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/presubmits/el98-src@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/releases/el102-lrel@ginkgo-tests.shtest/scenarios-bootc/el10/releases/el102-lrel@optional.shtest/scenarios-bootc/el10/releases/el102-lrel@standard1.shtest/scenarios-bootc/el10/releases/el102-lrel@standard2.shtest/scenarios-bootc/el9/periodics/el96-prel@el98-src@upgrade-fails-and-rolls-back.shtest/scenarios-bootc/el9/periodics/el96-prel@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/periodics/el96-prel@el98-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el9/periodics/el96-yminus2@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/periodics/el96-yminus2@el98-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el9/periodics/el98-src@ai-model-serving-offline.shtest/scenarios-bootc/el9/periodics/el98-src@cncf-conformance.shtest/scenarios-bootc/el9/periodics/el98-src@fault-tests-and-greenboot.shtest/scenarios-bootc/el9/periodics/el98-src@fips.shtest/scenarios-bootc/el9/periodics/el98-src@gitops-telemetry-clusterid-systemd.shtest/scenarios-bootc/el9/periodics/el98-src@isolated-net.shtest/scenarios-bootc/el9/periodics/el98-src@offline.shtest/scenarios-bootc/el9/periodics/el98-src@osconfig-cleanup-data.shtest/scenarios-bootc/el9/periodics/el98-src@osconfig-lifecycle.shtest/scenarios-bootc/el9/periodics/el98-src@upgrade-fails-cannot-backup.shtest/scenarios-bootc/el9/presubmits/el96-prel@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el96-yminus2@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el96-yminus2@prel@src@delta-upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el98-base@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el98-base@src@opt@delta-upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el98-src@ai-model-serving-online.shtest/scenarios-bootc/el9/presubmits/el98-src@auto-recovery.shtest/scenarios-bootc/el9/presubmits/el98-src@backup-and-restore-on-reboot.shtest/scenarios-bootc/el9/presubmits/el98-src@backups.shtest/scenarios-bootc/el9/presubmits/el98-src@configuration.shtest/scenarios-bootc/el9/presubmits/el98-src@downgrade-block.shtest/scenarios-bootc/el9/presubmits/el98-src@dual-stack.shtest/scenarios-bootc/el9/presubmits/el98-src@ipv6.shtest/scenarios-bootc/el9/presubmits/el98-src@multi-nic.shtest/scenarios-bootc/el9/presubmits/el98-src@optional.shtest/scenarios-bootc/el9/presubmits/el98-src@router.shtest/scenarios-bootc/el9/presubmits/el98-src@standard-suite1.shtest/scenarios-bootc/el9/presubmits/el98-src@standard-suite2.shtest/scenarios-bootc/el9/presubmits/el98-src@storage.shtest/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.sh.disabledtest/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails.shtest/scenarios-bootc/el9/releases/el96-lrel@published-images-standard1.shtest/scenarios-bootc/el9/releases/el96-lrel@published-images-standard2.shtest/scenarios-bootc/el9/releases/el96-y1@el98-lrel@lvms.shtest/scenarios-bootc/el9/releases/el96-y1@el98-lrel@standard1.shtest/scenarios-bootc/el9/releases/el96-y1@el98-lrel@standard2.shtest/scenarios-bootc/el9/releases/el96-y2@el98-lrel@lvms.shtest/scenarios-bootc/el9/releases/el96-y2@el98-lrel@standard1.shtest/scenarios-bootc/el9/releases/el96-y2@el98-lrel@standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@ai-model-serving-online.shtest/scenarios-bootc/el9/releases/el98-lrel@configuration.shtest/scenarios-bootc/el9/releases/el98-lrel@dual-stack.shtest/scenarios-bootc/el9/releases/el98-lrel@fips.shtest/scenarios-bootc/el9/releases/el98-lrel@ginkgo-tests.shtest/scenarios-bootc/el9/releases/el98-lrel@ipv6.shtest/scenarios-bootc/el9/releases/el98-lrel@iso-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@iso-standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@low-latency.shtest/scenarios-bootc/el9/releases/el98-lrel@multi-config-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@multi-config-standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@multi-nic.shtest/scenarios-bootc/el9/releases/el98-lrel@nightly-brew-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@nightly-brew-standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@optional.shtest/scenarios-bootc/el9/releases/el98-lrel@osconfig-router.shtest/scenarios-bootc/el9/releases/el98-lrel@storage-telemetry.shtest/scenarios-bootc/el9/releases/el98-lrel@tlsv13-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@tlsv13-standard2.sh
|
@vanhalenar: This pull request references USHIFT-6547 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 "4.22.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. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/bin/ci_phase_iso_build.sh (1)
70-73:⚠️ Potential issue | 🟠 MajorWarm the shared bootc templates in
update_build_cache().
run_bootc_image_build()now treats./image-blueprints-bootc/templatesas a prerequisite, but Lines 70-73 still warm the cache from the per-OS layer trees directly.-update_cachecan therefore publish stale or incomplete bootc artifacts.Suggested fix
# Build the bootc base layer and brew RPMs to be cached + $(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/templates for os in el9 el10; do $(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/${os}/layer1-base $(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/${os}/layer4-release done🤖 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` around lines 70 - 73, The cache warmup is using per-OS layer trees but run_bootc_image_build() now requires the shared ./image-blueprints-bootc/templates; update update_build_cache() (or replace the loop that calls build_bootc_images.sh per OS) to warm the shared templates directory instead of ./image-blueprints-bootc/${os}/layer1-base and layer4-release so the -update_cache step publishes the correct common bootc artifacts; specifically, change the calls to invoke ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/templates (or add an explicit warm of ./image-blueprints-bootc/templates) and remove the per-OS layer warmups so run_bootc_image_build() sees the expected prerequisite.
🤖 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/ci_phase_iso_build.sh`:
- Around line 123-145: The script currently silently skips building bootc images
when CI_JOB_NAME's last token (variable os) isn't "el9" or "el10"; update the
CI_JOB_NAME handling to fail fast for unsupported OS values by validating the
derived os and exiting non‑zero with an error message if it isn't one of the
supported values. Locate the block that assigns local -r os="${CI_JOB_NAME##*-}"
and the conditional if [[ "${os}" == "el9" || "${os}" == "el10" ]]; then and add
an else branch that prints a clear error mentioning CI_JOB_NAME and os and calls
exit 1 (respecting dry_run behavior if necessary), so mismatched job names do
not report success while skipping build_bootc_images.sh invocations.
---
Duplicate comments:
In `@test/bin/ci_phase_iso_build.sh`:
- Around line 70-73: The cache warmup is using per-OS layer trees but
run_bootc_image_build() now requires the shared
./image-blueprints-bootc/templates; update update_build_cache() (or replace the
loop that calls build_bootc_images.sh per OS) to warm the shared templates
directory instead of ./image-blueprints-bootc/${os}/layer1-base and
layer4-release so the -update_cache step publishes the correct common bootc
artifacts; specifically, change the calls to invoke ./bin/build_bootc_images.sh
-l ./image-blueprints-bootc/templates (or add an explicit warm of
./image-blueprints-bootc/templates) and remove the per-OS layer warmups so
run_bootc_image_build() sees the expected prerequisite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c763a6d0-8c66-446e-8751-635876ed56e6
📒 Files selected for processing (2)
test/bin/ci_phase_iso_build.shtest/image-blueprints-bootc/el10/layer3-periodic/group2/rhel102-bootc-source-isolated.image-bootc
🚧 Files skipped from review as they are similar to previous changes (1)
- test/image-blueprints-bootc/el10/layer3-periodic/group2/rhel102-bootc-source-isolated.image-bootc
Comment on lines
+123
to
+145
| if [ -v CI_JOB_NAME ] ; then | ||
|
|
||
| if [[ "${CI_JOB_NAME}" =~ .*periodic.* ]]; then | ||
| $(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/layer3-periodic | ||
| local -r os="${CI_JOB_NAME##*-}" | ||
|
|
||
| if [[ "${os}" == "el9" || "${os}" == "el10" ]]; then | ||
|
|
||
| $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer1-base" | ||
| $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer2-presubmit" | ||
|
|
||
| if [[ "${os}" == "el10" ]]; then | ||
| # Build el9 images for upgrade tests | ||
| $(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/el9/layer1-base | ||
| $(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/el9/layer2-presubmit | ||
| fi | ||
|
|
||
| if [[ "${CI_JOB_NAME}" =~ .*periodic.* ]]; then | ||
| $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer3-periodic" | ||
| fi | ||
| if [[ "${CI_JOB_NAME}" =~ .*release.* ]]; then | ||
| $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer4-release" | ||
| fi | ||
| fi | ||
|
|
There was a problem hiding this comment.
Fail fast when CI_JOB_NAME does not map to a supported OS.
Lines 125-144 only build OS layers when the last job-name token is exactly el9 or el10. Any other bootc job name currently exits successfully after building templates only, which makes a naming mismatch look green while skipping the artifacts.
🤖 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` around lines 123 - 145, The script currently
silently skips building bootc images when CI_JOB_NAME's last token (variable os)
isn't "el9" or "el10"; update the CI_JOB_NAME handling to fail fast for
unsupported OS values by validating the derived os and exiting non‑zero with an
error message if it isn't one of the supported values. Locate the block that
assigns local -r os="${CI_JOB_NAME##*-}" and the conditional if [[ "${os}" ==
"el9" || "${os}" == "el10" ]]; then and add an else branch that prints a clear
error mentioning CI_JOB_NAME and os and calls exit 1 (respecting dry_run
behavior if necessary), so mismatched job names do not report success while
skipping build_bootc_images.sh invocations.
|
@vanhalenar: This pull request references USHIFT-6547 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 "4.22.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. |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
test/bin/ci_phase_iso_build.sh (2)
70-73:⚠️ Potential issue | 🟠 MajorWarm the bootc templates in
-update_cache.
run_bootc_image_build()now treats./image-blueprints-bootc/templatesas a prerequisite, butupdate_build_cache()still jumps straight to the OS layers.-update_cachecan therefore publish a bootc cache without the generated template artifacts.Suggested fix
# Build the bootc base layer and brew RPMs to be cached + $(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/templates for os in el9 el10; do $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer1-base" $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer4-release" done🤖 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` around lines 70 - 73, update_build_cache() currently skips generating bootc templates before publishing; ensure it "warms" the templates by invoking the same build step run_bootc_image_build() (or calling bash ./bin/build_bootc_images.sh for the ./image-blueprints-bootc/templates target) prior to running the OS layer builds and publishing cache so ./image-blueprints-bootc/templates is present; modify update_build_cache() to run the template generation (or call run_bootc_image_build with the templates path) before looping over el9/el10 layers and publishing.
123-149:⚠️ Potential issue | 🟠 MajorFail fast when
CI_JOB_NAMEdoes not resolve to a supported OS.If
${CI_JOB_NAME##*-}is anything other thanel9orel10, this function still builds templates, may build upstream, and exits 0. That makes a job-name drift look green while skipping all OS-specific bootc artifacts.Suggested fix
if [[ "${os}" == "el9" || "${os}" == "el10" ]]; then $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer1-base" $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer2-presubmit" @@ if [[ "${CI_JOB_NAME}" =~ .*release.* ]]; then $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer4-release" fi + else + echo "ERROR: Unsupported bootc OS derived from CI_JOB_NAME='${CI_JOB_NAME}': '${os}'" + exit 1 fi🤖 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` around lines 123 - 149, After extracting os="${CI_JOB_NAME##*-}", add a fail-fast check that verifies os is either "el9" or "el10": if it is not, print a clear error mentioning CI_JOB_NAME and the unresolved os and exit with a non-zero status (e.g., exit 1) so the job fails rather than silently skipping OS-specific builds; place this check before any calls that invoke build_bootc_images.sh (the blocks that use $(dry_run) bash -x ./bin/build_bootc_images.sh) so upstream or other steps do not proceed when the OS is unsupported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/bin/ci_phase_iso_build.sh`:
- Around line 70-73: update_build_cache() currently skips generating bootc
templates before publishing; ensure it "warms" the templates by invoking the
same build step run_bootc_image_build() (or calling bash
./bin/build_bootc_images.sh for the ./image-blueprints-bootc/templates target)
prior to running the OS layer builds and publishing cache so
./image-blueprints-bootc/templates is present; modify update_build_cache() to
run the template generation (or call run_bootc_image_build with the templates
path) before looping over el9/el10 layers and publishing.
- Around line 123-149: After extracting os="${CI_JOB_NAME##*-}", add a fail-fast
check that verifies os is either "el9" or "el10": if it is not, print a clear
error mentioning CI_JOB_NAME and the unresolved os and exit with a non-zero
status (e.g., exit 1) so the job fails rather than silently skipping OS-specific
builds; place this check before any calls that invoke build_bootc_images.sh (the
blocks that use $(dry_run) bash -x ./bin/build_bootc_images.sh) so upstream or
other steps do not proceed when the OS is unsupported.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 63f727df-e091-4959-b38b-91e8aa35b25c
📒 Files selected for processing (132)
test/bin/ci_phase_iso_build.shtest/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/layer2-presubmit/group1/rhel102-bootc-source.containerfiletest/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-optionals.containerfiletest/image-blueprints-bootc/el10/layer3-periodic/group1/rhel102-bootc-source-isolated.containerfiletest/image-blueprints-bootc/el10/layer3-periodic/group2/rhel102-bootc-source-isolated.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.containerfiletest/image-blueprints-bootc/el10/layer4-release/group2/rhel102-bootc-brew-lrel-optional.image-bootctest/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/layer2-presubmit/group1/rhel98-bootc-source-base.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group1/rhel98-bootc-source.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-aux.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-fake-next-minor.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-fips.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-optionals.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group1/rhel98-bootc-source-ai-model-serving.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group1/rhel98-bootc-source-isolated.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group2/rhel98-bootc-source-ai-model-serving.image-bootctest/image-blueprints-bootc/el9/layer3-periodic/group2/rhel98-bootc-source-gitops.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group2/rhel98-bootc-source-isolated.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.containerfiletest/image-blueprints-bootc/templates/ai-model-serving-test-data.sh.templatetest/image-blueprints-bootc/templates/build-serialsim.sh.templatetest/image-blueprints-bootc/templates/microshift-copy-images.conf.templatetest/image-blueprints-bootc/templates/microshift-copy-images.sh.templatetest/image-blueprints-bootc/templates/microshift-ovsdb-ownership.conf.templatetest/image-blueprints-bootc/templates/rpm-repo-config.sh.templatetest/image-blueprints-bootc/upstream/group1/cos10-test-agent.containerfiletest/image-blueprints-bootc/upstream/group1/cos9-test-agent.containerfiletest/image-blueprints-bootc/upstream/group2/centos10-bootc.image-bootctest/image-blueprints-bootc/upstream/group2/centos9-bootc.image-bootctest/image-blueprints-bootc/upstream/group2/cos10-bootc-source.containerfiletest/image-blueprints-bootc/upstream/group2/cos9-bootc-source.containerfiletest/image-blueprints-bootc/upstream/group3/cos10-bootc-source-isolated.containerfiletest/image-blueprints-bootc/upstream/group3/cos10-bootc-source-optionals.containerfiletest/image-blueprints-bootc/upstream/group3/cos9-bootc-source-isolated.containerfiletest/image-blueprints-bootc/upstream/group3/cos9-bootc-source-optionals.containerfiletest/scenarios-bootc/el10/periodics/el102-src@optional.shtest/scenarios-bootc/el10/periodics/el102-src@standard-suite1.shtest/scenarios-bootc/el10/periodics/el102-src@standard-suite2.shtest/scenarios-bootc/el10/periodics/el96-prel@el102-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el10/periodics/el96-yminus2@el102-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el10/periodics/el98-src@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/presubmits/el96-prel@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/presubmits/el96-yminus2@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/presubmits/el98-src@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/releases/el102-lrel@ginkgo-tests.shtest/scenarios-bootc/el10/releases/el102-lrel@optional.shtest/scenarios-bootc/el10/releases/el102-lrel@standard1.shtest/scenarios-bootc/el10/releases/el102-lrel@standard2.shtest/scenarios-bootc/el9/periodics/el96-prel@el98-src@upgrade-fails-and-rolls-back.shtest/scenarios-bootc/el9/periodics/el96-prel@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/periodics/el96-prel@el98-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el9/periodics/el96-yminus2@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/periodics/el96-yminus2@el98-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el9/periodics/el98-src@ai-model-serving-offline.shtest/scenarios-bootc/el9/periodics/el98-src@cncf-conformance.shtest/scenarios-bootc/el9/periodics/el98-src@fault-tests-and-greenboot.shtest/scenarios-bootc/el9/periodics/el98-src@fips.shtest/scenarios-bootc/el9/periodics/el98-src@gitops-telemetry-clusterid-systemd.shtest/scenarios-bootc/el9/periodics/el98-src@isolated-net.shtest/scenarios-bootc/el9/periodics/el98-src@offline.shtest/scenarios-bootc/el9/periodics/el98-src@osconfig-cleanup-data.shtest/scenarios-bootc/el9/periodics/el98-src@osconfig-lifecycle.shtest/scenarios-bootc/el9/periodics/el98-src@upgrade-fails-cannot-backup.shtest/scenarios-bootc/el9/presubmits/el96-prel@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el96-yminus2@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el96-yminus2@prel@src@delta-upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el98-base@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el98-base@src@opt@delta-upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el98-src@ai-model-serving-online.shtest/scenarios-bootc/el9/presubmits/el98-src@auto-recovery.shtest/scenarios-bootc/el9/presubmits/el98-src@backup-and-restore-on-reboot.shtest/scenarios-bootc/el9/presubmits/el98-src@backups.shtest/scenarios-bootc/el9/presubmits/el98-src@configuration.shtest/scenarios-bootc/el9/presubmits/el98-src@downgrade-block.shtest/scenarios-bootc/el9/presubmits/el98-src@dual-stack.shtest/scenarios-bootc/el9/presubmits/el98-src@ipv6.shtest/scenarios-bootc/el9/presubmits/el98-src@multi-nic.shtest/scenarios-bootc/el9/presubmits/el98-src@optional.shtest/scenarios-bootc/el9/presubmits/el98-src@router.shtest/scenarios-bootc/el9/presubmits/el98-src@standard-suite1.shtest/scenarios-bootc/el9/presubmits/el98-src@standard-suite2.shtest/scenarios-bootc/el9/presubmits/el98-src@storage.shtest/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.sh.disabledtest/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails.shtest/scenarios-bootc/el9/releases/el96-lrel@published-images-standard1.shtest/scenarios-bootc/el9/releases/el96-lrel@published-images-standard2.shtest/scenarios-bootc/el9/releases/el96-y1@el98-lrel@lvms.shtest/scenarios-bootc/el9/releases/el96-y1@el98-lrel@standard1.shtest/scenarios-bootc/el9/releases/el96-y1@el98-lrel@standard2.shtest/scenarios-bootc/el9/releases/el96-y2@el98-lrel@lvms.shtest/scenarios-bootc/el9/releases/el96-y2@el98-lrel@standard1.shtest/scenarios-bootc/el9/releases/el96-y2@el98-lrel@standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@ai-model-serving-online.shtest/scenarios-bootc/el9/releases/el98-lrel@configuration.shtest/scenarios-bootc/el9/releases/el98-lrel@dual-stack.shtest/scenarios-bootc/el9/releases/el98-lrel@fips.shtest/scenarios-bootc/el9/releases/el98-lrel@ginkgo-tests.shtest/scenarios-bootc/el9/releases/el98-lrel@ipv6.shtest/scenarios-bootc/el9/releases/el98-lrel@iso-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@iso-standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@low-latency.shtest/scenarios-bootc/el9/releases/el98-lrel@multi-config-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@multi-config-standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@multi-nic.shtest/scenarios-bootc/el9/releases/el98-lrel@nightly-brew-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@nightly-brew-standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@optional.shtest/scenarios-bootc/el9/releases/el98-lrel@osconfig-router.shtest/scenarios-bootc/el9/releases/el98-lrel@storage-telemetry.shtest/scenarios-bootc/el9/releases/el98-lrel@tlsv13-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@tlsv13-standard2.sh
|
@vanhalenar: This pull request references USHIFT-6547 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 "4.22.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. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/bin/ci_phase_iso_build.sh (1)
126-147:⚠️ Potential issue | 🟠 MajorFail fast for unsupported
CI_JOB_NAMEOS suffixes.If
${CI_JOB_NAME##*-}is anything other thanel9orel10, this function currently succeeds after building templates only. That makes a naming mismatch look green while skipping the real bootc artifacts.Suggested fix
if [[ "${os}" == "el9" || "${os}" == "el10" ]]; then @@ if [[ "${CI_JOB_NAME}" =~ .*release.* ]]; then $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer4-release" fi + else + echo "ERROR: Unsupported bootc OS suffix '${os}' derived from CI_JOB_NAME='${CI_JOB_NAME}'" + exit 1 fi🤖 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` around lines 126 - 147, The current branch that checks CI_JOB_NAME and derives os (local -r os="${CI_JOB_NAME##*-}") only handles el9/el10 and silently continues for other suffixes; add a fail-fast path inside the if [ -v CI_JOB_NAME ] block after computing os: if os is not one of "el9" or "el10" log a clear error including the CI_JOB_NAME and os values (e.g. via echo or the repo’s logger) and exit non‑zero (exit 1) so unsupported suffixes fail the job instead of silently succeeding; update the block around the os check (the conditional that currently tests [[ "${os}" == "el9" || "${os}" == "el10" ]]) to include this else branch.
🤖 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/ci_phase_iso_build.sh`:
- Around line 25-26: The script currently unconditionally overrides
AWS_BUCKET_NAME (the export in ci_phase_iso_build.sh) which forces all CI runs
to use the test cache; remove that hard-coded override or replace it with a
conditional default so the caller-provided AWS_BUCKET_NAME is preserved (i.e.,
only set a fallback when AWS_BUCKET_NAME is empty) and update the export line
that references AWS_BUCKET_NAME accordingly so CI callers can control the
bucket.
---
Duplicate comments:
In `@test/bin/ci_phase_iso_build.sh`:
- Around line 126-147: The current branch that checks CI_JOB_NAME and derives os
(local -r os="${CI_JOB_NAME##*-}") only handles el9/el10 and silently continues
for other suffixes; add a fail-fast path inside the if [ -v CI_JOB_NAME ] block
after computing os: if os is not one of "el9" or "el10" log a clear error
including the CI_JOB_NAME and os values (e.g. via echo or the repo’s logger) and
exit non‑zero (exit 1) so unsupported suffixes fail the job instead of silently
succeeding; update the block around the os check (the conditional that currently
tests [[ "${os}" == "el9" || "${os}" == "el10" ]]) to include this else branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 2f5c05a4-3ffb-4f50-95a9-3fd2c46a72d9
📒 Files selected for processing (1)
test/bin/ci_phase_iso_build.sh
openshift-merge-robot
added
the
needs-rebase
openshift-merge-robot
removed
the
needs-rebase
|
@vanhalenar: This pull request references USHIFT-6547 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 "4.22.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. |
There was a problem hiding this comment.
♻️ Duplicate comments (3)
test/bin/ci_phase_iso_build.sh (2)
69-73:⚠️ Potential issue | 🟠 MajorWarm the bootc templates before caching OS layers.
run_bootc_image_build()now depends on./image-blueprints-bootc/templates, butupdate_build_cache()still skips that step.-update_cachecan therefore warm an incomplete bootc cache.Suggested fix
# Build the bootc base layer and brew RPMs to be cached + $(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/templates for os in el9 el10; do $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer1-base" $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer4-release" done🤖 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` around lines 69 - 73, The update_build_cache() path currently skips warming ./image-blueprints-bootc/templates so -update_cache can produce an incomplete bootc cache; modify update_build_cache() to warm the bootc templates before caching OS layers by invoking the same template warm-up used by run_bootc_image_build() (or by calling ./bin/build_bootc_images.sh for the templates layer), ensuring the templates under ./image-blueprints-bootc/templates are created/populated prior to the for-loop that builds layer1-base and layer4-release; reference run_bootc_image_build(), update_build_cache(), and ./bin/build_bootc_images.sh when making the change.
125-144:⚠️ Potential issue | 🟠 MajorFail fast when
CI_JOB_NAMEdoes not map to a supported OS.If the last job-name token is anything other than
el9orel10, this path silently skips the required layer builds and still reports success.Suggested fix
if [[ "${os}" == "el9" || "${os}" == "el10" ]]; then $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer1-base" $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer2-presubmit" if [[ "${os}" == "el10" ]]; then # Build el9 images for upgrade tests $(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/el9/layer1-base $(dry_run) bash -x ./bin/build_bootc_images.sh -l ./image-blueprints-bootc/el9/layer2-presubmit fi if [[ "${CI_JOB_NAME}" =~ .*periodic.* ]]; then $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer3-periodic" fi if [[ "${CI_JOB_NAME}" =~ .*release.* ]]; then $(dry_run) bash -x ./bin/build_bootc_images.sh -l "./image-blueprints-bootc/${os}/layer4-release" fi + else + echo "ERROR: Unsupported bootc OS suffix '${os}' derived from CI_JOB_NAME='${CI_JOB_NAME}'" + exit 1 fi🤖 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` around lines 125 - 144, The code extracts os from CI_JOB_NAME into the local -r os variable and silently skips builds when os is not "el9" or "el10"; add a fail-fast check right after computing local -r os="${CI_JOB_NAME##*-}" that validates os is either "el9" or "el10" and if not prints a clear error to stderr (e.g. echo "Unsupported OS token: ${os} from ${CI_JOB_NAME}" >&2) and exits non-zero (exit 1) so the build_bootc_images.sh calls are not skipped silently; update any callers expecting this script to return non-zero on unsupported OS.test/bin/manage_build_cache.sh (1)
10-11:⚠️ Potential issue | 🟠 MajorRestore the environment-based bucket selection.
This hard-codes every cache read/write/delete to a personal test bucket and ignores the caller-provided
AWS_BUCKET_NAME. That makes cache behavior misleading and can mutate the wrong S3 namespace.Suggested fix
-#AWS_BUCKET_NAME="${AWS_BUCKET_NAME:-microshift-build-cache}" -AWS_BUCKET_NAME="thalenar-test-cache-us-west-2" +AWS_BUCKET_NAME="${AWS_BUCKET_NAME:-microshift-build-cache}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/bin/manage_build_cache.sh` around lines 10 - 11, Replace the hard-coded AWS_BUCKET_NAME assignment that forces "thalenar-test-cache-us-west-2" with the environment-driven default using shell parameter expansion (restore AWS_BUCKET_NAME="${AWS_BUCKET_NAME:-microshift-build-cache}") in manage_build_cache.sh so callers can override AWS_BUCKET_NAME; remove the fixed literal assignment and keep or add a comment explaining the default.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/bin/ci_phase_iso_build.sh`:
- Around line 69-73: The update_build_cache() path currently skips warming
./image-blueprints-bootc/templates so -update_cache can produce an incomplete
bootc cache; modify update_build_cache() to warm the bootc templates before
caching OS layers by invoking the same template warm-up used by
run_bootc_image_build() (or by calling ./bin/build_bootc_images.sh for the
templates layer), ensuring the templates under
./image-blueprints-bootc/templates are created/populated prior to the for-loop
that builds layer1-base and layer4-release; reference run_bootc_image_build(),
update_build_cache(), and ./bin/build_bootc_images.sh when making the change.
- Around line 125-144: The code extracts os from CI_JOB_NAME into the local -r
os variable and silently skips builds when os is not "el9" or "el10"; add a
fail-fast check right after computing local -r os="${CI_JOB_NAME##*-}" that
validates os is either "el9" or "el10" and if not prints a clear error to stderr
(e.g. echo "Unsupported OS token: ${os} from ${CI_JOB_NAME}" >&2) and exits
non-zero (exit 1) so the build_bootc_images.sh calls are not skipped silently;
update any callers expecting this script to return non-zero on unsupported OS.
In `@test/bin/manage_build_cache.sh`:
- Around line 10-11: Replace the hard-coded AWS_BUCKET_NAME assignment that
forces "thalenar-test-cache-us-west-2" with the environment-driven default using
shell parameter expansion (restore
AWS_BUCKET_NAME="${AWS_BUCKET_NAME:-microshift-build-cache}") in
manage_build_cache.sh so callers can override AWS_BUCKET_NAME; remove the fixed
literal assignment and keep or add a comment explaining the default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4ae65279-a949-4d08-90bb-86a503ad5a91
📒 Files selected for processing (133)
test/bin/ci_phase_iso_build.shtest/bin/manage_build_cache.shtest/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/layer2-presubmit/group1/rhel102-bootc-source.containerfiletest/image-blueprints-bootc/el10/layer2-presubmit/group2/rhel102-bootc-source-optionals.containerfiletest/image-blueprints-bootc/el10/layer3-periodic/group1/rhel102-bootc-source-isolated.containerfiletest/image-blueprints-bootc/el10/layer3-periodic/group2/rhel102-bootc-source-isolated.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.containerfiletest/image-blueprints-bootc/el10/layer4-release/group2/rhel102-bootc-brew-lrel-optional.image-bootctest/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/layer2-presubmit/group1/rhel98-bootc-source-base.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group1/rhel98-bootc-source.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-aux.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-fake-next-minor.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-fips.containerfiletest/image-blueprints-bootc/el9/layer2-presubmit/group2/rhel98-bootc-source-optionals.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group1/rhel98-bootc-source-ai-model-serving.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group1/rhel98-bootc-source-isolated.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group2/rhel98-bootc-source-ai-model-serving.image-bootctest/image-blueprints-bootc/el9/layer3-periodic/group2/rhel98-bootc-source-gitops.containerfiletest/image-blueprints-bootc/el9/layer3-periodic/group2/rhel98-bootc-source-isolated.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.containerfiletest/image-blueprints-bootc/templates/ai-model-serving-test-data.sh.templatetest/image-blueprints-bootc/templates/build-serialsim.sh.templatetest/image-blueprints-bootc/templates/microshift-copy-images.conf.templatetest/image-blueprints-bootc/templates/microshift-copy-images.sh.templatetest/image-blueprints-bootc/templates/microshift-ovsdb-ownership.conf.templatetest/image-blueprints-bootc/templates/rpm-repo-config.sh.templatetest/image-blueprints-bootc/upstream/group1/cos10-test-agent.containerfiletest/image-blueprints-bootc/upstream/group1/cos9-test-agent.containerfiletest/image-blueprints-bootc/upstream/group2/centos10-bootc.image-bootctest/image-blueprints-bootc/upstream/group2/centos9-bootc.image-bootctest/image-blueprints-bootc/upstream/group2/cos10-bootc-source.containerfiletest/image-blueprints-bootc/upstream/group2/cos9-bootc-source.containerfiletest/image-blueprints-bootc/upstream/group3/cos10-bootc-source-isolated.containerfiletest/image-blueprints-bootc/upstream/group3/cos10-bootc-source-optionals.containerfiletest/image-blueprints-bootc/upstream/group3/cos9-bootc-source-isolated.containerfiletest/image-blueprints-bootc/upstream/group3/cos9-bootc-source-optionals.containerfiletest/scenarios-bootc/el10/periodics/el102-src@optional.shtest/scenarios-bootc/el10/periodics/el102-src@standard-suite1.shtest/scenarios-bootc/el10/periodics/el102-src@standard-suite2.shtest/scenarios-bootc/el10/periodics/el96-prel@el102-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el10/periodics/el96-yminus2@el102-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el10/periodics/el98-src@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/presubmits/el96-prel@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/presubmits/el96-yminus2@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/presubmits/el98-src@el102-src@upgrade-ok.shtest/scenarios-bootc/el10/releases/el102-lrel@ginkgo-tests.shtest/scenarios-bootc/el10/releases/el102-lrel@optional.shtest/scenarios-bootc/el10/releases/el102-lrel@standard1.shtest/scenarios-bootc/el10/releases/el102-lrel@standard2.shtest/scenarios-bootc/el9/periodics/el96-prel@el98-src@upgrade-fails-and-rolls-back.shtest/scenarios-bootc/el9/periodics/el96-prel@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/periodics/el96-prel@el98-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el9/periodics/el96-yminus2@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/periodics/el96-yminus2@el98-src@upgrade-ostree2bootc-ok.shtest/scenarios-bootc/el9/periodics/el98-src@ai-model-serving-offline.shtest/scenarios-bootc/el9/periodics/el98-src@cncf-conformance.shtest/scenarios-bootc/el9/periodics/el98-src@fault-tests-and-greenboot.shtest/scenarios-bootc/el9/periodics/el98-src@fips.shtest/scenarios-bootc/el9/periodics/el98-src@gitops-telemetry-clusterid-systemd.shtest/scenarios-bootc/el9/periodics/el98-src@isolated-net.shtest/scenarios-bootc/el9/periodics/el98-src@offline.shtest/scenarios-bootc/el9/periodics/el98-src@osconfig-cleanup-data.shtest/scenarios-bootc/el9/periodics/el98-src@osconfig-lifecycle.shtest/scenarios-bootc/el9/periodics/el98-src@upgrade-fails-cannot-backup.shtest/scenarios-bootc/el9/presubmits/el96-prel@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el96-yminus2@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el96-yminus2@prel@src@delta-upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el98-base@el98-src@upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el98-base@src@opt@delta-upgrade-ok.shtest/scenarios-bootc/el9/presubmits/el98-src@ai-model-serving-online.shtest/scenarios-bootc/el9/presubmits/el98-src@auto-recovery.shtest/scenarios-bootc/el9/presubmits/el98-src@backup-and-restore-on-reboot.shtest/scenarios-bootc/el9/presubmits/el98-src@backups.shtest/scenarios-bootc/el9/presubmits/el98-src@configuration.shtest/scenarios-bootc/el9/presubmits/el98-src@downgrade-block.shtest/scenarios-bootc/el9/presubmits/el98-src@dual-stack.shtest/scenarios-bootc/el9/presubmits/el98-src@ipv6.shtest/scenarios-bootc/el9/presubmits/el98-src@multi-nic.shtest/scenarios-bootc/el9/presubmits/el98-src@optional.shtest/scenarios-bootc/el9/presubmits/el98-src@router.shtest/scenarios-bootc/el9/presubmits/el98-src@standard-suite1.shtest/scenarios-bootc/el9/presubmits/el98-src@standard-suite2.shtest/scenarios-bootc/el9/presubmits/el98-src@storage.shtest/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails-then-recovers.sh.disabledtest/scenarios-bootc/el9/presubmits/el98-src@upgrade-fails.shtest/scenarios-bootc/el9/releases/el96-lrel@published-images-standard1.shtest/scenarios-bootc/el9/releases/el96-lrel@published-images-standard2.shtest/scenarios-bootc/el9/releases/el96-y1@el98-lrel@lvms.shtest/scenarios-bootc/el9/releases/el96-y1@el98-lrel@standard1.shtest/scenarios-bootc/el9/releases/el96-y1@el98-lrel@standard2.shtest/scenarios-bootc/el9/releases/el96-y2@el98-lrel@lvms.shtest/scenarios-bootc/el9/releases/el96-y2@el98-lrel@standard1.shtest/scenarios-bootc/el9/releases/el96-y2@el98-lrel@standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@ai-model-serving-online.shtest/scenarios-bootc/el9/releases/el98-lrel@configuration.shtest/scenarios-bootc/el9/releases/el98-lrel@dual-stack.shtest/scenarios-bootc/el9/releases/el98-lrel@fips.shtest/scenarios-bootc/el9/releases/el98-lrel@ginkgo-tests.shtest/scenarios-bootc/el9/releases/el98-lrel@ipv6.shtest/scenarios-bootc/el9/releases/el98-lrel@iso-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@iso-standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@low-latency.shtest/scenarios-bootc/el9/releases/el98-lrel@multi-config-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@multi-config-standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@multi-nic.shtest/scenarios-bootc/el9/releases/el98-lrel@nightly-brew-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@nightly-brew-standard2.shtest/scenarios-bootc/el9/releases/el98-lrel@optional.shtest/scenarios-bootc/el9/releases/el98-lrel@osconfig-router.shtest/scenarios-bootc/el9/releases/el98-lrel@storage-telemetry.shtest/scenarios-bootc/el9/releases/el98-lrel@tlsv13-standard1.shtest/scenarios-bootc/el9/releases/el98-lrel@tlsv13-standard2.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- test/image-blueprints-bootc/el10/layer3-periodic/group2/rhel102-bootc-source-isolated.image-bootc
- test/image-blueprints-bootc/el10/layer4-release/group2/rhel102-bootc-brew-lrel-optional.image-bootc
vanhalenar
force-pushed
the
dual-lane
branch
2 times, most recently
from
3a92406 to
86dabe9
Compare
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
3 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.
i accidentally deleted this PR before when moving branches around, adding it again. includes restructuring of bootc image blueprints and scenarios into two categories based on RHEL ver. the related PR in release repo is here.
Summary by CodeRabbit
New Features
Chores
Tests