Fix reconcile loop when cache definitions are invalid (#164)

The reconcile set the status two times with each reconcile: First to `WaitingForCacheSync` and then to `*Error`.
This PR changes the logic to set `WaitingForCacheSync` only after it is known if there were any errors.

Author: bastjan

controllers/managedresource_controller.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"sync/atomic"
 
 	"github.com/DmitriyVTitov/size"
 	"github.com/google/go-jsonnet"
@@ -73,6 +74,10 @@ type ManagedResourceReconciler struct {
 
 	configHash       string
 	configGeneration int64
+
+	// started is set to true after the first reconcile has been started.
+	// It's a bit of a hack to detect that the caches are synced as it's much easier than starting to track the cache sync state.
+	started atomic.Bool
 }
 
 type instanceCache struct {
@@ -170,6 +175,8 @@ func newEspejoteError(err error, t EspejoteErrorType) EspejoteError {
 }
 
 func (r *ManagedResourceReconciler) Reconcile(ctx context.Context, req Request) (ctrl.Result, error) {
+	r.started.CompareAndSwap(false, true)
+
 	// Since we are not using the default builder we need to add the request to the context ourselves
 	ctx = log.IntoContext(ctx, log.FromContext(ctx).WithValues("request", req))
 

controllers/managedresource_controller_manager.go

@@ -132,6 +132,12 @@ func (r *ManagedResourceControllerManager) Reconcile(ctx context.Context, req re
 				r.recordErr(ctx, fmt.Errorf("failed to ensure instance controller: %w", err), string(DependencyConfigurationError), managedResource, req),
 			)
 		}
+		if !ic.reconciler.started.Load() && managedResource.Status.Status != string(WaitingForCacheSync) {
+			managedResource.Status.Status = string(WaitingForCacheSync)
+			if err := r.Status().Update(ctx, &managedResource); err != nil {
+				return ctrl.Result{}, fmt.Errorf("failed to update status of managed resource %q: %w", req.NamespacedName, err)
+			}
+		}
 		if err := ic.StartErr(); err != nil {
 			return ctrl.Result{}, multierr.Combine(
 				fmt.Errorf("failed to start instance controller for managed resource %q: %w", req.NamespacedName, err),
@@ -156,14 +162,13 @@ func (r *ManagedResourceControllerManager) recordErr(ctx context.Context, err er
 	}
 
 	r.Recorder.Eventf(&managedResource, nil, "Warning", errType, "Reconcile", "%s: %s", errType, err.Error())
-	var statusUpdateErr error
 	if managedResource.Status.Status != errType {
 		managedResource.Status.Status = errType
 		if err := r.Status().Update(ctx, &managedResource); err != nil {
-			statusUpdateErr = fmt.Errorf("failed to update status of managed resource %q: %w", req.NamespacedName, err)
+			return fmt.Errorf("failed to update status of managed resource %q: %w", req.NamespacedName, err)
 		}
 	}
-	return statusUpdateErr
+	return nil
 }
 
 func (r *ManagedResourceControllerManager) ensureInstanceControllerFor(ctx context.Context, mr espejotev1alpha1.ManagedResource) (*resourceController, error) {
@@ -199,13 +204,6 @@ func (r *ManagedResourceControllerManager) ensureInstanceControllerFor(ctx conte
 		delete(r.controllers, mrKey)
 	}
 
-	if mr.Status.Status != string(WaitingForCacheSync) {
-		mr.Status.Status = string(WaitingForCacheSync)
-		if err := r.Status().Update(ctx, &mr); err != nil {
-			return nil, fmt.Errorf("failed to update status of managed resource %q: %w", mrKey, err)
-		}
-	}
-
 	instanceCtrlCtx, instanceCtrlCancel := context.WithCancel(r.ControllerLifetimeCtx)
 	reconciler := &ManagedResourceReconciler{
 		For: mrKey,