Conversation
WalkthroughA new Tekton PipelineRun resource for pull requests was added, and the existing push PipelineRun was refactored to reference an external pipeline definition from a remote Git repository instead of using an inline specification. Parameters and configuration were adjusted to match the new approach, with workspace and service account settings retained. Changes
Sequence Diagram(s)sequenceDiagram
participant PR_Event as Pull Request Event
participant Tekton as Tekton Controller
participant RemoteRepo as Remote Pipeline Repo
participant Pipeline as PipelineRun
PR_Event->>Tekton: Triggers PipelineRun (pull-request)
Tekton->>RemoteRepo: Fetch pipeline definition (container-build.yaml)
Tekton->>Pipeline: Start PipelineRun with parameters and workspace
Pipeline-->>Tekton: Executes build steps as defined in remote pipeline
sequenceDiagram
participant Push_Event as Push Event
participant Tekton as Tekton Controller
participant RemoteRepo as Remote Pipeline Repo
participant Pipeline as PipelineRun
Push_Event->>Tekton: Triggers PipelineRun (push)
Tekton->>RemoteRepo: Fetch pipeline definition (container-build.yaml)
Tekton->>Pipeline: Start PipelineRun with parameters and workspace
Pipeline-->>Tekton: Executes build steps as defined in remote pipeline
Suggested labels
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
![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
🔭 Outside diff range comments (2)
.tekton/odh-codeflare-operator-push.yaml (2)
11-13: Same multiline-string parsing error as in PR pipeline
pipelinesascode.tekton.dev/on-cel-expressionis also split, breaking YAML. Apply the same one-line or>fix.
10-11: Branch filter inconsistency
on-target-branchis[test]while the CEL expression expectsmain. Align them to avoid silent skips.
🧹 Nitpick comments (1)
.tekton/odh-codeflare-operator-pull-request.yaml (1)
28-34: Static tag will overwrite previous PR images
output-imageis hard-coded toquay.io/opendatahub/codeflare-operator:pr-v1.16.0.
Every pull-request build will push the same tag, racing with other PRs and making debugging harder.
Consider incorporating the commit SHA or PR number:- value: quay.io/opendatahub/codeflare-operator:pr-v1.16.0 + value: quay.io/opendatahub/codeflare-operator:{{revision}}-{{ pr_number }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.tekton/odh-codeflare-operator-pull-request.yaml(1 hunks).tekton/odh-codeflare-operator-push.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: precommit
- GitHub Check: build
🔇 Additional comments (3)
.tekton/odh-codeflare-operator-pull-request.yaml (1)
40-41: Undefined template variabletarget_branchThe Git resolver’s
revisionparameter references{{ target_branch }}, but no such placeholder is supplied inparams:or annotations processed by PaC.
If the intent is to use the incoming branch, reuse{{revision}}or define the variable explicitly..tekton/odh-codeflare-operator-push.yaml (2)
33-41: Missing definition fortarget_branchin Git resolver
{{ target_branch }}is used but never provided inparams.
Either:- - name: revision - value: '{{ target_branch }}' + - name: revision + value: '{{ revision }}'or add a matching param/annotation.
27-32: Consider aligning parameters with PR runThe push PipelineRun omits
image-expires-after, which the sharedcontainer-build.yamlmay require (it is present in the PR definition).
Double-check the remote pipeline’s parameter defaults.
| pipelinesascode.tekton.dev/on-target-branch: "[test]" | ||
| pipelinesascode.tekton.dev/on-event: "[pull_request]" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Target-branch mismatch between filters
on-target-branch is limited to the literal branch test, while the CEL expression filters on main.
This inconsistency will make the run never trigger (or trigger twice if both branches are used). Pick one branch.
🤖 Prompt for AI Agents
In .tekton/odh-codeflare-operator-pull-request.yaml around lines 10 to 11, the
on-target-branch filter is set to "test" while the CEL expression filters on
"main", causing a mismatch that prevents the pipeline from triggering correctly.
To fix this, ensure both filters specify the same branch name by either changing
on-target-branch to "main" or updating the CEL expression to "test", so the
pipeline triggers consistently on the intended branch.
| pipelinesascode.tekton.dev/on-target-branch: "[test]" | ||
| pipelinesascode.tekton.dev/on-event: "[pull_request]" | ||
| pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch | ||
| == "main" |
There was a problem hiding this comment.
Broken annotation value – YAML will not parse
The pipelinesascode.tekton.dev/on-cel-expression annotation is split across two lines.
YAML treats the newline as the end of the scalar, so == "main" becomes a second key that will be rejected by the API server.
- pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch
- == "main"
+ pipelinesascode.tekton.dev/on-cel-expression: >
+ event == "pull_request" && target_branch == "main"📝 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.
| pipelinesascode.tekton.dev/on-target-branch: "[test]" | |
| pipelinesascode.tekton.dev/on-event: "[pull_request]" | |
| pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch | |
| == "main" | |
| pipelinesascode.tekton.dev/on-target-branch: "[test]" | |
| pipelinesascode.tekton.dev/on-event: "[pull_request]" | |
| pipelinesascode.tekton.dev/on-cel-expression: > | |
| event == "pull_request" && target_branch == "main" |
🤖 Prompt for AI Agents
In .tekton/odh-codeflare-operator-pull-request.yaml around lines 10 to 13, the
annotation pipelinesascode.tekton.dev/on-cel-expression is split across two
lines, causing YAML parsing errors. Fix this by joining the expression into a
single line without a newline, ensuring the entire expression is a valid scalar
value in YAML.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.tekton/odh-codeflare-operator-pull-request.yaml (2)
10-11: Branch filter mismatch will prevent PR runs from triggering
on-target-branchis still set to"[test]"while the CEL expression below targets themainbranch. Pipelines-as-Code evaluates the intersection of all filters, so this configuration makes the PipelineRun un-triggerable (or double-triggered if both branches are pushed). Align the filters:- pipelinesascode.tekton.dev/on-target-branch: "[test]" + pipelinesascode.tekton.dev/on-target-branch: "[main]"
12-13: Multi-line annotation breaks YAML parsingThe
on-cel-expressionvalue is split across two lines, turning== "main"into a new key and causing a validation error. Use a folded scalar:- pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch - == "main" + pipelinesascode.tekton.dev/on-cel-expression: > + event == "pull_request" && target_branch == "main"
🧹 Nitpick comments (2)
.tekton/odh-codeflare-operator-pull-request.yaml (2)
28-31: Trailing spaces flagged by yamllintLines 28 and 31 have trailing whitespace. While harmless at runtime, they fail linters configured in many CI pipelines—worth cleaning.
- - name: additional-tags - value:␠ + - name: additional-tags + value: ... - - name: additional-labels - value:␠ + - name: additional-labels + value:
48-50: Inconsistent moustache syntax – could lead to substitution missEverywhere else variables are
{{revision}} / {{target_branch}}, but here a space sneaks in:'{{ target_branch }}'. Some templating engines treat the space literally and won’t substitute. Consider normalising:- value: '{{ target_branch }}' + value: '{{target_branch}}'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.tekton/odh-codeflare-operator-pull-request.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.tekton/odh-codeflare-operator-pull-request.yaml
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: precommit
- GitHub Check: build
- GitHub Check: precommit
- GitHub Check: build
| - name: build-source-image | ||
| value: true | ||
| - name: build-image-index | ||
| value: false |
There was a problem hiding this comment.
Booleans must be quoted – Tekton params are strings
Tekton treats all Param values as strings. Unquoted true/false become YAML booleans and will be rejected by the webhook.
- - name: build-source-image
- value: true
+ - name: build-source-image
+ value: "true"
- - name: build-image-index
- value: false
+ - name: build-image-index
+ value: "false"📝 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.
| - name: build-source-image | |
| value: true | |
| - name: build-image-index | |
| value: false | |
| - name: build-source-image | |
| value: "true" | |
| - name: build-image-index | |
| value: "false" |
🤖 Prompt for AI Agents
In .tekton/odh-codeflare-operator-pull-request.yaml around lines 40 to 43, the
boolean values true and false are unquoted, causing them to be interpreted as
YAML booleans instead of strings. To fix this, enclose the values true and false
in double quotes to ensure they are treated as strings by Tekton, for example,
change value: true to value: "true" and value: false to value: "false".
|
/retest |
maorfr
force-pushed
the
centralize-pipelines
branch
from
32b71b1 to
1f4e5d9
Compare
|
/retest |
|
@maorfr: The following test failed, say
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
part of https://issues.redhat.com/browse/RHOAIENG-28846
this PR is an attempt to experiment with building multi-arch images for this component, to eventually support ARM.
Summary by CodeRabbit
New Features
Refactor