Merge pull request #776 from fluxcd/remove-ready-check

Fix ImagePolicy reconciler spamming `no tags in database` after a restart

Author: matheuscscp

internal/controller/imagepolicy_controller.go

@@ -330,16 +330,6 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP
 		return
 	}
 
-	// Proceed only if the ImageRepository has scan result.
-	if !conditions.IsReady(repo) {
-		// Mark not ready but don't requeue. When the repository becomes ready,
-		// it'll trigger a policy reconciliation. No runtime error to prevent
-		// requeue.
-		conditions.MarkFalse(obj, meta.ReadyCondition, imagev1.DependencyNotReadyReason, "referenced ImageRepository has not been scanned yet")
-		result, retErr = ctrl.Result{}, nil
-		return
-	}
-
 	// Check if the image is valid and mark stalled if not.
 	if _, err := registry.ParseImageReference(repo.Spec.Image, repo.Spec.Insecure); err != nil {
 		conditions.MarkStalled(obj, imagev1.ImageURLInvalidReason, "%s", err)
@@ -368,10 +358,10 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP
 			return
 		}
 
-		// If there's no tag in the database, mark not ready and retry.
+		// If there's no tag in the database, mark not ready.
 		if err == errNoTagsInDatabase {
 			conditions.MarkFalse(obj, meta.ReadyCondition, imagev1.DependencyNotReadyReason, "%s", err)
-			result, retErr = ctrl.Result{}, err
+			result, retErr = ctrl.Result{}, nil
 			return
 		}
 

internal/controller/imagepolicy_controller_test.go

@@ -47,7 +47,7 @@ import (
 	"github.com/fluxcd/image-reflector-controller/internal/test"
 )
 
-func TestImagePolicyReconciler_imageRepoNotReady(t *testing.T) {
+func TestImagePolicyReconciler_imageRepoHasNoTags(t *testing.T) {
 	g := NewWithT(t)
 
 	namespaceName := "imagepolicy-" + randStringRunes(5)
@@ -65,19 +65,14 @@ func TestImagePolicyReconciler_imageRepoNotReady(t *testing.T) {
 			Name:      "repo",
 		},
 		Spec: imagev1.ImageRepositorySpec{
-			Image: "ghcr.io/stefanprodan/podinfo/foo:bar:zzz:qqq/aaa",
+			Image: "ghcr.io/doesnot/exist",
 		},
 	}
 	g.Expect(k8sClient.Create(ctx, imageRepo)).NotTo(HaveOccurred())
 	t.Cleanup(func() {
 		g.Expect(k8sClient.Delete(ctx, imageRepo)).NotTo(HaveOccurred())
 	})
 
-	g.Eventually(func() bool {
-		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imageRepo), imageRepo)
-		return err == nil && conditions.IsStalled(imageRepo)
-	}).Should(BeTrue())
-
 	imagePolicy := &imagev1.ImagePolicy{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: namespaceName,
@@ -87,6 +82,9 @@ func TestImagePolicyReconciler_imageRepoNotReady(t *testing.T) {
 			ImageRepositoryRef: meta.NamespacedObjectReference{
 				Name: imageRepo.Name,
 			},
+			Policy: imagev1.ImagePolicyChoice{
+				Alphabetical: &imagev1.AlphabeticalPolicy{},
+			},
 		},
 	}
 	g.Expect(k8sClient.Create(ctx, imagePolicy)).NotTo(HaveOccurred())
@@ -174,15 +172,17 @@ func TestImagePolicyReconciler_ignoresImageRepoNotReadyEvent(t *testing.T) {
 	}).Should(BeTrue())
 
 	// Check that the ImagePolicy is still ready and does not get updated.
-	err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imagePolicy), imagePolicy)
-	g.Expect(err).NotTo(HaveOccurred())
-	g.Expect(conditions.IsReady(imagePolicy)).To(BeTrue())
+	g.Eventually(func() bool {
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imagePolicy), imagePolicy)
+		return err == nil && conditions.IsReady(imagePolicy)
+	}).Should(BeTrue())
 
 	// Wait a bit and check that the ImagePolicy remains ready.
 	time.Sleep(time.Second)
-	err = k8sClient.Get(ctx, client.ObjectKeyFromObject(imagePolicy), imagePolicy)
-	g.Expect(err).NotTo(HaveOccurred())
-	g.Expect(conditions.IsReady(imagePolicy)).To(BeTrue())
+	g.Eventually(func() bool {
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imagePolicy), imagePolicy)
+		return err == nil && conditions.IsReady(imagePolicy)
+	}).Should(BeTrue())
 }
 
 func TestImagePolicyReconciler_invalidImage(t *testing.T) {

internal/controller/imagerepository_controller.go

@@ -290,7 +290,12 @@ func (r *ImageRepositoryReconciler) reconcile(ctx context.Context, sp *patch.Ser
 	// Scan the repository if it's scan time. No scan is a no-op reconciliation.
 	// The next scan time is not reset in case of no-op reconciliation.
 	if ok {
-		reconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "scanning: %s", reasonMsg)
+		// When the database is empty, we need to set the Ready condition to
+		// Unknown to force a transition when the controller restarts.
+		// This transition is required to unblock the ImagePolicy object
+		// from the Ready condition stuck in False with reason DependencyNotReady.
+		drift := reasonMsg == scanReasonEmptyDatabase
+		reconcile.ProgressiveStatus(drift, obj, meta.ProgressingReason, "scanning: %s", reasonMsg)
 		if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
 			result, retErr = ctrl.Result{}, err
 			return