Skip to content

Commit

Permalink
Merge pull request kubernetes-sigs#2318 from tariq1890/aws-refactor
Browse files Browse the repository at this point in the history 
remove unused parameters in aws change submit method
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 1, 2021
2 parents 1946d65 + 1aef05c commit f8a3868
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 36 deletions.
43 changes: 12 additions & 31 deletions provider/aws/aws.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,14 +414,9 @@ func (p *AWSProvider) doRecords(ctx context.Context, action string, endpoints []
return errors.Wrapf(err, "failed to list zones, aborting %s doRecords action", action)
}

records, err := p.records(ctx, zones)
if err != nil {
log.Errorf("failed to list records while preparing %s doRecords action: %s", action, err)
}

p.AdjustEndpoints(endpoints)

return p.submitChanges(ctx, p.newChanges(action, endpoints, records, zones), zones)
return p.submitChanges(ctx, p.newChanges(action, endpoints), zones)
}

// UpdateRecords updates a given set of old records to a new set of records in a given hosted zone.
Expand All @@ -431,15 +426,10 @@ func (p *AWSProvider) UpdateRecords(ctx context.Context, updates, current []*end
return errors.Wrapf(err, "failed to list zones, aborting UpdateRecords")
}

records, err := p.records(ctx, zones)
if err != nil {
log.Errorf("failed to list records while preparing UpdateRecords: %s", err)
}

return p.submitChanges(ctx, p.createUpdateChanges(updates, current, records, zones), zones)
return p.submitChanges(ctx, p.createUpdateChanges(updates, current), zones)
}

func (p *AWSProvider) createUpdateChanges(newEndpoints, oldEndpoints []*endpoint.Endpoint, recordsCache []*endpoint.Endpoint, zones map[string]*route53.HostedZone) []*route53.Change {
func (p *AWSProvider) createUpdateChanges(newEndpoints, oldEndpoints []*endpoint.Endpoint) []*route53.Change {
var deletes []*endpoint.Endpoint
var creates []*endpoint.Endpoint
var updates []*endpoint.Endpoint
Expand All @@ -459,9 +449,9 @@ func (p *AWSProvider) createUpdateChanges(newEndpoints, oldEndpoints []*endpoint
}

combined := make([]*route53.Change, 0, len(deletes)+len(creates)+len(updates))
combined = append(combined, p.newChanges(route53.ChangeActionCreate, creates, recordsCache, zones)...)
combined = append(combined, p.newChanges(route53.ChangeActionUpsert, updates, recordsCache, zones)...)
combined = append(combined, p.newChanges(route53.ChangeActionDelete, deletes, recordsCache, zones)...)
combined = append(combined, p.newChanges(route53.ChangeActionCreate, creates)...)
combined = append(combined, p.newChanges(route53.ChangeActionUpsert, updates)...)
combined = append(combined, p.newChanges(route53.ChangeActionDelete, deletes)...)
return combined
}

Expand All @@ -487,20 +477,11 @@ func (p *AWSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e
return errors.Wrap(err, "failed to list zones, not applying changes")
}

records, ok := ctx.Value(provider.RecordsContextKey).([]*endpoint.Endpoint)
if !ok {
var err error
records, err = p.records(ctx, zones)
if err != nil {
log.Errorf("failed to get records while preparing to applying changes: %s", err)
}
}

updateChanges := p.createUpdateChanges(changes.UpdateNew, changes.UpdateOld, records, zones)
updateChanges := p.createUpdateChanges(changes.UpdateNew, changes.UpdateOld)

combinedChanges := make([]*route53.Change, 0, len(changes.Delete)+len(changes.Create)+len(updateChanges))
combinedChanges = append(combinedChanges, p.newChanges(route53.ChangeActionCreate, changes.Create, records, zones)...)
combinedChanges = append(combinedChanges, p.newChanges(route53.ChangeActionDelete, changes.Delete, records, zones)...)
combinedChanges = append(combinedChanges, p.newChanges(route53.ChangeActionCreate, changes.Create)...)
combinedChanges = append(combinedChanges, p.newChanges(route53.ChangeActionDelete, changes.Delete)...)
combinedChanges = append(combinedChanges, updateChanges...)

return p.submitChanges(ctx, combinedChanges, zones)
Expand Down Expand Up @@ -567,11 +548,11 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes []*route53.Chan
}

// newChanges returns a collection of Changes based on the given records and action.
func (p *AWSProvider) newChanges(action string, endpoints []*endpoint.Endpoint, recordsCache []*endpoint.Endpoint, zones map[string]*route53.HostedZone) []*route53.Change {
func (p *AWSProvider) newChanges(action string, endpoints []*endpoint.Endpoint) []*route53.Change {
changes := make([]*route53.Change, 0, len(endpoints))

for _, endpoint := range endpoints {
change, dualstack := p.newChange(action, endpoint, recordsCache, zones)
change, dualstack := p.newChange(action, endpoint)
changes = append(changes, change)
if dualstack {
// make a copy of change, modify RRS type to AAAA, then add new change
Expand Down Expand Up @@ -619,7 +600,7 @@ func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoin
// returned Change is based on the given record by the given action, e.g.
// action=ChangeActionCreate returns a change for creation of the record and
// action=ChangeActionDelete returns a change for deletion of the record.
func (p *AWSProvider) newChange(action string, ep *endpoint.Endpoint, recordsCache []*endpoint.Endpoint, zones map[string]*route53.HostedZone) (*route53.Change, bool) {
func (p *AWSProvider) newChange(action string, ep *endpoint.Endpoint) (*route53.Change, bool) {
change := &route53.Change{
Action: aws.String(action),
ResourceRecordSet: &route53.ResourceRecordSet{
Expand Down
8 changes: 3 additions & 5 deletions provider/aws/aws_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ func TestAWSApplyChanges(t *testing.T) {
setup func(p *AWSProvider) context.Context
listRRSets int
}{
{"no cache", func(p *AWSProvider) context.Context { return context.Background() }, 3},
{"no cache", func(p *AWSProvider) context.Context { return context.Background() }, 0},
{"cached", func(p *AWSProvider) context.Context {
ctx := context.Background()
records, err := p.Records(ctx)
Expand Down Expand Up @@ -781,7 +781,7 @@ func TestAWSsubmitChanges(t *testing.T) {
zones, _ := provider.Zones(ctx)
records, _ := provider.Records(ctx)
cs := make([]*route53.Change, 0, len(endpoints))
cs = append(cs, provider.newChanges(route53.ChangeActionCreate, endpoints, records, zones)...)
cs = append(cs, provider.newChanges(route53.ChangeActionCreate, endpoints)...)

require.NoError(t, provider.submitChanges(ctx, cs, zones))

Expand All @@ -798,11 +798,9 @@ func TestAWSsubmitChangesError(t *testing.T) {
ctx := context.Background()
zones, err := provider.Zones(ctx)
require.NoError(t, err)
records, err := provider.Records(ctx)
require.NoError(t, err)

ep := endpoint.NewEndpointWithTTL("fail.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.0.0.1")
cs := provider.newChanges(route53.ChangeActionCreate, []*endpoint.Endpoint{ep}, records, zones)
cs := provider.newChanges(route53.ChangeActionCreate, []*endpoint.Endpoint{ep})

require.Error(t, provider.submitChanges(ctx, cs, zones))
}
Expand Down

0 comments on commit f8a3868

Please sign in to comment.