-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the right query params on infoblox #3301
Add the right query params on infoblox #3301
Conversation
/kind bug |
uh buh! this error is not from my change :D I can, tho, fix using this PR or a different one if required! |
@njuettner @seanmalloy Please could we retest this? It's a small change but fixes a significant performance issue in the Infoblox provider. We are running this internally in production. |
I'll try to review it today. |
Hey @rikatz this is failing on a small lint error |
@rikatz - Any idea when you can address the lint error? Would love to see this merged out. |
yes, right now. Let me take a look |
043458d
to
2484da5
Compare
ok folks, this is passing now. @szuecs @seanmalloy just so this doesn't fall off of your plate :D Thanks!! |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rikatz, szuecs The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Infoblox provider is not doing the proper query/search params. It ends up doing a huge query on infoblox, trying to fetch all registries from all zones and views for every zone external-dns manages, when it should get only the registries that belongs to that zone.
This PR adds the search parameter correctly, using zone and view to find just the managed entries.