NO-ISSUE: Update konflux manifests#2133
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@pacevedom: This pull request explicitly references no jira issue. 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdated Tekton pipeline YAMLs to replace several task bundle image digests. Refactored the release script to collect new bundle digests, validate presence of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can get early access to new features in CodeRabbit.Enable the |
openshift-ci
bot
added
the
size/L
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pacevedom 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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
release/hack/update-konflux-task-refs.sh (1)
1-2: Consider addingset -euo pipefailfor stricter error handling.Currently only
set -xis enabled. Addingset -eandpipefailwould catch unexpected failures earlier, especially in the yq/cut pipeline.Suggested improvement
#!/bin/bash -set -x +set -euo pipefail +set -x🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@release/hack/update-konflux-task-refs.sh` around lines 1 - 2, Replace the current shell options in the script (currently only set -x) with stricter error handling by enabling set -euo pipefail in addition to -x; update the top-of-file options (the existing set -x line) so the script exits on errors/unset variables and fails pipelines properly (for example, change the invocation around set -x to include -euo pipefail or add a separate set -euo pipefail line before/with set -x).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@release/hack/update-konflux-task-refs.sh`:
- Around line 20-23: The exit-code check after assigning new_digest is
unreliable because $? reflects the assignment, not the skopeo process; change
the pattern to capture skopeo's failure directly by running skopeo inspect into
the new_digest variable with an if/conditional that checks the command's success
(e.g., use "if ! new_digest=$(skopeo inspect --format='{{ .Digest }}'
\"docker://${image}\"); then ..." or equivalent) and on failure call echo and
exit 1; update the block that sets new_digest and the subsequent error handling
so the script fails when skopeo fails.
- Around line 34-36: The pmt migrate invocation currently runs without
pipeline-file flags and thus scans all .tekton files; update the script so pmt
migrate is called with explicit pipeline file arguments tied to the bundles
extracted: when iterating and using NEW_BUNDLES, invoke pmt migrate
"${NEW_BUNDLES[@]}" with one or more --pipeline-file "$PIPELINE_FILE" flags
(i.e., move the pmt migrate call into the loop that processes PIPELINE_FILE so
each extracted bundle is migrated only for that file), or alternatively collect
all PIPELINE_FILE values and pass them as multiple --pipeline-file flags to a
single pmt migrate call; ensure you reference NEW_BUNDLES, pmt migrate and
PIPELINE_FILE in the change.
---
Nitpick comments:
In `@release/hack/update-konflux-task-refs.sh`:
- Around line 1-2: Replace the current shell options in the script (currently
only set -x) with stricter error handling by enabling set -euo pipefail in
addition to -x; update the top-of-file options (the existing set -x line) so the
script exits on errors/unset variables and fails pipelines properly (for
example, change the invocation around set -x to include -euo pipefail or add a
separate set -euo pipefail line before/with set -x).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: b1fcef57-760e-4b99-b7ee-7f763c5cec9a
📒 Files selected for processing (3)
.tekton/multi-arch-build-pipeline.yaml.tekton/single-arch-build-pipeline.yamlrelease/hack/update-konflux-task-refs.sh
pacevedom
force-pushed
the
update-konflux-manifests
branch
from
e9150b7 to
e96691e
Compare
|
@pacevedom: 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. |
openshift-ci
bot
added
the
needs-rebase
|
PR needs rebase. 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. |
3 participants
No description provided.