From 6b8035e0adb29c39ea22c69927d2bbc0e4588f88 Mon Sep 17 00:00:00 2001 From: Thibault Jamet Date: Fri, 12 Mar 2021 09:35:11 +0100 Subject: [PATCH 1/2] Increase AWS pagination size From measurements, AWS by default has pagination of 100 items per page when listing hosted zone resources. This increases the number of requests required to list all our zones, and pushes a hard constraint on the rate limits. From the experiments, it seems that on the server-side, there is a hard limit of 300 elements per page, as per AWS documentation: https://docs.aws.amazon.com/Route53/latest/APIReference/API_ListResourceRecordSets.html > ListResourceRecordSets returns up to 300 resource record sets at a time in ASCII order, > beginning at a position specified by the name and type elements Hence raising the page size from 100 to 300 items would decrease by 3 the number of requests posted to Route53 We even set a higher limit so we can benefit from a lower number of requests if ever AWS increases the hard limit of 300. --- provider/aws/aws.go | 7 +++++++ provider/aws/aws_test.go | 9 +++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 2581af2a15..425bd654ca 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -376,6 +376,13 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*route53.Hos for _, z := range zones { params := &route53.ListResourceRecordSetsInput{ HostedZoneId: z.Id, + // From the experiments, it seems that the default MaxItems applied is 100, + // and that, on the server side, there is a hard limit of 300 elements per page. + // After a discussion with AWS representants, clients should accept + // when less items are returned, and still paginate accordingly. + // As we are using the standard AWS client, this should already be compliant. + // Hence, ifever AWS decides to raise this limit, we will automatically reduce the pressure on rate limits + MaxItems: aws.String("1000"), } if err := p.client.ListResourceRecordSetsPagesWithContext(ctx, params, f); err != nil { diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 40e522a491..09259c3dec 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -56,6 +56,7 @@ type Route53APIStub struct { recordSets map[string]map[string][]*route53.ResourceRecordSet zoneTags map[string][]*route53.Tag m dynamicMock + t *testing.T } // MockMethod starts a description of an expectation of the specified method @@ -67,16 +68,19 @@ func (r *Route53APIStub) MockMethod(method string, args ...interface{}) *mock.Ca } // NewRoute53APIStub returns an initialized Route53APIStub -func NewRoute53APIStub() *Route53APIStub { +func NewRoute53APIStub(t *testing.T) *Route53APIStub { return &Route53APIStub{ zones: make(map[string]*route53.HostedZone), recordSets: make(map[string]map[string][]*route53.ResourceRecordSet), zoneTags: make(map[string][]*route53.Tag), + t: t, } } func (r *Route53APIStub) ListResourceRecordSetsPagesWithContext(ctx context.Context, input *route53.ListResourceRecordSetsInput, fn func(p *route53.ListResourceRecordSetsOutput, lastPage bool) (shouldContinue bool), opts ...request.Option) error { output := route53.ListResourceRecordSetsOutput{} // TODO: Support optional input args. + require.NotNil(r.t, input.MaxItems) + assert.EqualValues(r.t, "1000", *input.MaxItems) if len(r.recordSets) == 0 { output.ResourceRecordSets = []*route53.ResourceRecordSet{} } else if _, ok := r.recordSets[aws.StringValue(input.HostedZoneId)]; !ok { @@ -1198,6 +1202,7 @@ func listAWSRecords(t *testing.T, client Route53API, zone string) []*route53.Res recordSets := []*route53.ResourceRecordSet{} require.NoError(t, client.ListResourceRecordSetsPagesWithContext(context.Background(), &route53.ListResourceRecordSetsInput{ HostedZoneId: aws.String(zone), + MaxItems: aws.String("1000"), }, func(resp *route53.ListResourceRecordSetsOutput, _ bool) bool { recordSets = append(recordSets, resp.ResourceRecordSets...) return true @@ -1255,7 +1260,7 @@ func newAWSProvider(t *testing.T, domainFilter endpoint.DomainFilter, zoneIDFilt } func newAWSProviderWithTagFilter(t *testing.T, domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zoneTypeFilter provider.ZoneTypeFilter, zoneTagFilter provider.ZoneTagFilter, evaluateTargetHealth, dryRun bool, records []*endpoint.Endpoint) (*AWSProvider, *Route53APIStub) { - client := NewRoute53APIStub() + client := NewRoute53APIStub(t) provider := &AWSProvider{ client: client, From d0120542ca0ab4a83fa1168252af755ac2b24f5a Mon Sep 17 00:00:00 2001 From: Thibault Jamet Date: Wed, 24 Mar 2021 11:15:02 +0100 Subject: [PATCH 2/2] Use a constant 300 AWS page size --- provider/aws/aws.go | 15 ++++++++------- provider/aws/aws_test.go | 4 ++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 425bd654ca..dd36d8fd83 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -40,6 +40,13 @@ import ( const ( recordTTL = 300 + // From the experiments, it seems that the default MaxItems applied is 100, + // and that, on the server side, there is a hard limit of 300 elements per page. + // After a discussion with AWS representants, clients should accept + // when less items are returned, and still paginate accordingly. + // As we are using the standard AWS client, this should already be compliant. + // Hence, ifever AWS decides to raise this limit, we will automatically reduce the pressure on rate limits + route53PageSize = "300" // provider specific key that designates whether an AWS ALIAS record has the EvaluateTargetHealth // field set to true. providerSpecificEvaluateTargetHealth = "aws/evaluate-target-health" @@ -376,13 +383,7 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*route53.Hos for _, z := range zones { params := &route53.ListResourceRecordSetsInput{ HostedZoneId: z.Id, - // From the experiments, it seems that the default MaxItems applied is 100, - // and that, on the server side, there is a hard limit of 300 elements per page. - // After a discussion with AWS representants, clients should accept - // when less items are returned, and still paginate accordingly. - // As we are using the standard AWS client, this should already be compliant. - // Hence, ifever AWS decides to raise this limit, we will automatically reduce the pressure on rate limits - MaxItems: aws.String("1000"), + MaxItems: aws.String(route53PageSize), } if err := p.client.ListResourceRecordSetsPagesWithContext(ctx, params, f); err != nil { diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 09259c3dec..4e5d3eaa8a 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -80,7 +80,7 @@ func NewRoute53APIStub(t *testing.T) *Route53APIStub { func (r *Route53APIStub) ListResourceRecordSetsPagesWithContext(ctx context.Context, input *route53.ListResourceRecordSetsInput, fn func(p *route53.ListResourceRecordSetsOutput, lastPage bool) (shouldContinue bool), opts ...request.Option) error { output := route53.ListResourceRecordSetsOutput{} // TODO: Support optional input args. require.NotNil(r.t, input.MaxItems) - assert.EqualValues(r.t, "1000", *input.MaxItems) + assert.EqualValues(r.t, route53PageSize, *input.MaxItems) if len(r.recordSets) == 0 { output.ResourceRecordSets = []*route53.ResourceRecordSet{} } else if _, ok := r.recordSets[aws.StringValue(input.HostedZoneId)]; !ok { @@ -1202,7 +1202,7 @@ func listAWSRecords(t *testing.T, client Route53API, zone string) []*route53.Res recordSets := []*route53.ResourceRecordSet{} require.NoError(t, client.ListResourceRecordSetsPagesWithContext(context.Background(), &route53.ListResourceRecordSetsInput{ HostedZoneId: aws.String(zone), - MaxItems: aws.String("1000"), + MaxItems: aws.String(route53PageSize), }, func(resp *route53.ListResourceRecordSetsOutput, _ bool) bool { recordSets = append(recordSets, resp.ResourceRecordSets...) return true