Skip to content

Commit

Permalink
Merge pull request #2913 from alebedev87/handle-missing-txt-record-fi…
Browse files Browse the repository at this point in the history
…x-batch

Handle the migration to the new TXT format: missing records to be created separately
  • Loading branch information
k8s-ci-robot committed Jul 27, 2022
2 parents f48408a + 0feb32d commit 4046257
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 1 deletion.
24 changes: 23 additions & 1 deletion controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,33 @@ func (c *Controller) RunOnce(ctx context.Context) error {
verifiedARecords.Set(float64(len(vRecords)))
endpoints = c.Registry.AdjustEndpoints(endpoints)

if len(missingRecords) > 0 {
// Add missing records before the actual plan is applied.
// This prevents the problems when the missing TXT record needs to be
// created and deleted/upserted in the same batch.
missingRecordsPlan := &plan.Plan{
Policies: []plan.Policy{c.Policy},
Missing: missingRecords,
DomainFilter: endpoint.MatchAllDomainFilters{c.DomainFilter, c.Registry.GetDomainFilter()},
PropertyComparator: c.Registry.PropertyValuesEqual,
ManagedRecords: c.ManagedRecordTypes,
}
missingRecordsPlan = missingRecordsPlan.Calculate()
if missingRecordsPlan.Changes.HasChanges() {
err = c.Registry.ApplyChanges(ctx, missingRecordsPlan.Changes)
if err != nil {
registryErrorsTotal.Inc()
deprecatedRegistryErrors.Inc()
return err
}
log.Info("All missing records are created")
}
}

plan := &plan.Plan{
Policies: []plan.Policy{c.Policy},
Current: records,
Desired: endpoints,
Missing: missingRecords,
DomainFilter: endpoint.MatchAllDomainFilters{c.DomainFilter, c.Registry.GetDomainFilter()},
PropertyComparator: c.Registry.PropertyValuesEqual,
ManagedRecords: c.ManagedRecordTypes,
Expand Down
99 changes: 99 additions & 0 deletions controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,51 @@ func testControllerFiltersDomains(t *testing.T, configuredEndpoints []*endpoint.
}
}

type noopRegistryWithMissing struct {
*registry.NoopRegistry
missingRecords []*endpoint.Endpoint
}

func (r *noopRegistryWithMissing) MissingRecords() []*endpoint.Endpoint {
return r.missingRecords
}

func testControllerFiltersDomainsWithMissing(t *testing.T, configuredEndpoints []*endpoint.Endpoint, domainFilter endpoint.DomainFilterInterface, providerEndpoints, missingEndpoints []*endpoint.Endpoint, expectedChanges []*plan.Changes) {
t.Helper()
cfg := externaldns.NewConfig()
cfg.ManagedDNSRecordTypes = []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME}

source := new(testutils.MockSource)
source.On("Endpoints").Return(configuredEndpoints, nil)

// Fake some existing records in our DNS provider and validate some desired changes.
provider := &filteredMockProvider{
RecordsStore: providerEndpoints,
}
noop, err := registry.NewNoopRegistry(provider)
require.NoError(t, err)

r := &noopRegistryWithMissing{
NoopRegistry: noop,
missingRecords: missingEndpoints,
}

ctrl := &Controller{
Source: source,
Registry: r,
Policy: &plan.SyncPolicy{},
DomainFilter: domainFilter,
ManagedRecordTypes: cfg.ManagedDNSRecordTypes,
}

assert.NoError(t, ctrl.RunOnce(context.Background()))
assert.Equal(t, 1, provider.RecordsCallCount)
require.Len(t, provider.ApplyChangesCalls, len(expectedChanges))
for i, change := range expectedChanges {
assert.Equal(t, *change, *provider.ApplyChangesCalls[i])
}
}

func TestControllerSkipsEmptyChanges(t *testing.T) {
testControllerFiltersDomains(
t,
Expand Down Expand Up @@ -517,3 +562,57 @@ func TestARecords(t *testing.T) {
assert.Equal(t, math.Float64bits(2), valueFromMetric(sourceARecords))
assert.Equal(t, math.Float64bits(1), valueFromMetric(registryARecords))
}

// TestMissingRecordsApply validates that the missing records result in the dedicated plan apply.
func TestMissingRecordsApply(t *testing.T) {
testControllerFiltersDomainsWithMissing(
t,
[]*endpoint.Endpoint{
{
DNSName: "record1.used.tld",
RecordType: endpoint.RecordTypeA,
Targets: endpoint.Targets{"1.2.3.4"},
},
{
DNSName: "record2.used.tld",
RecordType: endpoint.RecordTypeA,
Targets: endpoint.Targets{"8.8.8.8"},
},
},
endpoint.NewDomainFilter([]string{"used.tld"}),
[]*endpoint.Endpoint{
{
DNSName: "record1.used.tld",
RecordType: endpoint.RecordTypeA,
Targets: endpoint.Targets{"1.2.3.4"},
},
},
[]*endpoint.Endpoint{
{
DNSName: "a-record1.used.tld",
RecordType: endpoint.RecordTypeTXT,
Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""},
},
},
[]*plan.Changes{
// Missing record had its own plan applied.
{
Create: []*endpoint.Endpoint{
{
DNSName: "a-record1.used.tld",
RecordType: endpoint.RecordTypeTXT,
Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""},
},
},
},
{
Create: []*endpoint.Endpoint{
{
DNSName: "record2.used.tld",
RecordType: endpoint.RecordTypeA,
Targets: endpoint.Targets{"8.8.8.8"},
},
},
},
})
}

0 comments on commit 4046257

Please sign in to comment.