diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 0d8dab50c..8df0c8280 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -247,6 +247,10 @@ func ChangesetLess(c Changeset, i, j int) bool { // elements, and sort on the result. } +func CorrectionLess(c []*models.Correction, i, j int) bool { + return c[i].Msg < c[j].Msg +} + func (d *differ) ChangedGroups(existing []*models.RecordConfig) map[models.RecordKey][]string { changedKeys := map[models.RecordKey][]string{} _, create, delete, modify := d.IncrementalDiff(existing) diff --git a/providers/azuredns/azureDnsProvider.go b/providers/azuredns/azureDnsProvider.go index 42d6d9e59..3cd6440b9 100644 --- a/providers/azuredns/azureDnsProvider.go +++ b/providers/azuredns/azureDnsProvider.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "sort" "strings" "time" @@ -220,7 +221,7 @@ func (a *azureDNSProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod defer cancel() _, err := a.recordsClient.Delete(ctx, *a.resourceGroup, zoneName, *rrset.Name, nativeToRecordType(rrset.Type), "") // Artifically slow things down after a delete, as the API can take time to register it. The tests fail if we delete and then recheck too quickly. - time.Sleep(25 * time.Millisecond) + time.Sleep(10 * time.Millisecond) if err != nil { return err } @@ -252,7 +253,7 @@ func (a *azureDNSProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod defer cancel() _, err := a.recordsClient.Delete(ctx, *a.resourceGroup, zoneName, recordName, existingRecordType, "") // Artifically slow things down after a delete, as the API can take time to register it. The tests fail if we delete and then recheck too quickly. - time.Sleep(25 * time.Millisecond) + time.Sleep(10 * time.Millisecond) if err != nil { return err } @@ -271,7 +272,7 @@ func (a *azureDNSProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod defer cancel() _, err := a.recordsClient.CreateOrUpdate(ctx, *a.resourceGroup, zoneName, recordName, recordType, *rrset, "", "") // Artifically slow things down after a delete, as the API can take time to register it. The tests fail if we delete and then recheck too quickly. - time.Sleep(25 * time.Millisecond) + time.Sleep(10 * time.Millisecond) if err != nil { return err } @@ -280,6 +281,18 @@ func (a *azureDNSProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*mod }) } } + + // Sort the records for cosmetic reasons: It just makes a long list + // of deletes or adds easier to read if they are in sorted order. + // That said, it may be risky to sort them (sort key is the text + // message "Msg") if there are deletes that must happen before adds. + // Reading the above code it isn't clear that any of the updates are + // order-dependent. That said, all the tests pass. + // If in the future this causes a bug, we can either just remove + // this next line, or (even better) put any order-dependent + // operations in a single models.Correction{}. + sort.Slice(corrections, func(i, j int) bool { return diff.CorrectionLess(corrections, i, j) }) + return corrections, nil } diff --git a/providers/desec/desecProvider.go b/providers/desec/desecProvider.go index 178e8a45a..0b1f1c035 100644 --- a/providers/desec/desecProvider.go +++ b/providers/desec/desecProvider.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "fmt" + "sort" "github.com/StackExchange/dnscontrol/v3/models" "github.com/StackExchange/dnscontrol/v3/pkg/diff" @@ -215,5 +216,13 @@ func (c *api) GenerateDomainCorrections(dc *models.DomainConfig, existing models }, }) + // NB(tlim): This sort is just to make updates look pretty. It is + // cosmetic. The risk here is that there may be some updates that + // require a specific order (for example a delete before an add). + // However the code doesn't seem to have such situation. All tests + // pass. That said, if this breaks anything, the easiest fix might + // be to just remove the sort. + sort.Slice(corrections, func(i, j int) bool { return diff.CorrectionLess(corrections, i, j) }) + return corrections, nil } diff --git a/providers/gandi_v5/gandi_v5Provider.go b/providers/gandi_v5/gandi_v5Provider.go index b1c1f649d..204f53e11 100644 --- a/providers/gandi_v5/gandi_v5Provider.go +++ b/providers/gandi_v5/gandi_v5Provider.go @@ -283,6 +283,14 @@ func (client *api) GenerateDomainCorrections(dc *models.DomainConfig, existing m } } + // NB(tlim): This sort is just to make updates look pretty. It is + // cosmetic. The risk here is that there may be some updates that + // require a specific order (for example a delete before an add). + // However the code doesn't seem to have such situation. All tests + // pass. That said, if this breaks anything, the easiest fix might + // be to just remove the sort. + sort.Slice(corrections, func(i, j int) bool { return diff.CorrectionLess(corrections, i, j) }) + return corrections, nil }