Merge pull request #4682 from EWK20/fix/gcp_softerror

adds soft error for google provider

provider/google/google.go

@@ -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 {
@@ -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))
 		}
 	}
 
@@ -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)

provider/google/google_test.go

@@ -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 {
@@ -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
+	project            string
-	managedZone string
+	managedZone        string
+	recordsListSoftErr error
 }
 
 func (m *mockResourceRecordSetsListCall) Pages(ctx context.Context, f func(*dns.ResourceRecordSetsListResponse) error) error {
@@ -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 {
@@ -242,25 +256,12 @@ func TestGoogleZonesVisibilityFilterPrivatePeering(t *testing.T) {
 
 	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."},
-	})
-}
-
 func TestGoogleRecords(t *testing.T) {
 	originalEndpoints := []*endpoint.Endpoint{
 		endpoint.NewEndpointWithTTL("list-test.zone-1.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeA, endpoint.TTL(1), "1.2.3.4"),
@@ -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)
@@ -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.
@@ -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{
@@ -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"),
@@ -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"),
@@ -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
@@ -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,
@@ -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.",
@@ -703,27 +727,31 @@ func newGoogleProviderZoneOverlap(t *testing.T, domainFilter endpoint.DomainFilt
 	})
 
 	createZone(t, provider, &dns.ManagedZone{
-		Name:       "svc-local-peer",
+		Name:          "svc-local-peer",
-		DnsName:    "svc.local.",
+		DnsName:       "svc.local.",
-		Id:         10006,
+		Id:            10006,
-		Visibility: "private",
+		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",
+		project:      "zalando-external-dns-test",
-		dryRun:                   false,
+		dryRun:       false,
-		domainFilter:             domainFilter,
+		domainFilter: domainFilter,
-		zoneIDFilter:             zoneIDFilter,
+		zoneIDFilter: zoneIDFilter,
-		resourceRecordSetsClient: &mockResourceRecordSetsClient{},
+		resourceRecordSetsClient: &mockResourceRecordSetsClient{
-		managedZonesClient:       &mockManagedZonesClient{},
+			recordsErr: recordsErr,
-		changesClient:            &mockChangesClient{},
+		},
+		managedZonesClient: &mockManagedZonesClient{
+			zonesErr: zonesErr,
+		},
+		changesClient: &mockChangesClient{},
 	}
 
 	createZone(t, provider, &dns.ManagedZone{
@@ -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)
 		}
@@ -770,8 +798,7 @@ 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)
+	records, _ := provider.Records(ctx)
-	require.NoError(t, err)
 
 	validateEndpoints(t, records, []*endpoint.Endpoint{})
 
@@ -779,15 +806,15 @@ func setupGoogleRecords(t *testing.T, provider *GoogleProvider, endpoints []*end
 		Create: endpoints,
 	}))
 
-	records, err = provider.Records(ctx)
+	records, _ = provider.Records(ctx)
-	require.NoError(t, err)
 
 	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:
@@ -795,7 +822,7 @@ func clearGoogleRecords(t *testing.T, provider *GoogleProvider, zone string) {
 			}
 		}
 		return nil
-	}))
+	})
 
 	if len(recordSets) != 0 {
 		_, err := provider.changesClient.Create(provider.project, zone, &dns.Change{