Fail over to another resolver if the current one returns SERVFAIL (#101)

* Fail over to another resolver if the current one returns SERVFAIL

* Handle drops in fail-rotate and fail-back groups

doc/configuration.md

@@ -365,7 +365,7 @@ type = "round-robin"
 
 ### Fail-Rotate group
 
-In a Fail-Rotate group, one of the upstream resolvers or modifiers is active and receives all queries. If the active resolver fails, the next becomes active and the request is retried. If the last resolver fails the first becomes the active again. There's no time-based automatic fail-back.
+In a Fail-Rotate group, one of the upstream resolvers or modifiers is active and receives all queries. If the active resolver fails, i.e. no response or returns SERVFAIL, the next becomes active and the request is retried. If the last resolver fails the first becomes the active again. There's no time-based automatic fail-back.
 
 #### Configuration
 
@@ -385,7 +385,7 @@ type = "fail-rotate"
 
 ### Fail-Back group
 
-Similar to [fail-rotate](#Fail-Rotate-group) but will attempt to fall back to the original order (prioritizing the first) if there are no failures for a minute.
+Similar to [fail-rotate](#Fail-Rotate-group) but will attempt to fall back to the original order (prioritizing the first) if there are no failures for a minute. Failure means either no response or it returns SERVFAIL.
 
 #### Configuration
 

failback.go

@@ -77,7 +77,7 @@ func (r *FailBack) Resolve(q *dns.Msg, ci ClientInfo) (*dns.Msg, error) {
 		log.WithField("resolver", resolver.String()).Debug("forwarding query to resolver")
 		r.metrics.route.Add(resolver.String(), 1)
 		a, err := resolver.Resolve(q, ci)
-		if err == nil { // Return immediately if successful
+		if err == nil && (a == nil || a.Rcode != dns.RcodeServerFailure) { // Return immediately if successful
 			return a, err
 		}
 		log.WithField("resolver", resolver.String()).WithError(err).Debug("resolver returned failure")

failback_test.go

@@ -47,3 +47,39 @@ func TestFailBack(t *testing.T) {
 	require.Equal(t, 5, r1.HitCount())
 	require.Equal(t, 1, r2.HitCount())
 }
+
+func TestFailBackSERVFAIL(t *testing.T) {
+	// Build 2 resolvers that count the number of invocations
+	var ci ClientInfo
+	opt := StaticResolverOptions{
+		RCode: dns.RcodeServerFailure,
+	}
+	r1, err := NewStaticResolver("test-static", opt)
+	require.NoError(t, err)
+
+	r2 := new(TestResolver)
+
+	g := NewFailBack("test-fb", FailBackOptions{ResetAfter: time.Second}, r1, r2)
+	q := new(dns.Msg)
+	q.SetQuestion("test.com.", dns.TypeA)
+
+	// Send the first query, the first resolver will return SERVFAIL and the request will go to the 2nd
+	_, err = g.Resolve(q, ci)
+	require.NoError(t, err)
+	require.Equal(t, 1, r2.HitCount())
+}
+
+func TestFailBackDrop(t *testing.T) {
+	var ci ClientInfo
+	r1 := NewDropResolver("test-drop")
+	r2 := new(TestResolver)
+
+	g := NewFailBack("test-fb", FailBackOptions{ResetAfter: time.Second}, r1, r2)
+	q := new(dns.Msg)
+	q.SetQuestion("test.com.", dns.TypeA)
+
+	// The query should be dropped, so no failover
+	_, err := g.Resolve(q, ci)
+	require.NoError(t, err)
+	require.Equal(t, 0, r2.HitCount())
+}

failrotate.go

@@ -41,7 +41,7 @@ func (r *FailRotate) Resolve(q *dns.Msg, ci ClientInfo) (*dns.Msg, error) {
 		log.WithField("resolver", resolver.String()).Trace("forwarding query to resolver")
 		r.metrics.route.Add(resolver.String(), 1)
 		a, err := resolver.Resolve(q, ci)
-		if err == nil { // Return immediately if successful
+		if err == nil && (a == nil || a.Rcode != dns.RcodeServerFailure) { // Return immediately if successful
 			return a, err
 		}
 		log.WithField("resolver", resolver.String()).WithError(err).Debug("resolver returned failure")

failrotate_test.go

@@ -61,3 +61,39 @@ func TestFailRotate(t *testing.T) {
 	require.Equal(t, 5, r1.HitCount())
 	require.Equal(t, 5, r2.HitCount())
 }
+
+func TestFailRotateSERVFAIL(t *testing.T) {
+	// Build 2 resolvers that count the number of invocations
+	var ci ClientInfo
+	opt := StaticResolverOptions{
+		RCode: dns.RcodeServerFailure,
+	}
+	r1, err := NewStaticResolver("test-static", opt)
+	require.NoError(t, err)
+
+	r2 := new(TestResolver)
+
+	g := NewFailRotate("test-rotate", r1, r2)
+	q := new(dns.Msg)
+	q.SetQuestion("test.com.", dns.TypeA)
+
+	// Send the first query, the first resolver will return SERVFAIL and the request will go to the 2nd
+	_, err = g.Resolve(q, ci)
+	require.NoError(t, err)
+	require.Equal(t, 1, r2.HitCount())
+}
+
+func TestFailRotateDrop(t *testing.T) {
+	var ci ClientInfo
+	r1 := NewDropResolver("test-drop")
+	r2 := new(TestResolver)
+
+	g := NewFailRotate("test-rotate", r1, r2)
+	q := new(dns.Msg)
+	q.SetQuestion("test.com.", dns.TypeA)
+
+	// The query should be dropped, so no failover
+	_, err := g.Resolve(q, ci)
+	require.NoError(t, err)
+	require.Equal(t, 0, r2.HitCount())
+}