Allow to override options for specific controllers #2030
Conversation
Later some resources may be monitored using EventBridge events and these controller will have longer poll intervals. Resources without EventBridge events will have short poll intervals. Options are parsed from env variables with `PROVIDER_AWS_` prefix, for example: `PROVIDER_AWS_ec2.instance.pollInterval=10m`. Signed-off-by: Max Melentyev <max.melentyev@reddit.com>
max-melentyev
force-pushed
the
controller-specific-options
branch
from
23b7bad
to
13bbdd1
Compare
|
I'm happy to add more docs on this feature, but I didn't find any appropriate place for it other than source code comments. Please let me know if there's a better option. |
|
|
||
| // Collects all env variables with the prefix OPTION_ENV_PREFIX and returns them as a map | ||
| // with the prefix removed. | ||
| func optionsOverridesFromEnv() map[string]string { |
There was a problem hiding this comment.
Doesn't kingpin support natively loading flags from environment variables?
There was a problem hiding this comment.
I see it has binding of particular flag to an env variable. This will add ~320 new flags and it'll still require some code to collect these flags into overrides map.
There was a problem hiding this comment.
I'm not a maintainer of this provider anymore, so I'll defer to @MisterMX and @chlunde.
That said, I'm a bit concerned about:
- Adding a feature to only this provider to tune poll interval per resource type
- Introducing a new path to configure the provider (environment variables)
I think if this is a problem for this provider it's probably a problem for other providers. It would be ideal to agree on the problem, and a standard way to address it. Usually to do that we'd write a one-pager in the crossplane/crossplane repo, that provider owners can adopt.
@ulucinar is the TL of Upbound's extensions team (he owns tools like upjet that generate a lot of the other providers). He might be a good person to review a one-pager if you're interested in writing one.
|
This looks like a very drastic change to me. And I honestly don't think that we are going merge that since it is very custom solution for a single provider which does not comply to the other providers. I would prefer a more general approach that can be included in https://github.com/crossplane/crossplane-runtime. As @negz suggested it is probably good to start with a design proposal in https://github.com/crossplane/crossplane. |
|
Also discussed here: |
|
Closing this for now as the goal is rather to find a generic solution that works across all Crossplane providers. |
Description of your changes
We're facing long poll queues when using crossplane to manage thousands of EC2 instances,
and facing AWS API throttling while observing thousands of route53 records (#2029).
As an intermediate step we want to reduce poll interval for these resources and keep it short for other resources.
In further PRs I plan to make these resources to be monitored using EventBridge events
and these controller to have even longer poll intervals.
Resources without EventBridge events will still have short poll intervals.
Options are parsed from env variables with
PROVIDER_AWS_prefix, for example:PROVIDER_AWS_ec2.instance.pollInterval=10m.I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested