Merge pull request #257 from gardener/fix-remote-data-race

Don't cleanup entries belonging to provider of equivalent zone

pkg/dns/provider/changemodel.go

@@ -74,6 +74,9 @@ func (this *ChangeGroup) cleanup(logger logger.LogContext, model *ChangeModel) b
 		_, ok := model.applied[s.Name]
 		if !ok {
 			if s.IsOwnedBy(model.ownership) {
+				if model.ExistsInEquivalentZone(s.Name) {
+					continue
+				}
 				if e := model.IsStale(ZonedDNSName{ZoneID: model.ZoneId(), DNSName: s.Name}); e != nil {
 					if e.IsDeleting() {
 						model.failedDNSNames.Add(s.Name) // preventing deletion of stale entry
@@ -183,6 +186,10 @@ func (this *ChangeModel) IsStale(dns ZonedDNSName) *Entry {
 	return this.context.stale[dns]
 }
 
+func (this *ChangeModel) ExistsInEquivalentZone(dnsName string) bool {
+	return this.context.equivEntries != nil && this.context.equivEntries.Contains(dnsName)
+}
+
 func (this *ChangeModel) getProviderView(p DNSProvider) *ChangeGroup {
 	v := this.providergroups[p.AccountHash()]
 	if v == nil {

pkg/dns/provider/interface.go

@@ -250,6 +250,7 @@ type DNSProvider interface {
 
 	GetZones() DNSHostedZones
 	IncludesZone(zoneID dns.ZoneID) bool
+	HasEquivalentZone(zoneID dns.ZoneID) bool
 
 	GetZoneState(zone DNSHostedZone) (DNSZoneState, error)
 	ExecuteRequests(logger logger.LogContext, zone DNSHostedZone, state DNSZoneState, requests []*ChangeRequest) error

pkg/dns/provider/provider.go

@@ -600,6 +600,14 @@ func (this *dnsProviderVersion) IncludesZone(zoneID dns.ZoneID) bool {
 	return this.TypeCode() == zoneID.ProviderType && this.included_zones != nil && this.included_zones.Contains(zoneID.ID)
 }
 
+// HasEquivalentZone returns true for same provider specific zone id but different provider type and
+// one zoneid has provider type "remote".
+func (this *dnsProviderVersion) HasEquivalentZone(zoneID dns.ZoneID) bool {
+	return this.TypeCode() != zoneID.ProviderType &&
+		(this.TypeCode() == "remote" || zoneID.ProviderType == "remote") &&
+		this.included_zones != nil && this.included_zones.Contains(zoneID.ID)
+}
+
 func (this *dnsProviderVersion) GetDedicatedDNSAccess() DedicatedDNSAccess {
 	h, _ := this.account.handler.(DedicatedDNSAccess)
 	return h

pkg/dns/provider/state.go

@@ -51,16 +51,19 @@ func (z ZonedDNSName) String() string {
 
 type DNSNames map[ZonedDNSName]*Entry
 
+type DNSNameSet = utils.StringSet
+
 type zoneReconciliation struct {
-	zone      *dnsHostedZone
-	providers DNSProviders
-	entries   Entries
-	ownership dns.Ownership
-	stale     DNSNames
-	dedicated bool
-	deleting  bool
-	fhandler  FinalizerHandler
-	dnsTicker *Ticker
+	zone         *dnsHostedZone
+	providers    DNSProviders
+	entries      Entries
+	equivEntries DNSNameSet
+	ownership    dns.Ownership
+	stale        DNSNames
+	dedicated    bool
+	deleting     bool
+	fhandler     FinalizerHandler
+	dnsTicker    *Ticker
 }
 
 type setup struct {
@@ -456,18 +459,20 @@ func (this *state) GetEntriesForZone(logger logger.LogContext, zoneid dns.ZoneID
 	entries := Entries{}
 	zone := this.zones[zoneid]
 	if zone != nil {
-		return this.addEntriesForZone(logger, entries, DNSNames{}, zone)
+		entries, _, stale, deleting := this.addEntriesForZone(logger, entries, DNSNames{}, zone)
+		return entries, stale, deleting
 	}
 	return entries, nil, false
 }
 
-func (this *state) addEntriesForZone(logger logger.LogContext, entries Entries, stale DNSNames, zone DNSHostedZone) (Entries, DNSNames, bool) {
+func (this *state) addEntriesForZone(logger logger.LogContext, entries Entries, stale DNSNames, zone DNSHostedZone) (Entries, DNSNameSet, DNSNames, bool) {
 	if entries == nil {
 		entries = Entries{}
 	}
 	if stale == nil {
 		stale = DNSNames{}
 	}
+	equivEntries := DNSNameSet{}
 	deleting := true // TODO check
 	domain := zone.Domain()
 	// fallback if no forwarded domains are reported
@@ -477,7 +482,6 @@ func (this *state) addEntriesForZone(logger logger.LogContext, entries Entries,
 			nested.Add(z.Domain())
 		}
 	}
-loop:
 	for dns, e := range this.dnsnames {
 		if e.Kind() == api.DNSLockKind {
 			continue
@@ -498,15 +502,15 @@ loop:
 					stale[e.ZonedDNSName()] = e
 					continue
 				}
-			} else if provider == nil || !provider.IncludesZone(zone.Id()) {
+			} else if provider == nil {
 				continue
-			}
-			if dns.ZoneID == zone.Id() && zone.Match(dns.DNSName) > 0 {
-				for excl := range nested { // fallback if no forwarded domains are reported
-					if dnsutils.Match(dns.DNSName, excl) {
-						continue loop
-					}
+			} else if !provider.IncludesZone(zone.Id()) {
+				if provider.HasEquivalentZone(zone.Id()) && e.IsActive() && !forwarded(nested, dns.DNSName) {
+					equivEntries.Add(dns.DNSName)
 				}
+				continue
+			}
+			if dns.ZoneID == zone.Id() && zone.Match(dns.DNSName) > 0 && !forwarded(nested, dns.DNSName) {
 				if e.IsActive() {
 					deleting = deleting || e.IsDeleting()
 					entries[e.ObjectName()] = e
@@ -525,7 +529,7 @@ loop:
 			}
 		}
 	}
-	return entries, stale, deleting
+	return entries, equivEntries, stale, deleting
 }
 
 func (this *state) GetZoneForEntry(e *Entry) *dns.ZoneID {
@@ -716,3 +720,12 @@ func (this *state) ObjectUpdated(key resources.ClusterObjectKey) {
 	this.context.Infof("requeue %s because of change in annotation resource", key)
 	this.context.EnqueueKey(key)
 }
+
+func forwarded(nested utils.StringSet, dnsname string) bool {
+	for excl := range nested {
+		if dnsutils.Match(dnsname, excl) {
+			return true
+		}
+	}
+	return false
+}

pkg/dns/provider/state_zone.go

@@ -75,7 +75,7 @@ func (this *state) GetZoneReconcilation(logger logger.LogContext, zoneid dns.Zon
 	if now.Before(next) {
 		return next.Sub(now), hasProviders, req
 	}
-	req.entries, req.stale, req.deleting = this.addEntriesForZone(logger, nil, nil, zone)
+	req.entries, req.equivEntries, req.stale, req.deleting = this.addEntriesForZone(logger, nil, nil, zone)
 	req.providers = this.getProvidersForZone(zoneid)
 	req.dnsTicker = this.dnsTicker
 	return 0, hasProviders, req