From f03bcaef15efcdf031eb34f9b908ade6d0788786 Mon Sep 17 00:00:00 2001 From: dongjiang1989 Date: Wed, 19 Jun 2024 13:42:30 +0800 Subject: [PATCH 1/5] fix external_dns_registry_errors_total metrics value 0 Signed-off-by: dongjiang1989 --- provider/azure/azure.go | 23 ++++++++++++++++++----- provider/azure/azure_private_dns.go | 25 ++++++++++++++++++++----- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/provider/azure/azure.go b/provider/azure/azure.go index 904fa62239..48b6703625 100644 --- a/provider/azure/azure.go +++ b/provider/azure/azure.go @@ -19,6 +19,7 @@ package azure import ( "context" + "errors" "fmt" "strings" @@ -160,9 +161,9 @@ func (p *AzureProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) } deleted, updated := p.mapChanges(zones, changes) - p.deleteRecords(ctx, deleted) - p.updateRecords(ctx, updated) - return nil + err1 := p.deleteRecords(ctx, deleted) + err2 := p.updateRecords(ctx, updated) + return errors.Join(err1, err2) } func (p *AzureProvider) zones(ctx context.Context) ([]dns.Zone, error) { @@ -235,8 +236,9 @@ func (p *AzureProvider) mapChanges(zones []dns.Zone, changes *plan.Changes) (azu return deleted, updated } -func (p *AzureProvider) deleteRecords(ctx context.Context, deleted azureChangeMap) { +func (p *AzureProvider) deleteRecords(ctx context.Context, deleted azureChangeMap) error { // Delete records first + var failedCount uint64 for zone, endpoints := range deleted { for _, ep := range endpoints { name := p.recordSetNameForZone(zone, ep) @@ -249,6 +251,7 @@ func (p *AzureProvider) deleteRecords(ctx context.Context, deleted azureChangeMa } else { log.Infof("Deleting %s record named '%s' for Azure DNS zone '%s'.", ep.RecordType, name, zone) if _, err := p.recordSetsClient.Delete(ctx, p.resourceGroup, zone, name, dns.RecordType(ep.RecordType), nil); err != nil { + failedCount++ log.Errorf( "Failed to delete %s record named '%s' for Azure DNS zone '%s': %v", ep.RecordType, @@ -260,9 +263,14 @@ func (p *AzureProvider) deleteRecords(ctx context.Context, deleted azureChangeMa } } } + if failedCount > 0 { + return fmt.Errorf("Failed to delete records for Azure DNS zone: %d", failedCount) + } + return nil } -func (p *AzureProvider) updateRecords(ctx context.Context, updated azureChangeMap) { +func (p *AzureProvider) updateRecords(ctx context.Context, updated azureChangeMap) error { + var failedCount uint64 for zone, endpoints := range updated { for _, ep := range endpoints { name := p.recordSetNameForZone(zone, ep) @@ -302,6 +310,7 @@ func (p *AzureProvider) updateRecords(ctx context.Context, updated azureChangeMa ) } if err != nil { + failedCount++ log.Errorf( "Failed to update %s record named '%s' to '%s' for DNS zone '%s': %v", ep.RecordType, @@ -313,6 +322,10 @@ func (p *AzureProvider) updateRecords(ctx context.Context, updated azureChangeMa } } } + if failedCount > 0 { + return fmt.Errorf("Failed to update records for Azure DNS zone: %d", failedCount) + } + return nil } func (p *AzureProvider) recordSetNameForZone(zone string, endpoint *endpoint.Endpoint) string { diff --git a/provider/azure/azure_private_dns.go b/provider/azure/azure_private_dns.go index 051b13c877..fef769b790 100644 --- a/provider/azure/azure_private_dns.go +++ b/provider/azure/azure_private_dns.go @@ -19,6 +19,7 @@ package azure import ( "context" + "errors" "fmt" "strings" @@ -170,9 +171,9 @@ func (p *AzurePrivateDNSProvider) ApplyChanges(ctx context.Context, changes *pla } deleted, updated := p.mapChanges(zones, changes) - p.deleteRecords(ctx, deleted) - p.updateRecords(ctx, updated) - return nil + err1 := p.deleteRecords(ctx, deleted) + err2 := p.updateRecords(ctx, updated) + return errors.Join(err1, err2) } func (p *AzurePrivateDNSProvider) zones(ctx context.Context) ([]privatedns.PrivateZone, error) { @@ -241,9 +242,10 @@ func (p *AzurePrivateDNSProvider) mapChanges(zones []privatedns.PrivateZone, cha return deleted, updated } -func (p *AzurePrivateDNSProvider) deleteRecords(ctx context.Context, deleted azurePrivateDNSChangeMap) { +func (p *AzurePrivateDNSProvider) deleteRecords(ctx context.Context, deleted azurePrivateDNSChangeMap) error { log.Debugf("Records to be deleted: %d", len(deleted)) // Delete records first + var failedCount uint64 for zone, endpoints := range deleted { for _, ep := range endpoints { name := p.recordSetNameForZone(zone, ep) @@ -256,6 +258,7 @@ func (p *AzurePrivateDNSProvider) deleteRecords(ctx context.Context, deleted azu } else { log.Infof("Deleting %s record named '%s' for Azure Private DNS zone '%s'.", ep.RecordType, name, zone) if _, err := p.recordSetsClient.Delete(ctx, p.resourceGroup, zone, privatedns.RecordType(ep.RecordType), name, nil); err != nil { + failedCount++ log.Errorf( "Failed to delete %s record named '%s' for Azure Private DNS zone '%s': %v", ep.RecordType, @@ -267,10 +270,17 @@ func (p *AzurePrivateDNSProvider) deleteRecords(ctx context.Context, deleted azu } } } + + if failedCount > 0 { + return fmt.Errorf("Failed to delete records for Azure Private DNS zone: %d", failedCount) + } + + return nil } -func (p *AzurePrivateDNSProvider) updateRecords(ctx context.Context, updated azurePrivateDNSChangeMap) { +func (p *AzurePrivateDNSProvider) updateRecords(ctx context.Context, updated azurePrivateDNSChangeMap) error { log.Debugf("Records to be updated: %d", len(updated)) + var failedCount uint64 for zone, endpoints := range updated { for _, ep := range endpoints { name := p.recordSetNameForZone(zone, ep) @@ -310,6 +320,7 @@ func (p *AzurePrivateDNSProvider) updateRecords(ctx context.Context, updated azu ) } if err != nil { + failedCount++ log.Errorf( "Failed to update %s record named '%s' to '%s' for Azure Private DNS zone '%s': %v", ep.RecordType, @@ -321,6 +332,10 @@ func (p *AzurePrivateDNSProvider) updateRecords(ctx context.Context, updated azu } } } + if failedCount > 0 { + return fmt.Errorf("Failed to update records for Azure Private DNS zone: %d", failedCount) + } + return nil } func (p *AzurePrivateDNSProvider) recordSetNameForZone(zone string, endpoint *endpoint.Endpoint) string { From f423b60d020638355d8901fd9ca8262b1b892a35 Mon Sep 17 00:00:00 2001 From: dongjiang1989 Date: Thu, 27 Jun 2024 12:04:01 +0800 Subject: [PATCH 2/5] add ApplyChanges return error case Signed-off-by: dongjiang1989 --- provider/azure/azure_privatedns_test.go | 49 +++++++++++++++++++++++++ provider/azure/azure_test.go | 46 +++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/provider/azure/azure_privatedns_test.go b/provider/azure/azure_privatedns_test.go index dea74c10b1..3ba875a206 100644 --- a/provider/azure/azure_privatedns_test.go +++ b/provider/azure/azure_privatedns_test.go @@ -18,6 +18,7 @@ package azure import ( "context" + "errors" "testing" azcoreruntime "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" @@ -352,6 +353,54 @@ func TestAzurePrivateDNSApplyChanges(t *testing.T) { }) } +func TestAzurePrivateDNSApplyChangesRetrunError(t *testing.T) { + recordsClient := mockPrivateRecordSetsClient{} + zones := []*privatedns.PrivateZone{ + createMockPrivateZone("example.com", "/privateDnsZones/example.com"), + } + + zonesClient := newMockPrivateZonesClient(zones) + + provider := newAzurePrivateDNSProvider( + endpoint.NewDomainFilter([]string{"foo.example.com"}), + endpoint.NewDomainFilter([]string{"example.com"}), + provider.NewZoneIDFilter([]string{""}), + false, + "group", + &zonesClient, + &recordsClient, + ) + + createRecords := []*endpoint.Endpoint{} + + currentRecords := []*endpoint.Endpoint{ + endpoint.NewEndpoint("old.foo.example.com", endpoint.RecordTypeA, "121.212.121.212"), + endpoint.NewEndpoint("oldcname.foo.example.com", endpoint.RecordTypeCNAME, "other.com"), + endpoint.NewEndpoint("old.nope.example.com", endpoint.RecordTypeA, "121.212.121.212"), + } + updatedRecords := []*endpoint.Endpoint{ + endpoint.NewEndpointWithTTL("new.foo.example.com", endpoint.RecordTypeA, 3600, "111.222.111.222"), + endpoint.NewEndpointWithTTL("new.foo.example.com", endpoint.RecordTypeAAAA, 3600, "2001::111:222:111:222"), + endpoint.NewEndpointWithTTL("newcname.foo.example.com", endpoint.RecordTypeCNAME, 10, "other.com"), + endpoint.NewEndpoint("new.nope.example.com", endpoint.RecordTypeA, "222.111.222.111"), + endpoint.NewEndpoint("new.nope.example.com", endpoint.RecordTypeAAAA, "2001::222:111:222:111"), + endpoint.NewEndpointWithTTL("@.foo.example.com", endpoint.RecordTypeNAPTR, 10, "other.com"), + } + + deleteRecords := []*endpoint.Endpoint{} + + changes := &plan.Changes{ + Create: createRecords, + UpdateNew: updatedRecords, + UpdateOld: currentRecords, + Delete: deleteRecords, + } + + if err := provider.ApplyChanges(context.Background(), changes); err == nil { + t.Fatal(errors.New("ApplyChanges NotError")) + } +} + func TestAzurePrivateDNSApplyChangesDryRun(t *testing.T) { recordsClient := mockRecordSetsClient{} diff --git a/provider/azure/azure_test.go b/provider/azure/azure_test.go index f2031d1398..fe23049484 100644 --- a/provider/azure/azure_test.go +++ b/provider/azure/azure_test.go @@ -18,6 +18,7 @@ package azure import ( "context" + "errors" "testing" azcoreruntime "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" @@ -358,6 +359,51 @@ func TestAzureApplyChanges(t *testing.T) { }) } +func TestAzureApplyChangesRetrunError(t *testing.T) { + recordsClient := mockRecordSetsClient{} + zonesClient := newMockZonesClient([]*dns.Zone{createMockZone("example.com", "/dnszones/example.com")}) + provider := newAzureProvider( + endpoint.NewDomainFilter([]string{"foo.example.com"}), + endpoint.NewDomainFilter([]string{"example.com"}), + provider.NewZoneIDFilter([]string{""}), + false, + "group", + "", + "", + &zonesClient, + &recordsClient, + ) + + createRecords := []*endpoint.Endpoint{} + + currentRecords := []*endpoint.Endpoint{ + endpoint.NewEndpoint("old.foo.example.com", endpoint.RecordTypeA, "121.212.121.212"), + endpoint.NewEndpoint("oldcname.foo.example.com", endpoint.RecordTypeCNAME, "other.com"), + endpoint.NewEndpoint("old.nope.example.com", endpoint.RecordTypeA, "121.212.121.212"), + } + updatedRecords := []*endpoint.Endpoint{ + endpoint.NewEndpointWithTTL("new.foo.example.com", endpoint.RecordTypeA, 3600, "111.222.111.222"), + endpoint.NewEndpointWithTTL("new.foo.example.com", endpoint.RecordTypeAAAA, 3600, "2001::111:222:111:222"), + endpoint.NewEndpointWithTTL("newcname.foo.example.com", endpoint.RecordTypeCNAME, 10, "other.com"), + endpoint.NewEndpoint("new.nope.example.com", endpoint.RecordTypeA, "222.111.222.111"), + endpoint.NewEndpoint("new.nope.example.com", endpoint.RecordTypeAAAA, "2001::222:111:222:111"), + endpoint.NewEndpoint("@.new.example.com", endpoint.RecordTypeAAAA, "aabb::222:111:222:111"), + } + + deleteRecords := []*endpoint.Endpoint{} + + changes := &plan.Changes{ + Create: createRecords, + UpdateNew: updatedRecords, + UpdateOld: currentRecords, + Delete: deleteRecords, + } + + if err := provider.ApplyChanges(context.Background(), changes); err != nil { + t.Fatal(errors.New("ApplyChanges NotError")) + } +} + func TestAzureApplyChangesDryRun(t *testing.T) { recordsClient := mockRecordSetsClient{} From 3b2174281379109646d96b806e2cf0671bdef9ad Mon Sep 17 00:00:00 2001 From: dongjiang Date: Thu, 4 Jul 2024 21:24:30 +0800 Subject: [PATCH 3/5] =?UTF-8?q?=E6=9B=B4=E6=96=B0=20azure=5Fprivatedns=5Ft?= =?UTF-8?q?est.go?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com> --- provider/azure/azure_privatedns_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/azure/azure_privatedns_test.go b/provider/azure/azure_privatedns_test.go index 3ba875a206..a0f701914c 100644 --- a/provider/azure/azure_privatedns_test.go +++ b/provider/azure/azure_privatedns_test.go @@ -353,7 +353,7 @@ func TestAzurePrivateDNSApplyChanges(t *testing.T) { }) } -func TestAzurePrivateDNSApplyChangesRetrunError(t *testing.T) { +func TestAzurePrivateDNSApplyChangesReturnError(t *testing.T) { recordsClient := mockPrivateRecordSetsClient{} zones := []*privatedns.PrivateZone{ createMockPrivateZone("example.com", "/privateDnsZones/example.com"), From fff87316e096f3c9992bd2496d6645b3b88f50e8 Mon Sep 17 00:00:00 2001 From: dongjiang Date: Thu, 4 Jul 2024 21:24:37 +0800 Subject: [PATCH 4/5] =?UTF-8?q?=E6=9B=B4=E6=96=B0=20azure=5Ftest.go?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com> --- provider/azure/azure_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/azure/azure_test.go b/provider/azure/azure_test.go index fe23049484..271f98f302 100644 --- a/provider/azure/azure_test.go +++ b/provider/azure/azure_test.go @@ -359,7 +359,7 @@ func TestAzureApplyChanges(t *testing.T) { }) } -func TestAzureApplyChangesRetrunError(t *testing.T) { +func TestAzureApplyChangesReturnError(t *testing.T) { recordsClient := mockRecordSetsClient{} zonesClient := newMockZonesClient([]*dns.Zone{createMockZone("example.com", "/dnszones/example.com")}) provider := newAzureProvider( From f2e4deef8f97d1dcc34fad5bd6ca82946d4e808d Mon Sep 17 00:00:00 2001 From: dongjiang Date: Fri, 5 Jul 2024 09:29:30 +0800 Subject: [PATCH 5/5] Update provider/azure/azure_test.go --- provider/azure/azure_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/azure/azure_test.go b/provider/azure/azure_test.go index 271f98f302..5e7e1cd42c 100644 --- a/provider/azure/azure_test.go +++ b/provider/azure/azure_test.go @@ -400,7 +400,7 @@ func TestAzureApplyChangesReturnError(t *testing.T) { } if err := provider.ApplyChanges(context.Background(), changes); err != nil { - t.Fatal(errors.New("ApplyChanges NotError")) + t.Fatal(errors.New("ApplyChanges Error")) } }