From d9b743922ad7fbec94cb765bd384043742ff1fa3 Mon Sep 17 00:00:00 2001 From: Leonardo Quatrocchi Date: Sat, 6 Apr 2024 03:25:05 -0300 Subject: [PATCH] Specify and clarify root cause of issue 4241 --- controller/controller.go | 7 +++++-- endpoint/endpoint.go | 2 ++ plan/plan.go | 19 +++++++++++++++++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/controller/controller.go b/controller/controller.go index f651f103e..c1979a6c4 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -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) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 08c9c6b23..26828f020 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -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{} diff --git a/plan/plan.go b/plan/plan.go index 0dc4854ac..1f913942f 100644 --- a/plan/plan.go +++ b/plan/plan.go @@ -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) @@ -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) }