Normalize and Improve Error Handling of "External" errors (i.e. Openstack)

[fwiesel]

See : https://github.com/cobaltcore-dev/openstack-hypervisor-operator/pull/134/files#r2413934009 for details

[fwiesel]

We need to normalize somehow the way we handle "external" errors.

I hope, we agree that we need to retry on errors, and that it should back-off the error somehow.

I'd divide the problem in two almost disjoint decisions: What do we return from the reconcile-function to the reconcileHandler, and what do we do in it.

What to return to the reconcileHandler?

Each item has their own metric counter.

• Return the error:
  • will requeue the reconciliation request into the queue (which should do exponential back-off, but does not?)
  • print a fairly use-less stack-dump into the linked function.
• Return RequeueAfter>0 (and no error):
  • will re-queue the event with the specified delay
  • reset any back-off / rate-limit metrics
  • will not log an error
• Return Requeue (and no error)
  • Will re-queue the request the same way as with an error
  • will not log anything
  • Is deprecated: "either an error or RequeueAfter should be used instead."
• Return nothing, and let a status update re-trigger the reconciliation
  • Not really a solution, as we cannot do any back-off that way.

Due to the deprecation, we are either left with returning an error or RequeueAfter.

• The first means figuring out, why the back-off doesn't seem to work why it works as intended, and optionally maybe improve the logging (i.e. drop the use-less stack trace?)
• The second means implementing our own back-off somehow (via the condition?)

What do to in the reconcile-function?

These are not necessarily either/or and can be combined.

• Log the error
  • Duplicate when returning the error, but we might get a better stack-trace, as the one in the controller-loop is use-less.
• Reflect the error as an update in conditions
  • Gives operator/developers easier introspection of the state than grepping logs
  • Triggers itself reconciliation loop
    • Can be filtered out by checking resource-version of the resource
  • may raise own errors
  • Can be joined with (errors.Join)
  • When? (To be verified)
    • On each attempt
      • Updates the timestamp of the condition
      • Creates more load
  • Only when it changes
    • Less load
    • Reflect when the error started to occur
    • Cannot use the update event for retry

[fwiesel]

kubernetes-sigs:

• aws-load-balancer-controller: Seems to simpy return the error
• kro: Also returns error, but also Requeue, because they have implemented their own queue
• cluster-api-provider-openstack: Also seems to bubble the error up.