Repository: caddyserver/caddy (language: go)

Issue:
Support falling back to cached upstreams IPs when DNS lookups fail
We'd like to request support for temporarily falling back to cached upstream IP addresses in the event an upstream DNS query fails. We have a situation where an upstream service cluster can temporarily report as unhealthy, even though the vast majority of the upstream node IPs are continuing to function, and we'd like to not immediately fail all requests.

This situation looks like:
* We have a dynamic upstream service which is doing load balancing by an HTTP header that resolves to multiple IPs
* This cluster becomes temporarily unavailable and SRV lookups return an NXDOMAIN from our service mesh
* Caddy fails the DNS lookup and returns a 502

We'd like to be able to tell Caddy to use the cached IPs for a specified period of time, this would help insulate flakey services.

Let me know if I can provide more information, thank you! 


Discussion / hints:
Ahh, I didn't consider the DNS lookup to be a point of failure... :upside_down_face: (Insert "It's always DNS" meme here)

> We'd like to be able to tell Caddy to use the cached IPs for a specified period of time, this would help insulate flakey services.

To be clear, this already happens (that's the "refresh" config parameter, default is 1 minute) -- but currently, after that period of time, it will try to get new ones, and if it can't it will error.

The simplest fix I can think of is to log the error and return the cached IPs. Would that work for you?

This may potentially perform a lot of failing lookups though (although only ever 1 at a time) resulting in slight service degradation (because higher latency).

So I guess that's why you're asking for a way to use the cached IPs "for a specified period of time" -- so maybe we add a new config parameter like a `grace_period` that specifies a length of time between lookup attempts. (Or a better name for this param, if you can think of one...)

Am I on the right track? I'll try to push a branch soon as a starting point.

Fix patch:
diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go
index 2d21a5ca4ed..46e45c64652 100644
--- a/modules/caddyhttp/reverseproxy/upstreams.go
+++ b/modules/caddyhttp/reverseproxy/upstreams.go
@@ -49,6 +49,13 @@ type SRVUpstreams struct {
 	// Results are cached between lookups. Default: 1m
 	Refresh caddy.Duration `json:"refresh,omitempty"`
 
+	// If > 0 and there is an error with the lookup,
+	// continue to use the cached results for up to
+	// this long before trying again, (even though they
+	// are stale) instead of returning an error to the
+	// client. Default: 0s.
+	GracePeriod caddy.Duration `json:"grace_period,omitempty"`
+
 	// Configures the DNS resolver used to resolve the
 	// SRV address to SRV records.
 	Resolver *UpstreamResolver `json:"resolver,omitempty"`
@@ -140,6 +147,12 @@ func (su SRVUpstreams) GetUpstreams(r *http.Request) ([]*Upstream, error) {
 		// out and an error will be returned alongside the remaining results, if any." Thus, we
 		// only return an error if no records were also returned.
 		if len(records) == 0 {
+			if su.GracePeriod > 0 {
+				su.logger.Error("SRV lookup failed; using previously cached", zap.Error(err))
+				cached.freshness = time.Now().Add(time.Duration(su.GracePeriod) - time.Duration(su.Refresh))
+				srvs[suAddr] = cached
+				return allNew(cached.upstreams), nil
+			}
 			return nil, err
 		}
 		su.logger.Warn("SRV records filtered", zap.Error(err))


Test patch: