Allow errors to be quiet (#429)

ErrQuiet errors are not logged within reconciler-runtime and do not
trigger events to be recorded. This behavior is conventional rather than
technical. An implementor may choose to ignore these conventions when it
fits their purpose. Quite is not silent.

Also redefines ErrHaltSubReconcilers to wrap ErrQuiet.
HaltSubReconcilers is deprecated in favor of ErrHaltSubReconcilers.

Signed-off-by: Scott Andrews <andrewssc@vmware.com>

README.md

@@ -1076,8 +1076,11 @@ reconciler-runtime is rapidly evolving. While we strive for API compatability be
 
 ### Current Deprecations
 
-- status `InitializeConditions()` is deprecated in favor of `InitializeConditions(context.Context)`. Support may be removed in a future release, users are encouraged to migrate.
-- `ConditionSet#Manage` is deprecated in favor of `ConditionSet#ManageWithContext`. Support may be removed in a future release, users are encuraged to migrate.
+Backwards support may be removed in a future release, users are encouraged to migrate.
+
+- status `InitializeConditions()` is deprecated in favor of `InitializeConditions(context.Context)`.
+- `ConditionSet#Manage` is deprecated in favor of `ConditionSet#ManageWithContext`.
+- `HaltSubReconcilers` is deprecated in favor of `ErrHaltSubReconcilers`.
 
 ## Contributing
 

reconcilers/aggregate.go

@@ -62,7 +62,7 @@ type AggregateReconciler[Type client.Object] struct {
 	// Reconciler is called for each reconciler request with the resource being reconciled.
 	// Typically, Reconciler is a Sequence of multiple SubReconcilers.
 	//
-	// When HaltSubReconcilers is returned as an error, execution continues as if no error was
+	// When ErrHaltSubReconcilers is returned as an error, execution continues as if no error was
 	// returned.
 	//
 	// +optional
@@ -227,7 +227,9 @@ func (r *AggregateReconciler[T]) Reconcile(ctx context.Context, req Request) (Re
 			resource.SetNamespace(r.Request.Namespace)
 			resource.SetName(r.Request.Name)
 		} else {
-			log.Error(err, "unable to fetch resource")
+			if !errors.Is(err, ErrQuiet) {
+				log.Error(err, "unable to fetch resource")
+			}
 			return Result{}, err
 		}
 	}
@@ -238,7 +240,7 @@ func (r *AggregateReconciler[T]) Reconcile(ctx context.Context, req Request) (Re
 	}
 
 	result, err := r.Reconciler.Reconcile(ctx, resource)
-	if err != nil && !errors.Is(err, HaltSubReconcilers) {
+	if err != nil && !errors.Is(err, ErrHaltSubReconcilers) {
 		return result, err
 	}
 

reconcilers/aggregate_test.go

@@ -342,7 +342,7 @@ func TestAggregateReconciler(t *testing.T) {
 					r.Reconciler = reconcilers.Sequence[*corev1.ConfigMap]{
 						&reconcilers.SyncReconciler[*corev1.ConfigMap]{
 							Sync: func(ctx context.Context, resource *corev1.ConfigMap) error {
-								return reconcilers.HaltSubReconcilers
+								return reconcilers.ErrHaltSubReconcilers
 							},
 						},
 						&reconcilers.SyncReconciler[*corev1.ConfigMap]{
@@ -371,7 +371,7 @@ func TestAggregateReconciler(t *testing.T) {
 					r.Reconciler = reconcilers.Sequence[*corev1.ConfigMap]{
 						&reconcilers.SyncReconciler[*corev1.ConfigMap]{
 							SyncWithResult: func(ctx context.Context, resource *corev1.ConfigMap) (reconcilers.Result, error) {
-								return reconcilers.Result{Requeue: true}, reconcilers.HaltSubReconcilers
+								return reconcilers.Result{Requeue: true}, reconcilers.ErrHaltSubReconcilers
 							},
 						},
 						&reconcilers.SyncReconciler[*corev1.ConfigMap]{

reconcilers/child.go

@@ -274,7 +274,9 @@ func (r *ChildReconciler[T, CT, CLT]) Reconcile(ctx context.Context, resource T)
 			r.ReflectChildStatusOnParent(ctx, resource, child, err)
 			return Result{}, nil
 		}
-		log.Error(err, "unable to reconcile child")
+		if !errors.Is(err, ErrQuiet) {
+			log.Error(err, "unable to reconcile child")
+		}
 		return Result{}, err
 	}
 	r.ReflectChildStatusOnParent(ctx, resource, child, nil)
@@ -301,8 +303,10 @@ func (r *ChildReconciler[T, CT, CLT]) reconcile(ctx context.Context, resource T)
 		for _, extra := range items {
 			log.Info("deleting extra child", "child", namespaceName(extra))
 			if err := c.Delete(ctx, extra); err != nil {
-				pc.Recorder.Eventf(resource, corev1.EventTypeWarning, "DeleteFailed",
-					"Failed to delete %s %q: %v", typeName(r.ChildType), extra.GetName(), err)
+				if !errors.Is(err, ErrQuiet) {
+					pc.Recorder.Eventf(resource, corev1.EventTypeWarning, "DeleteFailed",
+						"Failed to delete %s %q: %v", typeName(r.ChildType), extra.GetName(), err)
+				}
 				return nilCT, err
 			}
 			pc.Recorder.Eventf(resource, corev1.EventTypeNormal, "Deleted",

reconcilers/finalizer.go

@@ -7,6 +7,7 @@ package reconcilers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/go-logr/logr"
@@ -142,9 +143,11 @@ func ensureFinalizer(ctx context.Context, resource client.Object, finalizer stri
 
 				patch := client.MergeFromWithOptions(current, client.MergeFromWithOptimisticLock{})
 				if err := config.Patch(ctx, desired, patch); err != nil {
-					log.Error(err, "unable to patch finalizers", "finalizer", finalizer)
-					config.Recorder.Eventf(current, corev1.EventTypeWarning, "FinalizerPatchFailed",
-						"Failed to patch finalizer %q: %s", finalizer, err)
+					if !errors.Is(err, ErrQuiet) {
+						log.Error(err, "unable to patch finalizers", "finalizer", finalizer)
+						config.Recorder.Eventf(current, corev1.EventTypeWarning, "FinalizerPatchFailed",
+							"Failed to patch finalizer %q: %s", finalizer, err)
+					}
 					return err
 				}
 				config.Recorder.Eventf(current, corev1.EventTypeNormal, "FinalizerPatched",

reconcilers/reconcilers.go

@@ -29,12 +29,23 @@ type SubReconciler[Type client.Object] interface {
 }
 
 var (
-	// HaltSubReconcilers is an error that instructs SubReconcilers to stop processing the request,
-	// while the root reconciler proceeds as if there was no error. HaltSubReconcilers may be
+	// ErrQuiet are not logged or recorded as events within reconciler-runtime.
+	// They are propagated as errors unless otherwise defined.
+	//
+	// Test for ErrQuiet with errors.Is(err, ErrQuiet)
+	ErrQuiet = fmt.Errorf("quiet errors are returned as errors, but not logged or recorded")
+
+	// ErrHaltSubReconcilers is an error that instructs SubReconcilers to stop processing the request,
+	// while the root reconciler proceeds as if there was no error. ErrHaltSubReconcilers may be
 	// wrapped by other errors.
 	//
+	// ErrHaltSubReconcilers wraps ErrQuiet to suppress spurious logs.
+	//
 	// See documentation for the specific SubReconciler caller to see how they handle this case.
-	HaltSubReconcilers = errors.New("stop processing SubReconcilers, without returning an error")
+	ErrHaltSubReconcilers = fmt.Errorf("stop processing SubReconcilers, without returning an error: %w", ErrQuiet)
+
+	// Deprecated HaltSubReconcilers use ErrHaltSubReconcilers instead
+	HaltSubReconcilers = ErrHaltSubReconcilers
 )
 
 const requestStashKey StashKey = "reconciler-runtime:request"
@@ -211,7 +222,9 @@ func EnqueueTracked(ctx context.Context) handler.EventHandler {
 
 			items, err := c.Tracker.GetObservers(obj)
 			if err != nil {
-				log.Error(err, "unable to get tracked requests")
+				if !errors.Is(err, ErrQuiet) {
+					log.Error(err, "unable to get tracked requests")
+				}
 				return nil
 			}
 

reconcilers/resource.go

@@ -68,7 +68,7 @@ type ResourceReconciler[Type client.Object] struct {
 	// Reconciler is called for each reconciler request with the resource being reconciled.
 	// Typically, Reconciler is a Sequence of multiple SubReconcilers.
 	//
-	// When HaltSubReconcilers is returned as an error, execution continues as if no error was
+	// When ErrHaltSubReconcilers is returned as an error, execution continues as if no error was
 	// returned.
 	Reconciler SubReconciler[Type]
 
@@ -206,7 +206,9 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res
 			// on deleted requests.
 			return Result{}, nil
 		}
-		log.Error(err, "unable to fetch resource")
+		if !errors.Is(err, ErrQuiet) {
+			log.Error(err, "unable to fetch resource")
+		}
 		return Result{}, err
 	}
 	resource := originalResource.DeepCopyObject().(T)
@@ -233,9 +235,11 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res
 			// patch status
 			log.Info("patching status", "diff", cmp.Diff(originalResourceStatus, resourceStatus))
 			if patchErr := c.Status().Patch(ctx, resource, client.MergeFrom(originalResource)); patchErr != nil {
-				log.Error(patchErr, "unable to patch status")
-				c.Recorder.Eventf(resource, corev1.EventTypeWarning, "StatusPatchFailed",
-					"Failed to patch status: %v", patchErr)
+				if !errors.Is(patchErr, ErrQuiet) {
+					log.Error(patchErr, "unable to patch status")
+					c.Recorder.Eventf(resource, corev1.EventTypeWarning, "StatusPatchFailed",
+						"Failed to patch status: %v", patchErr)
+				}
 				return Result{}, patchErr
 			}
 			c.Recorder.Eventf(resource, corev1.EventTypeNormal, "StatusPatched",
@@ -244,9 +248,11 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res
 			// update status
 			log.Info("updating status", "diff", cmp.Diff(originalResourceStatus, resourceStatus))
 			if updateErr := c.Status().Update(ctx, resource); updateErr != nil {
-				log.Error(updateErr, "unable to update status")
-				c.Recorder.Eventf(resource, corev1.EventTypeWarning, "StatusUpdateFailed",
-					"Failed to update status: %v", updateErr)
+				if !errors.Is(updateErr, ErrQuiet) {
+					log.Error(updateErr, "unable to update status")
+					c.Recorder.Eventf(resource, corev1.EventTypeWarning, "StatusUpdateFailed",
+						"Failed to update status: %v", updateErr)
+				}
 				return Result{}, updateErr
 			}
 			c.Recorder.Eventf(resource, corev1.EventTypeNormal, "StatusUpdated",
@@ -265,7 +271,7 @@ func (r *ResourceReconciler[T]) reconcile(ctx context.Context, resource T) (Resu
 	}
 
 	result, err := r.Reconciler.Reconcile(ctx, resource)
-	if err != nil && !errors.Is(err, HaltSubReconcilers) {
+	if err != nil && !errors.Is(err, ErrHaltSubReconcilers) {
 		return Result{}, err
 	}
 

reconcilers/resource_test.go

@@ -596,7 +596,7 @@ func TestResourceReconciler_Duck(t *testing.T) {
 								resource.Status.Fields = map[string]string{
 									"want": "this to run",
 								}
-								return reconcilers.HaltSubReconcilers
+								return reconcilers.ErrHaltSubReconcilers
 							},
 						},
 						&reconcilers.SyncReconciler[*resources.TestDuck]{
@@ -642,7 +642,7 @@ func TestResourceReconciler_Duck(t *testing.T) {
 								resource.Status.Fields = map[string]string{
 									"want": "this to run",
 								}
-								return reconcilers.Result{Requeue: true}, reconcilers.HaltSubReconcilers
+								return reconcilers.Result{Requeue: true}, reconcilers.ErrHaltSubReconcilers
 							},
 						},
 						&reconcilers.SyncReconciler[*resources.TestDuck]{
@@ -1104,7 +1104,7 @@ func TestResourceReconciler(t *testing.T) {
 								resource.Status.Fields = map[string]string{
 									"want": "this to run",
 								}
-								return reconcilers.Result{Requeue: true}, reconcilers.HaltSubReconcilers
+								return reconcilers.Result{Requeue: true}, reconcilers.ErrHaltSubReconcilers
 							},
 						},
 						&reconcilers.SyncReconciler[*resources.TestResource]{

reconcilers/resourcemanager.go

@@ -131,9 +131,11 @@ func (r *ResourceManager[T]) Manage(ctx context.Context, resource client.Object,
 		if !actual.GetCreationTimestamp().Time.IsZero() && actual.GetDeletionTimestamp() == nil {
 			log.Info("deleting unwanted resource", "resource", namespaceName(actual))
 			if err := c.Delete(ctx, actual); err != nil {
-				log.Error(err, "unable to delete unwanted resource", "resource", namespaceName(actual))
-				pc.Recorder.Eventf(resource, corev1.EventTypeWarning, "DeleteFailed",
-					"Failed to delete %s %q: %v", typeName(actual), actual.GetName(), err)
+				if !errors.Is(err, ErrQuiet) {
+					log.Error(err, "unable to delete unwanted resource", "resource", namespaceName(actual))
+					pc.Recorder.Eventf(resource, corev1.EventTypeWarning, "DeleteFailed",
+						"Failed to delete %s %q: %v", typeName(actual), actual.GetName(), err)
+				}
 				return nilT, err
 			}
 			pc.Recorder.Eventf(resource, corev1.EventTypeNormal, "Deleted",
@@ -151,9 +153,11 @@ func (r *ResourceManager[T]) Manage(ctx context.Context, resource client.Object,
 	if internal.IsNil(actual) || actual.GetCreationTimestamp().Time.IsZero() {
 		log.Info("creating resource", "resource", r.sanitize(desired))
 		if err := c.Create(ctx, desired); err != nil {
-			log.Error(err, "unable to create resource", "resource", namespaceName(desired))
-			pc.Recorder.Eventf(resource, corev1.EventTypeWarning, "CreationFailed",
-				"Failed to create %s %q: %v", typeName(desired), desired.GetName(), err)
+			if !errors.Is(err, ErrQuiet) {
+				log.Error(err, "unable to create resource", "resource", namespaceName(desired))
+				pc.Recorder.Eventf(resource, corev1.EventTypeWarning, "CreationFailed",
+					"Failed to create %s %q: %v", typeName(desired), desired.GetName(), err)
+			}
 			return nilT, err
 		}
 		if r.TrackDesired {
@@ -200,9 +204,11 @@ func (r *ResourceManager[T]) Manage(ctx context.Context, resource client.Object,
 	}
 	log.Info("updating resource", "diff", cmp.Diff(r.sanitize(actual), r.sanitize(current)))
 	if err := c.Update(ctx, current); err != nil {
-		log.Error(err, "unable to update resource", "resource", namespaceName(current))
-		pc.Recorder.Eventf(resource, corev1.EventTypeWarning, "UpdateFailed",
-			"Failed to update %s %q: %v", typeName(current), current.GetName(), err)
+		if !errors.Is(err, ErrQuiet) {
+			log.Error(err, "unable to update resource", "resource", namespaceName(current))
+			pc.Recorder.Eventf(resource, corev1.EventTypeWarning, "UpdateFailed",
+				"Failed to update %s %q: %v", typeName(current), current.GetName(), err)
+		}
 		return nilT, err
 	}
 
@@ -211,7 +217,9 @@ func (r *ResourceManager[T]) Manage(ctx context.Context, resource client.Object,
 	r.MergeBeforeUpdate(base, desired)
 	patch, err := NewPatch(base, current)
 	if err != nil {
-		log.Error(err, "unable to generate mutation patch", "snapshot", r.sanitize(desired), "base", r.sanitize(base))
+		if !errors.Is(err, ErrQuiet) {
+			log.Error(err, "unable to generate mutation patch", "snapshot", r.sanitize(desired), "base", r.sanitize(base))
+		}
 	} else {
 		r.mutationCache.Set(current.GetUID(), patch, 1*time.Hour)
 	}

reconcilers/sequence_test.go

@@ -66,7 +66,7 @@ func TestSequence(t *testing.T) {
 							},
 						}, &reconcilers.SyncReconciler[*resources.TestResource]{
 							SyncWithResult: func(ctx context.Context, resource *resources.TestResource) (reconcilers.Result, error) {
-								return reconcilers.Result{RequeueAfter: 2 * time.Minute}, reconcilers.HaltSubReconcilers
+								return reconcilers.Result{RequeueAfter: 2 * time.Minute}, reconcilers.ErrHaltSubReconcilers
 							},
 						},
 					}

reconcilers/sync.go

@@ -7,6 +7,7 @@ package reconcilers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/go-logr/logr"
@@ -118,7 +119,9 @@ func (r *SyncReconciler[T]) Reconcile(ctx context.Context, resource T) (Result,
 		syncResult, err := r.sync(ctx, resource)
 		result = AggregateResults(result, syncResult)
 		if err != nil {
-			log.Error(err, "unable to sync")
+			if !errors.Is(err, ErrQuiet) {
+				log.Error(err, "unable to sync")
+			}
 			return result, err
 		}
 	}
@@ -127,7 +130,9 @@ func (r *SyncReconciler[T]) Reconcile(ctx context.Context, resource T) (Result,
 		finalizeResult, err := r.finalize(ctx, resource)
 		result = AggregateResults(result, finalizeResult)
 		if err != nil {
-			log.Error(err, "unable to finalize")
+			if !errors.Is(err, ErrQuiet) {
+				log.Error(err, "unable to finalize")
+			}
 			return result, err
 		}
 	}

reconcilers/sync_test.go

@@ -62,7 +62,7 @@ func TestSyncReconciler(t *testing.T) {
 				"SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] {
 					return &reconcilers.SyncReconciler[*resources.TestResource]{
 						SyncWithResult: func(ctx context.Context, resource *resources.TestResource) (reconcilers.Result, error) {
-							return reconcilers.Result{Requeue: true}, reconcilers.HaltSubReconcilers
+							return reconcilers.Result{Requeue: true}, reconcilers.ErrHaltSubReconcilers
 						},
 					}
 				},
@@ -148,7 +148,7 @@ func TestSyncReconciler(t *testing.T) {
 							return reconcilers.Result{RequeueAfter: 2 * time.Hour}, nil
 						},
 						FinalizeWithResult: func(ctx context.Context, resource *resources.TestResource) (reconcilers.Result, error) {
-							return reconcilers.Result{RequeueAfter: 3 * time.Hour}, reconcilers.HaltSubReconcilers
+							return reconcilers.Result{RequeueAfter: 3 * time.Hour}, reconcilers.ErrHaltSubReconcilers
 						},
 					}
 				},

reconcilers/webhook.go

@@ -8,6 +8,7 @@ package reconcilers
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"sync"
@@ -139,7 +140,9 @@ func (r *AdmissionWebhookAdapter[T]) Handle(ctx context.Context, req admission.R
 	})
 
 	if err := r.reconcile(ctx, req, resp); err != nil {
-		log.Error(err, "reconcile error")
+		if !errors.Is(err, ErrQuiet) {
+			log.Error(err, "reconcile error")
+		}
 		resp.Allowed = false
 		if resp.Result == nil {
 			resp.Result = &metav1.Status{