AGENT-1577: Use master branch of appliance as image builder - #333
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@zaneb: This pull request references AGENT-1577 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. |
WalkthroughThe ISO builder now uses ChangesISO Builder Image Update
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zaneb 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/iso_builder/Dockerfile (1)
2-30: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftAdd a separate final stage.
AS builderon Line 2 does not create a multi-stage build. The builder becomes the final image and retains build tools,/openshift-appliance, and the generatedappliance-config.yaml. The configuration is created from the mounted pull secret, and the cleanup commands do not remove it.Add a final stage that copies only the generated ISO and runs as a non-root user.
As per path instructions, Dockerfiles must use multi-stage builds and must not keep build tools or secrets in the final image.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/iso_builder/Dockerfile` around lines 2 - 30, Add a separate final stage after the existing builder stage, copying only the generated ISO from builder and configuring a non-root runtime user. Ensure the final image excludes build tools, /openshift-appliance, appliance-config.yaml, and any secret-derived files, while preserving the ISO output.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tools/iso_builder/Dockerfile`:
- Around line 2-30: Add a separate final stage after the existing builder stage,
copying only the generated ISO from builder and configuring a non-root runtime
user. Ensure the final image excludes build tools, /openshift-appliance,
appliance-config.yaml, and any secret-derived files, while preserving the ISO
output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 681f43aa-64de-42a9-95dc-f81855d71215
📒 Files selected for processing (3)
tools/iso_builder/CLAUDE.mdtools/iso_builder/Dockerfiletools/iso_builder/hack/build-ove-image.sh
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
CI config merged, so this should be good to go. |
|
@zaneb We haven't really been maintaining the Konflux tekton files for the master branch since we only release out of stable branches, its still using 4.22 here to build an ISO https://github.com/openshift/agent-installer-utils/blob/main/.tekton/ove-ui-iso-pull-request.yaml#L43-L45. As a result the Konflux build fails with: I assume that this image is not available yet in 4.22? We should probably:
That will avoid us having to override this test with each PR. |
|
I haven't changed the CI image name on branches prior to 5.1 because that would mess up builds on release-* branches. So yeah, we need master building using the master openshift/appliance image. I'm not completely clear on how this is set up - it looks like the tekton config is generally managed by Konflux itself? |
|
/override "Red Hat Konflux kflux-prd-rh02 / Red Hat Konflux / ove-ui-enterprise-contract / ove-ui-iso" |
|
@bfournie: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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. |
We are in the process of migrating the Konflux builds to the ART namespace and these .texton files will no longer be maintained. Until then we'll get Konflux build failures since the appliance release is named agent-release-iso-builder in the main branch only, see #333. This is an attempt to get the Konflux builds to pass for now using a nightly release since we currently don't have any stable releases for 5.1.
|
/retest |
1 similar comment
|
/retest |
|
@zaneb Even though the Konflux files were updated to use 5.1 instead of 4.22, the I think you just need to rebase this PR to pick up the .tekton change for this version. |
Switch to the agent-release-iso-builder CI image that is built from the current OCP release branch. This replaces agent-preinstall-image-builder, which is now built from the branch of the same name and does not use stable branches.
e17ddff to
16743ca
Compare
|
That seemed to do the trick, thanks Bob. |
|
/lgtm |
|
Scheduling tests matching the |
|
/test ? |
|
/test e2e-agent-compact-ipv4-iso-no-registry |
|
@zaneb: 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. |
|
/verified by ci/prow/e2e-agent-compact-ipv4-iso-no-registry |
openshift-ci-robot
added
the
verified
|
@bfournie: 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. |
3 participants
Switch to the agent-release-iso-builder CI image that is built from the current OCP release branch.
This replaces agent-preinstall-image-builder, which is now built from the branch of the same name and does not use stable branches.
This depends on openshift/release#83022
Summary by CodeRabbit