SPLAT-2837: Add fbc build pipeline - #76
Conversation
|
@AnnaZivkovic: This pull request references SPLAT-2837 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 story 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-robot
added
the
jira/valid-reference
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AnnaZivkovic 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
WalkthroughThis PR adds a new Tekton Pipeline definition, ChangesFBC Build Pipeline Definition
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Init
participant CloneRepository
participant RunOpmCommand
participant PrefetchDependencies
participant BuildTask as BuildahRemoteOciTa
participant BuildImageIndex
participant ChecksAndTags
Init->>CloneRepository: clone repo into OCI storage
CloneRepository->>RunOpmCommand: generate source artifacts
RunOpmCommand->>PrefetchDependencies: prefetch dependencies
PrefetchDependencies->>BuildTask: build per-platform images (matrix)
BuildTask->>BuildImageIndex: aggregate images into index
BuildImageIndex->>ChecksAndTags: run deprecated-base-image-check, apply-tags, validate-fbc, pruning-check, fips-check (guarded by skip-checks)
Related PRs: None found. Suggested labels: tekton, ci/cd Suggested reviewers: None found. 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.tekton/fbc-build-pipeline.yaml (1)
56-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing explicit
typeonenable-cache-proxyparam.Every other param declares
type: string/arrayexplicitly; this one omits it. Tekton defaults tostring, so it's functionally fine, but the inconsistency is worth tidying.♻️ Proposed fix
- default: "false" description: Enable cache proxy configuration name: enable-cache-proxy + type: string🤖 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 @.tekton/fbc-build-pipeline.yaml around lines 56 - 58, The enable-cache-proxy parameter in the Tekton pipeline is missing an explicit type, unlike the other params. Update the parameter definition in the pipeline spec to include type: string for enable-cache-proxy so it matches the surrounding parameter declarations and stays consistent with the rest of the config.
🤖 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 @.tekton/fbc-build-pipeline.yaml:
- Around line 67-75: The build-platforms parameter default in the Tekton
pipeline is currently missing linux/amd64 and still includes unsupported non-x86
targets. Update the build-platforms default in the pipeline definition to use
only linux/amd64, and ensure any related PipelineRun or parameter overrides that
reference build-platforms are aligned with that amd64-only constraint.
---
Nitpick comments:
In @.tekton/fbc-build-pipeline.yaml:
- Around line 56-58: The enable-cache-proxy parameter in the Tekton pipeline is
missing an explicit type, unlike the other params. Update the parameter
definition in the pipeline spec to include type: string for enable-cache-proxy
so it matches the surrounding parameter declarations and stays consistent with
the rest of the config.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c0d393bf-4695-4dc4-bd49-ef261e2876e2
📒 Files selected for processing (1)
.tekton/fbc-build-pipeline.yaml
| - default: | ||
| - localhost | ||
| - linux/arm64 | ||
| - linux/ppc64le | ||
| - linux/s390x | ||
| description: List of platforms to build the container images on. The available | ||
| set of values is determined by the configuration of the multi-platform-controller. | ||
| name: build-platforms | ||
| type: array |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
build-platforms default omits linux/amd64 and includes disallowed non-x86 platforms.
This repo targets vSphere/amd64 only. The default list here has no linux/amd64 entry and instead builds linux/arm64, linux/ppc64le, and linux/s390x.
🛠️ Proposed fix
- default:
- localhost
- - linux/arm64
- - linux/ppc64le
- - linux/s390x
+ - linux/amd64
description: List of platforms to build the container images on. The available
set of values is determined by the configuration of the multi-platform-controller.
name: build-platforms
type: arrayBased on learnings, "any Tekton PipelineRun in this repo that defines/sets the build-platforms parameter must restrict it to linux/amd64 only. Do not include non-x86 platforms".
📝 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.
| - default: | |
| - localhost | |
| - linux/arm64 | |
| - linux/ppc64le | |
| - linux/s390x | |
| description: List of platforms to build the container images on. The available | |
| set of values is determined by the configuration of the multi-platform-controller. | |
| name: build-platforms | |
| type: array | |
| - default: | |
| - localhost | |
| - linux/amd64 | |
| description: List of platforms to build the container images on. The available | |
| set of values is determined by the configuration of the multi-platform-controller. | |
| name: build-platforms | |
| type: array |
🤖 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 @.tekton/fbc-build-pipeline.yaml around lines 67 - 75, The build-platforms
parameter default in the Tekton pipeline is currently missing linux/amd64 and
still includes unsupported non-x86 targets. Update the build-platforms default
in the pipeline definition to use only linux/amd64, and ensure any related
PipelineRun or parameter overrides that reference build-platforms are aligned
with that amd64-only constraint.
Source: Learnings
|
@AnnaZivkovic: 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. |
2 participants
Summary by CodeRabbit