airship/vino
Code Issues Proposed changes

Don't log reconcile errors

Browse Source 
Since controller-runtime already logs reconcile errors, there is
no need to log them ourselves, and we instead wrap errors as
needed to provide extra context.

Change-Id: I4fcfd1cf1e8bf2829efc1877884c46166f9927c0
This commit is contained in:
Sean Eagan 2021-01-22 13:53:03 -06:00 committed by Sean Eagan
parent 9207b6364c
commit 3185f685c3
1 changed files with 8 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
pkg/controllers/vino_controller.go
View File

@ -65,7 +65,7 @@ func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
if apierror.IsNotFound(err) { if apierror.IsNotFound(err) {
return ctrl.Result{}, nil return ctrl.Result{}, nil
} }
logger.Error(err, "Failed to get vino CR") err = fmt.Errorf("failed to get vino CR: %w", err)
return ctrl.Result{}, err return ctrl.Result{}, err
} }
@ -73,7 +73,7 @@ func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
logger.Info("adding finalizer to new vino object") logger.Info("adding finalizer to new vino object")
controllerutil.AddFinalizer(vino, vinov1.VinoFinalizer) controllerutil.AddFinalizer(vino, vinov1.VinoFinalizer)
if err = r.Update(ctx, vino); err != nil { if err = r.Update(ctx, vino); err != nil {
logger.Error(err, "unable to register finalizer") err = fmt.Errorf("unable to register finalizer: %w", err)
return ctrl.Result{}, err return ctrl.Result{}, err
} }
} }
@ -86,7 +86,7 @@ func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
if readyCondition == nil || readyCondition.ObservedGeneration != vino.GetGeneration() { if readyCondition == nil || readyCondition.ObservedGeneration != vino.GetGeneration() {
vinov1.VinoProgressing(vino) vinov1.VinoProgressing(vino)
if err = r.patchStatus(ctx, vino); err != nil { if err = r.patchStatus(ctx, vino); err != nil {
logger.Error(err, "unable to patch status after progressing") err = fmt.Errorf("unable to patch status after progressing: %w", err)
return ctrl.Result{Requeue: true}, err return ctrl.Result{Requeue: true}, err
} }
} }
@ -103,7 +103,7 @@ func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
vinov1.VinoReady(vino) vinov1.VinoReady(vino)
if err := r.patchStatus(ctx, vino); err != nil { if err := r.patchStatus(ctx, vino); err != nil {
logger.Error(err, "unable to patch status after reconciliation") err = fmt.Errorf("unable to patch status after reconciliation: %w", err)
return ctrl.Result{Requeue: true}, err return ctrl.Result{Requeue: true}, err
} }
logger.Info("successfully reconciled VINO CR") logger.Info("successfully reconciled VINO CR")
@ -111,7 +111,6 @@ func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
} }
func (r *VinoReconciler) reconcileConfigMap(ctx context.Context, name types.NamespacedName, vino *vinov1.Vino) error { func (r *VinoReconciler) reconcileConfigMap(ctx context.Context, name types.NamespacedName, vino *vinov1.Vino) error {
logger := logr.FromContext(ctx)
err := r.ensureConfigMap(ctx, name, vino) err := r.ensureConfigMap(ctx, name, vino)
if err != nil { if err != nil {
err = fmt.Errorf("could not reconcile ConfigMap: %w", err) err = fmt.Errorf("could not reconcile ConfigMap: %w", err)
@ -131,7 +130,7 @@ func (r *VinoReconciler) reconcileConfigMap(ctx context.Context, name types.Name
}) })
if patchStatusErr := r.patchStatus(ctx, vino); patchStatusErr != nil { if patchStatusErr := r.patchStatus(ctx, vino); patchStatusErr != nil {
err = kerror.NewAggregate([]error{err, patchStatusErr}) err = kerror.NewAggregate([]error{err, patchStatusErr})
logger.Error(err, "unable to patch status after ConfigMap reconciliation failed") err = fmt.Errorf("unable to patch status after ConfigMap reconciliation failed: %w", err)
} }
return err return err
} }
@ -143,7 +142,7 @@ func (r *VinoReconciler) reconcileConfigMap(ctx context.Context, name types.Name
ObservedGeneration: vino.GetGeneration(), ObservedGeneration: vino.GetGeneration(),
}) })
if err = r.patchStatus(ctx, vino); err != nil { if err = r.patchStatus(ctx, vino); err != nil {
logger.Error(err, "unable to patch status after ConfigMap reconciliation succeeded") err = fmt.Errorf("unable to patch status after ConfigMap reconciliation succeeded: %w", err)
return err return err
} }
@ -238,7 +237,6 @@ func needsUpdate(generated, current *corev1.ConfigMap) bool {
} }
func (r *VinoReconciler) reconcileDaemonSet(ctx context.Context, vino *vinov1.Vino) error { func (r *VinoReconciler) reconcileDaemonSet(ctx context.Context, vino *vinov1.Vino) error {
logger := logr.FromContext(ctx)
err := r.ensureDaemonSet(ctx, vino) err := r.ensureDaemonSet(ctx, vino)
if err != nil { if err != nil {
err = fmt.Errorf("could not reconcile DaemonSet: %w", err) err = fmt.Errorf("could not reconcile DaemonSet: %w", err)
@ -258,7 +256,7 @@ func (r *VinoReconciler) reconcileDaemonSet(ctx context.Context, vino *vinov1.Vi
}) })
if patchStatusErr := r.patchStatus(ctx, vino); patchStatusErr != nil { if patchStatusErr := r.patchStatus(ctx, vino); patchStatusErr != nil {
err = kerror.NewAggregate([]error{err, patchStatusErr}) err = kerror.NewAggregate([]error{err, patchStatusErr})
logger.Error(err, "unable to patch status after DaemonSet reconciliation failed") err = fmt.Errorf("unable to patch status after DaemonSet reconciliation failed: %w", err)
} }
return err return err
} }
@ -270,7 +268,7 @@ func (r *VinoReconciler) reconcileDaemonSet(ctx context.Context, vino *vinov1.Vi
ObservedGeneration: vino.GetGeneration(), ObservedGeneration: vino.GetGeneration(),
}) })
if err := r.patchStatus(ctx, vino); err != nil { if err := r.patchStatus(ctx, vino); err != nil {
logger.Error(err, "unable to patch status after DaemonSet reconciliation succeeded") err = fmt.Errorf("unable to patch status after DaemonSet reconciliation succeeded: %w", err)
return err return err
} }