Skip to content

Commit 9e5b7eb

Browse files
kiurchvkiurchv
committed
refactor: move duplicated service endpoint merge logic into mergeAndSortEndpoints function
1 parent 0959ee2 commit 9e5b7eb

File tree

3 files changed

+51
-87
lines changed

3 files changed

+51
-87
lines changed

‎source/nomad_service.go

Lines changed: 1 addition & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"bytes"
2121
"context"
2222
"fmt"
23-
"sort"
2423
"strings"
2524
"text/template"
2625
"unicode"
@@ -121,49 +120,7 @@ func (ns *nomadServiceSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoi
121120
}
122121
}
123122

124-
// this sorting is required to make merging work.
125-
// after we merge endpoints that have same DNS, we want to ensure that we end up with the same service being an "owner"
126-
// of all those records, as otherwise each time we update, we will end up with a different service that gets data merged in
127-
// and that will cause external-dns to recreate dns record due to different service owner in TXT record.
128-
// if new service is added or old one removed, that might cause existing record to get re-created due to potentially new
129-
// owner being selected. Which is fine, since it shouldn't happen often and shouldn't cause any disruption.
130-
if len(endpoints) > 1 {
131-
sort.Slice(endpoints, func(i, j int) bool {
132-
return endpoints[i].Labels[endpoint.ResourceLabelKey] < endpoints[j].Labels[endpoint.ResourceLabelKey]
133-
})
134-
// Use stable sort to not disrupt the order of services
135-
sort.SliceStable(endpoints, func(i, j int) bool {
136-
if endpoints[i].DNSName != endpoints[j].DNSName {
137-
return endpoints[i].DNSName < endpoints[j].DNSName
138-
}
139-
return endpoints[i].RecordType < endpoints[j].RecordType
140-
})
141-
mergedEndpoints := []*endpoint.Endpoint{}
142-
mergedEndpoints = append(mergedEndpoints, endpoints[0])
143-
for i := 1; i < len(endpoints); i++ {
144-
lastMergedEndpoint := len(mergedEndpoints) - 1
145-
if mergedEndpoints[lastMergedEndpoint].DNSName == endpoints[i].DNSName &&
146-
mergedEndpoints[lastMergedEndpoint].RecordType == endpoints[i].RecordType &&
147-
mergedEndpoints[lastMergedEndpoint].RecordType != endpoint.RecordTypeCNAME && // It is against RFC-1034 for CNAME records to have multiple targets, so skip merging
148-
mergedEndpoints[lastMergedEndpoint].SetIdentifier == endpoints[i].SetIdentifier &&
149-
mergedEndpoints[lastMergedEndpoint].RecordTTL == endpoints[i].RecordTTL {
150-
mergedEndpoints[lastMergedEndpoint].Targets = append(mergedEndpoints[lastMergedEndpoint].Targets, endpoints[i].Targets[0])
151-
} else {
152-
mergedEndpoints = append(mergedEndpoints, endpoints[i])
153-
}
154-
155-
if mergedEndpoints[lastMergedEndpoint].DNSName == endpoints[i].DNSName &&
156-
mergedEndpoints[lastMergedEndpoint].RecordType == endpoints[i].RecordType &&
157-
mergedEndpoints[lastMergedEndpoint].RecordType == endpoint.RecordTypeCNAME {
158-
log.Debugf("CNAME %s with multiple targets found", endpoints[i].DNSName)
159-
}
160-
}
161-
endpoints = mergedEndpoints
162-
}
163-
164-
for _, ep := range endpoints {
165-
sort.Sort(ep.Targets)
166-
}
123+
endpoints = mergeAndSortEndpoints(endpoints)
167124

168125
return endpoints, nil
169126
}

‎source/service.go

Lines changed: 1 addition & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -203,49 +203,7 @@ func (sc *serviceSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, e
203203
endpoints = append(endpoints, svcEndpoints...)
204204
}
205205

206-
// this sorting is required to make merging work.
207-
// after we merge endpoints that have same DNS, we want to ensure that we end up with the same service being an "owner"
208-
// of all those records, as otherwise each time we update, we will end up with a different service that gets data merged in
209-
// and that will cause external-dns to recreate dns record due to different service owner in TXT record.
210-
// if new service is added or old one removed, that might cause existing record to get re-created due to potentially new
211-
// owner being selected. Which is fine, since it shouldn't happen often and shouldn't cause any disruption.
212-
if len(endpoints) > 1 {
213-
sort.Slice(endpoints, func(i, j int) bool {
214-
return endpoints[i].Labels[endpoint.ResourceLabelKey] < endpoints[j].Labels[endpoint.ResourceLabelKey]
215-
})
216-
// Use stable sort to not disrupt the order of services
217-
sort.SliceStable(endpoints, func(i, j int) bool {
218-
if endpoints[i].DNSName != endpoints[j].DNSName {
219-
return endpoints[i].DNSName < endpoints[j].DNSName
220-
}
221-
return endpoints[i].RecordType < endpoints[j].RecordType
222-
})
223-
mergedEndpoints := []*endpoint.Endpoint{}
224-
mergedEndpoints = append(mergedEndpoints, endpoints[0])
225-
for i := 1; i < len(endpoints); i++ {
226-
lastMergedEndpoint := len(mergedEndpoints) - 1
227-
if mergedEndpoints[lastMergedEndpoint].DNSName == endpoints[i].DNSName &&
228-
mergedEndpoints[lastMergedEndpoint].RecordType == endpoints[i].RecordType &&
229-
mergedEndpoints[lastMergedEndpoint].RecordType != endpoint.RecordTypeCNAME && // It is against RFC-1034 for CNAME records to have multiple targets, so skip merging
230-
mergedEndpoints[lastMergedEndpoint].SetIdentifier == endpoints[i].SetIdentifier &&
231-
mergedEndpoints[lastMergedEndpoint].RecordTTL == endpoints[i].RecordTTL {
232-
mergedEndpoints[lastMergedEndpoint].Targets = append(mergedEndpoints[lastMergedEndpoint].Targets, endpoints[i].Targets[0])
233-
} else {
234-
mergedEndpoints = append(mergedEndpoints, endpoints[i])
235-
}
236-
237-
if mergedEndpoints[lastMergedEndpoint].DNSName == endpoints[i].DNSName &&
238-
mergedEndpoints[lastMergedEndpoint].RecordType == endpoints[i].RecordType &&
239-
mergedEndpoints[lastMergedEndpoint].RecordType == endpoint.RecordTypeCNAME {
240-
log.Debugf("CNAME %s with multiple targets found", endpoints[i].DNSName)
241-
}
242-
}
243-
endpoints = mergedEndpoints
244-
}
245-
246-
for _, ep := range endpoints {
247-
sort.Sort(ep.Targets)
248-
}
206+
endpoints = mergeAndSortEndpoints(endpoints)
249207

250208
return endpoints, nil
251209
}

‎source/source.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"math"
2424
"net/netip"
2525
"reflect"
26+
"sort"
2627
"strconv"
2728
"strings"
2829
"text/template"
@@ -384,3 +385,51 @@ func waitForDynamicCacheSync(ctx context.Context, factory dynamicInformerFactory
384385
}
385386
return nil
386387
}
388+
389+
func mergeAndSortEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
390+
// this sorting is required to make merging work.
391+
// after we merge endpoints that have same DNS, we want to ensure that we end up with the same service being an "owner"
392+
// of all those records, as otherwise each time we update, we will end up with a different service that gets data merged in
393+
// and that will cause external-dns to recreate dns record due to different service owner in TXT record.
394+
// if new service is added or old one removed, that might cause existing record to get re-created due to potentially new
395+
// owner being selected. Which is fine, since it shouldn't happen often and shouldn't cause any disruption.
396+
if len(endpoints) > 1 {
397+
sort.Slice(endpoints, func(i, j int) bool {
398+
return endpoints[i].Labels[endpoint.ResourceLabelKey] < endpoints[j].Labels[endpoint.ResourceLabelKey]
399+
})
400+
// Use stable sort to not disrupt the order of services
401+
sort.SliceStable(endpoints, func(i, j int) bool {
402+
if endpoints[i].DNSName != endpoints[j].DNSName {
403+
return endpoints[i].DNSName < endpoints[j].DNSName
404+
}
405+
return endpoints[i].RecordType < endpoints[j].RecordType
406+
})
407+
mergedEndpoints := []*endpoint.Endpoint{}
408+
mergedEndpoints = append(mergedEndpoints, endpoints[0])
409+
for i := 1; i < len(endpoints); i++ {
410+
lastMergedEndpoint := len(mergedEndpoints) - 1
411+
if mergedEndpoints[lastMergedEndpoint].DNSName == endpoints[i].DNSName &&
412+
mergedEndpoints[lastMergedEndpoint].RecordType == endpoints[i].RecordType &&
413+
mergedEndpoints[lastMergedEndpoint].RecordType != endpoint.RecordTypeCNAME && // It is against RFC-1034 for CNAME records to have multiple targets, so skip merging
414+
mergedEndpoints[lastMergedEndpoint].SetIdentifier == endpoints[i].SetIdentifier &&
415+
mergedEndpoints[lastMergedEndpoint].RecordTTL == endpoints[i].RecordTTL {
416+
mergedEndpoints[lastMergedEndpoint].Targets = append(mergedEndpoints[lastMergedEndpoint].Targets, endpoints[i].Targets[0])
417+
} else {
418+
mergedEndpoints = append(mergedEndpoints, endpoints[i])
419+
}
420+
421+
if mergedEndpoints[lastMergedEndpoint].DNSName == endpoints[i].DNSName &&
422+
mergedEndpoints[lastMergedEndpoint].RecordType == endpoints[i].RecordType &&
423+
mergedEndpoints[lastMergedEndpoint].RecordType == endpoint.RecordTypeCNAME {
424+
log.Debugf("CNAME %s with multiple targets found", endpoints[i].DNSName)
425+
}
426+
}
427+
endpoints = mergedEndpoints
428+
}
429+
430+
for _, ep := range endpoints {
431+
sort.Sort(ep.Targets)
432+
}
433+
434+
return endpoints
435+
}

0 commit comments

Comments
 (0)