OCM-24478 | ci: add ROSA changelog tag job#79632
Conversation
WalkthroughA postsubmit Prow job ChangesChangelog History Job
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 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)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: olucasfreitas 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
openshift-merge-bot
Bot
added
the
rehearsals-ack
![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/rosa/openshift-rosa-master.yaml (1)
66-68: ⚖️ Poor tradeoffConsider ensuring jq is available in the base image rather than installing at runtime.
Installing packages at runtime via
dnfadds latency to every job execution and creates an external dependency on package repositories. Best practice for CI jobs is to ensure required tools are present in the base container image.Consider one of these approaches:
- Request that jq be added to the
rhel-9-golang-1.25-openshift-4.22builder image- Use a different base image that includes jq
- Accept this pattern if the base image is outside your control and resilience to image changes is preferred
🤖 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 `@ci-operator/config/openshift/rosa/openshift-rosa-master.yaml` around lines 66 - 68, The current runtime-install snippet (the if block checking `command -v jq` and running `dnf install -y jq`) should be removed and replaced by ensuring jq is present in the base image; either request `jq` be added to the `rhel-9-golang-1.25-openshift-4.22` builder image, change the job's base image to one that already includes `jq`, or explicitly bake `jq` into your project image/Dockerfile used by this CI job so the `if ! command -v jq` / `dnf install -y jq` fallback is not required at runtime.
🤖 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.
Nitpick comments:
In `@ci-operator/config/openshift/rosa/openshift-rosa-master.yaml`:
- Around line 66-68: The current runtime-install snippet (the if block checking
`command -v jq` and running `dnf install -y jq`) should be removed and replaced
by ensuring jq is present in the base image; either request `jq` be added to the
`rhel-9-golang-1.25-openshift-4.22` builder image, change the job's base image
to one that already includes `jq`, or explicitly bake `jq` into your project
image/Dockerfile used by this CI job so the `if ! command -v jq` / `dnf install
-y jq` fallback is not required at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 07c364e4-4de0-4e7a-a2ce-43167c754a21
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/rosa/openshift-rosa-master-postsubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/openshift/rosa/openshift-rosa-master.yaml
olucasfreitas
force-pushed
the
OCM-24478-git-cliff-changelog
branch
from
5b7cd92 to
874424d
Compare
|
/retest-required |
Add a changelog-history postsubmit test to the ROSA ci-operator config so Prow runs the changelog PR helper after merges to master. The job file is fully generated by ci-operator-prowgen; no hand-edited job stanzas are included. Tag-triggered execution is deferred to a follow-up once the repo-side changelog tooling is validated on master merges first.
olucasfreitas
force-pushed
the
OCM-24478-git-cliff-changelog
branch
from
874424d to
116c7c5
Compare
|
[REHEARSALNOTIFIER] Note: If this PR includes changes to step registry files ( Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@ci-operator/config/openshift/rosa/openshift-rosa-master.yaml`:
- Around line 66-68: The script currently attempts to install jq with "dnf
install -y jq" but doesn’t check the install exit code, so if installation fails
the script proceeds to run "make changelog-pr" and fails later; update the block
that uses "command -v jq" and "dnf install -y jq" to test the dnf command’s exit
status and immediately print a clear error and exit non‑zero on failure (or set
errexit/pipefail at the top), e.g., ensure the failure path for "dnf install -y
jq" stops execution with a descriptive message about jq installation failure;
alternatively, document or switch to a base image that already includes jq to
avoid runtime installation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 19591f64-2f4e-4606-9402-8c1a16793c54
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/rosa/openshift-rosa-master-postsubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/openshift/rosa/openshift-rosa-master.yaml
| if ! command -v jq >/dev/null 2>&1; then | ||
| dnf install -y jq >/dev/null | ||
| fi |
There was a problem hiding this comment.
Add error handling for jq installation.
If dnf install fails, the script continues to make changelog-pr without checking the exit code, which may result in unclear error messages if the changelog target depends on jq.
🛡️ Proposed fix to add error handling
if ! command -v jq >/dev/null 2>&1; then
- dnf install -y jq >/dev/null
+ if ! dnf install -y jq >/dev/null 2>&1; then
+ echo "ERROR: Failed to install jq dependency"
+ exit 1
+ fi
fiAlternatively, consider using a base image that already includes jq to avoid runtime installation overhead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ! command -v jq >/dev/null 2>&1; then | |
| dnf install -y jq >/dev/null | |
| fi | |
| if ! command -v jq >/dev/null 2>&1; then | |
| if ! dnf install -y jq >/dev/null 2>&1; then | |
| echo "ERROR: Failed to install jq dependency" | |
| exit 1 | |
| fi | |
| fi |
🤖 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 `@ci-operator/config/openshift/rosa/openshift-rosa-master.yaml` around lines 66
- 68, The script currently attempts to install jq with "dnf install -y jq" but
doesn’t check the install exit code, so if installation fails the script
proceeds to run "make changelog-pr" and fails later; update the block that uses
"command -v jq" and "dnf install -y jq" to test the dnf command’s exit status
and immediately print a clear error and exit non‑zero on failure (or set
errexit/pipefail at the top), e.g., ensure the failure path for "dnf install -y
jq" stops execution with a descriptive message about jq installation failure;
alternatively, document or switch to a base image that already includes jq to
avoid runtime installation.
|
@olucasfreitas: 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. |
1 participant
Summary
Why
ROSA is adding a historical
CHANGELOG.mdthat should be updated after stable tags, but all automation for this flow needs to remain on Prow rather than moving into GitHub Actions.Testing
python3YAML parse forci-operator/config/openshift/rosa/openshift-rosa-master.yamlpython3YAML parse forci-operator/jobs/openshift/rosa/openshift-rosa-master-postsubmits.yamlgit diff --checkNotes
openshift/rosachange at OCM-24478 | chore: add historical changelog automation rosa#3245.github-credentials-openshift-ci-robot-private-git-clonersecret. If that token does not have the required write scope for branch push / PR creation, the job wiring will need a follow-up secret update.Summary by CodeRabbit
This change adds OpenShift CI (ci-operator / Prow) automation so the openshift/rosa repository can maintain a historical CHANGELOG.md from Prow after stable tags.
What changed (practical terms)
Impact / rationale
Testing performed
Notes / follow-ups