From 17b6a18c379abae09441987c72b73609d478b7ac Mon Sep 17 00:00:00 2001 From: Pierre Delagrave Date: Tue, 23 Aug 2016 16:59:38 -0400 Subject: [PATCH] Added a configurable timeout to the client making DNS queries to downstream name servers --- command/agent/config.go | 23 ++++++++++++++++++-- command/agent/config_test.go | 5 ++++- command/agent/dns.go | 4 ++-- command/agent/dns_test.go | 41 ++++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 5 deletions(-) diff --git a/command/agent/config.go b/command/agent/config.go index 3675e100f..151f7ca61 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -109,6 +109,13 @@ type DNSConfig struct { // compressed. In Consul 0.7 this was turned on by default and this // config was added as an opt-out. DisableCompression bool `mapstructure:"disable_compression"` + + // InternalClientTimeout specifies the timeout in seconds + // for Consul's internal dns client. This value is used for the + // connection, read and write timeout. + // Default: 2s + InternalClientTimeout time.Duration `mapstructure:"-"` + InternalClientTimeoutRaw string `mapstructure:"internal_client_timeout" json:"-"` } // Telemetry is the telemetry configuration for the server @@ -634,8 +641,9 @@ func DefaultConfig() *Config { Server: 8300, }, DNSConfig: DNSConfig{ - UDPAnswerLimit: 3, - MaxStale: 5 * time.Second, + UDPAnswerLimit: 3, + MaxStale: 5 * time.Second, + InternalClientTimeout: 2 * time.Second, }, Telemetry: Telemetry{ StatsitePrefix: "consul", @@ -828,6 +836,14 @@ func DecodeConfig(r io.Reader) (*Config, error) { result.DNSConfig.MaxStale = dur } + if raw := result.DNSConfig.InternalClientTimeoutRaw; raw != "" { + dur, err := time.ParseDuration(raw) + if err != nil { + return nil, fmt.Errorf("InternalClientTimeout invalid: %v", err) + } + result.DNSConfig.InternalClientTimeout = dur + } + if len(result.DNSConfig.ServiceTTLRaw) != 0 { if result.DNSConfig.ServiceTTL == nil { result.DNSConfig.ServiceTTL = make(map[string]time.Duration) @@ -1333,6 +1349,9 @@ func MergeConfig(a, b *Config) *Config { if b.DNSConfig.DisableCompression { result.DNSConfig.DisableCompression = true } + if b.DNSConfig.InternalClientTimeout != 0 { + result.DNSConfig.InternalClientTimeout = b.DNSConfig.InternalClientTimeout + } if b.CheckUpdateIntervalRaw != "" || b.CheckUpdateInterval != 0 { result.CheckUpdateInterval = b.CheckUpdateInterval } diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 3b4de35ed..a7fb8bf22 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -544,7 +544,7 @@ func TestDecodeConfig(t *testing.T) { } // DNS node ttl, max stale - input = `{"dns_config": {"allow_stale": true, "enable_truncate": false, "max_stale": "15s", "node_ttl": "5s", "only_passing": true, "udp_answer_limit": 6}}` + input = `{"dns_config": {"allow_stale": true, "enable_truncate": false, "max_stale": "15s", "node_ttl": "5s", "only_passing": true, "udp_answer_limit": 6, "internal_client_timeout": "7s"}}` config, err = DecodeConfig(bytes.NewReader([]byte(input))) if err != nil { t.Fatalf("err: %s", err) @@ -568,6 +568,9 @@ func TestDecodeConfig(t *testing.T) { if config.DNSConfig.UDPAnswerLimit != 6 { t.Fatalf("bad: %#v", config) } + if config.DNSConfig.InternalClientTimeout != 7*time.Second { + t.Fatalf("bad: %#v", config) + } // DNS service ttl input = `{"dns_config": {"service_ttl": {"*": "1s", "api": "10s", "web": "30s"}}}` diff --git a/command/agent/dns.go b/command/agent/dns.go index e75c5a791..d062877e7 100644 --- a/command/agent/dns.go +++ b/command/agent/dns.go @@ -842,7 +842,7 @@ func (d *DNSServer) handleRecurse(resp dns.ResponseWriter, req *dns.Msg) { } // Recursively resolve - c := &dns.Client{Net: network} + c := &dns.Client{Net: network, Timeout: d.config.InternalClientTimeout} var r *dns.Msg var rtt time.Duration var err error @@ -887,7 +887,7 @@ func (d *DNSServer) resolveCNAME(name string) []dns.RR { m.SetQuestion(name, dns.TypeA) // Make a DNS lookup request - c := &dns.Client{Net: "udp"} + c := &dns.Client{Net: "udp", Timeout: d.config.InternalClientTimeout} var r *dns.Msg var rtt time.Duration var err error diff --git a/command/agent/dns_test.go b/command/agent/dns_test.go index a90905073..acda488dc 100644 --- a/command/agent/dns_test.go +++ b/command/agent/dns_test.go @@ -1400,6 +1400,47 @@ func TestDNS_Recurse(t *testing.T) { } } +func TestDNS_InternalClientTimeout(t *testing.T) { + serverClientTimeout := 3 * time.Second + testClientTimeout := serverClientTimeout + 5*time.Second + + dir, srv := makeDNSServerConfig(t, func(c *Config) { + c.DNSRecursor = "127.0.0.77" // must be an unreachable host + }, func(c *DNSConfig) { + c.InternalClientTimeout = serverClientTimeout + }) + defer os.RemoveAll(dir) + defer srv.agent.Shutdown() + + m := new(dns.Msg) + m.SetQuestion("apple.com.", dns.TypeANY) + + // This client calling the server under test must have a longer timeout than the one we set internally + c := &dns.Client{Timeout: testClientTimeout} + addr, _ := srv.agent.config.ClientListener("", srv.agent.config.Ports.DNS) + + start := time.Now() + in, _, err := c.Exchange(m, addr.String()) + + duration := time.Now().Sub(start) + + if err != nil { + t.Fatalf("err: %v", err) + } + + if len(in.Answer) != 0 { + t.Fatalf("Bad: %#v", in) + } + if in.Rcode != dns.RcodeServerFailure { + t.Fatalf("Bad: %#v", in) + } + + if duration < serverClientTimeout { + t.Fatalf("Expected the call to return after at least %f seconds but lastest only %f", serverClientTimeout.Seconds(), duration.Seconds()) + } + +} + func TestDNS_ServiceLookup_FilterCritical(t *testing.T) { dir, srv := makeDNSServer(t) defer os.RemoveAll(dir)