Don't force requeuing after adding finalizer in GitRepo controller

integrationtests/gitjob/controller/controller_test.go

@@ -64,15 +64,7 @@ var _ = Describe("GitJob controller", func() {
 			gitRepo.Spec.CABundle = caBundle
 
 			Expect(k8sClient.Create(ctx, &gitRepo)).ToNot(HaveOccurred())
-			Eventually(func() string {
-				var gitRepoFromCluster v1alpha1.GitRepo
-				err := k8sClient.Get(ctx, types.NamespacedName{Name: gitRepo.Name, Namespace: gitRepo.Namespace}, &gitRepoFromCluster)
-				if err != nil {
-					// maybe the resource gitrepo is not created yet
-					return ""
-				}
-				return gitRepoFromCluster.Status.Display.ReadyBundleDeployments
-			}).Should(ContainSubstring("0/0"))
+			waitGitrepoCreated(gitRepo)
 
 			By("Creating a job")
 			Eventually(func() error {
@@ -367,6 +359,7 @@ var _ = Describe("GitJob controller", func() {
 			expectedCommit = commit
 			gitRepo = createGitRepo(gitRepoName)
 			Expect(k8sClient.Create(ctx, &gitRepo)).ToNot(HaveOccurred())
+			waitGitrepoCreated(gitRepo)
 
 			By("creating a Job")
 			Eventually(func() error {
@@ -402,6 +395,8 @@ var _ = Describe("GitJob controller", func() {
 		JustBeforeEach(func() {
 			gitRepo = createGitRepo(gitRepoName)
 			Expect(k8sClient.Create(ctx, &gitRepo)).ToNot(HaveOccurred())
+			waitGitrepoCreated(gitRepo)
+
 			Eventually(func() error {
 				jobName = names.SafeConcatName(gitRepoName, names.Hex(repo+commit, 5))
 				return k8sClient.Get(ctx, types.NamespacedName{Name: jobName, Namespace: gitRepoNamespace}, &job)
@@ -517,6 +512,8 @@ var _ = Describe("GitJob controller", func() {
 		JustBeforeEach(func() {
 			gitRepo = createGitRepo(gitRepoName)
 			Expect(k8sClient.Create(ctx, &gitRepo)).ToNot(HaveOccurred())
+			waitGitrepoCreated(gitRepo)
+
 			Eventually(func() error {
 				jobName = names.SafeConcatName(gitRepoName, names.Hex(repo+commit, 5))
 				return k8sClient.Get(ctx, types.NamespacedName{Name: jobName, Namespace: gitRepoNamespace}, &job)
@@ -582,6 +579,7 @@ var _ = Describe("GitJob controller", func() {
 		JustBeforeEach(func() {
 			gitRepo = createGitRepoWithDisablePolling(gitRepoName)
 			Expect(k8sClient.Create(ctx, &gitRepo)).To(Succeed())
+			waitGitrepoCreated(gitRepo)
 
 			By("Creating a job")
 			Eventually(func() error {
@@ -697,9 +695,10 @@ var _ = Describe("GitJob controller", func() {
 			gitRepo = createGitRepo(gitRepoName)
 			gitRepo.Spec.HelmSecretNameForPaths = helmSecretNameForPaths
 			gitRepo.Spec.HelmSecretName = helmSecretName
-			// Create should return an error
+			// Create should not return an error
 			err := k8sClient.Create(ctx, &gitRepo)
 			Expect(err).ToNot(HaveOccurred())
+			waitGitrepoCreated(gitRepo)
 		})
 
 		AfterEach(func() {

integrationtests/gitjob/controller/gitrepo_test.go

@@ -56,6 +56,9 @@ var _ = Describe("GitRepo", func() {
 			expectedCommit = commit
 			err := k8sClient.Create(ctx, gitrepo)
 			Expect(err).NotTo(HaveOccurred())
+			waitGitrepoCreated(*gitrepo)
+			// simulate update of generation after adding finalizer (the test environment does not do it)
+			Expect(simulateIncreaseForceSyncGeneration(*gitrepo)).ToNot(HaveOccurred())
 		})
 
 		It("updates the gitrepo status", func() {

integrationtests/gitjob/controller/suite_test.go

@@ -205,6 +205,18 @@ func waitDeleteGitrepo(gitRepo v1alpha1.GitRepo) {
 	}).Should(BeTrue())
 }
 
+func waitGitrepoCreated(gitRepo v1alpha1.GitRepo) {
+	Eventually(func() string {
+		var gitRepoFromCluster v1alpha1.GitRepo
+		err := k8sClient.Get(ctx, types.NamespacedName{Name: gitRepo.Name, Namespace: gitRepo.Namespace}, &gitRepoFromCluster)
+		if err != nil {
+			// maybe the resource gitrepo is not created yet
+			return ""
+		}
+		return gitRepoFromCluster.Status.Display.ReadyBundleDeployments
+	}).Should(ContainSubstring("0/0"))
+}
+
 func beOwnedBy(expected interface{}) gomegatypes.GomegaMatcher {
 	return gcustom.MakeMatcher(func(meta metav1.ObjectMeta) (bool, error) {
 		ref, ok := expected.(metav1.OwnerReference)

internal/cmd/controller/gitops/reconciler/gitjob_controller.go

@@ -157,7 +157,6 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 			return ctrl.Result{}, err
 		}
 
-		// requeue as adding the finalizer changes the spec
 		return ctrl.Result{Requeue: true}, nil
 	}
 

internal/cmd/controller/imagescan/gitcommit_job.go

@@ -333,7 +333,10 @@ func setupKnownHosts(gitrepo *fleet.GitRepo, data []byte) error {
 	return nil
 }
 
-func commitAllAndPush(ctx context.Context, repo *gogit.Repository, auth transport.AuthMethod, commit fleet.CommitSpec) (string, error) {
+func commitAllAndPush(ctx context.Context, repo *gogit.Repository, auth transport.AuthMethod, commit *fleet.CommitSpec) (string, error) {
+	if commit == nil {
+		return "", nil
+	}
 	working, err := repo.Worktree()
 	if err != nil {
 		return "", err

pkg/apis/fleet.cattle.io/v1alpha1/gitrepo_types.go

@@ -121,7 +121,7 @@ type GitRepoSpec struct {
 	ImageSyncInterval *metav1.Duration `json:"imageScanInterval,omitempty"`
 
 	// Commit specifies how to commit to the git repo when a new image is scanned and written back to git repo.
-	ImageScanCommit CommitSpec `json:"imageScanCommit,omitempty"`
+	ImageScanCommit *CommitSpec `json:"imageScanCommit,omitempty"`
 
 	// KeepResources specifies if the resources created must be kept after deleting the GitRepo.
 	KeepResources bool `json:"keepResources,omitempty"`