diff --git a/command/agent/dns.go b/command/agent/dns.go index a5816b3d6..83bc980b4 100644 --- a/command/agent/dns.go +++ b/command/agent/dns.go @@ -587,13 +587,10 @@ RPC: goto RPC } - // TODO (slackpad) Do we want to apply the DNS server's per-service TTL - // configs if the query's TTL is not set? That seems like it adds a lot - // of complexity (we'd have to plumb the service name back with the query - // results to do this), but is it what people would expect? - // Determine the TTL. The parse should never fail since we vet it when - // the query is created, but we check anyway. + // the query is created, but we check anyway. If the query didn't + // specify a TTL then we will try to use the agent's service-specific + // TTL configs. var ttl time.Duration if out.DNS.TTL != "" { var err error @@ -601,6 +598,12 @@ RPC: if err != nil { d.logger.Printf("[WARN] dns: Failed to parse TTL '%s' for prepared query '%s', ignoring", out.DNS.TTL, query) } + } else if d.config.ServiceTTL != nil { + var ok bool + ttl, ok = d.config.ServiceTTL[out.Service] + if !ok { + ttl = d.config.ServiceTTL["*"] + } } // If we have no nodes, return not found! diff --git a/command/agent/dns_test.go b/command/agent/dns_test.go index 41a2356a8..b27befcea 100644 --- a/command/agent/dns_test.go +++ b/command/agent/dns_test.go @@ -2087,7 +2087,6 @@ func TestDNS_ServiceLookup_TTL(t *testing.T) { } c.AllowStale = true c.MaxStale = time.Second - } dir, srv := makeDNSServerConfig(t, nil, confFn) defer os.RemoveAll(dir) @@ -2184,7 +2183,15 @@ func TestDNS_ServiceLookup_TTL(t *testing.T) { } func TestDNS_PreparedQuery_TTL(t *testing.T) { - dir, srv := makeDNSServer(t) + confFn := func(c *DNSConfig) { + c.ServiceTTL = map[string]time.Duration{ + "db": 10 * time.Second, + "*": 5 * time.Second, + } + c.AllowStale = true + c.MaxStale = time.Second + } + dir, srv := makeDNSServerConfig(t, nil, confFn) defer os.RemoveAll(dir) defer srv.agent.Shutdown() @@ -2207,20 +2214,34 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) { if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) } + + args = &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "127.0.0.1", + Service: &structs.NodeService{ + Service: "api", + Port: 2222, + }, + } + if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } } - // Register prepared queries with and without a TTL set. + // Register prepared queries with and without a TTL set for "db", as + // well as one for "api". { args := &structs.PreparedQueryRequest{ Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ - Name: "ttl", + Name: "db-ttl", Service: structs.ServiceQuery{ Service: "db", }, DNS: structs.QueryDNSOptions{ - TTL: "10s", + TTL: "18s", }, }, } @@ -2234,7 +2255,7 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) { Datacenter: "dc1", Op: structs.PreparedQueryCreate, Query: &structs.PreparedQuery{ - Name: "nottl", + Name: "db-nottl", Service: structs.ServiceQuery{ Service: "db", }, @@ -2244,11 +2265,27 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) { if err := srv.agent.RPC("PreparedQuery.Apply", args, &id); err != nil { t.Fatalf("err: %v", err) } + + args = &structs.PreparedQueryRequest{ + Datacenter: "dc1", + Op: structs.PreparedQueryCreate, + Query: &structs.PreparedQuery{ + Name: "api-nottl", + Service: structs.ServiceQuery{ + Service: "api", + }, + }, + } + + if err := srv.agent.RPC("PreparedQuery.Apply", args, &id); err != nil { + t.Fatalf("err: %v", err) + } } - // Make sure the TTL is set when requested. + // Make sure the TTL is set when requested, and overrides the agent- + // specific config since the query takes precedence. m := new(dns.Msg) - m.SetQuestion("ttl.query.consul.", dns.TypeSRV) + m.SetQuestion("db-ttl.query.consul.", dns.TypeSRV) c := new(dns.Client) addr, _ := srv.agent.config.ClientListener("", srv.agent.config.Ports.DNS) @@ -2265,7 +2302,7 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) { if !ok { t.Fatalf("Bad: %#v", in.Answer[0]) } - if srvRec.Hdr.Ttl != 10 { + if srvRec.Hdr.Ttl != 18 { t.Fatalf("Bad: %#v", in.Answer[0]) } @@ -2273,13 +2310,14 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) { if !ok { t.Fatalf("Bad: %#v", in.Extra[0]) } - if aRec.Hdr.Ttl != 10 { + if aRec.Hdr.Ttl != 18 { t.Fatalf("Bad: %#v", in.Extra[0]) } - // And the TTL should default to 0 otherwise. + // And the TTL should take the service-specific value from the agent's + // config otherwise. m = new(dns.Msg) - m.SetQuestion("nottl.query.consul.", dns.TypeSRV) + m.SetQuestion("db-nottl.query.consul.", dns.TypeSRV) in, _, err = c.Exchange(m, addr.String()) if err != nil { t.Fatalf("err: %v", err) @@ -2293,7 +2331,7 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) { if !ok { t.Fatalf("Bad: %#v", in.Answer[0]) } - if srvRec.Hdr.Ttl != 0 { + if srvRec.Hdr.Ttl != 10 { t.Fatalf("Bad: %#v", in.Answer[0]) } @@ -2301,7 +2339,36 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) { if !ok { t.Fatalf("Bad: %#v", in.Extra[0]) } - if aRec.Hdr.Ttl != 0 { + if aRec.Hdr.Ttl != 10 { + t.Fatalf("Bad: %#v", in.Extra[0]) + } + + // If there's no query TTL and no service-specific value then the wild + // card value should be used. + m = new(dns.Msg) + m.SetQuestion("api-nottl.query.consul.", dns.TypeSRV) + 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) + } + + srvRec, ok = in.Answer[0].(*dns.SRV) + if !ok { + t.Fatalf("Bad: %#v", in.Answer[0]) + } + if srvRec.Hdr.Ttl != 5 { + t.Fatalf("Bad: %#v", in.Answer[0]) + } + + aRec, ok = in.Extra[0].(*dns.A) + if !ok { + t.Fatalf("Bad: %#v", in.Extra[0]) + } + if aRec.Hdr.Ttl != 5 { t.Fatalf("Bad: %#v", in.Extra[0]) } } diff --git a/consul/prepared_query_endpoint.go b/consul/prepared_query_endpoint.go index 18a647691..77b8e68bb 100644 --- a/consul/prepared_query_endpoint.go +++ b/consul/prepared_query_endpoint.go @@ -386,6 +386,7 @@ func (p *PreparedQuery) execute(query *structs.PreparedQuery, } // Capture the nodes and pass the DNS information through to the reply. + reply.Service = query.Service.Service reply.Nodes = nodes reply.DNS = query.DNS diff --git a/consul/prepared_query_endpoint_test.go b/consul/prepared_query_endpoint_test.go index dfc2a5193..adb7dceea 100644 --- a/consul/prepared_query_endpoint_test.go +++ b/consul/prepared_query_endpoint_test.go @@ -1077,7 +1077,7 @@ func TestPreparedQuery_Execute(t *testing.T) { var reply structs.PreparedQueryExecuteResponse err := msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply) - if err == nil || err.Error() != ErrQueryNotFound.Error() { + if err == nil || err.Error() != ErrQueryNotFound.Error() { t.Fatalf("bad: %v", err) } @@ -1100,6 +1100,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 10 || reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) @@ -1121,6 +1122,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 3 || reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) @@ -1162,6 +1164,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 10 || reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) @@ -1186,6 +1189,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 10 || reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) @@ -1244,6 +1248,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 9 || reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) @@ -1270,6 +1275,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 10 || reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) @@ -1297,6 +1303,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 9 || reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) @@ -1331,6 +1338,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 8 || reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) @@ -1359,6 +1367,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 0 || reply.Datacenter != "dc1" || reply.Failovers != 0 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) @@ -1385,6 +1394,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 9 || reply.Datacenter != "dc2" || reply.Failovers != 1 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) @@ -1412,6 +1422,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 3 || reply.Datacenter != "dc2" || reply.Failovers != 1 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) @@ -1438,6 +1449,7 @@ func TestPreparedQuery_Execute(t *testing.T) { if len(reply.Nodes) != 9 || reply.Datacenter != "dc2" || reply.Failovers != 1 || + reply.Service != query.Query.Service.Service || !reflect.DeepEqual(reply.DNS, query.Query.DNS) || !reply.QueryMeta.KnownLeader { t.Fatalf("bad: %v", reply) diff --git a/consul/structs/prepared_query.go b/consul/structs/prepared_query.go index e737323fd..a38adb52c 100644 --- a/consul/structs/prepared_query.go +++ b/consul/structs/prepared_query.go @@ -186,6 +186,9 @@ func (q *PreparedQueryExecuteRemoteRequest) RequestDatacenter() string { // PreparedQueryExecuteResponse has the results of executing a query. type PreparedQueryExecuteResponse struct { + // Service is the service that was queried. + Service string + // Nodes has the nodes that were output by the query. Nodes CheckServiceNodes