AWSSD: Utilize DiscoverInstances instead of ListInstances #2506

voro015 · 2022-01-09T23:49:50Z

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 almost a year of running in prod.

Fixes #2505

Checklist

Unit tests updated
End user documentation updated

Renamed instanceToHttpInstanceSummary to instanceToHTTPInstanceSummary

# Conflicts: # provider/awssd/aws_sd.go

k8s-triage-robot · 2022-04-12T19:05:44Z

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

voro015 · 2022-05-05T01:42:21Z

/remove-lifecycle stale

k8s-triage-robot · 2022-08-03T01:57:06Z

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

voro015 · 2022-08-04T02:20:24Z

/remove-lifecycle stale

eakraly · 2022-08-10T23:13:29Z

Bumping this PR

k8s-triage-robot · 2022-11-08T23:24:28Z

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

voro015 · 2022-11-10T17:52:02Z

/remove-lifecycle stale

k8s-triage-robot · 2023-02-08T18:37:33Z

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 · 2023-03-10T19:13:41Z

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 · 2023-04-09T19:53:11Z

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

provider/awssd/aws_sd_test.go

szuecs · 2023-06-26T09:25:45Z

provider/awssd/aws_sd_test.go

-		Instances: instances,
-	}, true)
+	if len(instances) == 0 {
+		return nil, errors.New("instances not found")


Please create an error var that can be referenced. For tests or for top level error handling.

provider/awssd/aws_sd_test.go

Co-authored-by: John Gardiner Myers <[email protected]>

provider/awssd/aws_sd_test.go

mloiseleur · 2023-10-03T07:54:39Z

/lgtm

johngmyers · 2023-10-04T00:10:37Z

/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?

k8s-ci-robot · 2023-10-04T00:10:57Z

[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:

~~OWNERS~~ [johngmyers]

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

johngmyers · 2023-10-04T00:11:46Z

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

johngmyers · 2023-10-04T00:17:59Z

/label tide/merge-method-squash

voro015 added 2 commits November 14, 2021 16:25

AWSSD: Utilize DiscoverInstances instead of ListInstances

4199785

Merge branch 'master' into aws-sd_discover-instances

b860d21

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 9, 2022

k8s-ci-robot requested review from njuettner and seanmalloy January 9, 2022 23:50

Fixed stylecheck

16356cf

Renamed instanceToHttpInstanceSummary to instanceToHTTPInstanceSummary

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 12, 2022

Merge branch 'master' into aws-sd_discover-instances

cf139be

# Conflicts: # provider/awssd/aws_sd.go

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 12, 2022

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 12, 2022

Merge branch 'kubernetes-sigs:master' into aws-sd_discover-instances

a39b86a

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 5, 2022

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 3, 2022

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 4, 2022

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 8, 2022

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 10, 2022

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 8, 2023

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 10, 2023

szuecs reviewed Jun 26, 2023

View reviewed changes

provider/awssd/aws_sd_test.go Outdated Show resolved Hide resolved

szuecs reviewed Jun 26, 2023

View reviewed changes

provider/awssd/aws_sd_test.go Outdated Show resolved Hide resolved

voro015 added 2 commits July 18, 2023 15:43

awssd log error on failed DeleteService

97fe079

updated awssd tests

454ac69

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 18, 2023

voro015 and others added 3 commits July 18, 2023 15:48

Merge branch 'kubernetes-sigs:master' into aws-sd_discover-instances

283f3b2

fix missing import

f9b236c

awssd tests handle not found NS with DiscoverInstancesWithContext

eda0e59

voro015 requested a review from szuecs July 19, 2023 00:33

johngmyers reviewed Jul 19, 2023

View reviewed changes

provider/awssd/aws_sd_test.go Outdated Show resolved Hide resolved

voro015 and others added 2 commits July 18, 2023 17:58

Update provider/awssd/aws_sd_test.go

3b6d816

Co-authored-by: John Gardiner Myers <[email protected]>

Update provider/awssd/aws_sd_test.go

2a3c977

Co-authored-by: John Gardiner Myers <[email protected]>

voro015 requested a review from johngmyers July 19, 2023 19:08

mloiseleur reviewed Oct 2, 2023

View reviewed changes

provider/awssd/aws_sd_test.go Show resolved Hide resolved

k8s-ci-robot assigned mloiseleur Oct 3, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 3, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 4, 2023

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Oct 4, 2023

k8s-ci-robot merged commit 4eb7e75 into kubernetes-sigs:master Oct 4, 2023
6 checks passed

voro015 deleted the aws-sd_discover-instances branch October 4, 2023 04:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AWSSD: Utilize DiscoverInstances instead of ListInstances #2506

AWSSD: Utilize DiscoverInstances instead of ListInstances #2506

voro015 commented Jan 9, 2022 •

edited

Loading

k8s-triage-robot commented Apr 12, 2022

voro015 commented May 5, 2022

k8s-triage-robot commented Aug 3, 2022

voro015 commented Aug 4, 2022

eakraly commented Aug 10, 2022

k8s-triage-robot commented Nov 8, 2022

voro015 commented Nov 10, 2022

k8s-triage-robot commented Feb 8, 2023

k8s-triage-robot commented Mar 10, 2023

k8s-triage-robot commented Apr 9, 2023

szuecs Jun 26, 2023

voro015 Jul 19, 2023

mloiseleur commented Oct 3, 2023

johngmyers commented Oct 4, 2023

k8s-ci-robot commented Oct 4, 2023

johngmyers commented Oct 4, 2023

johngmyers commented Oct 4, 2023

AWSSD: Utilize DiscoverInstances instead of ListInstances #2506

AWSSD: Utilize DiscoverInstances instead of ListInstances #2506

Conversation

voro015 commented Jan 9, 2022 • edited Loading

k8s-triage-robot commented Apr 12, 2022

voro015 commented May 5, 2022

k8s-triage-robot commented Aug 3, 2022

voro015 commented Aug 4, 2022

eakraly commented Aug 10, 2022

k8s-triage-robot commented Nov 8, 2022

voro015 commented Nov 10, 2022

k8s-triage-robot commented Feb 8, 2023

k8s-triage-robot commented Mar 10, 2023

k8s-triage-robot commented Apr 9, 2023

szuecs Jun 26, 2023

Choose a reason for hiding this comment

voro015 Jul 19, 2023

Choose a reason for hiding this comment

mloiseleur commented Oct 3, 2023

johngmyers commented Oct 4, 2023

k8s-ci-robot commented Oct 4, 2023

johngmyers commented Oct 4, 2023

johngmyers commented Oct 4, 2023

voro015 commented Jan 9, 2022 •

edited

Loading