openshift/os: update ci-operator configs for build-args changes#78212
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 59 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughUpdated CI operator configuration files for OpenShift OS image builds. Added new build arguments ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 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 |
In openshift/os#1919, we're changing the openshift/os Containerfile to use ARG IMAGE_FROM=overridden instead of a hardcoded FROM, and to require IMAGE_NAME/IMAGE_CPE build args for RHCOS builds. So we need to adapt the `as` substitution here accordingly. Unfortunately, ci-operator (but really, Builds v1) doesn't support build args files so here we're just hardcoding values for the parameter for CI to build. This is something we'd be able to fix once CI also uses Konflux. Yes, the literal `${IMAGE_FROM}` is intended because the substitution needs to match against the raw string in the Dockerfile AIUI. Keep the old value too so that we can merge this PR and not break openshift/os CI before merging openshift/os#1919. Assisted-by: OpenCode (Claude Opus 4.6)
jlebon
force-pushed
the
pr/openshift-os-build-args
branch
from
a0f1466 to
0f8bfa0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/config/openshift/os/openshift-os-master.yaml (1)
16-21: Nit: comment placement inside thebuild_argssequence.The explanatory comment on lines 16-17 sits between the first and second entries of the
build_argslist. It reads naturally as applying toIMAGE_NAME/IMAGE_CPEspecifically (which is the intent), but some YAML tooling/formatters may relocate or strip comments embedded mid-sequence. Consider either placing it directly above- name: IMAGE_NAMEwith matching list-item indentation, or inlining it on each entry, to reduce the chance of it getting detached on future regenerations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/config/openshift/os/openshift-os-master.yaml` around lines 16 - 21, The comment explaining the source-of-truth for the build args is currently inside the build_args sequence between items and may be lost by YAML tooling; move that comment so it sits directly above the "- name: IMAGE_NAME" list item with the same list-item indentation (or alternatively convert it to inline comments on the IMAGE_NAME and IMAGE_CPE entries), ensuring the comment remains adjacent to the IMAGE_NAME and IMAGE_CPE entries and preserves indentation so formatters won't detach it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ci-operator/config/openshift/os/openshift-os-master.yaml`:
- Around line 16-21: The comment explaining the source-of-truth for the build
args is currently inside the build_args sequence between items and may be lost
by YAML tooling; move that comment so it sits directly above the "- name:
IMAGE_NAME" list item with the same list-item indentation (or alternatively
convert it to inline comments on the IMAGE_NAME and IMAGE_CPE entries), ensuring
the comment remains adjacent to the IMAGE_NAME and IMAGE_CPE entries and
preserves indentation so formatters won't detach it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 18ccde7f-9d01-4c95-b172-d89c4695a6bb
📒 Files selected for processing (2)
ci-operator/config/openshift/os/openshift-os-master.yamlci-operator/config/openshift/os/openshift-os-master__okd-scos.yaml
openshift-ci
Bot
added
the
approved
|
[REHEARSALNOTIFIER]
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@jlebon: 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. |
|
/lgtm A lot of nice context in that git commit message. Part of me wonders if some of that should live in the config file too. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dustymabe, jlebon 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 |
I initially had that, but |
|
/pj-rehearse ack |
|
@jlebon: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
…shift#78212) In openshift/os#1919, we're changing the openshift/os Containerfile to use ARG IMAGE_FROM=overridden instead of a hardcoded FROM, and to require IMAGE_NAME/IMAGE_CPE build args for RHCOS builds. So we need to adapt the `as` substitution here accordingly. Unfortunately, ci-operator (but really, Builds v1) doesn't support build args files so here we're just hardcoding values for the parameter for CI to build. This is something we'd be able to fix once CI also uses Konflux. Yes, the literal `${IMAGE_FROM}` is intended because the substitution needs to match against the raw string in the Dockerfile AIUI. Keep the old value too so that we can merge this PR and not break openshift/os CI before merging openshift/os#1919. Assisted-by: OpenCode (Claude Opus 4.6)
…shift#78212) In openshift/os#1919, we're changing the openshift/os Containerfile to use ARG IMAGE_FROM=overridden instead of a hardcoded FROM, and to require IMAGE_NAME/IMAGE_CPE build args for RHCOS builds. So we need to adapt the `as` substitution here accordingly. Unfortunately, ci-operator (but really, Builds v1) doesn't support build args files so here we're just hardcoding values for the parameter for CI to build. This is something we'd be able to fix once CI also uses Konflux. Yes, the literal `${IMAGE_FROM}` is intended because the substitution needs to match against the raw string in the Dockerfile AIUI. Keep the old value too so that we can merge this PR and not break openshift/os CI before merging openshift/os#1919. Assisted-by: OpenCode (Claude Opus 4.6)
…shift#78212) In openshift/os#1919, we're changing the openshift/os Containerfile to use ARG IMAGE_FROM=overridden instead of a hardcoded FROM, and to require IMAGE_NAME/IMAGE_CPE build args for RHCOS builds. So we need to adapt the `as` substitution here accordingly. Unfortunately, ci-operator (but really, Builds v1) doesn't support build args files so here we're just hardcoding values for the parameter for CI to build. This is something we'd be able to fix once CI also uses Konflux. Yes, the literal `${IMAGE_FROM}` is intended because the substitution needs to match against the raw string in the Dockerfile AIUI. Keep the old value too so that we can merge this PR and not break openshift/os CI before merging openshift/os#1919. Assisted-by: OpenCode (Claude Opus 4.6)
2 participants
In openshift/os#1919, we're changing the openshift/os Containerfile to use ARG IMAGE_FROM=overridden instead of a hardcoded FROM, and to require IMAGE_NAME/IMAGE_CPE build args for RHCOS builds.
So we need to adapt the
assubstitution here accordingly.Unfortunately, ci-operator (but really, Builds v1) doesn't support build args files so here we're just hardcoding values for the parameter for CI to build. This is something we'd be able to fix once CI also uses Konflux.
Yes, the literal
${IMAGE_FROM}is intended because the substitution needs to match against the raw string in the Dockerfile AIUI.Keep the old value too so that we can merge this
PR and not break openshift/os CI before merging
openshift/os#1919.
Assisted-by: OpenCode (Claude Opus 4.6)
Summary by CodeRabbit