Skip to content

Commit

Permalink
Specify and clarify root cause of issue 4241
Browse files Browse the repository at this point in the history
  • Loading branch information
leonardocaylent committed Apr 6, 2024
1 parent ba56b7a commit d9b7439
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 4 deletions.
7 changes: 5 additions & 2 deletions controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,12 @@ func (c *Controller) RunOnce(ctx context.Context) error {
return err
}

//records = endpoint.RemoveDuplicates(records)
//This deduplication could be a different but valid solution
records = endpoint.RemoveDuplicates(records)
//This deduplication seems to be the a good fit
//Duplicated endpoints were living on this variable
//In all versions for the overlapping zones
//With this in place the change on plan.go is not needed
//Also it saves some rounds of cpu
//Keeping this here until we decide what's best
registryEndpointsTotal.Set(float64(len(records)))
regARecords, regAAAARecords := countAddressRecords(records)
Expand Down
2 changes: 2 additions & 0 deletions endpoint/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,8 @@ type DNSEndpointList struct {
}

// RemoveDuplicates returns a slice holding the unique endpoints.
// This function doesn't contemplate the Targets of an Endpoint
// as part of the primary Key
func RemoveDuplicates(endpoints []*Endpoint) []*Endpoint {
visited := make(map[EndpointKey]struct{})
result := []*Endpoint{}
Expand Down
19 changes: 17 additions & 2 deletions plan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,20 @@ func (p *Plan) Calculate() *Plan {
if p.DomainFilter == nil {
p.DomainFilter = endpoint.MatchAllDomainFilters(nil)
}
// Root cause of the issue:
// Behavior on 0.14.0 on Deletes
// ------------------------------------------------------------------------------------------------------
// DNSName | Current record | Desired Records (Candidate) |
// ------------------------------------------------------------------------------------------------------
// dnsname.zone.com |[dnsname.zone.com IN A]dnsname.zone.com IN A (dup ep)| [] |
// ------------------------------------------------------------------------------------------------------
//
// Behavior on 0.13.6 on Deletes
// ------------------------------------------------------------------------------------------------------
// DNSName | Current record | Desired Records (Candidate) |
// ------------------------------------------------------------------------------------------------------
// dnsname.zone.com | [dnsname.zone.com IN CNAME ] | [] |
// ------------------------------------------------------------------------------------------------------

for _, current := range filterRecordsForPlan(p.Current, p.DomainFilter, p.ManagedRecords, p.ExcludeRecords) {
t.addCurrent(current)
Expand Down Expand Up @@ -252,10 +266,11 @@ func (p *Plan) Calculate() *Plan {
// filter out updates this external dns does not have ownership claim over
if p.OwnerID != "" {
changes.Delete = endpoint.FilterEndpointsByOwnerID(p.OwnerID, changes.Delete)
// Remove duplicated endpoints generated by plan.go on Line 196
changes.Delete = endpoint.RemoveDuplicates(changes.Delete)
// Remove duplicated endpoints generated by plan.go on Line 210
// changes.Delete = endpoint.RemoveDuplicates(changes.Delete)
// This was not needed on version 0.13.6, but it seems like
// the old function/code had the ability of removing duplicated endpoints
// While the filterRecordsForPlan and addCurrent functions were running
changes.UpdateOld = endpoint.FilterEndpointsByOwnerID(p.OwnerID, changes.UpdateOld)
changes.UpdateNew = endpoint.FilterEndpointsByOwnerID(p.OwnerID, changes.UpdateNew)
}
Expand Down

0 comments on commit d9b7439

Please sign in to comment.