Feat: pipelinerun reconciliation #1972

hasanawad94 · 2025-08-19T14:35:38Z

This will be relevant to shipwright-io/community#277.
This pr only covers introducing PipelineRuns as an alternative to taskruns to be used for buildruns.
With these changes when BUILDRUN_EXECUTOR controller environment flag is set to PipelineRun the original taskrun will be embedded into a pipelinerun.

Future prs will cover making PipelineRun to define a set of TaskRuns that can run in parallel. This shp covers the details.

Changes

Add pipelinerun_runner.go with TektonPipelineRunWrapper implementation
that wraps Tekton PipelineRuns and injects TaskRun specs into PipelineRun
structures.
Implement ImageBuildRunner and ImageBuildRunnerFactory interfaces for
PipelineRun support.
Add reconciliation logic for PipelineRun status updates, including
condition handling and failure detection while maintaining original reconcile logic for taskruns.
Add UpdateBuildRunUsingPipelineRunCondition function to update BuildRun
status based on PipelineRun conditions (timeout, cancellation, success,
failure)
Refactor volume checking to use GetUnderlyingTaskRun() method for both
TaskRun and PipelineRun executors, eliminating the need for separate
volume checking methods
Add RunnerFactories map to support configurable executor selection
between TaskRun and PipelineRun implementations
Update error handling in CreateImageBuildRunner
Added unit, integration and e2e tests to ci.

Submitter Checklist

Includes tests if functionality changed/was added
Includes docs if changes are user-facing
Set a kind label on this PR
Release notes block has been filled in, or marked NONE

Release Notes

Added support for Tekton PipelineRuns as an alternative to TaskRuns for build execution. Configurable executor selection - users can now choose between TaskRun and PipelineRun executors via configuration.

adambkaplan · 2025-09-24T19:12:46Z

nit - it looks like you are using merge commits to keep this branch up to date. This makes it harder to identify which changes are net-new code.

Please use git rebase instead.

docs/configuration.md

pkg/reconciler/buildrun/buildrun.go

pkg/reconciler/buildrun/controller.go

SaschaSchwarze0 · 2025-10-06T11:44:12Z

pkg/reconciler/buildrun/pipelinerun_runner.go

+	// Since we only have one task for now in our PipelineRun, we can use the first ChildReference.
+	// The pod name will be the same as the TaskRun name with a suffix.
+	firstTaskRef := t.PipelineRun.Status.ChildReferences[0]
+	return firstTaskRef.Name + "-pod"


Can the TaskRun name be too long so that Tekton does not append -pod but applies some additional magic to shorten it ? In general relying on an implementation detail of Tekton feels incorrect.

And general question: why do we return the name of the first Pod here ? In which context are we using this function (I guess maybe error reporting) and would it make sense to revisit those to see which pod name we should return here.

test/utils/v1beta1/controllers.go

hasanawad94 · 2025-10-08T12:53:31Z

Thanks for the review @SaschaSchwarze0. For the GetPodName() method its needed for the metrics collection.
I've removed that uncertain logic relying on the naming and moved it to fetch the pod name from the taskrun in the reconcile function incase of using pipelinerun

build/pkg/reconciler/buildrun/buildrun.go

Lines 491 to 498 in 6e897bf

    
           // For PipelineRuns, GetPodName() returns empty string, so we need to get the actual pod name 
        
           // from the underlying TaskRun. For now, there is only 1 TaskRun per PipelineRun, so we can 
        
           // safely use the first TaskRun to get the pod name. 
        
           if podName == "" && buildRunner.GetExecutorKind() == "PipelineRun" { 
        
           	if taskRuns, err := buildRunner.GetUnderlyingTaskRuns(r.client); err == nil && len(taskRuns) > 0 { 
        
           		podName = taskRuns[0].Status.PodName 
        
           	} 
        
           }

This commit introduces PipelineRun support to the BuildRun reconciler, enabling Shipwright to use Tekton PipelineRuns as an alternative to TaskRuns for build execution. Key changes: - Add pipelinerun_runner.go with TektonPipelineRunWrapper implementation that wraps Tekton PipelineRuns and injects TaskRun specs into PipelineRun structures. - Implement ImageBuildRunner and ImageBuildRunnerFactory interfaces for PipelineRun support. - Add reconciliation logic for PipelineRun status updates, including condition handling and failure detection - Add UpdateBuildRunUsingPipelineRunCondition function to update BuildRun status based on PipelineRun conditions (timeout, cancellation, success, failure) - Refactor volume checking to use GetUnderlyingTaskRun() method for both TaskRun and PipelineRun executors, eliminating the need for separate volume checking methods - Add RunnerFactories map to support configurable executor selection between TaskRun and PipelineRun implementations - Update error handling in CreateImageBuildRunner - Add UpdateImageBuildRunFromExecutor to handle updating Buildrun based on used executor (pipelinerun or taskrun) The implementation maintains backward compatibility. Signed-off-by: Hasan Awad <[email protected]>

ayushsatyam146 · 2025-10-17T08:39:10Z

/lgtm

ayushsatyam146 · 2025-10-21T10:54:02Z

/approve

adambkaplan

/approve

This PR has undergone sufficient review and has demonstrated thorough test coverage. Furthermore, the test suites ensure that the current TaskRun reconciliation behavior is enabled by default, and that PipelineRun reconciliation is gated by a feature flag.

I have a few nits around simplifying the make commands invoked for the integration and e2e testing, but these can be addressed in a followup PR/issue.

adambkaplan · 2025-10-22T14:35:20Z

.github/workflows/ci.yml

          make test-integration
+      - name: Test-PipelineRun
+        run: |
+          BUILDRUN_EXECUTOR=PipelineRun ginkgo --focus-file="buildruns_to_pipelineruns_test.go" -v test/integration/...


nit (not blocking): ideally would like to have these be arguments to the make target, but I can live with this for now.

adambkaplan · 2025-10-22T14:36:15Z

.github/workflows/ci.yml

+          TEST_E2E_SERVICEACCOUNT_NAME=pipeline \
+          TEST_E2E_TIMEOUT_MULTIPLIER=${TEST_E2E_TIMEOUT_MULTIPLIER} \
+          TEST_E2E_VERIFY_TEKTONOBJECTS=true \
+          ginkgo --focus="PipelineRun E2E Tests" --procs 8 --timeout=1h --vv test/e2e/v1beta1/


nit (not blocking): ditto on incorporating this into the make target.

adambkaplan · 2025-10-22T14:37:11Z

pkg/config/config.go

 		TerminationLogPath:            terminationLogPathDefault,
 		GitRewriteRule:                false,
 		VulnerabilityCountLimit:       50,
+		BuildrunExecutor:              "TaskRun",


👍 on default value.

adambkaplan · 2025-10-22T14:38:30Z

pkg/config/config.go

+	// set environment variable for executor type
+	if executor := os.Getenv(controllerBuildrunExecutorEnvVar); executor != "" {
+		c.BuildrunExecutor = executor
+	}


nit (not blocking): ideally we should have some validation on this env var value and not accept it at face value.

adambkaplan · 2025-10-22T14:40:52Z

Makefile

 		--randomize-all \
 		--randomize-suites \
 		--fail-on-pending \
+		--skip-file=buildruns_to_pipelineruns_test.go \


nit (not blocking): we should use labels or other Ginkgo-native mechanisms for skipping these tests in the default integration and e2e test suites.

openshift-ci · 2025-10-22T14:49:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, ayushsatyam146

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [adambkaplan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Label for when a PR has specified a release note labels Aug 19, 2025

pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 19, 2025

shipwright-ci-bot added this to Shipwright Overview Aug 19, 2025

hasanawad94 force-pushed the add-pipelinerun-reconciliation branch 8 times, most recently from 920c1fe to 83ea44c Compare August 26, 2025 11:03

hasanawad94 force-pushed the add-pipelinerun-reconciliation branch from 83ea44c to b7cc447 Compare August 27, 2025 11:55

pull-request-size bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 27, 2025

hasanawad94 force-pushed the add-pipelinerun-reconciliation branch 11 times, most recently from 3625cbe to 33f0016 Compare September 2, 2025 07:21

hasanawad94 marked this pull request as ready for review September 2, 2025 07:26

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 2, 2025

openshift-ci bot requested review from dorzel and rxinui September 2, 2025 07:26

hasanawad94 force-pushed the add-pipelinerun-reconciliation branch from 5a94962 to edf6f28 Compare October 4, 2025 10:33

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 4, 2025

hasanawad94 force-pushed the add-pipelinerun-reconciliation branch 3 times, most recently from 8f8d704 to 2c6d078 Compare October 5, 2025 07:19

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 5, 2025

SaschaSchwarze0 reviewed Oct 6, 2025

View reviewed changes

hasanawad94 force-pushed the add-pipelinerun-reconciliation branch 7 times, most recently from 15ffaa5 to 0d89f13 Compare October 8, 2025 12:15

hasanawad94 requested a review from SaschaSchwarze0 October 8, 2025 12:56

hasanawad94 force-pushed the add-pipelinerun-reconciliation branch from 0d89f13 to 02f1aaa Compare October 15, 2025 08:38

openshift-ci bot assigned ayushsatyam146 Oct 17, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2025

sayan-biswas approved these changes Oct 21, 2025

View reviewed changes

adambkaplan approved these changes Oct 22, 2025

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 22, 2025

adambkaplan mentioned this pull request Oct 22, 2025

Clean Up CI Commands for PipelineRun Reconcile #2017

Open

openshift-merge-bot bot merged commit b431ca8 into shipwright-io:main Oct 22, 2025
15 checks passed

github-project-automation bot moved this to Done in Shipwright Overview Oct 22, 2025

Feat: pipelinerun reconciliation #1972

Feat: pipelinerun reconciliation #1972

Uh oh!

Conversation

hasanawad94 commented Aug 19, 2025 • edited by SaschaSchwarze0 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Submitter Checklist

Release Notes

Uh oh!

adambkaplan commented Sep 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SaschaSchwarze0 Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hasanawad94 commented Oct 8, 2025

Uh oh!

ayushsatyam146 commented Oct 17, 2025

Uh oh!

ayushsatyam146 commented Oct 21, 2025

Uh oh!

adambkaplan left a comment

Choose a reason for hiding this comment

Uh oh!

adambkaplan Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

adambkaplan Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

adambkaplan Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

adambkaplan Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

adambkaplan Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

hasanawad94 commented Aug 19, 2025 •

edited by SaschaSchwarze0

Loading