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.

Sign up for GitHub

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

adds soft error for google provider #4682

Merged
merged 2 commits into from
Sep 19, 2024
Merged

adds soft error for google provider #4682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f73f86
adds soft error for google provider
EWK20 Aug 15, 2024
920e682
adds soft error test cases
EWK20 Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions provider/google/google.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (p *GoogleProvider) Zones(ctx context.Context) (map[string]*dns.ManagedZone

log.Debugf("Matching zones against domain filters: %v", p.domainFilter)
if err := p.managedZonesClient.List(p.project).Pages(ctx, f); err != nil {
return nil, err
return nil, provider.NewSoftError(fmt.Errorf("failed to list zones: %w", err))
}

if len(zones) == 0 {
Expand Down Expand Up @@ -228,7 +228,7 @@ func (p *GoogleProvider) Records(ctx context.Context) (endpoints []*endpoint.End

for _, z := range zones {
if err := p.resourceRecordSetsClient.List(p.project, z.Name).Pages(ctx, f); err != nil {
return nil, err
return nil, provider.NewSoftError(fmt.Errorf("failed to list records in zone %s: %w", z.Name, err))
}
}

Expand Down Expand Up @@ -302,7 +302,7 @@ func (p *GoogleProvider) submitChange(ctx context.Context, change *dns.Change) e
}

if _, err := p.changesClient.Create(p.project, zone, c).Do(); err != nil {
return err
return provider.NewSoftError(fmt.Errorf("failed to create changes: %w", err))
}

time.Sleep(p.batchChangeInterval)
Expand Down
123 changes: 75 additions & 48 deletions provider/google/google_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ func (m *mockManagedZonesCreateCall) Do(opts ...googleapi.CallOption) (*dns.Mana
}

type mockManagedZonesListCall struct {
project string
project string
zonesListSoftErr error
}

func (m *mockManagedZonesListCall) Pages(ctx context.Context, f func(*dns.ManagedZonesListResponse) error) error {
Expand All @@ -71,22 +72,29 @@ func (m *mockManagedZonesListCall) Pages(ctx context.Context, f func(*dns.Manage
}
}

if m.zonesListSoftErr != nil {
return m.zonesListSoftErr
}

return f(&dns.ManagedZonesListResponse{ManagedZones: zones})
}

type mockManagedZonesClient struct{}
type mockManagedZonesClient struct {
zonesErr error
}

func (m *mockManagedZonesClient) Create(project string, managedZone *dns.ManagedZone) managedZonesCreateCallInterface {
return &mockManagedZonesCreateCall{project: project, managedZone: managedZone}
}

func (m *mockManagedZonesClient) List(project string) managedZonesListCallInterface {
return &mockManagedZonesListCall{project: project}
return &mockManagedZonesListCall{project: project, zonesListSoftErr: m.zonesErr}
}

type mockResourceRecordSetsListCall struct {
project string
managedZone string
project string
managedZone string
recordsListSoftErr error
}

func (m *mockResourceRecordSetsListCall) Pages(ctx context.Context, f func(*dns.ResourceRecordSetsListResponse) error) error {
Expand All @@ -102,13 +110,19 @@ func (m *mockResourceRecordSetsListCall) Pages(ctx context.Context, f func(*dns.
resp = append(resp, v)
}

if m.recordsListSoftErr != nil {
return m.recordsListSoftErr
}

return f(&dns.ResourceRecordSetsListResponse{Rrsets: resp})
}

type mockResourceRecordSetsClient struct{}
type mockResourceRecordSetsClient struct {
recordsErr error
}

func (m *mockResourceRecordSetsClient) List(project string, managedZone string) resourceRecordSetsListCallInterface {
return &mockResourceRecordSetsListCall{project: project, managedZone: managedZone}
return &mockResourceRecordSetsListCall{project: project, managedZone: managedZone, recordsListSoftErr: m.recordsErr}
}

type mockChangesCreateCall struct {
Expand Down Expand Up @@ -240,24 +254,11 @@ func TestGoogleZonesVisibilityFilterPrivate(t *testing.T) {
func TestGoogleZonesVisibilityFilterPrivatePeering(t *testing.T) {
provider := newGoogleProviderZoneOverlap(t, endpoint.NewDomainFilter([]string{"svc.local."}), provider.NewZoneIDFilter([]string{""}), provider.NewZoneTypeFilter("private"), false, []*endpoint.Endpoint{})

zones, err := provider.Zones(context.Background())
require.NoError(t, err)

validateZones(t, zones, map[string]*dns.ManagedZone{
"svc-local": {Name: "svc-local", DnsName: "svc.local.", Id: 1005, Visibility: "private"},
})
}

func TestGoogleZones(t *testing.T) {
provider := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), false, []*endpoint.Endpoint{})

zones, err := provider.Zones(context.Background())
require.NoError(t, err)

validateZones(t, zones, map[string]*dns.ManagedZone{
"zone-1-ext-dns-test-2-gcp-zalan-do": {Name: "zone-1-ext-dns-test-2-gcp-zalan-do", DnsName: "zone-1.ext-dns-test-2.gcp.zalan.do."},
"zone-2-ext-dns-test-2-gcp-zalan-do": {Name: "zone-2-ext-dns-test-2-gcp-zalan-do", DnsName: "zone-2.ext-dns-test-2.gcp.zalan.do."},
"zone-3-ext-dns-test-2-gcp-zalan-do": {Name: "zone-3-ext-dns-test-2-gcp-zalan-do", DnsName: "zone-3.ext-dns-test-2.gcp.zalan.do."},
"svc-local": {Name: "svc-local", DnsName: "svc.local.", Id: 1005, Visibility: "private"},
})
}

Expand All @@ -268,7 +269,7 @@ func TestGoogleRecords(t *testing.T) {
endpoint.NewEndpointWithTTL("list-test-alias.zone-1.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeCNAME, endpoint.TTL(3), "foo.elb.amazonaws.com"),
}

provider := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), false, originalEndpoints)
provider := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), false, originalEndpoints, nil, nil)

records, err := provider.Records(context.Background())
require.NoError(t, err)
Expand Down Expand Up @@ -299,6 +300,8 @@ func TestGoogleRecordsFilter(t *testing.T) {
provider.NewZoneIDFilter([]string{""}),
false,
originalEndpoints,
nil,
nil,
)

// these records should be filtered out since they don't match a hosted zone or domain filter.
Expand Down Expand Up @@ -343,6 +346,8 @@ func TestGoogleApplyChanges(t *testing.T) {
endpoint.NewEndpointWithTTL("update-test-cname.zone-1.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeCNAME, googleRecordTTL, "bar.elb.amazonaws.com"),
endpoint.NewEndpointWithTTL("delete-test-cname.zone-1.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeCNAME, googleRecordTTL, "qux.elb.amazonaws.com"),
},
nil,
nil,
)

createRecords := []*endpoint.Endpoint{
Expand Down Expand Up @@ -407,7 +412,7 @@ func TestGoogleApplyChangesDryRun(t *testing.T) {
endpoint.NewEndpointWithTTL("delete-test-cname.zone-1.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeCNAME, googleRecordTTL, "qux.elb.amazonaws.com"),
}

provider := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), true, originalEndpoints)
provider := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), true, originalEndpoints, nil, nil)

createRecords := []*endpoint.Endpoint{
endpoint.NewEndpoint("create-test.zone-1.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeA, "8.8.8.8"),
Expand Down Expand Up @@ -449,12 +454,12 @@ func TestGoogleApplyChangesDryRun(t *testing.T) {
}

func TestGoogleApplyChangesEmpty(t *testing.T) {
provider := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), false, []*endpoint.Endpoint{})
provider := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), false, []*endpoint.Endpoint{}, nil, nil)
assert.NoError(t, provider.ApplyChanges(context.Background(), &plan.Changes{}))
}

func TestNewFilteredRecords(t *testing.T) {
provider := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), false, []*endpoint.Endpoint{})
provider := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{""}), false, []*endpoint.Endpoint{}, nil, nil)

records := provider.newFilteredRecords([]*endpoint.Endpoint{
endpoint.NewEndpointWithTTL("update-test.zone-2.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeA, 1, "8.8.4.4"),
Expand Down Expand Up @@ -603,6 +608,26 @@ func TestGoogleBatchChangeSetExceedingNameChange(t *testing.T) {
require.Equal(t, 0, len(batchCs))
}

func TestSoftErrListZonesConflict(t *testing.T) {
p := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{}), false, []*endpoint.Endpoint{}, provider.NewSoftError(fmt.Errorf("failed to list zones")), nil)

zones, err := p.Zones(context.Background())
require.Error(t, err)
require.ErrorIs(t, err, provider.SoftError)

require.Empty(t, zones)
}

func TestSoftErrListRecordsConflict(t *testing.T) {
p := newGoogleProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.gcp.zalan.do."}), provider.NewZoneIDFilter([]string{}), false, []*endpoint.Endpoint{}, nil, provider.NewSoftError(fmt.Errorf("failed to list records in zone")))

records, err := p.Records(context.Background())
require.Error(t, err)
require.ErrorIs(t, err, provider.SoftError)

require.Empty(t, records)
}

func sortChangesByName(cs *dns.Change) {
sort.SliceStable(cs.Additions, func(i, j int) bool {
return cs.Additions[i].Name < cs.Additions[j].Name
Expand Down Expand Up @@ -647,7 +672,7 @@ func validateChangeRecord(t *testing.T, record *dns.ResourceRecordSet, expected
assert.Equal(t, expected.Type, record.Type)
}

func newGoogleProviderZoneOverlap(t *testing.T, domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zoneTypeFilter provider.ZoneTypeFilter, dryRun bool, records []*endpoint.Endpoint) *GoogleProvider {
func newGoogleProviderZoneOverlap(t *testing.T, domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zoneTypeFilter provider.ZoneTypeFilter, dryRun bool, _ []*endpoint.Endpoint) *GoogleProvider {
provider := &GoogleProvider{
project: "zalando-external-dns-test",
dryRun: false,
Expand Down Expand Up @@ -694,7 +719,6 @@ func newGoogleProviderZoneOverlap(t *testing.T, domainFilter endpoint.DomainFilt
Visibility: "private",
})


createZone(t, provider, &dns.ManagedZone{
Name: "svc-local",
DnsName: "svc.local.",
Expand All @@ -703,27 +727,31 @@ func newGoogleProviderZoneOverlap(t *testing.T, domainFilter endpoint.DomainFilt
})

createZone(t, provider, &dns.ManagedZone{
Name: "svc-local-peer",
DnsName: "svc.local.",
Id: 10006,
Visibility: "private",
Name: "svc-local-peer",
DnsName: "svc.local.",
Id: 10006,
Visibility: "private",
PeeringConfig: &dns.ManagedZonePeeringConfig{TargetNetwork: nil},
})

provider.dryRun = dryRun

return provider
}

func newGoogleProvider(t *testing.T, domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, dryRun bool, records []*endpoint.Endpoint) *GoogleProvider {
func newGoogleProvider(t *testing.T, domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, dryRun bool, records []*endpoint.Endpoint, zonesErr, recordsErr error) *GoogleProvider {
provider := &GoogleProvider{
project: "zalando-external-dns-test",
dryRun: false,
domainFilter: domainFilter,
zoneIDFilter: zoneIDFilter,
resourceRecordSetsClient: &mockResourceRecordSetsClient{},
managedZonesClient: &mockManagedZonesClient{},
changesClient: &mockChangesClient{},
project: "zalando-external-dns-test",
dryRun: false,
domainFilter: domainFilter,
zoneIDFilter: zoneIDFilter,
resourceRecordSetsClient: &mockResourceRecordSetsClient{
recordsErr: recordsErr,
},
managedZonesClient: &mockManagedZonesClient{
zonesErr: zonesErr,
},
changesClient: &mockChangesClient{},
}

createZone(t, provider, &dns.ManagedZone{
Expand Down Expand Up @@ -754,10 +782,10 @@ func newGoogleProvider(t *testing.T, domainFilter endpoint.DomainFilter, zoneIDF
return provider
}

func createZone(t *testing.T, provider *GoogleProvider, zone *dns.ManagedZone) {
func createZone(t *testing.T, p *GoogleProvider, zone *dns.ManagedZone) {
zone.Description = "Testing zone for kubernetes.io/external-dns"

if _, err := provider.managedZonesClient.Create("zalando-external-dns-test", zone).Do(); err != nil {
if _, err := p.managedZonesClient.Create("zalando-external-dns-test", zone).Do(); err != nil {
if err, ok := err.(*googleapi.Error); !ok || err.Code != http.StatusConflict {
require.NoError(t, err)
}
Expand All @@ -770,32 +798,31 @@ func setupGoogleRecords(t *testing.T, provider *GoogleProvider, endpoints []*end
clearGoogleRecords(t, provider, "zone-3-ext-dns-test-2-gcp-zalan-do")

ctx := context.Background()
records, err := provider.Records(ctx)
require.NoError(t, err)
records, _ := provider.Records(ctx)

validateEndpoints(t, records, []*endpoint.Endpoint{})

require.NoError(t, provider.ApplyChanges(context.Background(), &plan.Changes{
Create: endpoints,
}))

records, err = provider.Records(ctx)
require.NoError(t, err)
records, _ = provider.Records(ctx)

validateEndpoints(t, records, endpoints)
}

func clearGoogleRecords(t *testing.T, provider *GoogleProvider, zone string) {
recordSets := []*dns.ResourceRecordSet{}
require.NoError(t, provider.resourceRecordSetsClient.List(provider.project, zone).Pages(context.Background(), func(resp *dns.ResourceRecordSetsListResponse) error {

provider.resourceRecordSetsClient.List(provider.project, zone).Pages(context.Background(), func(resp *dns.ResourceRecordSetsListResponse) error {
for _, r := range resp.Rrsets {
switch r.Type {
case endpoint.RecordTypeA, endpoint.RecordTypeCNAME:
recordSets = append(recordSets, r)
}
}
return nil
}))
})

if len(recordSets) != 0 {
_, err := provider.changesClient.Create(provider.project, zone, &dns.Change{
Expand Down