don't requeue if dashboard CR are broken #1236
Conversation
|
@weisdd is it worth doing some kind of status update, or should we just skip the reconcile? |
controllers/dashboard_controller.go
Outdated
| @@ -194,13 +194,13 @@ func (r *GrafanaDashboardReconciler) Reconcile(ctx context.Context, req ctrl.Req | |||
| dashboardJson, err := r.fetchDashboardJson(ctx, cr) | |||
| if err != nil { | |||
| controllerLog.Error(err, "error fetching dashboard", "dashboard", cr.Name) | |||
| return ctrl.Result{RequeueAfter: RequeueDelay}, nil | |||
| return ctrl.Result{Requeue: false}, nil | |||
There was a problem hiding this comment.
This can fail due to multiple reasons including temporary connectivity loss. So, I would say we can judge whether it's worth re-queueing only if we make fetchDashboardJson return easily distinguishable errors.
There was a problem hiding this comment.
Yes, you are right, I will create a custom error and update the PR. Hopefully sometime next week.
There was a problem hiding this comment.
I gave this some more thought.
In many of these cases we actually want a retry a and in the case of dashboard.Spec.Json we don't even return an error since it's more or less not possible to generate one.
So I didn't really see the need to do a bunch of custom errors for this use case.
There was a problem hiding this comment.
Yeah, I think the updated implementation should help with the broken JSON, and, indeed, custom errors are not needed in this case.
NissesSenap
force-pushed
the
broken_json_spec
branch
from
c187e35
to
ad7529f
Compare
fixes #1091