Requeue when the pipeline isn't ready

If a cache has not yet synced, or a target is not to be found, and the
controller has incomplete information, then the safest thing to do is
not make any decisions on promotions, and instead requeue the
pipeline for another try later.

This fixes a problem that was exposed by adding a predicate to the
watch on Pipeline: if a cluster cache isn't ready when the controller
looks, and the target doesn't exist, there's nothing to trigger
another go-round. This presented itself as a problem with the remote
and Kustomization tests, which explicitly check for a "not found"
target status.

(I fixed another problem with those tests, in passing -- if they are
going to check for an updated target status, they had better fetch the
object again.)

Signed-off-by: Michael Bridgen <[email protected]>

controllers/leveltriggered/controller.go

@@ -215,6 +215,11 @@ func (r *PipelineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		pipeline.GetNamespace(), pipeline.GetName(),
 	)
 
+	// If it's not ready, we can't make any promotion decisions. Requeue, presuming backoff.
+	if unready {
+		return ctrl.Result{Requeue: true}, nil
+	}
+
 	firstEnv := pipeline.Spec.Environments[0]
 
 	firstEnvStatus, ok := pipeline.Status.Environments[firstEnv.Name]
@@ -470,7 +475,9 @@ func (r *PipelineReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	}
 
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&v1alpha1.Pipeline{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
+		For(&v1alpha1.Pipeline{},
+			builder.WithPredicates(predicate.GenerationChangedPredicate{}),
+		).
 		Watches(
 			&clusterctrlv1alpha1.GitopsCluster{},
 			handler.EnqueueRequestsFromMapFunc(r.requestsForCluster(gitopsClusterIndexKey)),

controllers/leveltriggered/controller_remote_test.go

@@ -99,6 +99,7 @@ func TestRemoteTargets(t *testing.T) {
 		g.Expect(getTargetStatus(g, p, "test", 0).Ready).NotTo(BeTrue())
 		// we can see "target cluster client not synced" before "not found"
 		g.Eventually(func() string {
+			p = getPipeline(ctx, g, client.ObjectKeyFromObject(pipeline))
 			return getTargetStatus(g, p, "test", 0).Error
 		}).Should(ContainSubstring("not found"))
 

controllers/leveltriggered/controller_test.go

@@ -139,7 +139,10 @@ func TestReconcile(t *testing.T) {
 		// the application hasn't been created, so we expect "not found"
 		p := getPipeline(ctx, g, client.ObjectKeyFromObject(pipeline))
 		g.Expect(getTargetStatus(g, p, "test", 0).Ready).NotTo(BeTrue())
-		g.Expect(getTargetStatus(g, p, "test", 0).Error).To(ContainSubstring("not found"))
+		g.Eventually(func() string {
+			p = getPipeline(ctx, g, client.ObjectKeyFromObject(pipeline))
+			return getTargetStatus(g, p, "test", 0).Error
+		}, "2s").Should(ContainSubstring("not found"))
 
 		// FIXME create the app
 		app := kustomv1.Kustomization{