Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle migration to the new TXT format: use ForceUpdate strategy #3635

Merged
25 changes: 0 additions & 25 deletions controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,6 @@ func (c *Controller) RunOnce(ctx context.Context) error {
return err
}

missingRecords := c.Registry.MissingRecords()

registryEndpointsTotal.Set(float64(len(records)))
regARecords, regAAAARecords := countAddressRecords(records)
registryARecords.Set(float64(regARecords))
Expand All @@ -218,29 +216,6 @@ func (c *Controller) RunOnce(ctx context.Context) error {
verifiedAAAARecords.Set(float64(vAAAARecords))
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,
Expand Down
99 changes: 0 additions & 99 deletions controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,51 +311,6 @@ 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 @@ -683,60 +638,6 @@ func TestARecords(t *testing.T) {
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"},
},
},
},
})
}

func TestAAAARecords(t *testing.T) {
testControllerFiltersDomains(
t,
Expand Down
7 changes: 0 additions & 7 deletions plan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ type Plan struct {
Current []*endpoint.Endpoint
// List of desired records
Desired []*endpoint.Endpoint
// List of missing records to be created, use for the migrations (e.g. old-new TXT format)
Missing []*endpoint.Endpoint
// Policies under which the desired changes are calculated
Policies []Policy
// List of changes necessary to move towards desired state
Expand Down Expand Up @@ -177,11 +175,6 @@ func (p *Plan) Calculate() *Plan {
changes = pol.Apply(changes)
}

// Handle the migration of the TXT records created before the new format (introduced in v0.12.0)
if len(p.Missing) > 0 {
changes.Create = append(changes.Create, filterRecordsForPlan(p.Missing, p.DomainFilter, append(p.ManagedRecords, endpoint.RecordTypeTXT))...)
}

plan := &Plan{
Current: p.Current,
Desired: p.Desired,
Expand Down
33 changes: 0 additions & 33 deletions plan/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@ type PlanTestSuite struct {
domainFilterFiltered2 *endpoint.Endpoint
domainFilterFiltered3 *endpoint.Endpoint
domainFilterExcluded *endpoint.Endpoint
domainFilterFilteredTXT1 *endpoint.Endpoint
domainFilterFilteredTXT2 *endpoint.Endpoint
domainFilterExcludedTXT *endpoint.Endpoint
}

func (suite *PlanTestSuite) SetupTest() {
Expand Down Expand Up @@ -233,21 +230,6 @@ func (suite *PlanTestSuite) SetupTest() {
Targets: endpoint.Targets{"1.1.1.1"},
RecordType: "A",
}
suite.domainFilterFilteredTXT1 = &endpoint.Endpoint{
DNSName: "a-foo.domain.tld",
Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""},
RecordType: "TXT",
}
suite.domainFilterFilteredTXT2 = &endpoint.Endpoint{
DNSName: "cname-bar.domain.tld",
Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""},
RecordType: "TXT",
}
suite.domainFilterExcludedTXT = &endpoint.Endpoint{
DNSName: "cname-bar.otherdomain.tld",
Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""},
RecordType: "TXT",
}
}

func (suite *PlanTestSuite) TestSyncFirstRound() {
Expand Down Expand Up @@ -661,21 +643,6 @@ func (suite *PlanTestSuite) TestDomainFiltersUpdate() {
validateEntries(suite.T(), changes.Delete, expectedDelete)
}

func (suite *PlanTestSuite) TestMissing() {
missing := []*endpoint.Endpoint{suite.domainFilterFilteredTXT1, suite.domainFilterFilteredTXT2, suite.domainFilterExcludedTXT}
expectedCreate := []*endpoint.Endpoint{suite.domainFilterFilteredTXT1, suite.domainFilterFilteredTXT2}

p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Missing: missing,
DomainFilter: endpoint.NewDomainFilter([]string{"domain.tld"}),
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
}

changes := p.Calculate().Changes
validateEntries(suite.T(), changes.Create, expectedCreate)
}

func (suite *PlanTestSuite) TestAAAARecords() {

current := []*endpoint.Endpoint{}
Expand Down
5 changes: 0 additions & 5 deletions registry/aws_sd_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,6 @@ func (sdr *AWSSDRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, er
return records, nil
}

// MissingRecords returns nil because there is no missing records for AWSSD registry
func (sdr *AWSSDRegistry) MissingRecords() []*endpoint.Endpoint {
return nil
}

// ApplyChanges filters out records not owned the External-DNS, additionally it adds the required label
// inserted in the AWS SD instance as a CreateID field
func (sdr *AWSSDRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) error {
Expand Down
5 changes: 0 additions & 5 deletions registry/noop.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ func (im *NoopRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, erro
return im.provider.Records(ctx)
}

// MissingRecords returns nil because there is no missing records for Noop registry
func (im *NoopRegistry) MissingRecords() []*endpoint.Endpoint {
return nil
}

// ApplyChanges propagates changes to the dns provider
func (im *NoopRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) error {
return im.provider.ApplyChanges(ctx, changes)
Expand Down
1 change: 0 additions & 1 deletion registry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ type Registry interface {
PropertyValuesEqual(attribute string, previous string, current string) bool
AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint
GetDomainFilter() endpoint.DomainFilterInterface
MissingRecords() []*endpoint.Endpoint
}

// TODO(ideahitme): consider moving this to Plan
Expand Down
25 changes: 5 additions & 20 deletions registry/txt.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ import (
"sigs.k8s.io/external-dns/provider"
)

const recordTemplate = "%{record_type}"
const (
recordTemplate = "%{record_type}"
providerSpecificForceUpdate = "txt/force-update"
)

// TXTRegistry implements registry interface with ownership implemented via associated TXT records
type TXTRegistry struct {
Expand All @@ -50,9 +53,6 @@ type TXTRegistry struct {

managedRecordTypes []string

// missingTXTRecords stores TXT records which are missing after the migration to the new format
missingTXTRecords []*endpoint.Endpoint

// encrypt text records
txtEncryptEnabled bool
txtEncryptAESKey []byte
Expand Down Expand Up @@ -117,7 +117,6 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error
}

endpoints := []*endpoint.Endpoint{}
missingEndpoints := []*endpoint.Endpoint{}

labelMap := map[string]endpoint.Labels{}
txtRecordsMap := map[string]struct{}{}
Expand Down Expand Up @@ -174,17 +173,11 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error
if plan.IsManagedRecord(ep.RecordType, im.managedRecordTypes) {
// Get desired TXT records and detect the missing ones
desiredTXTs := im.generateTXTRecord(ep)
missingDesiredTXTs := []*endpoint.Endpoint{}
for _, desiredTXT := range desiredTXTs {
if _, exists := txtRecordsMap[desiredTXT.DNSName]; !exists {
missingDesiredTXTs = append(missingDesiredTXTs, desiredTXT)
ep.WithProviderSpecific(providerSpecificForceUpdate, "true")
}
}
if len(desiredTXTs) > len(missingDesiredTXTs) {
// Add missing TXT records only if those are managed (by externaldns) ones.
// The unmanaged record has both of the desired TXT records missing.
missingEndpoints = append(missingEndpoints, missingDesiredTXTs...)
}
}
}
}
Expand All @@ -195,17 +188,9 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error
im.recordsCacheRefreshTime = time.Now()
}

im.missingTXTRecords = missingEndpoints

return endpoints, nil
}

// MissingRecords returns the TXT record to be created.
// The missing records are collected during the run of Records method.
func (im *TXTRegistry) MissingRecords() []*endpoint.Endpoint {
return im.missingTXTRecords
}

// generateTXTRecord generates both "old" and "new" TXT records.
// Once we decide to drop old format we need to drop toTXTName() and rename toNewTXTName
func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) []*endpoint.Endpoint {
Expand Down
Loading