Allow controller authors to configure queue rate limits

[negz]

Controllers created using controller-runtime implicitly use the DefaultControllerRateLimiter for the work queue they use to process reconcile requests. If I follow the logic correctly, this means that under the following conditions...

• A watched object has changed.
• A reconcile attempt has returned an error.
• A reconcile attempt has returned Result{Requeue: true}

...the earliest time the resulting reconcile will be processed will be the maximum of:

• The enqueue time plus a duration increasing exponentially from 5ms to ~16 minutes per object until said object is reconciled successfully (i.e. a reconcile attempt for that object returns Result{Requeue: false}, nil or Result{RequeueAfter: N}, nil).
• The enqueue time plus a duration calculated to limit the controller to 10 requeues per second on average.

While building Crossplane we've found these requeue semantics are not ideal for reconciling Kubernetes resources with cloud provider APIs. There are two parts to this:

• Typically we don't want to retry as soon as 5ms from the time we encountered an error, and also don't want to reach a point where we're waiting over 16 minutes between each reconcile.
• There is no way for users who do not have access to our controller manager pod logs (i.e. most users) to learn that the object they're waiting on is suffering exponential backoff. Ideally upon encountering an error we'd emit an event explaining the error and informing the user that we'll try again after a particular time.

I believe we could solve both of the above issues if we were able to override the default rate limiter, perhaps by adding it to controller.Options. This would allow us to tweak the exponential backoff parameters, and to plumb the rate limiter through to our Reconciler so it could inform concerned parties when an object was next going to be reconciled.

Relates to #195 and #417.

[DirectXMan12]

/kind feature
/priority backlog

As long as we're careful not to tie ourselves to closely to the exact interfaces or semantics of the underlying apimachinery libraries, this seems like a reasonable request

[negz]

https://godoc.org/k8s.io/client-go/util/workqueue#RateLimiter is the interface controller.New currently uses behind the scenes. If we were unconcerned with exposing that interface I'd likely do something like:

// Options are the arguments for creating a new Controller
type Options struct {
	// ... existing fields omitted

	// A RateLimiter used to limit how frequently requests may be queued.
	// Defaults to <brief summary of DefaultControllerRateLimiter semantics>.
	RateLimiter workqueue.RateLimiter
}

// New returns a new Controller registered with the Manager.  The Manager will ensure that shared Caches have
// been synced before the Controller is Started.
func New(name string, mgr manager.Manager, options Options) (Controller, error) {
	if options.RateLimiter == nil {
		options.RateLimiter = workqueue.DefaultControllerRateLimiter()
	}

	// Make a great new controller...
}

@DirectXMan12 Would you be concerned about leaking that interface to controller-runtime users? If so, would you want to try to alter the interface, or would it be sufficient to define an identical interface inside the controller package?

[DirectXMan12]

the more we expose from apimachinery, the more likely something explodes dramatically in our faces when someone decides that a method name needs to change, or an interface needs to go away, or whatnot (cough 1.16 apimachinery cough). That's one of my big concerns.

To clarify: what's the exact desire here -- set the backoff? Control the backoff in a more granular manner? That interface is decent if we need the latter -- we just need to make sure we can always support it.

[negz]

To clarify: what's the exact desire here -- set the backoff? Control the backoff in a more granular manner?

It's not immediately obvious to me what the difference between those two options is, so "yes". :) I'd like:

• Control over backoff semantics. For example in our case where we're orchestrating (fairly slow moving) cloud APIs we might want our first retry after ~1 seconds (not 5ms), backing off up to ~5 minutes (not ~16 minutes).
• Insight into the current state of the backoff. I feel it's a much better user experience to report "Encountered error 'foo' while attempting to bar the baz. Will try again no sooner than 15:03:30Z" rather than simply reporting that there was an error and letting the user guess at when the next reconcile will happen.

[DirectXMan12]

Yeah, that makes sense. Exposing an option to configure the rate-limiter is probably fine.

[rajathagasthya]

Control over backoff semantics. For example in our case where we're orchestrating (fairly slow moving) cloud APIs we might want our first retry after ~1 seconds (not 5ms), backing off up to ~5 minutes (not ~16 minutes).

We have the exact same use case in controlling backoff. So this would be really useful.

@DirectXMan12 Does it make sense to add a new ratelimiter package and define the RateLimiter interface (identical to client-go's RateLimiter) there? And then use that as the type for the rate limiter option?

[vincepri]

/kind design