Emit events for BuildRun objects

deploy/200-role.yaml

@@ -74,3 +74,8 @@ rules:
 - apiGroups: ['']
   resources: ['serviceaccounts']
   verbs:     ['get', 'list', 'watch', 'create', 'update', 'delete']
+
+# Allow events to be created in any namespace related to BuildRun objects
+- apiGroups: ['']
+  resources: ['events']
+  verbs:     ['create']

docs/buildrun.md

@@ -15,6 +15,8 @@ SPDX-License-Identifier: Apache-2.0
 - [BuildRun Status](#buildrun-status)
   - [Understanding the state of a BuildRun](#understanding-the-state-of-a-BuildRun)
   - [Understanding failed BuildRuns](#understanding-failed-buildruns)
+  - [Build Snapshot](#build-snapshot)
+  - [Events](#events)
 - [Relationship with Tekton Tasks](#relationship-with-tekton-tasks)
 
 ## Overview
@@ -193,6 +195,11 @@ In addition, the `Status.Conditions` will host under the `Message` field a compa
 
 For every BuildRun controller reconciliation, the `buildSpec` in the Status of the `BuildRun` is updated if an existing owned `TaskRun` is present. During this update, a `Build` resource snapshot is generated and embedded into the `status.buildSpec` path of the `BuildRun`. A `buildSpec` is just a copy of the original `Build` spec, from where the `BuildRun` executed a particular image build. The snapshot approach allows developers to see the original `Build` configuration.
 
+### Events
+
+The BuildRun controller will emit Kubernetes events when a BuildRun starts and completes.
+If the BuildRun ends in a failure, a `Warning` event is emitted; otherwise a `Normal` event is emitted.
+
 ## Relationship with Tekton Tasks
 
 The `BuildRun` resource abstracts the image construction by delegating this work to the Tekton Pipeline [TaskRun](https://github.com/tektoncd/pipeline/blob/main/docs/taskruns.md). Compared to a Tekton Pipeline [Task](https://github.com/tektoncd/pipeline/blob/main/docs/tasks.md), a `TaskRun` runs all `steps` until completion of the `Task` or until a failure occurs in the `Task`.

pkg/reconciler/buildrun/buildrun.go

@@ -17,6 +17,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -46,6 +47,7 @@ type ReconcileBuildRun struct {
 	client                client.Client
 	scheme                *runtime.Scheme
 	setOwnerReferenceFunc setOwnerReferenceFunc
+	eventRecorder         record.EventRecorder
 }
 
 // NewReconciler returns a new reconcile.Reconciler
@@ -56,6 +58,7 @@ func NewReconciler(ctx context.Context, c *config.Config, mgr manager.Manager, o
 		client:                mgr.GetClient(),
 		scheme:                mgr.GetScheme(),
 		setOwnerReferenceFunc: ownerRef,
+		eventRecorder:         mgr.GetEventRecorderFor("buildruns"),
 	}
 }
 
@@ -111,11 +114,8 @@ func (r *ReconcileBuildRun) Reconcile(request reconcile.Request) (reconcile.Resu
 				// stop reconciling and mark the BuildRun as Failed
 				// we only reconcile again if the status.Update call fails
 				message := fmt.Sprintf("the Build is not registered correctly, build: %s, registered status: %s, reason: %s", build.Name, build.Status.Registered, build.Status.Reason)
-				if updateErr := resources.UpdateConditionWithFalseStatus(ctx, r.client, buildRun, message, resources.ConditionBuildRegistrationFailed); updateErr != nil {
-					return reconcile.Result{}, updateErr
-				}
-
-				return reconcile.Result{}, nil
+				updateErr := resources.UpdateConditionWithFalseStatus(ctx, r.client, buildRun, message, resources.ConditionBuildRegistrationFailed)
+				return r.handleError(buildRun, updateErr)
 			}
 
 			// Ensure the build-related labels on the BuildRun
@@ -126,6 +126,7 @@ func (r *ReconcileBuildRun) Reconcile(request reconcile.Request) (reconcile.Resu
 			// Set OwnerReference for Build and BuildRun only when build.shipwright.io/build-run-deletion is set "true"
 			if build.GetAnnotations()[buildv1alpha1.AnnotationBuildRunDeletion] == "true" && !resources.IsOwnedByBuild(build, buildRun.OwnerReferences) {
 				if err := r.setOwnerReferenceFunc(build, buildRun, r.scheme); err != nil {
+					// TODO - why are we setting this on the Build object instead of the BuildRun object?
 					build.Status.Reason = buildv1alpha1.SetOwnerReferenceFailed
 					build.Status.Message = fmt.Sprintf("unexpected error when trying to set the ownerreference: %v", err)
 					if err := r.client.Status().Update(ctx, build); err != nil {
@@ -161,30 +162,18 @@ func (r *ReconcileBuildRun) Reconcile(request reconcile.Request) (reconcile.Resu
 			// Choose a service account to use
 			svcAccount, err := resources.RetrieveServiceAccount(ctx, r.client, build, buildRun)
 			if err != nil {
-				if !resources.IsClientStatusUpdateError(err) && buildRun.Status.IsFailed(buildv1alpha1.Succeeded) {
-					return reconcile.Result{}, nil
-				}
-				// system call failure, reconcile again
-				return reconcile.Result{}, err
+				return r.handleError(buildRun, err)
 			}
 
 			strategy, err := r.getReferencedStrategy(ctx, build, buildRun)
 			if err != nil {
-				if !resources.IsClientStatusUpdateError(err) && buildRun.Status.IsFailed(buildv1alpha1.Succeeded) {
-					return reconcile.Result{}, nil
-				}
-				return reconcile.Result{}, err
+				return r.handleError(buildRun, err)
 			}
 
 			// Create the TaskRun, this needs to be the last step in this block to be idempotent
 			generatedTaskRun, err := r.createTaskRun(ctx, svcAccount, strategy, build, buildRun)
 			if err != nil {
-				if !resources.IsClientStatusUpdateError(err) && buildRun.Status.IsFailed(buildv1alpha1.Succeeded) {
-					ctxlog.Info(ctx, "taskRun generation failed", namespace, request.Namespace, name, request.Name)
-					return reconcile.Result{}, nil
-				}
-				// system call failure, reconcile again
-				return reconcile.Result{}, err
+				return r.handleError(buildRun, err)
 			}
 
 			ctxlog.Info(ctx, "creating TaskRun from BuildRun", namespace, request.Namespace, name, generatedTaskRun.GenerateName, "BuildRun", buildRun.Name)
@@ -267,6 +256,7 @@ func (r *ReconcileBuildRun) Reconcile(request reconcile.Request) (reconcile.Resu
 			if buildRun.Status.StartTime == nil && lastTaskRun.Status.StartTime != nil {
 				buildRun.Status.StartTime = lastTaskRun.Status.StartTime
 
+				r.recordBuildRunStarted(buildRun)
 				// Report the buildrun established duration (time between the creation of the buildrun and the start of the buildrun)
 				buildmetrics.BuildRunEstablishObserve(
 					buildRun.Status.BuildSpec.StrategyName(),
@@ -280,6 +270,7 @@ func (r *ReconcileBuildRun) Reconcile(request reconcile.Request) (reconcile.Resu
 			if lastTaskRun.Status.CompletionTime != nil && buildRun.Status.CompletionTime == nil {
 				buildRun.Status.CompletionTime = lastTaskRun.Status.CompletionTime
 
+				r.recordBuildRunCompleted(buildRun)
 				// buildrun completion duration (total time between the creation of the buildrun and the buildrun completion)
 				buildmetrics.BuildRunCompletionObserve(
 					buildRun.Status.BuildSpec.StrategyName(),
@@ -318,6 +309,7 @@ func (r *ReconcileBuildRun) Reconcile(request reconcile.Request) (reconcile.Resu
 						pod.CreationTimestamp.Time.Sub(lastTaskRun.CreationTimestamp.Time),
 					)
 				}
+
 			}
 
 			ctxlog.Info(ctx, "updating buildRun status", namespace, request.Namespace, name, request.Name)
@@ -332,6 +324,43 @@ func (r *ReconcileBuildRun) Reconcile(request reconcile.Request) (reconcile.Resu
 	return reconcile.Result{}, nil
 }
 
+// handleError determines the appropriate reconciliation result for the provided error.
+// It also ensures the build success/failure event is recorded.
+func (r *ReconcileBuildRun) handleError(buildRun *buildv1alpha1.BuildRun, err error) (reconcile.Result, error) {
+	if resources.IsClientStatusUpdateError(err) {
+		return reconcile.Result{}, err
+	}
+	if !buildRun.Status.IsFailed(buildv1alpha1.Succeeded) {
+		return reconcile.Result{}, err
+	}
+	r.recordBuildRunCompleted(buildRun)
+	return reconcile.Result{}, nil
+}
+
+// recordBuildRunCompleted records the appropriate event if the provided BuildRun has completed.
+func (r *ReconcileBuildRun) recordBuildRunCompleted(buildRun *buildv1alpha1.BuildRun) {
+	if buildRun.Status.CompletionTime == nil {
+		return
+	}
+	successStatus := buildRun.Status.GetCondition(buildv1alpha1.Succeeded)
+	if successStatus == nil {
+		return
+	}
+	if successStatus.Status == corev1.ConditionTrue {
+		r.eventRecorder.Event(buildRun, "Normal", "Succeeded", "Completed with no errors")
+	}
+	if successStatus.Status == corev1.ConditionFalse {
+		r.eventRecorder.Eventf(buildRun, "Warning", "Failed", "Failure reason: %s", successStatus.Reason)
+	}
+}
+
+func (r *ReconcileBuildRun) recordBuildRunStarted(buildRun *buildv1alpha1.BuildRun) {
+	if buildRun.Status.StartTime == nil {
+		return
+	}
+	r.eventRecorder.Event(buildRun, "Normal", "Started", "")
+}
+
 // GetBuildRunObject retrieves an existing BuildRun based on a name and namespace
 func (r *ReconcileBuildRun) GetBuildRunObject(ctx context.Context, objectName string, objectNS string, buildRun *buildv1alpha1.BuildRun) error {
 	if err := r.client.Get(ctx, types.NamespacedName{Name: objectName, Namespace: objectNS}, buildRun); err != nil {

pkg/reconciler/buildrun/buildrun_test.go

@@ -19,6 +19,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/pointer"
 	knativeapi "knative.dev/pkg/apis"
 	knativev1beta1 "knative.dev/pkg/apis/duck/v1beta1"
@@ -47,6 +48,7 @@ var _ = Describe("Reconcile BuildRun", func() {
 		buildRunSample                                         *build.BuildRun
 		taskRunSample                                          *v1beta1.TaskRun
 		statusWriter                                           *fakes.FakeStatusWriter
+		eventRecorder                                          *record.FakeRecorder
 		taskRunName, buildRunName, buildName, strategyName, ns string
 	)
 
@@ -101,6 +103,9 @@ var _ = Describe("Reconcile BuildRun", func() {
 		client.StatusCalls(func() crc.StatusWriter { return statusWriter })
 		manager.GetClientReturns(client)
 
+		eventRecorder = record.NewFakeRecorder(10)
+		manager.GetEventRecorderForReturns(eventRecorder)
+
 		// init the Build resource, this never change throughout this test suite
 		buildSample = ctl.DefaultBuild(buildName, strategyName, build.ClusterBuildStrategyKind)
 
@@ -139,7 +144,7 @@ var _ = Describe("Reconcile BuildRun", func() {
 				result, err := reconciler.Reconcile(taskRunRequest)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(reconcile.Result{}).To(Equal(result))
-				Expect(client.GetCallCount()).To(Equal(2))
+				Expect(client.GetCallCount()).To(Equal(3))
 			})
 			It("does not fail when the BuildRun does not exist", func() {
 
@@ -163,7 +168,7 @@ var _ = Describe("Reconcile BuildRun", func() {
 				result, err := reconciler.Reconcile(taskRunRequest)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(reconcile.Result{}).To(Equal(result))
-				Expect(client.GetCallCount()).To(Equal(2))
+				Expect(client.GetCallCount()).To(Equal(3))
 			})
 			It("updates the BuildRun status", func() {
 
@@ -189,7 +194,7 @@ var _ = Describe("Reconcile BuildRun", func() {
 				result, err := reconciler.Reconcile(taskRunRequest)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(reconcile.Result{}).To(Equal(result))
-				Expect(client.GetCallCount()).To(Equal(2))
+				Expect(client.GetCallCount()).To(Equal(3))
 				Expect(client.StatusCallCount()).To(Equal(1))
 			})
 
@@ -300,6 +305,8 @@ var _ = Describe("Reconcile BuildRun", func() {
 				Expect(reconcile.Result{}).To(Equal(result))
 				Expect(client.GetCallCount()).To(Equal(2))
 				Expect(client.StatusCallCount()).To(Equal(1))
+				// Build started event should have been emitted
+				Expect(len(eventRecorder.Events)).To(Equal(1))
 			})
 
 			It("updates the BuildRun status with a RUNNING reason", func() {
@@ -327,6 +334,8 @@ var _ = Describe("Reconcile BuildRun", func() {
 				Expect(reconcile.Result{}).To(Equal(result))
 				Expect(client.GetCallCount()).To(Equal(2))
 				Expect(client.StatusCallCount()).To(Equal(1))
+				// Build started event should have been emitted
+				Expect(len(eventRecorder.Events)).To(Equal(1))
 			})
 
 			It("updates the BuildRun status with a SUCCEEDED reason", func() {
@@ -352,8 +361,13 @@ var _ = Describe("Reconcile BuildRun", func() {
 				result, err := reconciler.Reconcile(taskRunRequest)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(reconcile.Result{}).To(Equal(result))
-				Expect(client.GetCallCount()).To(Equal(2))
+				Expect(client.GetCallCount()).To(Equal(3))
 				Expect(client.StatusCallCount()).To(Equal(1))
+
+				// Two events should be fired in this reconcile loop
+				// 1. Build started
+				// 2. Build succeeded
+				Expect(len(eventRecorder.Events)).To(Equal(2))
 			})
 
 			It("updates the BuildRun status when a FALSE status occurs", func() {
@@ -380,6 +394,11 @@ var _ = Describe("Reconcile BuildRun", func() {
 				result, err := reconciler.Reconcile(taskRunRequest)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(reconcile.Result{}).To(Equal(result))
+
+				// Two events should be fired in this reconcile loop
+				// 1. Build started
+				// 2. Build failed
+				Expect(len(eventRecorder.Events)).To(Equal(2))
 			})
 
 			It("does not break the reconcile when a taskrun pod initcontainers are not ready", func() {
@@ -574,6 +593,9 @@ var _ = Describe("Reconcile BuildRun", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(client.GetCallCount()).To(Equal(4))
 				Expect(client.StatusCallCount()).To(Equal(2))
+
+				// Updating the status to "Succeeded = false" should fire a failure event
+				Expect(len(eventRecorder.Events)).To(Equal(1))
 			})
 			It("fails on a TaskRun creation due to issues when retrieving the service account", func() {
 
@@ -636,6 +658,9 @@ var _ = Describe("Reconcile BuildRun", func() {
 				_, err := reconciler.Reconcile(buildRunRequest)
 				// we mark the BuildRun as Failed and do not reconcile again
 				Expect(err).ToNot(HaveOccurred())
+
+				// Updating the status to "Succeeded = false" should fire a failure event
+				Expect(len(eventRecorder.Events)).To(Equal(1))
 			})
 
 			It("fails on a TaskRun creation due to issues when retrieving the buildstrategy", func() {
@@ -704,6 +729,9 @@ var _ = Describe("Reconcile BuildRun", func() {
 				_, err := reconciler.Reconcile(buildRunRequest)
 				// we mark the BuildRun as Failed and do not reconcile again
 				Expect(err).ToNot(HaveOccurred())
+
+				// Updating the status to "Succeeded  false" should fire a failure event
+				Expect(len(eventRecorder.Events)).To(Equal(1))
 			})
 
 			It("fails on a TaskRun creation due to issues when retrieving the clusterbuildstrategy", func() {
@@ -771,6 +799,9 @@ var _ = Describe("Reconcile BuildRun", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(client.GetCallCount()).To(Equal(4))
 				Expect(client.StatusCallCount()).To(Equal(2))
+
+				// Updating the status to "Succeeded" should fire a failure event
+				Expect(len(eventRecorder.Events)).To(Equal(1))
 			})
 
 			It("defaults to a namespaced strategy if strategy kind is not set", func() {
@@ -829,6 +860,9 @@ var _ = Describe("Reconcile BuildRun", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(client.GetCallCount()).To(Equal(5))
 				Expect(client.StatusCallCount()).To(Equal(2))
+
+				// Updating the status to "Succeeded = false" should fire an event
+				Expect(len(eventRecorder.Events)).To(Equal(1))
 			})
 
 			It("fails on a TaskRun creation due to owner references errors", func() {
@@ -871,6 +905,9 @@ var _ = Describe("Reconcile BuildRun", func() {
 				_, err := reconciler.Reconcile(buildRunRequest)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(client.StatusCallCount()).To(Equal(2))
+
+				// Updating the status to "Succeeded = false" should fire an event
+				Expect(len(eventRecorder.Events)).To(Equal(1))
 			})
 
 			It("succeeds creating a TaskRun from a namespaced buildstrategy", func() {
@@ -965,6 +1002,9 @@ var _ = Describe("Reconcile BuildRun", func() {
 				_, err := reconciler.Reconcile(buildRunRequest)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(client.StatusCallCount()).To(Equal(1))
+
+				// Updating the status to "Succeeded = false" should fire an event
+				Expect(len(eventRecorder.Events)).To(Equal(1))
 			})
 
 			It("delays creation if the registered status of the build is not yet set", func() {

pkg/reconciler/buildrun/resources/errors.go

@@ -21,18 +21,20 @@ func (e ClientStatusUpdateError) Error() string {
 // IsClientStatusUpdateError checks whether the given error is of type ClientStatusUpdateError or
 // in case this is a list of errors, that it contains at least one error of type ClientStatusUpdateError
 func IsClientStatusUpdateError(err error) bool {
+	if err == nil {
+		return false
+	}
+
 	switch terr := err.(type) {
 	case *ClientStatusUpdateError, ClientStatusUpdateError:
 		return true
-
 	case Errors:
 		for _, e := range terr.errors {
 			if IsClientStatusUpdateError(e) {
 				return true
 			}
 		}
 	}
-
 	return false
 }