From 65ada1a62dce2f3c5194989089b68351befac3f8 Mon Sep 17 00:00:00 2001 From: Matt Good Date: Tue, 31 Mar 2015 16:48:48 -0700 Subject: [PATCH 1/2] Use DNS server startup callbacks Simplify waiting for the DNS server to start with the newer "NotifyStartedFunc" callback. --- command/agent/dns.go | 50 ++++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/command/agent/dns.go b/command/agent/dns.go index d6c2517a8ec0..3631302a9cef 100644 --- a/command/agent/dns.go +++ b/command/agent/dns.go @@ -7,6 +7,7 @@ import ( "math/rand" "net" "strings" + "sync" "time" "github.com/hashicorp/consul/consul/structs" @@ -51,17 +52,21 @@ func NewDNSServer(agent *Agent, config *DNSConfig, logOutput io.Writer, domain s // Construct the DNS components mux := dns.NewServeMux() + var wg sync.WaitGroup + // Setup the servers server := &dns.Server{ - Addr: bind, - Net: "udp", - Handler: mux, - UDPSize: 65535, + Addr: bind, + Net: "udp", + Handler: mux, + UDPSize: 65535, + NotifyStartedFunc: wg.Done, } serverTCP := &dns.Server{ - Addr: bind, - Net: "tcp", - Handler: mux, + Addr: bind, + Net: "tcp", + Handler: mux, + NotifyStartedFunc: wg.Done, } // Create the server @@ -99,6 +104,8 @@ func NewDNSServer(agent *Agent, config *DNSConfig, logOutput io.Writer, domain s mux.HandleFunc(".", srv.handleRecurse) } + wg.Add(2) + // Async start the DNS Servers, handle a potential error errCh := make(chan error, 1) go func() { @@ -116,28 +123,11 @@ func NewDNSServer(agent *Agent, config *DNSConfig, logOutput io.Writer, domain s } }() - // Check the server is running, do a test lookup - checkCh := make(chan error, 1) + // Wait for NotifyStartedFunc callbacks indicating server has started + startCh := make(chan struct{}) go func() { - // This is jank, but we have no way to edge trigger on - // the start of our server, so we just wait and hope it is up. - time.Sleep(50 * time.Millisecond) - - m := new(dns.Msg) - m.SetQuestion(testQuery, dns.TypeANY) - - c := new(dns.Client) - in, _, err := c.Exchange(m, bind) - if err != nil { - checkCh <- fmt.Errorf("dns test query failed: %v", err) - return - } - - if len(in.Answer) == 0 { - checkCh <- fmt.Errorf("no response to test message") - return - } - close(checkCh) + wg.Wait() + close(startCh) }() // Wait for either the check, listen error, or timeout @@ -146,8 +136,8 @@ func NewDNSServer(agent *Agent, config *DNSConfig, logOutput io.Writer, domain s return srv, e case e := <-errChTCP: return srv, e - case e := <-checkCh: - return srv, e + case <-startCh: + return srv, nil case <-time.After(time.Second): return srv, fmt.Errorf("timeout setting up DNS server") } From 062e4f94c038bd9a064c3184dd216c6d75a09c7a Mon Sep 17 00:00:00 2001 From: Matt Good Date: Tue, 31 Mar 2015 16:50:44 -0700 Subject: [PATCH 2/2] Remove unnecessary DNS test entry By using the startup callbacks, the DNS test entry is not needed to check that the server is alive. --- command/agent/dns.go | 41 +-------------------------------------- command/agent/dns_test.go | 28 -------------------------- 2 files changed, 1 insertion(+), 68 deletions(-) diff --git a/command/agent/dns.go b/command/agent/dns.go index 3631302a9cef..83b1d4ef8cf5 100644 --- a/command/agent/dns.go +++ b/command/agent/dns.go @@ -15,8 +15,6 @@ import ( ) const ( - testQuery = "_test.consul." - consulDomain = "consul." maxServiceResponses = 3 // For UDP only maxRecurseRecords = 3 ) @@ -84,11 +82,8 @@ func NewDNSServer(agent *Agent, config *DNSConfig, logOutput io.Writer, domain s // Register mux handler, for reverse lookup mux.HandleFunc("arpa.", srv.handlePtr) - // Register mux handlers, always handle "consul." + // Register mux handlers mux.HandleFunc(domain, srv.handleQuery) - if domain != consulDomain { - mux.HandleFunc(consulDomain, srv.handleTest) - } if len(recursors) > 0 { validatedRecursors := make([]string, len(recursors)) @@ -224,12 +219,6 @@ func (d *DNSServer) handleQuery(resp dns.ResponseWriter, req *dns.Msg) { d.logger.Printf("[DEBUG] dns: request for %v (%v)", q, time.Now().Sub(s)) }(time.Now()) - // Check if this is potentially a test query - if q.Name == testQuery { - d.handleTest(resp, req) - return - } - // Switch to TCP if the client is network := "udp" if _, ok := resp.RemoteAddr().(*net.TCPAddr); ok { @@ -256,34 +245,6 @@ func (d *DNSServer) handleQuery(resp dns.ResponseWriter, req *dns.Msg) { } } -// handleTest is used to handle DNS queries in the ".consul." domain -func (d *DNSServer) handleTest(resp dns.ResponseWriter, req *dns.Msg) { - q := req.Question[0] - defer func(s time.Time) { - d.logger.Printf("[DEBUG] dns: request for %v (%v)", q, time.Now().Sub(s)) - }(time.Now()) - - if !(q.Qtype == dns.TypeANY || q.Qtype == dns.TypeTXT) { - return - } - if q.Name != testQuery { - return - } - - // Always respond with TXT "ok" - m := new(dns.Msg) - m.SetReply(req) - m.Authoritative = true - m.RecursionAvailable = true - header := dns.RR_Header{Name: q.Name, Rrtype: dns.TypeTXT, Class: dns.ClassINET, Ttl: 0} - txt := &dns.TXT{Hdr: header, Txt: []string{"ok"}} - m.Answer = append(m.Answer, txt) - d.addSOA(consulDomain, m) - if err := resp.WriteMsg(m); err != nil { - d.logger.Printf("[WARN] dns: failed to respond: %v", err) - } -} - // addSOA is used to add an SOA record to a message for the given domain func (d *DNSServer) addSOA(domain string, msg *dns.Msg) { soa := &dns.SOA{ diff --git a/command/agent/dns_test.go b/command/agent/dns_test.go index 257b6c41dfd5..440e72d9d520 100644 --- a/command/agent/dns_test.go +++ b/command/agent/dns_test.go @@ -39,34 +39,6 @@ func TestRecursorAddr(t *testing.T) { } } -func TestDNS_IsAlive(t *testing.T) { - dir, srv := makeDNSServer(t) - defer os.RemoveAll(dir) - defer srv.agent.Shutdown() - - m := new(dns.Msg) - m.SetQuestion("_test.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) - } - - if len(in.Answer) != 1 { - t.Fatalf("Bad: %#v", in) - } - - txt, ok := in.Answer[0].(*dns.TXT) - if !ok { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if txt.Txt[0] != "ok" { - t.Fatalf("Bad: %#v", in.Answer[0]) - } -} - func TestDNS_NodeLookup(t *testing.T) { dir, srv := makeDNSServer(t) defer os.RemoveAll(dir)