Repository: rancher/fleet
Pull request #2698 (2024-08-05 07:52:54)

Adds k8s events to the gitopts controller
Adds events for failures so the user gets information faster when there's no access to logs.

It also adds events when we get a new commit, a new job is created or deleted to inform the user of the status

Refers to: https://github.com/rancher/fleet/issues/2674



Patch:
diff --git a/internal/cmd/controller/gitops/operator.go b/internal/cmd/controller/gitops/operator.go
index ccde36a3..30f4a03f 100644
--- a/internal/cmd/controller/gitops/operator.go
+++ b/internal/cmd/controller/gitops/operator.go
@@ -133,6 +133,7 @@ func (g *GitOperator) Run(cmd *cobra.Command, args []string) error {
 		JobNodeSelector: g.ShardNodeSelector,
 		GitFetcher:      &git.Fetch{},
 		Clock:           reconciler.RealClock{},
+		Recorder:        mgr.GetEventRecorderFor(fmt.Sprintf("gitjob-controller%s", g.ShardID)),
 	}
 
 	configReconciler := &fcreconciler.ConfigReconciler{
diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
index ae12f15a..11cf6b28 100644
--- a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
+++ b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go
@@ -23,6 +23,7 @@ import (
 
 	"github.com/rancher/wrangler/v3/pkg/name"
 
+	fleetevent "github.com/rancher/fleet/pkg/event"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -30,6 +31,7 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -84,6 +86,7 @@ type GitJobReconciler struct {
 	JobNodeSelector string
 	GitFetcher      GitFetcher
 	Clock           TimeGetter
+	Recorder        record.EventRecorder
 }
 
 func (r *GitJobReconciler) SetupWithManager(mgr ctrl.Manager) error {
@@ -144,8 +147,9 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 
 	// Restrictions / Overrides, gitrepo reconciler is responsible for setting error in status
 	oldStatus := gitrepo.Status.DeepCopy()
-	gitrepo, err := grutil.AuthorizeAndAssignDefaults(ctx, r.Client, gitrepo)
+	_, err := grutil.AuthorizeAndAssignDefaults(ctx, r.Client, gitrepo)
 	if err != nil {
+		r.Recorder.Event(gitrepo, fleetevent.Warning, "FailedToApplyRestrictions", err.Error())
 		return ctrl.Result{}, grutil.UpdateErrorStatus(ctx, r.Client, req.NamespacedName, *oldStatus, err)
 	}
 
@@ -178,7 +182,13 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		return ctrl.Result{}, nil
 	}
 
-	gitPollerWasExecuted, _ := r.checkPollingTask(ctx, gitrepo)
+	commitBefore := gitrepo.Status.Commit
+	gitPollerWasExecuted, err := r.checkPollingTask(ctx, gitrepo)
+	if err != nil {
+		r.Recorder.Event(gitrepo, fleetevent.Warning, "FailedToCheckCommit", err.Error())
+	} else if gitPollerWasExecuted && commitBefore != gitrepo.Status.Commit {
+		r.Recorder.Event(gitrepo, fleetevent.Normal, "GotNewCommit", gitrepo.Status.Commit)
+	}
 	// From this point onwards we have to take into account if the poller
 	// task was executed.
 	// If so, we need to return a Result with EnqueueAfter set.
@@ -189,7 +199,9 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		Name:      jobName(gitrepo),
 	}, &job)
 	if err != nil && !errors.IsNotFound(err) {
-		return reconcileResult(gitPollerWasExecuted, gitrepo), fmt.Errorf("error retrieving git job: %w", err)
+		err = fmt.Errorf("error retrieving git job: %w", err)
+		r.Recorder.Event(gitrepo, fleetevent.Warning, "FailedToGetGitJob", err.Error())
+		return reconcileResult(gitPollerWasExecuted, gitrepo), err
 	}
 
 	// Gitjob handling
@@ -200,10 +212,18 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 			if err == nil && commit != "" {
 				gitrepo.Status.Commit = commit
 			}
+			if err != nil {
+				r.Recorder.Event(gitrepo, fleetevent.Warning, "Failed", err.Error())
+			} else {
+				if gitPollerWasExecuted && commitBefore != gitrepo.Status.Commit {
+					r.Recorder.Event(gitrepo, fleetevent.Normal, "GotNewCommit", gitrepo.Status.Commit)
+				}
+			}
 		}
 
 		if gitrepo.Status.Commit != "" {
 			if err := r.validateExternalSecretExist(ctx, gitrepo); err != nil {
+				r.Recorder.Event(gitrepo, fleetevent.Warning, "FailedValidatingSecret", err.Error())
 				return reconcileResult(gitPollerWasExecuted, gitrepo), grutil.UpdateErrorStatus(ctx, r.Client, req.NamespacedName, gitrepo.Status, err)
 			}
 			logger.V(1).Info("Creating Git job resources")
@@ -216,6 +236,7 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 			if err := r.createJob(ctx, gitrepo); err != nil {
 				return reconcileResult(gitPollerWasExecuted, gitrepo), fmt.Errorf("error creating git job: %w", err)
 			}
+			r.Recorder.Event(gitrepo, fleetevent.Normal, "Created", "GitJob was created")
 		}
 	} else if gitrepo.Status.Commit != "" {
 		if err = r.deleteJobIfNeeded(ctx, gitrepo, &job); err != nil {
@@ -389,27 +410,48 @@ func (r *GitJobReconciler) createJob(ctx context.Context, gitRepo *v1alpha1.GitR
 
 func (r *GitJobReconciler) deleteJobIfNeeded(ctx context.Context, gitRepo *v1alpha1.GitRepo, job *batchv1.Job) error {
 	logger := log.FromContext(ctx)
+	jobDeleted := false
+	jobDeletedMessage := ""
 	// if force delete is set, delete the job to make sure a new job is created
 	if gitRepo.Spec.ForceSyncGeneration != gitRepo.Status.UpdateGeneration {
 		gitRepo.Status.UpdateGeneration = gitRepo.Spec.ForceSyncGeneration
-		logger.Info("job deletion triggered because of ForceUpdateGeneration")
+		jobDeletedMessage = "job deletion triggered because of ForceUpdateGeneration"
+		logger.Info(jobDeletedMessage)
 		if err := r.Delete(ctx, job, client.PropagationPolicy(metav1.DeletePropagationBackground)); err != nil && !errors.IsNotFound(err) {
 			return err
 		}
+		jobDeleted = true
 	}
 
 	// k8s Jobs are immutable. Recreate the job if the GitRepo Spec has changed.
-	if gitRepo.Generation != gitRepo.Status.ObservedGeneration {
+	// Avoid deleting the job twice
+	if !jobDeleted && generationChanged(gitRepo) {
+		jobDeletedMessage = "job deletion triggered because of generation change"
 		gitRepo.Status.ObservedGeneration = gitRepo.Generation
-		logger.Info("job deletion triggered because of generation change")
+		logger.Info(jobDeletedMessage)
 		if err := r.Delete(ctx, job, client.PropagationPolicy(metav1.DeletePropagationBackground)); err != nil && !errors.IsNotFound(err) {
 			return err
 		}
+		jobDeleted = true
+	}
+
+	if jobDeleted {
+		r.Recorder.Event(gitRepo, fleetevent.Normal, "JobDeleted", jobDeletedMessage)
 	}
 
 	return nil
 }
 
+func generationChanged(r *v1alpha1.GitRepo) bool {
+	// checks if generation changed.
+	// it ignores the case when Status.ObservedGeneration=0 because that's
+	// the initial value of a just created GitRepo and the initial value
+	// for Generation in k8s is 1.
+	// If we don't ignore we would be deleting the gitjob that was just created
+	// until later we reconcile ObservedGeneration with Generation
+	return (r.Generation != r.Status.ObservedGeneration) && r.Status.ObservedGeneration > 0
+}
+
 func jobName(obj *v1alpha1.GitRepo) string {
 	return name.SafeConcatName(obj.Name, name.Hex(obj.Spec.Repo+obj.Status.Commit, 5))
 }
diff --git a/internal/mocks/eventrecorder_mock.go b/internal/mocks/eventrecorder_mock.go
new file mode 100644
index 00000000..edf5528b
--- /dev/null
+++ b/internal/mocks/eventrecorder_mock.go
@@ -0,0 +1,81 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: k8s.io/client-go/tools/record (interfaces: EventRecorder)
+
+// Package mocks is a generated GoMock package.
+package mocks
+
+import (
+	reflect "reflect"
+
+	gomock "go.uber.org/mock/gomock"
+	runtime "k8s.io/apimachinery/pkg/runtime"
+)
+
+// MockEventRecorder is a mock of EventRecorder interface.
+type MockEventRecorder struct {
+	ctrl     *gomock.Controller
+	recorder *MockEventRecorderMockRecorder
+}
+
+// MockEventRecorderMockRecorder is the mock recorder for MockEventRecorder.
+type MockEventRecorderMockRecorder struct {
+	mock *MockEventRecorder
+}
+
+// NewMockEventRecorder creates a new mock instance.
+func NewMockEventRecorder(ctrl *gomock.Controller) *MockEventRecorder {
+	mock := &MockEventRecorder{ctrl: ctrl}
+	mock.recorder = &MockEventRecorderMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockEventRecorder) EXPECT() *MockEventRecorderMockRecorder {
+	return m.recorder
+}
+
+// AnnotatedEventf mocks base method.
+func (m *MockEventRecorder) AnnotatedEventf(arg0 runtime.Object, arg1 map[string]string, arg2, arg3, arg4 string, arg5 ...interface{}) {
+	m.ctrl.T.Helper()
+	varargs := []interface{}{arg0, arg1, arg2, arg3, arg4}
+	for _, a := range arg5 {
+		varargs = append(varargs, a)
+	}
+	m.ctrl.Call(m, "AnnotatedEventf", varargs...)
+}
+
+// AnnotatedEventf indicates an expected call of AnnotatedEventf.
+func (mr *MockEventRecorderMockRecorder) AnnotatedEventf(arg0, arg1, arg2, arg3, arg4 interface{}, arg5 ...interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	varargs := append([]interface{}{arg0, arg1, arg2, arg3, arg4}, arg5...)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AnnotatedEventf", reflect.TypeOf((*MockEventRecorder)(nil).AnnotatedEventf), varargs...)
+}
+
+// Event mocks base method.
+func (m *MockEventRecorder) Event(arg0 runtime.Object, arg1, arg2, arg3 string) {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "Event", arg0, arg1, arg2, arg3)
+}
+
+// Event indicates an expected call of Event.
+func (mr *MockEventRecorderMockRecorder) Event(arg0, arg1, arg2, arg3 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Event", reflect.TypeOf((*MockEventRecorder)(nil).Event), arg0, arg1, arg2, arg3)
+}
+
+// Eventf mocks base method.
+func (m *MockEventRecorder) Eventf(arg0 runtime.Object, arg1, arg2, arg3 string, arg4 ...interface{}) {
+	m.ctrl.T.Helper()
+	varargs := []interface{}{arg0, arg1, arg2, arg3}
+	for _, a := range arg4 {
+		varargs = append(varargs, a)
+	}
+	m.ctrl.Call(m, "Eventf", varargs...)
+}
+
+// Eventf indicates an expected call of Eventf.
+func (mr *MockEventRecorderMockRecorder) Eventf(arg0, arg1, arg2, arg3 interface{}, arg4 ...interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	varargs := append([]interface{}{arg0, arg1, arg2, arg3}, arg4...)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Eventf", reflect.TypeOf((*MockEventRecorder)(nil).Eventf), varargs...)
+}
diff --git a/pkg/event/event.go b/pkg/event/event.go
new file mode 100644
index 00000000..ca955ab0
--- /dev/null
+++ b/pkg/event/event.go
@@ -0,0 +1,6 @@
+package event
+
+const (
+	Normal  = "Normal"
+	Warning = "Warning"
+)


Test patch:
diff --git a/integrationtests/gitjob/controller/controller_test.go b/integrationtests/gitjob/controller/controller_test.go
index 30baa117..621549a0 100644
--- a/integrationtests/gitjob/controller/controller_test.go
+++ b/integrationtests/gitjob/controller/controller_test.go
@@ -1,6 +1,7 @@
 package controller
 
 import (
+	"context"
 	"encoding/hex"
 	"fmt"
 	"math/rand"
@@ -145,6 +146,27 @@ var _ = Describe("GitJob controller", func() {
 					g.Expect(checkCondition(&gitRepo, "Ready", corev1.ConditionTrue, "")).To(BeTrue())
 					g.Expect(checkCondition(&gitRepo, "Accepted", corev1.ConditionTrue, "")).To(BeTrue())
 				}).Should(Succeed())
+
+				// it should log 2 events
+				// first one is to log the new commit from the poller
+				// second one is to inform that the job was created
+				Eventually(func(g Gomega) {
+					events, _ := k8sClientSet.CoreV1().Events(gitRepo.Namespace).List(context.TODO(),
+						metav1.ListOptions{
+							FieldSelector: "involvedObject.name=success",
+							TypeMeta:      metav1.TypeMeta{Kind: "GitRepo"},
+						})
+					g.Expect(events).ToNot(BeNil())
+					g.Expect(len(events.Items)).To(Equal(2))
+					g.Expect(events.Items[0].Reason).To(Equal("GotNewCommit"))
+					g.Expect(events.Items[0].Message).To(Equal("9ca3a0ad308ed8bffa6602572e2a1343af9c3d2e"))
+					g.Expect(events.Items[0].Type).To(Equal("Normal"))
+					g.Expect(events.Items[0].Source.Component).To(Equal("gitjob-controller"))
+					g.Expect(events.Items[1].Reason).To(Equal("Created"))
+					g.Expect(events.Items[1].Message).To(Equal("GitJob was created"))
+					g.Expect(events.Items[1].Type).To(Equal("Normal"))
+					g.Expect(events.Items[1].Source.Component).To(Equal("gitjob-controller"))
+				}).Should(Succeed())
 			})
 		})
 
@@ -345,6 +367,31 @@ var _ = Describe("GitJob controller", func() {
 
 				return string(job.UID) != string(newJob.UID)
 			}).Should(BeTrue())
+			// it should log 3 events
+			// first one is to log the new commit from the poller
+			// second one is to inform that the job was created
+			// third one reports on the job being deleted because of ForceUpdateGeneration
+			Eventually(func(g Gomega) {
+				events, _ := k8sClientSet.CoreV1().Events(gitRepo.Namespace).List(context.TODO(),
+					metav1.ListOptions{
+						FieldSelector: "involvedObject.name=force-deletion",
+						TypeMeta:      metav1.TypeMeta{Kind: "GitRepo"},
+					})
+				g.Expect(events).ToNot(BeNil())
+				g.Expect(len(events.Items)).To(Equal(3))
+				g.Expect(events.Items[0].Reason).To(Equal("GotNewCommit"))
+				g.Expect(events.Items[0].Message).To(Equal("9ca3a0ad308ed8bffa6602572e2a1343af9c3d2e"))
+				g.Expect(events.Items[0].Type).To(Equal("Normal"))
+				g.Expect(events.Items[0].Source.Component).To(Equal("gitjob-controller"))
+				g.Expect(events.Items[1].Reason).To(Equal("Created"))
+				g.Expect(events.Items[1].Message).To(Equal("GitJob was created"))
+				g.Expect(events.Items[1].Type).To(Equal("Normal"))
+				g.Expect(events.Items[1].Source.Component).To(Equal("gitjob-controller"))
+				g.Expect(events.Items[2].Reason).To(Equal("JobDeleted"))
+				g.Expect(events.Items[2].Message).To(Equal("job deletion triggered because of ForceUpdateGeneration"))
+				g.Expect(events.Items[2].Type).To(Equal("Normal"))
+				g.Expect(events.Items[2].Source.Component).To(Equal("gitjob-controller"))
+			}).Should(Succeed())
 		})
 
 		It("verifies that UpdateGeneration in Status is equal to ForceSyncGeneration", func() {
diff --git a/integrationtests/gitjob/controller/suite_test.go b/integrationtests/gitjob/controller/suite_test.go
index 6f692eb5..237e85cf 100644
--- a/integrationtests/gitjob/controller/suite_test.go
+++ b/integrationtests/gitjob/controller/suite_test.go
@@ -11,12 +11,6 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/reugn/go-quartz/quartz"
 	"go.uber.org/mock/gomock"
-	"k8s.io/client-go/kubernetes/scheme"
-	"k8s.io/client-go/rest"
-	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/envtest"
-	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
 	"github.com/rancher/fleet/internal/cmd/controller/gitops/reconciler"
 	ctrlreconciler "github.com/rancher/fleet/internal/cmd/controller/reconciler"
@@ -25,6 +19,14 @@ import (
 	"github.com/rancher/fleet/internal/manifest"
 	v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 	"github.com/rancher/fleet/pkg/git/mocks"
+
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/rest"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/envtest"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 )
 
 const (
@@ -40,6 +42,7 @@ var (
 	logsBuffer     bytes.Buffer
 	namespace      string
 	expectedCommit string
+	k8sClientSet   *kubernetes.Clientset
 )
 
 func TestGitJobController(t *testing.T) {
@@ -63,6 +66,9 @@ var _ = BeforeSuite(func() {
 	err = v1alpha1.AddToScheme(scheme.Scheme)
 	Expect(err).NotTo(HaveOccurred())
 
+	k8sClientSet, err = kubernetes.NewForConfig(cfg)
+	Expect(err).NotTo(HaveOccurred())
+
 	k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
 	Expect(err).NotTo(HaveOccurred())
 	Expect(k8sClient).NotTo(BeNil())
@@ -98,6 +104,7 @@ var _ = BeforeSuite(func() {
 		Scheduler:  sched,
 		GitFetcher: fetcherMock,
 		Clock:      reconciler.RealClock{},
+		Recorder:   mgr.GetEventRecorderFor("gitjob-controller"),
 	}).SetupWithManager(mgr)
 	Expect(err).ToNot(HaveOccurred())
 
diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_test.go b/internal/cmd/controller/gitops/reconciler/gitjob_test.go
index 1d367a1b..58e7564d 100644
--- a/internal/cmd/controller/gitops/reconciler/gitjob_test.go
+++ b/internal/cmd/controller/gitops/reconciler/gitjob_test.go
@@ -16,13 +16,17 @@ import (
 	"github.com/rancher/fleet/internal/cmd/controller/finalize"
 	"github.com/rancher/fleet/internal/mocks"
 	fleetv1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
+	v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 	"github.com/rancher/wrangler/v3/pkg/genericcondition"
 
+	fleetevent "github.com/rancher/fleet/pkg/event"
 	gitmocks "github.com/rancher/fleet/pkg/git/mocks"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -31,6 +35,15 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
+func getCondition(gitrepo *v1alpha1.GitRepo, condType string) (genericcondition.GenericCondition, bool) {
+	for _, cond := range gitrepo.Status.Conditions {
+		if cond.Type == condType {
+			return cond, true
+		}
+	}
+	return genericcondition.GenericCondition{}, false
+}
+
 type ClockMock struct {
 	t time.Time
 }
@@ -38,6 +51,36 @@ type ClockMock struct {
 func (m ClockMock) Now() time.Time                  { return m.t }
 func (m ClockMock) Since(t time.Time) time.Duration { return m.t.Sub(t) }
 
+// gitRepoMatcher implements a gomock matcher that checks for gitrepos.
+// It only checks for the expected name and namespace so far
+type gitRepoMatcher struct {
+	gitrepo fleetv1.GitRepo
+}
+
+func (m gitRepoMatcher) Matches(x interface{}) bool {
+	gitrepo, ok := x.(*fleetv1.GitRepo)
+	if !ok {
+		return false
+	}
+	return m.gitrepo.Name == gitrepo.Name && m.gitrepo.Namespace == gitrepo.Namespace
+}
+
+func (m gitRepoMatcher) String() string {
+	return fmt.Sprintf("Gitrepo %s-%s", m.gitrepo.Name, m.gitrepo.Namespace)
+}
+
+type gitRepoPointerMatcher struct {
+}
+
+func (m gitRepoPointerMatcher) Matches(x interface{}) bool {
+	_, ok := x.(*fleetv1.GitRepo)
+	return ok
+}
+
+func (m gitRepoPointerMatcher) String() string {
+	return ""
+}
+
 func getGitPollingCondition(gitrepo *fleetv1.GitRepo) (genericcondition.GenericCondition, bool) {
 	for _, cond := range gitrepo.Status.Conditions {
 		if cond.Type == gitPollingCondition {
@@ -146,12 +189,20 @@ func TestReconcile_LatestCommitErrorIsSetInConditions(t *testing.T) {
 		},
 	).Times(1)
 
+	recorderMock := mocks.NewMockEventRecorder(mockCtrl)
+	recorderMock.EXPECT().Event(
+		&gitRepoMatcher{gitRepo},
+		fleetevent.Warning,
+		"FailedToCheckCommit",
+		"TEST ERROR",
+	)
 	r := GitJobReconciler{
 		Client:     client,
 		Scheme:     scheme,
 		Image:      "",
 		GitFetcher: fetcher,
 		Clock:      RealClock{},
+		Recorder:   recorderMock,
 	}
 
 	ctx := context.TODO()
@@ -214,12 +265,21 @@ func TestReconcile_LatestCommitIsOkay(t *testing.T) {
 		},
 	).Times(1)
 
+	recorderMock := mocks.NewMockEventRecorder(mockCtrl)
+	recorderMock.EXPECT().Event(
+		&gitRepoMatcher{gitRepo},
+		fleetevent.Normal,
+		"GotNewCommit",
+		"1883fd54bc5dfd225acf02aecbb6cb8020458e33",
+	)
+
 	r := GitJobReconciler{
 		Client:     client,
 		Scheme:     scheme,
 		Image:      "",
 		GitFetcher: fetcher,
 		Clock:      RealClock{},
+		Recorder:   recorderMock,
 	}
 
 	ctx := context.TODO()
@@ -348,12 +408,21 @@ func TestReconcile_LatestCommitShouldBeCalled(t *testing.T) {
 		},
 	).Times(1)
 
+	recorderMock := mocks.NewMockEventRecorder(mockCtrl)
+	recorderMock.EXPECT().Event(
+		&gitRepoMatcher{gitRepo},
+		fleetevent.Normal,
+		"GotNewCommit",
+		"1883fd54bc5dfd225acf02aecbb6cb8020458e33",
+	)
+
 	r := GitJobReconciler{
 		Client:     client,
 		Scheme:     scheme,
 		Image:      "",
 		GitFetcher: fetcher,
 		Clock:      RealClock{},
+		Recorder:   recorderMock,
 	}
 
 	ctx := context.TODO()
@@ -387,7 +456,14 @@ func TestReconcile_Error_WhenGitrepoRestrictionsAreNotMet(t *testing.T) {
 			return nil
 		},
 	)
-	mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
+
+	mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), &gitRepoPointerMatcher{}, gomock.Any()).Times(2).DoAndReturn(
+		func(ctx context.Context, req types.NamespacedName, gitrepo *fleetv1.GitRepo, opts ...interface{}) error {
+			gitrepo.Name = gitRepo.Name
+			gitrepo.Namespace = gitRepo.Namespace
+			return nil
+		},
+	)
 	statusClient := mocks.NewMockSubResourceWriter(mockCtrl)
 	mockClient.EXPECT().Status().Times(1).Return(statusClient)
 	statusClient.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any()).Do(
@@ -401,11 +477,20 @@ func TestReconcile_Error_WhenGitrepoRestrictionsAreNotMet(t *testing.T) {
 		},
 	)
 
+	recorderMock := mocks.NewMockEventRecorder(mockCtrl)
+	recorderMock.EXPECT().Event(
+		&gitRepoMatcher{gitRepo},
+		fleetevent.Warning,
+		"FailedToApplyRestrictions",
+		"empty targetNamespace denied, because allowedTargetNamespaces restriction is present",
+	)
+
 	r := GitJobReconciler{
-		Client: mockClient,
-		Scheme: scheme,
-		Image:  "",
-		Clock:  RealClock{},
+		Client:   mockClient,
+		Scheme:   scheme,
+		Image:    "",
+		Clock:    RealClock{},
+		Recorder: recorderMock,
 	}
 
 	ctx := context.TODO()
@@ -418,6 +503,164 @@ func TestReconcile_Error_WhenGitrepoRestrictionsAreNotMet(t *testing.T) {
 	}
 }
 
+func TestReconcile_Error_WhenGetGitJobErrors(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	scheme := runtime.NewScheme()
+	utilruntime.Must(batchv1.AddToScheme(scheme))
+	gitRepo := fleetv1.GitRepo{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "gitrepo",
+			Namespace: "default",
+		},
+	}
+	namespacedName := types.NamespacedName{Name: gitRepo.Name, Namespace: gitRepo.Namespace}
+	mockClient := mocks.NewMockClient(mockCtrl)
+	mockClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
+
+	mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).DoAndReturn(
+		func(ctx context.Context, req types.NamespacedName, gitrepo *fleetv1.GitRepo, opts ...interface{}) error {
+			gitrepo.Name = gitRepo.Name
+			gitrepo.Namespace = gitRepo.Namespace
+			gitrepo.Spec.Repo = "repo"
+			controllerutil.AddFinalizer(gitrepo, finalize.GitRepoFinalizer)
+			gitrepo.Status.Commit = "dd45c7ad68e10307765104fea4a1f5997643020f"
+			return nil
+		},
+	)
+	mockFetcher := gitmocks.NewMockGitFetcher(mockCtrl)
+	commit := "1883fd54bc5dfd225acf02aecbb6cb8020458e33"
+	mockFetcher.EXPECT().LatestCommit(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(commit, nil)
+
+	mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).DoAndReturn(
+		func(ctx context.Context, req types.NamespacedName, job *batchv1.Job, opts ...interface{}) error {
+			return fmt.Errorf("GITJOB ERROR")
+		},
+	)
+
+	recorderMock := mocks.NewMockEventRecorder(mockCtrl)
+	recorderMock.EXPECT().Event(
+		&gitRepoMatcher{gitRepo},
+		fleetevent.Normal,
+		"GotNewCommit",
+		"1883fd54bc5dfd225acf02aecbb6cb8020458e33",
+	)
+	recorderMock.EXPECT().Event(
+		&gitRepoMatcher{gitRepo},
+		fleetevent.Warning,
+		"FailedToGetGitJob",
+		"error retrieving git job: GITJOB ERROR",
+	)
+
+	r := GitJobReconciler{
+		Client:     mockClient,
+		Scheme:     scheme,
+		Image:      "",
+		Clock:      RealClock{},
+		Recorder:   recorderMock,
+		GitFetcher: mockFetcher,
+	}
+
+	ctx := context.TODO()
+	_, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: namespacedName})
+	if err == nil {
+		t.Errorf("expecting an error, got nil")
+	}
+	if err.Error() != "error retrieving git job: GITJOB ERROR" {
+		t.Errorf("unexpected error %v", err)
+	}
+}
+
+func TestReconcile_Error_WhenSecretDoesNotExist(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	scheme := runtime.NewScheme()
+	utilruntime.Must(batchv1.AddToScheme(scheme))
+	gitRepo := fleetv1.GitRepo{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "gitrepo",
+			Namespace: "default",
+		},
+	}
+	namespacedName := types.NamespacedName{Name: gitRepo.Name, Namespace: gitRepo.Namespace}
+	mockClient := mocks.NewMockClient(mockCtrl)
+	mockClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
+
+	mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), &gitRepoPointerMatcher{}, gomock.Any()).Times(2).DoAndReturn(
+		func(ctx context.Context, req types.NamespacedName, gitrepo *fleetv1.GitRepo, opts ...interface{}) error {
+			gitrepo.Name = gitRepo.Name
+			gitrepo.Namespace = gitRepo.Namespace
+			gitrepo.Spec.Repo = "repo"
+			gitrepo.Spec.HelmSecretNameForPaths = "somevalue"
+			controllerutil.AddFinalizer(gitrepo, finalize.GitRepoFinalizer)
+			gitrepo.Status.Commit = "dd45c7ad68e10307765104fea4a1f5997643020f"
+			return nil
+		},
+	)
+	mockFetcher := gitmocks.NewMockGitFetcher(mockCtrl)
+	commit := "1883fd54bc5dfd225acf02aecbb6cb8020458e33"
+	mockFetcher.EXPECT().LatestCommit(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(commit, nil)
+
+	// we need to return a NotFound error, so the code tries to create it.
+	mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).DoAndReturn(
+		func(ctx context.Context, req types.NamespacedName, job *batchv1.Job, opts ...interface{}) error {
+			return errors.NewNotFound(schema.GroupResource{}, "TEST ERROR")
+		},
+	)
+
+	mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).DoAndReturn(
+		func(ctx context.Context, req types.NamespacedName, job *corev1.Secret, opts ...interface{}) error {
+			return fmt.Errorf("SECRET ERROR")
+		},
+	)
+
+	recorderMock := mocks.NewMockEventRecorder(mockCtrl)
+	recorderMock.EXPECT().Event(
+		&gitRepoMatcher{gitRepo},
+		fleetevent.Normal,
+		"GotNewCommit",
+		"1883fd54bc5dfd225acf02aecbb6cb8020458e33",
+	)
+	recorderMock.EXPECT().Event(
+		&gitRepoMatcher{gitRepo},
+		fleetevent.Warning,
+		"FailedValidatingSecret",
+		"failed to look up HelmSecretNameForPaths, error: SECRET ERROR",
+	)
+
+	statusClient := mocks.NewMockSubResourceWriter(mockCtrl)
+	mockClient.EXPECT().Status().Times(1).Return(statusClient)
+	statusClient.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any()).Do(
+		func(ctx context.Context, repo *fleetv1.GitRepo, opts ...interface{}) {
+			c, found := getCondition(repo, fleetv1.GitRepoAcceptedCondition)
+			if !found {
+				t.Errorf("expecting to find the %s condition and could not find it.", fleetv1.GitRepoAcceptedCondition)
+			}
+			if c.Message != "failed to look up HelmSecretNameForPaths, error: SECRET ERROR" {
+				t.Errorf("expecting message [failed to look up HelmSecretNameForPaths, error: SECRET ERROR] in condition, got [%s]", c.Message)
+			}
+		},
+	)
+
+	r := GitJobReconciler{
+		Client:     mockClient,
+		Scheme:     scheme,
+		Image:      "",
+		Clock:      RealClock{},
+		Recorder:   recorderMock,
+		GitFetcher: mockFetcher,
+	}
+
+	ctx := context.TODO()
+	_, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: namespacedName})
+	if err == nil {
+		t.Errorf("expecting an error, got nil")
+	}
+	if err.Error() != "failed to look up HelmSecretNameForPaths, error: SECRET ERROR" {
+		t.Errorf("unexpected error %v", err)
+	}
+}
+
 func TestNewJob(t *testing.T) { // nolint:funlen
 	securityContext := &corev1.SecurityContext{
 		AllowPrivilegeEscalation: &[]bool{false}[0],