negz
commented
3 years ago I'd prefer to start simple here, at least until we have more confidence that folks would benefit from different poll intervals for different individual managed resources (or kinds of managed resources). I personally suspect adding a --poll-interval provider flag to each provider that sets WithPollInterval for all of the provider's controllers would be a huge win that requires little more than a touch of plumbing.
If we did want to expose this at the ProviderConfig level (which does seem like the logical next most flexible place to do this after the provider level), or even the managed resource level, I'd prefer to do that by updating the the return value of the ExternalConnector, which is typically what first reads the managed resource and its provider config. For example:
// An ExternalConnection represents a connection to an external system
type ExternalConnection struct {
Client ExternalClient
PollInterval time.Duration
// Using a struct future proofs us in case we want to surface
// other things up to the managed.Reconciler in future.
}
// An ExternalConnecter produces a new ExternalClient given the supplied
// Managed resource.
type ExternalConnecter interface {
// Connect to the provider specified by the supplied managed resource.
Connect(ctx context.Context, mg resource.Managed) (ExternalConnection, error)
}
hasheddan
smcavallo
mcavoyk
benagricola
ytsarev
janwillies
muvaf
srueg
stale[bot]
What problem are you facing?
Currently, all controllers that use the managed reconciler have a default poll interval of 1 minute, which can be overridden during controller setup. However, this means that all objects reconciled by that controller are forced to use the same interval, which may not make sense (e.g. a production database may need to be checked for drift more frequently then a staging one).
How could Crossplane help solve your problem?
Crossplane users should be able to define the happy path poll interval for all of their resources. This could be accomplished by exposing
pollIntervalfield embedded in every managed resource, then getting the value when returning withRequeueAfterin the managed reconciler. Another option would be to expose the field on aProviderConfig, which @muvaf has pointed out could make sense due to the fact that rate limiting behavior is typically scoped to a single account on providers, such as AWS.A third option would be to just define an additional method on the
ExternalClientinterface, potentially something likeGetPollInterval()that would allow provider authors to determine how the poll interval is chosen. It could be a field on theProviderConfigor managed resource, provided as an argument to controller entrypoint, or some combination of the three with different options taking precedence over others.To encourage consistency, it would be good for us to suggest a common implementation, likely by adding
pollIntervalfields to the embeddedProviderConfigand managed resource structs, as well as supplying a defaultGetPollInterval()that accepted both objects (and potentially a hardcoded default fallback) and then selected one at a chose precedence, while also allowing provider authors to override this behavior.