external-dns icon indicating copy to clipboard operation
external-dns copied to clipboard

AWSSD: Utilize DiscoverInstances instead of ListInstances

Open voro015 opened this issue 3 years ago • 6 comments

Description

AWS ServiceDiscovery provider should use DiscoverInstances API call when possible instead of ListInstances, as with default API limits external-dns will start throttling its API on a scale of several thousands of nodes.

  • Updated AWSSDProvider to utilize DiscoverInstances API. Its input is expecting ServiceName *string instead of ServiceID *string Ref which pulled some additional changes. Outputs are different too but pretty easy to convert.
  • Updated AWSSDClientStub, same tests are used to cover new function.

Benchmarked before/after on same cluster - new DiscoverInstances variant took 26 seconds to reconcile, while ListInstances took 28 seconds. Tested it on aprox a month on live.

Fixes #2505

Checklist

  • [x] Unit tests updated
  • [ ] End user documentation updated

voro015 avatar Jan 09 '22 23:01 voro015

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: voro015 To complete the pull request process, please assign seanmalloy after the PR has been reviewed. You can assign the PR to them by writing /assign @seanmalloy in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot avatar Jan 09 '22 23:01 k8s-ci-robot

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Apr 12 '22 19:04 k8s-triage-robot

/remove-lifecycle stale

voro015 avatar May 05 '22 01:05 voro015

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Aug 03 '22 01:08 k8s-triage-robot

/remove-lifecycle stale

voro015 avatar Aug 04 '22 02:08 voro015

Bumping this PR

eakraly avatar Aug 10 '22 23:08 eakraly

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Nov 08 '22 23:11 k8s-triage-robot

/remove-lifecycle stale

voro015 avatar Nov 10 '22 17:11 voro015

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Feb 08 '23 18:02 k8s-triage-robot

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle rotten
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot avatar Mar 10 '23 19:03 k8s-triage-robot

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-triage-robot avatar Apr 09 '23 19:04 k8s-triage-robot

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot avatar Apr 09 '23 19:04 k8s-ci-robot

/reopen

voro015 avatar Apr 10 '23 06:04 voro015

@voro015: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot avatar Apr 10 '23 06:04 k8s-ci-robot

Hey @seanmalloy, Please give a chance to this PR, we run this code more than 1 year on scale >10k AWS CloudMap records and this helps us to remove throttling and improve performance.

volk1234 avatar Apr 26 '23 22:04 volk1234

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-triage-robot avatar May 26 '23 23:05 k8s-triage-robot

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot avatar May 26 '23 23:05 k8s-ci-robot

/reopen

voro015 avatar May 26 '23 23:05 voro015

@voro015: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot avatar May 26 '23 23:05 k8s-ci-robot

/remove-lifecycle rotten

voro015 avatar May 26 '23 23:05 voro015

/assign johngmyers

szuecs avatar Jun 16 '23 09:06 szuecs

CLA Missing ID CLA Not Signed

  • :white_check_mark: login: voro015 / name: Artem Voronin (4199785a7f5a6738b225c6af657acd5356d0b24c, b860d216dc955775b56ce9017736d29245f91886, 16356cf1b79f3960f6dcc4ca8c28beafd380cce4, cf139be7a599dd850c17056d5647e5eaa7837067, a39b86ac69c0ad628c6aeaddf24cecf546ea43e5, 0464a3b8c06d9766ac6520c6093c9f7741748a92)
  • :x: The email address for the commit (643df36c4bcb6275c27c33f02d09da872dcc21fb, cbb4df6e06fdaad774cdaaf588e68ae32cc0c439) is not linked to the GitHub account, preventing the EasyCLA check. Consult this Help Article and GitHub Help to resolve. (To view the commit's email address, add .patch at the end of this PR page's URL.) For further assistance with EasyCLA, please submit a support request ticket.

/lgtm /assign @seanmalloy

johngmyers avatar Jun 24 '23 18:06 johngmyers

/ok-to-test

szuecs avatar Jun 26 '23 09:06 szuecs

/lgtm

mloiseleur avatar Oct 03 '23 07:10 mloiseleur

/approve

On another topic, I was wondering if it would make sense to move the aws-sd registry logic into the provider. That registry can't be used with any other provider and the aws-sd provider can only be used with either the aws-sd or noop registries. Is there a use case for running the aws-sd provider with the noop registry?

johngmyers avatar Oct 04 '23 00:10 johngmyers

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johngmyers

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 /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot avatar Oct 04 '23 00:10 k8s-ci-robot

/test pull-external-dns-lint /test pull-external-dns-unit-test

johngmyers avatar Oct 04 '23 00:10 johngmyers

/label tide/merge-method-squash

johngmyers avatar Oct 04 '23 00:10 johngmyers