Skip to content

Commit 83ea44c

Browse files
committed
feat: Add PipelineRun support to BuildRun reconciler
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 <hasan.m.awad94@gmail.com>
1 parent 572e9a5 commit 83ea44c

File tree

10 files changed

+228
-125
lines changed

10 files changed

+228
-125
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ jobs:
150150
export IMAGE_PROCESSING_CONTAINER_IMAGE="$(KO_DOCKER_REPO=kind.local ko publish ./cmd/image-processing)"
151151
152152
make test-integration
153+
BUILDRUN_EXECUTOR=PipelineRun ginkgo --focus-file="buildruns_to_pipelineruns_test.go" -v test/integration/...
153154
154155
e2e:
155156
strategy:

Makefile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ TAG ?= latest
7979
# options for generating crds with controller-gen
8080
CONTROLLER_GEN="${GOBIN}/controller-gen"
8181

82+
# BuildRun executor configuration
83+
BUILDRUN_EXECUTOR ?= TaskRun
84+
8285
.EXPORT_ALL_VARIABLES:
8386

8487
default: build
@@ -213,6 +216,7 @@ test-integration: install-apis ginkgo
213216
--randomize-all \
214217
--randomize-suites \
215218
--fail-on-pending \
219+
--skip-file=buildruns_to_pipelineruns_test.go \
216220
-trace \
217221
test/integration/...
218222

deploy/500-controller.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ spec:
4545
value: ko://github.com/shipwright-io/build/cmd/bundle
4646
- name: WAITER_CONTAINER_IMAGE
4747
value: ko://github.com/shipwright-io/build/cmd/waiter
48+
- name: BUILDRUN_EXECUTOR
49+
value: "TaskRun"
4850
ports:
4951
- containerPort: 8383
5052
name: metrics-port

pkg/config/config.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ const (
6666
controllerBuildRunMaxConcurrentReconciles = "BUILDRUN_MAX_CONCURRENT_RECONCILES"
6767
controllerBuildStrategyMaxConcurrentReconciles = "BUILDSTRATEGY_MAX_CONCURRENT_RECONCILES"
6868
controllerClusterBuildStrategyMaxConcurrentReconciles = "CLUSTERBUILDSTRATEGY_MAX_CONCURRENT_RECONCILES"
69+
controllerBuildrunExecutorEnvVar = "BUILDRUN_EXECUTOR"
6970

7071
// environment variables for the kube API
7172
kubeAPIBurst = "KUBE_API_BURST"
@@ -107,6 +108,7 @@ type Config struct {
107108
KubeAPIOptions KubeAPIOptions
108109
GitRewriteRule bool
109110
VulnerabilityCountLimit int
111+
BuildrunExecutor string
110112
}
111113

112114
// PrometheusConfig contains the specific configuration for the
@@ -163,6 +165,7 @@ func NewDefaultConfig() *Config {
163165
TerminationLogPath: terminationLogPathDefault,
164166
GitRewriteRule: false,
165167
VulnerabilityCountLimit: 50,
168+
BuildrunExecutor: "TaskRun",
166169

167170
GitContainerTemplate: Step{
168171
Image: gitDefaultImage,
@@ -455,6 +458,10 @@ func (c *Config) SetConfigFromEnv() error {
455458
c.TerminationLogPath = terminationLogPath
456459
}
457460

461+
if executor := os.Getenv(controllerBuildrunExecutorEnvVar); executor != "" {
462+
c.BuildrunExecutor = executor
463+
}
464+
458465
return nil
459466
}
460467

pkg/reconciler/buildrun/buildrun.go

Lines changed: 68 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func NewReconciler(c *config.Config, mgr manager.Manager, ownerRef setOwnerRefer
5858
client: client.WithFieldOwner(mgr.GetClient(), "shipwright-buildrun-controller"),
5959
scheme: mgr.GetScheme(),
6060
setOwnerReferenceFunc: ownerRef,
61-
taskRunnerFactory: &TektonTaskRunImageBuildRunnerFactory{},
61+
taskRunnerFactory: RunnerFactories[c.BuildrunExecutor],
6262
}
6363
}
6464

@@ -81,26 +81,26 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
8181
buildRun = &buildv1beta1.BuildRun{}
8282
getBuildRunErr := r.GetBuildRunObject(ctx, request.Name, request.Namespace, buildRun)
8383

84-
// Try to get the ImageBuildRunner (TaskRun or PipelineRun) by name first
85-
var lastTaskRun ImageBuildRunner
86-
var getTaskRunErr error
84+
// Try to get the BuildRunner (TaskRun or PipelineRun) by name first
85+
var buildRunner ImageBuildRunner
86+
var buildRunnerErr error
8787

8888
// If we have a BuildRun with an executor reference, try to get the executor by its name
89-
if getBuildRunErr == nil && buildRun.Status.Executor != nil {
90-
lastTaskRun, getTaskRunErr = r.taskRunnerFactory.GetImageBuildRunner(ctx, r.client, types.NamespacedName{Name: buildRun.Status.Executor.Name, Namespace: request.Namespace})
89+
if getBuildRunErr == nil && buildRun.Status.Executor != nil && buildRun.Status.Executor.Name != "" {
90+
buildRunner, buildRunnerErr = r.taskRunnerFactory.GetImageBuildRunner(ctx, r.client, types.NamespacedName{Name: buildRun.Status.Executor.Name, Namespace: request.Namespace})
9191
} else {
9292
// Otherwise, try to get by the request name (this handles the case where the request comes from a TaskRun/PipelineRun event)
93-
lastTaskRun, getTaskRunErr = r.taskRunnerFactory.GetImageBuildRunner(ctx, r.client, types.NamespacedName{Name: request.Name, Namespace: request.Namespace})
93+
buildRunner, buildRunnerErr = r.taskRunnerFactory.GetImageBuildRunner(ctx, r.client, types.NamespacedName{Name: request.Name, Namespace: request.Namespace})
9494
}
9595

96-
if getBuildRunErr != nil && getTaskRunErr != nil {
96+
if getBuildRunErr != nil && buildRunnerErr != nil {
9797
if !apierrors.IsNotFound(getBuildRunErr) {
9898
return reconcile.Result{}, getBuildRunErr
9999
}
100-
if !apierrors.IsNotFound(getTaskRunErr) {
101-
return reconcile.Result{}, getTaskRunErr
100+
if !apierrors.IsNotFound(buildRunnerErr) {
101+
return reconcile.Result{}, buildRunnerErr
102102
}
103-
// If the BuildRun and TaskRun are not found, it might mean that we are running a Reconcile after a TaskRun was deleted. If this is the case, we need
103+
// If the BuildRun and BuildRunner are not found, it might mean that we are running a Reconcile after a TaskRun/PipelineRun was deleted. If this is the case, we need
104104
// to identify from the request the BuildRun name associate to it and update the BuildRun Status.
105105
r.VerifyRequestName(ctx, request, buildRun)
106106
return reconcile.Result{}, nil
@@ -132,9 +132,9 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
132132
}
133133
}
134134

135-
// for existing TaskRuns update the BuildRun Status, if there is no TaskRun, then create one
136-
if getTaskRunErr != nil {
137-
if apierrors.IsNotFound(getTaskRunErr) {
135+
// for existing BuildRunners update the BuildRun Status, if there is no BuildRunner, then create one
136+
if buildRunnerErr != nil {
137+
if apierrors.IsNotFound(buildRunnerErr) {
138138
build = &buildv1beta1.Build{}
139139
if err := resources.GetBuildObject(ctx, r.client, buildRun, build); err != nil {
140140
if !resources.IsClientStatusUpdateError(err) && buildRun.Status.IsFailed(buildv1beta1.Succeeded) {
@@ -325,29 +325,34 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
325325
}
326326

327327
// Create the ImageBuildRunner (TaskRun or PipelineRun)
328-
imageBuildRunner, err := r.taskRunnerFactory.CreateImageBuildRunner(r.config, svcAccount, strategy, build, buildRun, r.scheme, r.setOwnerReferenceFunc)
328+
imageBuildRunner, err := r.taskRunnerFactory.CreateImageBuildRunner(ctx, r.client, r.config, svcAccount, strategy, build, buildRun, r.scheme, r.setOwnerReferenceFunc)
329329
if err != nil {
330-
// Handle owner reference errors gracefully by marking the BuildRun as failed
331-
if err := resources.UpdateConditionWithFalseStatus(ctx, r.client, buildRun, err.Error(), resources.ConditionSetOwnerReferenceFailed); err != nil {
332-
return reconcile.Result{}, err
330+
if !resources.IsClientStatusUpdateError(err) && buildRun.Status.IsFailed(buildv1beta1.Succeeded) {
331+
ctxlog.Info(ctx, "buildRunner generation failed", namespace, request.Namespace, name, request.Name)
332+
return reconcile.Result{}, nil
333333
}
334-
return reconcile.Result{}, nil
334+
// system call failure, reconcile again
335+
return reconcile.Result{}, err
335336
}
336337

337-
// Check volumes exist using the interface method
338-
if err := imageBuildRunner.CheckVolumesExist(ctx, r.client); err != nil {
338+
// Check volumes exist using the TaskRun from the interface
339+
generatedTaskRun := imageBuildRunner.GetUnderlyingTaskRun()
340+
if generatedTaskRun != nil {
341+
err = resources.CheckTaskRunVolumesExist(ctx, r.client, generatedTaskRun)
339342
// if resource is not found, fail the build run
340-
if apierrors.IsNotFound(err) {
341-
if err := resources.UpdateConditionWithFalseStatus(ctx, r.client, buildRun, err.Error(), string(buildv1beta1.VolumeDoesNotExist)); err != nil {
342-
return reconcile.Result{}, err
343+
if err != nil {
344+
if apierrors.IsNotFound(err) {
345+
if err := resources.UpdateConditionWithFalseStatus(ctx, r.client, buildRun, err.Error(), string(buildv1beta1.VolumeDoesNotExist)); err != nil {
346+
return reconcile.Result{}, err
347+
}
348+
349+
// end of reconciliation
350+
return reconcile.Result{}, nil
343351
}
344352

345-
// end of reconciliation
346-
return reconcile.Result{}, nil
353+
// some other error might have happened, return it and reconcile again
354+
return reconcile.Result{}, err
347355
}
348-
349-
// some other error might have happened, return it and reconcile again
350-
return reconcile.Result{}, err
351356
}
352357

353358
ctxlog.Info(ctx, "creating ImageBuildRunner from BuildRun", namespace, request.Namespace, name, imageBuildRunner.GetName(), "BuildRun", buildRun.Name)
@@ -389,16 +394,16 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
389394
imageBuildRunner.GetCreationTimestamp().Time.Sub(buildRun.CreationTimestamp.Time),
390395
)
391396
} else {
392-
return reconcile.Result{}, getTaskRunErr
397+
return reconcile.Result{}, buildRunnerErr
393398
}
394399
} else {
395-
ctxlog.Info(ctx, "taskRun already exists", namespace, request.Namespace, name, request.Name)
400+
ctxlog.Info(ctx, "buildRunner already exists", namespace, request.Namespace, name, request.Name)
396401

397402
if getBuildRunErr != nil && !apierrors.IsNotFound(getBuildRunErr) {
398403
return reconcile.Result{}, getBuildRunErr
399404
} else if apierrors.IsNotFound(getBuildRunErr) {
400-
// this is a TR event, try getting the br from the label on the tr
401-
labels := lastTaskRun.GetLabels()
405+
// this is a TaskRun/PipelineRun event, try getting the br from the label on the executor
406+
labels := buildRunner.GetLabels()
402407
if labels != nil {
403408
err := r.GetBuildRunObject(ctx, labels[buildv1beta1.LabelBuildRun], request.Namespace, buildRun)
404409
if err != nil && !apierrors.IsNotFound(err) {
@@ -410,47 +415,38 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
410415
}
411416
}
412417

413-
if buildRun.IsCanceled() && !lastTaskRun.IsCancelled() {
414-
ctxlog.Info(ctx, "buildRun marked for cancellation, patching task run", namespace, request.Namespace, name, request.Name)
415-
if err := lastTaskRun.Cancel(ctx, r.client); err != nil {
416-
return reconcile.Result{}, fmt.Errorf("failed to cancel TaskRun: %v", err)
418+
if buildRun.IsCanceled() && !buildRunner.IsCancelled() {
419+
ctxlog.Info(ctx, "buildRun marked for cancellation, patching executor", namespace, request.Namespace, name, request.Name)
420+
if err := buildRunner.Cancel(ctx, r.client); err != nil {
421+
return reconcile.Result{}, fmt.Errorf("failed to cancel executor: %v", err)
417422
}
418423
}
419424

420425
// Check if the BuildRun is already finished, this happens if the build controller is restarted.
421-
// It then reconciles all TaskRuns. This is valuable if the build controller was down while the TaskRun
422-
// finishes which would be missed otherwise. But, if the TaskRun was already completed and the status
426+
// It then reconciles all executors. This is valuable if the build controller was down while the executor
427+
// finishes which would be missed otherwise. But, if the executor was already completed and the status
423428
// synchronized into the BuildRun, then yet another reconciliation is not necessary.
424429
if buildRun.Status.CompletionTime != nil {
425430
ctxlog.Info(ctx, "buildRun already marked completed", namespace, request.Namespace, name, request.Name)
426431
return reconcile.Result{}, nil
427432
}
428433

429-
taskRunResults := lastTaskRun.GetResults()
430-
if len(taskRunResults) > 0 {
431-
ctxlog.Info(ctx, "surfacing taskRun results to BuildRun status", namespace, request.Namespace, name, request.Name)
432-
resources.UpdateBuildRunUsingTaskResults(ctx, buildRun, taskRunResults, request)
434+
executorResults := buildRunner.GetResults()
435+
if len(executorResults) > 0 {
436+
ctxlog.Info(ctx, "surfacing executor results to BuildRun status", namespace, request.Namespace, name, request.Name)
437+
resources.UpdateBuildRunUsingTaskResults(ctx, buildRun, executorResults, request)
433438
}
434439

435-
trCondition := lastTaskRun.GetCondition(apis.ConditionSucceeded)
436-
if trCondition != nil {
437-
// Update BuildRun status based on the condition
438-
// Use the new interface methods to get the underlying objects
439-
if taskRunObj := lastTaskRun.GetUnderlyingTaskRun(); taskRunObj != nil {
440-
if err := resources.UpdateBuildRunUsingTaskRunCondition(ctx, r.client, buildRun, taskRunObj, trCondition); err != nil {
441-
return reconcile.Result{}, err
442-
}
443-
resources.UpdateBuildRunUsingTaskFailures(ctx, r.client, buildRun, taskRunObj)
444-
} else if pipelineRunObj := lastTaskRun.GetUnderlyingPipelineRun(); pipelineRunObj != nil {
445-
// For PipelineRuns, we need to handle the status update differently
446-
if err := resources.UpdateBuildRunUsingPipelineRunCondition(ctx, r.client, buildRun, pipelineRunObj, trCondition); err != nil {
447-
return reconcile.Result{}, err
448-
}
440+
executorCondition := buildRunner.GetCondition(apis.ConditionSucceeded)
441+
if executorCondition != nil {
442+
// Update BuildRun status based on the condition using the unified function
443+
if err := resources.UpdateImageBuildRunFromExecutor(ctx, r.client, buildRun, buildRunner.GetObject(), executorCondition); err != nil {
444+
return reconcile.Result{}, err
449445
}
450-
taskRunStatus := trCondition.Status
446+
executorStatus := executorCondition.Status
451447

452-
// check if we should delete the generated service account by checking the build run spec and that the task run is complete
453-
if taskRunStatus == corev1.ConditionTrue || taskRunStatus == corev1.ConditionFalse {
448+
// check if we should delete the generated service account by checking the build run spec and that the executor is complete
449+
if executorStatus == corev1.ConditionTrue || executorStatus == corev1.ConditionFalse {
454450
if err := resources.DeleteServiceAccount(ctx, r.client, buildRun); err != nil {
455451
ctxlog.Error(ctx, err, "Error during deletion of generated service account.")
456452
return reconcile.Result{}, err
@@ -459,18 +455,18 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
459455

460456
// Update the BuildExecutor if not already set
461457
if buildRun.Status.Executor == nil {
462-
executorName := lastTaskRun.GetName()
458+
executorName := buildRunner.GetName()
463459
buildRun.Status.Executor = &buildv1beta1.BuildExecutor{
464460
Name: executorName,
465-
Kind: lastTaskRun.GetExecutorKind(),
461+
Kind: buildRunner.GetExecutorKind(),
466462
}
467463
// Set the deprecated TaskRunName field for backward compatibility
468464
buildRun.Status.TaskRunName = &executorName
469465
}
470466

471-
taskRunStartTime := lastTaskRun.GetStartTime()
472-
if buildRun.Status.StartTime == nil && taskRunStartTime != nil {
473-
buildRun.Status.StartTime = taskRunStartTime
467+
executorStartTime := buildRunner.GetStartTime()
468+
if buildRun.Status.StartTime == nil && executorStartTime != nil {
469+
buildRun.Status.StartTime = executorStartTime
474470

475471
// Report the buildrun established duration (time between the creation of the buildrun and the start of the buildrun)
476472
buildmetrics.BuildRunEstablishObserve(
@@ -482,8 +478,8 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
482478
)
483479
}
484480

485-
if lastTaskRun.GetCompletionTime() != nil && buildRun.Status.CompletionTime == nil {
486-
buildRun.Status.CompletionTime = lastTaskRun.GetCompletionTime()
481+
if buildRunner.GetCompletionTime() != nil && buildRun.Status.CompletionTime == nil {
482+
buildRun.Status.CompletionTime = buildRunner.GetCompletionTime()
487483

488484
// buildrun completion duration (total time between the creation of the buildrun and the buildrun completion)
489485
buildmetrics.BuildRunCompletionObserve(
@@ -494,16 +490,16 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
494490
buildRun.Status.CompletionTime.Time.Sub(buildRun.CreationTimestamp.Time),
495491
)
496492

497-
// Look for the pod created by the taskrun
493+
// Look for the pod created by the executor
498494
var pod = &corev1.Pod{}
499-
if err := r.client.Get(ctx, types.NamespacedName{Namespace: request.Namespace, Name: lastTaskRun.GetPodName()}, pod); err == nil {
495+
if err := r.client.Get(ctx, types.NamespacedName{Namespace: request.Namespace, Name: buildRunner.GetPodName()}, pod); err == nil {
500496
if len(pod.Status.InitContainerStatuses) > 0 {
501497

502498
lastInitPodIdx := len(pod.Status.InitContainerStatuses) - 1
503499
lastInitPod := pod.Status.InitContainerStatuses[lastInitPodIdx]
504500

505501
if lastInitPod.State.Terminated != nil {
506-
// taskrun pod ramp-up (time between pod creation and last init container completion)
502+
// executor pod ramp-up (time between pod creation and last init container completion)
507503
buildmetrics.TaskRunPodRampUpDurationObserve(
508504
buildRun.Status.BuildSpec.StrategyName(),
509505
buildRun.Namespace,
@@ -514,13 +510,13 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
514510
}
515511
}
516512

517-
// taskrun ramp-up duration (time between taskrun creation and taskrun pod creation)
513+
// executor ramp-up duration (time between executor creation and executor pod creation)
518514
buildmetrics.TaskRunRampUpDurationObserve(
519515
buildRun.Status.BuildSpec.StrategyName(),
520516
buildRun.Namespace,
521517
buildRun.Spec.BuildName(),
522518
buildRun.Name,
523-
pod.CreationTimestamp.Time.Sub(lastTaskRun.GetCreationTimestamp().Time),
519+
pod.CreationTimestamp.Time.Sub(buildRunner.GetCreationTimestamp().Time),
524520
)
525521
}
526522
}

0 commit comments

Comments
 (0)