Skip to content

Commit

Permalink
Add flag to ensure new behaviour only impacts ruler AM resolution.
Browse files Browse the repository at this point in the history
Signed-off-by: Kevin Hellemun <[email protected]>
  • Loading branch information
OGKevin committed Sep 30, 2020
1 parent 84824e6 commit e30a981
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 10 deletions.
2 changes: 2 additions & 0 deletions cmd/thanos/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ func runQuery(
logger,
extprom.WrapRegistererWithPrefix("thanos_querier_store_apis_", reg),
dns.ResolverType(dnsSDResolver),
true,
)

for _, store := range strictStores {
Expand All @@ -277,6 +278,7 @@ func runQuery(
logger,
extprom.WrapRegistererWithPrefix("thanos_querier_rule_apis_", reg),
dns.ResolverType(dnsSDResolver),
true,
)

var (
Expand Down
2 changes: 2 additions & 0 deletions cmd/thanos/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ func runRule(
logger,
extprom.WrapRegistererWithPrefix("thanos_ruler_query_apis_", reg),
dns.ResolverType(dnsSDResolver),
true,
)
var queryClients []*http_util.Client
for _, cfg := range queryCfg {
Expand Down Expand Up @@ -401,6 +402,7 @@ func runRule(
logger,
extprom.WrapRegistererWithPrefix("thanos_ruler_alertmanagers_", reg),
dns.ResolverType(dnsSDResolver),
false,
)
var alertmgrs []*alert.Alertmanager
for _, cfg := range alertingCfg.Alertmanagers {
Expand Down
1 change: 1 addition & 0 deletions pkg/cacheutil/memcached_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ func newMemcachedClient(
logger,
extprom.WrapRegistererWithPrefix("thanos_memcached_", reg),
dns.GolangResolverType,
true,
)

c := &memcachedClient{
Expand Down
5 changes: 2 additions & 3 deletions pkg/discovery/dns/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ type ResolverType string
const (
GolangResolverType ResolverType = "golang"
MiekgdnsResolverType ResolverType = "miekgdns"
MockdnsResolverType ResolverType = "mockdns"
)

func (t ResolverType) ToResolver(logger log.Logger) ipLookupResolver {
Expand All @@ -56,9 +55,9 @@ func (t ResolverType) ToResolver(logger log.Logger) ipLookupResolver {

// NewProvider returns a new empty provider with a given resolver type.
// If empty resolver type is net.DefaultResolver.
func NewProvider(logger log.Logger, reg prometheus.Registerer, resolverType ResolverType) *Provider {
func NewProvider(logger log.Logger, reg prometheus.Registerer, resolverType ResolverType, returnErrOnNotFound bool) *Provider {
p := &Provider{
resolver: NewResolver(resolverType.ToResolver(logger), resolverType),
resolver: NewResolver(resolverType.ToResolver(logger), logger, returnErrOnNotFound),
resolved: make(map[string][]string),
logger: logger,
resolverAddrs: extprom.NewTxGaugeVec(reg, prometheus.GaugeOpts{
Expand Down
2 changes: 1 addition & 1 deletion pkg/discovery/dns/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestProvider(t *testing.T) {
"127.0.0.5:19095",
}

prv := NewProvider(log.NewNopLogger(), nil, "")
prv := NewProvider(log.NewNopLogger(), nil, "", true)
prv.resolver = &mockResolver{
res: map[string][]string{
"a": ips[:2],
Expand Down
26 changes: 21 additions & 5 deletions pkg/discovery/dns/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import (
"strconv"
"strings"

"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"

"github.com/pkg/errors"
)

Expand Down Expand Up @@ -37,13 +40,16 @@ type ipLookupResolver interface {
}

type dnsSD struct {
resolver ipLookupResolver
resolverType ResolverType
resolver ipLookupResolver
logger log.Logger
// https://github.com/thanos-io/thanos/issues/3186
// This flag is used to prevent components from crashing if hosts are not found.
returnErrOnNotFound bool
}

// NewResolver creates a resolver with given underlying resolver.
func NewResolver(resolver ipLookupResolver, resolverType ResolverType) Resolver {
return &dnsSD{resolver: resolver, resolverType: resolverType}
func NewResolver(resolver ipLookupResolver, logger log.Logger, returnErrOnNotFound bool) Resolver {
return &dnsSD{resolver: resolver, logger: logger, returnErrOnNotFound: returnErrOnNotFound}
}

func (s *dnsSD) Resolve(ctx context.Context, name string, qtype QType) ([]string, error) {
Expand Down Expand Up @@ -73,9 +79,13 @@ func (s *dnsSD) Resolve(ctx context.Context, name string, qtype QType) ([]string
ips, err := s.resolver.LookupIPAddr(ctx, host)
if err != nil {
dnsErr, ok := err.(*net.DNSError)
if !(s.resolverType == GolangResolverType && ok && dnsErr.IsNotFound) {
// https://github.com/thanos-io/thanos/issues/3186
// Default DNS resolver can make thanos components crash if DSN resolutions results in EAI_NONAME.
// the flag returnErrOnNotFound can be used to prevent such crash.
if !(!s.returnErrOnNotFound && ok && dnsErr.IsNotFound) {
return nil, errors.Wrapf(err, "lookup IP addresses %q", host)
}
level.Error(s.logger).Log("msg", "failed to lookup IP addresses", "host", host, "err", err)
}
for _, ip := range ips {
res = append(res, appendScheme(scheme, net.JoinHostPort(ip.String(), port)))
Expand Down Expand Up @@ -110,6 +120,12 @@ func (s *dnsSD) Resolve(ctx context.Context, name string, qtype QType) ([]string
return nil, errors.Errorf("invalid lookup scheme %q", qtype)
}

// https://github.com/thanos-io/thanos/issues/3186
// This happens when miekg is used as resolver. When the host cannot be found, nothing is returned.
if res == nil && err == nil {
level.Warn(s.logger).Log("msg", "IP address lookup yielded no results nor errors", "host", host)
}

return res, nil
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/discovery/dns/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"sort"
"testing"

"github.com/go-kit/kit/log"

"github.com/pkg/errors"
"github.com/thanos-io/thanos/pkg/testutil"
)
Expand Down Expand Up @@ -184,7 +186,7 @@ func TestDnsSD_Resolve(t *testing.T) {

func testDnsSd(t *testing.T, tt DNSSDTest) {
ctx := context.TODO()
dnsSD := dnsSD{tt.resolver, MockdnsResolverType}
dnsSD := dnsSD{tt.resolver, log.NewNopLogger(), false}

result, err := dnsSD.Resolve(ctx, tt.addr, tt.qtype)
if tt.expectedErr != nil {
Expand Down

0 comments on commit e30a981

Please sign in to comment.