From 42648438a01c7f42d4b3d2846aa1d395ccef9f35 Mon Sep 17 00:00:00 2001 From: Ryan Breen Date: Wed, 29 Jul 2015 17:16:48 -0400 Subject: [PATCH 1/2] Check NXDOMAIN after filtering nodes Move the check for NXDOMAIN below the service health filter. --- command/agent/dns.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/command/agent/dns.go b/command/agent/dns.go index 15c343228d..7940102e3c 100644 --- a/command/agent/dns.go +++ b/command/agent/dns.go @@ -478,13 +478,6 @@ RPC: goto RPC } - // If we have no nodes, return not found! - if len(out.Nodes) == 0 { - d.addSOA(d.domain, resp) - resp.SetRcode(req, dns.RcodeNameError) - return - } - // Determine the TTL var ttl time.Duration if d.config.ServiceTTL != nil { @@ -498,6 +491,13 @@ RPC: // Filter out any service nodes due to health checks out.Nodes = d.filterServiceNodes(out.Nodes) + // If we have no nodes, return not found! + if len(out.Nodes) == 0 { + d.addSOA(d.domain, resp) + resp.SetRcode(req, dns.RcodeNameError) + return + } + // Perform a random shuffle shuffleServiceNodes(out.Nodes) From 0a7dc85076daadf23ca001655118d9e378e0bd25 Mon Sep 17 00:00:00 2001 From: Ryan Breen Date: Wed, 29 Jul 2015 18:21:16 -0400 Subject: [PATCH 2/2] Test for GH-1142. --- command/agent/dns_test.go | 88 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/command/agent/dns_test.go b/command/agent/dns_test.go index fe430b7158..ee656568fb 100644 --- a/command/agent/dns_test.go +++ b/command/agent/dns_test.go @@ -1098,6 +1098,94 @@ func TestDNS_ServiceLookup_FilterCritical(t *testing.T) { } } +func TestDNS_ServiceLookup_OnlyFailing(t *testing.T) { + dir, srv := makeDNSServer(t) + defer os.RemoveAll(dir) + defer srv.agent.Shutdown() + + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") + + // Register nodes + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "127.0.0.1", + Service: &structs.NodeService{ + Service: "db", + Tags: []string{"master"}, + Port: 12345, + }, + Check: &structs.HealthCheck{ + CheckID: "serf", + Name: "serf", + Status: structs.HealthCritical, + }, + } + + var out struct{} + if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + + args2 := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "bar", + Address: "127.0.0.2", + Service: &structs.NodeService{ + Service: "db", + Tags: []string{"master"}, + Port: 12345, + }, + Check: &structs.HealthCheck{ + CheckID: "serf", + Name: "serf", + Status: structs.HealthCritical, + }, + } + if err := srv.agent.RPC("Catalog.Register", args2, &out); err != nil { + t.Fatalf("err: %v", err) + } + + args3 := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "bar", + Address: "127.0.0.2", + Service: &structs.NodeService{ + Service: "db", + Tags: []string{"master"}, + Port: 12345, + }, + Check: &structs.HealthCheck{ + CheckID: "db", + Name: "db", + ServiceID: "db", + Status: structs.HealthCritical, + }, + } + if err := srv.agent.RPC("Catalog.Register", args3, &out); err != nil { + t.Fatalf("err: %v", err) + } + + m := new(dns.Msg) + m.SetQuestion("db.service.consul.", dns.TypeANY) + + c := new(dns.Client) + addr, _ := srv.agent.config.ClientListener("", srv.agent.config.Ports.DNS) + in, _, err := c.Exchange(m, addr.String()) + if err != nil { + t.Fatalf("err: %v", err) + } + + // All 3 are failing, so we should get 0 answers and an NXDOMAIN response + if len(in.Answer) != 0 { + t.Fatalf("Bad: %#v", in) + } + + if in.Rcode != dns.RcodeNameError { + t.Fatalf("Bad: %#v", in) + } +} + func TestDNS_ServiceLookup_OnlyPassing(t *testing.T) { dir, srv := makeDNSServerConfig(t, nil, func(c *DNSConfig) { c.OnlyPassing = true