Fix empty errors in logs when a reconciled resource is not ready

`NotReadyError` ensures that its error message is not empty.

The initial issue was that when the app droplet is not set, the error
gets its reson set to `DropletNotAssigned`, but the reason is not
reflected in the error message. This commit includes the reason, and
ensures that the message is at least `resource not ready`

While being here, do not requeue reconcile when the app droplet is not
set in its spec. It makes no sense to requeue the reconcile event as it
is expected that the droplet is set on the spec externally. When that
happens the app would be reconciled anyway

fixes #3862

Author: danail-branekov

controllers/controllers/workloads/apps/controller.go

@@ -146,7 +146,7 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, cfApp *korifiv1alpha
 	cfApp.Status.VCAPServicesSecretName = secretName
 
 	if cfApp.Spec.CurrentDropletRef.Name == "" {
-		return ctrl.Result{}, k8s.NewNotReadyError().WithReason("DropletNotAssigned")
+		return ctrl.Result{}, k8s.NewNotReadyError().WithReason("DropletNotAssigned").WithNoRequeue()
 	}
 
 	droplet, err := r.getDroplet(ctx, cfApp)

kpack-image-builder/controllers/builderinfo_controller_test.go

@@ -171,7 +171,7 @@ var _ = Describe("BuilderInfoReconciler", func() {
 				g.Expect(readyCondition).NotTo(BeNil())
 				g.Expect(readyCondition.Status).To(Equal(metav1.ConditionFalse))
 				g.Expect(readyCondition.Reason).To(Equal("ClusterBuilderNotReady"))
-				g.Expect(readyCondition.Message).To(Equal(fmt.Sprintf("ClusterBuilder %q is not ready: something happened", clusterBuilderName)))
+				g.Expect(readyCondition.Message).To(ContainSubstring(fmt.Sprintf("ClusterBuilder %q is not ready: something happened", clusterBuilderName)))
 				g.Expect(readyCondition.ObservedGeneration).To(Equal(info.Generation))
 			}).Should(Succeed())
 		})

tools/k8s/reconcile.go

@@ -73,12 +73,7 @@ func (r *PatchingReconciler[T]) Reconcile(ctx context.Context, req ctrl.Request)
 
 		var notReadyErr NotReadyError
 		if errors.As(delegateErr, &notReadyErr) {
-			reason := notReadyErr.reason
-			if reason == "" {
-				reason = "Unknown"
-			}
-
-			readyConditionBuilder.WithReason(reason).WithMessage(notReadyErr.message)
+			readyConditionBuilder.WithReason(tools.IfZero(notReadyErr.reason, "Unknown")).WithMessage(notReadyErr.message)
 
 			if notReadyErr.noRequeue {
 				result = ctrl.Result{}
@@ -118,16 +113,21 @@ type NotReadyError struct {
 }
 
 func (e NotReadyError) Error() string {
-	if e.cause == nil {
-		return e.message
+	composedMessage := "resource not ready"
+
+	if e.reason != "" {
+		composedMessage = composedMessage + ": " + e.reason
+	}
+
+	if e.message != "" {
+		composedMessage = composedMessage + ": " + e.message
 	}
 
-	message := e.message
-	if message != "" {
-		message = message + ": "
+	if e.cause != nil {
+		composedMessage = composedMessage + ": " + e.cause.Error()
 	}
 
-	return fmt.Sprintf("%s%s", message, e.cause.Error())
+	return composedMessage
 }
 
 func NewNotReadyError() NotReadyError {

tools/k8s/reconcile_test.go

@@ -233,10 +233,14 @@ var _ = Describe("Reconcile", func() {
 					HasType(Equal(korifiv1alpha1.StatusConditionReady)),
 					HasStatus(Equal(metav1.ConditionFalse)),
 					HasReason(Equal("Unknown")),
-					HasMessage(BeEmpty()),
+					HasMessage(ContainSubstring("resource not ready")),
 				)))
 			})
 
+			It("returns a non-empty error", func() {
+				Expect(err.Error()).To(ContainSubstring("resource not ready"))
+			})
+
 			When("reason is specified", func() {
 				BeforeEach(func() {
 					objectReconciler.reconcileResourceError = k8s.NewNotReadyError().WithReason("TestReason")
@@ -270,7 +274,7 @@ var _ = Describe("Reconcile", func() {
 					Expect(updatedOrg.Status.Conditions).To(ContainElement(SatisfyAll(
 						HasType(Equal(korifiv1alpha1.StatusConditionReady)),
 						HasStatus(Equal(metav1.ConditionFalse)),
-						HasMessage(Equal("TestMessage")),
+						HasMessage(ContainSubstring("TestMessage")),
 					)))
 				})
 			})
@@ -289,7 +293,7 @@ var _ = Describe("Reconcile", func() {
 					Expect(updatedOrg.Status.Conditions).To(ContainElement(SatisfyAll(
 						HasType(Equal(korifiv1alpha1.StatusConditionReady)),
 						HasStatus(Equal(metav1.ConditionFalse)),
-						HasMessage(Equal("test-err")),
+						HasMessage(ContainSubstring(("test-err"))),
 					)))
 				})
 			})
@@ -310,7 +314,7 @@ var _ = Describe("Reconcile", func() {
 					Expect(updatedOrg.Status.Conditions).To(ContainElement(SatisfyAll(
 						HasType(Equal(korifiv1alpha1.StatusConditionReady)),
 						HasStatus(Equal(metav1.ConditionFalse)),
-						HasMessage(Equal("TestMessage: test-err")),
+						HasMessage(ContainSubstring("TestMessage: test-err")),
 					)))
 				})
 			})