[release-4.20] OCPBUGS-79537: EgressFirewall: Use exponential backoff to refresh IP addresses for DNS names

go-controller/pkg/util/dns.go

@@ -16,8 +16,12 @@ import (
 )
 
 const (
-	// defaultTTL is used if an invalid or zero TTL is provided.
-	defaultTTL = 30 * time.Minute
+	// defaultMinTTL is the minimum TTL value that will be used for a domain name if an invalid or zero TTL is found
+	defaultMinTTL = 5 * time.Second
+	// defaultMaxTTL is the maximum TTL value that will be used for a domain name if an invalid or zero TTL is found
+	defaultMaxTTL = 2 * time.Minute
+	// maxRetryBeforeBackoff is the maximum number of times to retry a DNS lookup before exponential backoff starts
+	maxRetryBeforeBackoff = 10
 )
 
 type dnsValue struct {
@@ -27,6 +31,8 @@ type dnsValue struct {
 	ttl time.Duration
 	// Holds (last dns lookup time + ttl), tells when to refresh IPs next time
 	nextQueryTime time.Time
+	// Number of times the DNS lookup has been retried before backoff starts
+	retryCount int
 }
 
 type DNS struct {
@@ -105,11 +111,22 @@ func (d *DNS) updateOne(dns string) (bool, error) {
 		return false, fmt.Errorf("DNS value not found in dnsMap for domain: %q", dns)
 	}
 
-	ips, ttl, err := d.getIPsAndMinTTL(dns)
-	if err != nil {
-		res.nextQueryTime = time.Now().Add(defaultTTL)
-		d.dnsMap[dns] = res
-		return false, err
+	ips, ttl, retry, err := d.getIPsAndMinTTL(dns)
+	if retry {
+		// If the DNS lookup has been retried maxRetryCount times, use exponential backoff
+		// by doubling the previous TTL. The TTL is capped at defaultMaxTTL.
+		if res.retryCount >= maxRetryBeforeBackoff {
+			ttl = min(res.ttl*2, defaultMaxTTL)
+		} else {
+			// Increment the retry count
+			res.retryCount++
+		}
+		// If no valid IPs were found, use the previous IPs as fallback.
+		if len(ips) == 0 {
+			ips = res.ips
+		}
+	} else {
+		res.retryCount = 0
 	}
 
 	changed := false
@@ -120,10 +137,10 @@ func (d *DNS) updateOne(dns string) (bool, error) {
 	res.ttl = ttl
 	res.nextQueryTime = time.Now().Add(res.ttl)
 	d.dnsMap[dns] = res
-	return changed, nil
+	return changed, err
 }
 
-func (d *DNS) getIPsAndMinTTL(domain string) ([]net.IP, time.Duration, error) {
+func (d *DNS) getIPsAndMinTTL(domain string) ([]net.IP, time.Duration, bool, error) {
 	ips := []net.IP{}
 	ttlSet := false
 	var ttlSeconds uint32
@@ -197,19 +214,27 @@ func (d *DNS) getIPsAndMinTTL(domain string) ([]net.IP, time.Duration, error) {
 	}
 
 	if !ttlSet || (len(ips) == 0) {
-		return nil, defaultTTL, fmt.Errorf("IPv4 or IPv6 addr not found for domain: %q, nameservers: %v", domain, d.nameservers)
+		return nil, defaultMinTTL, true, fmt.Errorf("IPv4 or IPv6 addr not found for domain: %q, nameservers: %v", domain, d.nameservers)
 	}
 
+	ips = removeDuplicateIPs(ips)
+
 	ttl, err := time.ParseDuration(fmt.Sprintf("%ds", minTTL))
 	if err != nil {
-		utilruntime.HandleError(fmt.Errorf("invalid TTL value for domain: %q, err: %v, defaulting ttl=%s", domain, err, defaultTTL.String()))
-		ttl = defaultTTL
+		utilruntime.HandleError(fmt.Errorf("invalid TTL value for domain: %q, err: %v", domain, err))
+		return ips, defaultMinTTL, true, nil
 	}
 	if ttl == 0 {
-		ttl = defaultTTL
+		// If the TTL is 0, return the default minimum TTL. The retry is set to false as this
+		// is not an error scenario. TTL being 0 is a valid scenario for some DNS servers
+		// and it means that the IP addresses should be refreshed everytime whenever the DNS
+		// name is being used. From the point of view of OVN-Kubernetes, the IP addresses are
+		// refreshed every defaultMinTTL.
+		klog.V(5).Infof("TTL value is 0 for domain: %q, defaulting ttl=%s", domain, defaultMinTTL.String())
+		return ips, defaultMinTTL, false, nil
 	}
 
-	return removeDuplicateIPs(ips), ttl, nil
+	return ips, ttl, false, nil
 }
 
 func (d *DNS) GetNextQueryTime() (time.Time, string, bool) {