diff --git a/controller/controller.go b/controller/controller.go index 689fc9ef9d..83df2f45c2 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -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, diff --git a/controller/controller_test.go b/controller/controller_test.go index 27d01777c2..b18f07e629 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -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, @@ -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"}, + }, + }, + }, + }) +}