From 155f6e948de1fdba511fa7fc43e50c6d66714c75 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Thu, 20 Aug 2020 14:45:43 +0100 Subject: [PATCH] Be consistent about returning an error It's mooted in https://github.com/fluxcd/toolkit/discussions/164 that a distinct metric is used for not completing reconciliation, as opposed to an unexpected error. Until that discussion has run its course, we should just do what the other controllers do, and that's returning an error when the controller is unable to reconcile to completion. This also adds a comment noting the purpose of the redundant `Requeue: true` fields, for the avoidance of confusion later. --- controllers/imagerepository_controller.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/controllers/imagerepository_controller.go b/controllers/imagerepository_controller.go index e93bbc8..05837b1 100644 --- a/controllers/imagerepository_controller.go +++ b/controllers/imagerepository_controller.go @@ -64,8 +64,14 @@ type ImageRepositoryReconciler struct { func (r *ImageRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { ctx := context.Background() + // NB: In general, if an error is returned then controller-runtime + // will requeue the request with back-off. In the following this + // is usually made explicit by _also_ returning + // `ctrl.Result{Requeue: true}`. + var imageRepo imagev1alpha1.ImageRepository if err := r.Get(ctx, req.NamespacedName, &imageRepo); err != nil { + // _Might_ get requeued return ctrl.Result{}, client.IgnoreNotFound(err) } @@ -83,7 +89,7 @@ func (r *ImageRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er return ctrl.Result{Requeue: true}, err } log.Error(err, "Unable to parse image name", "imageName", imageRepo.Spec.Image) - return ctrl.Result{}, nil + return ctrl.Result{Requeue: true}, err } imageRepo.Status.CanonicalImageName = ref.Context().String()