Skip to content

Commit

Permalink
[Ruler][DNS] Don't propagate no such host error if using default reso…
Browse files Browse the repository at this point in the history
…lver (#3257)

* Don't propagate DNS not found error.

Signed-off-by: Kevin Hellemun <17928966+OGKevin@users.noreply.github.com>

* Update change log.

Signed-off-by: Kevin Hellemun <17928966+OGKevin@users.noreply.github.com>

* Add DNS resolver type for mock resolver.

Signed-off-by: Kevin Hellemun <17928966+OGKevin@users.noreply.github.com>

* Remove trailing white space in changelog.

Signed-off-by: Kevin Hellemun <17928966+OGKevin@users.noreply.github.com>

* Add flag to ensure new behaviour only impacts ruler AM resolution.

Signed-off-by: Kevin Hellemun <17928966+OGKevin@users.noreply.github.com>

* Make method signature change backwards compatible

Signed-off-by: Kevin Hellemun <17928966+OGKevin@users.noreply.github.com>

* Minor refactoring of logical sequence and dub code.

Signed-off-by: Kevin Hellemun <17928966+OGKevin@users.noreply.github.com>

* Remove flag and make it default behaviour

Signed-off-by: Kevin Hellemun <17928966+OGKevin@users.noreply.github.com>

* Remove unneeded todo comment.

Signed-off-by: Kevin Hellemun <17928966+OGKevin@users.noreply.github.com>
  • Loading branch information
OGKevin authored Oct 15, 2020
1 parent 57ced35 commit 7a9a077
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 6 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re

## Unreleased
- [#3259](https://github.com/thanos-io/thanos/pull/3259) Thanos BlockViewer: Added a button in the blockviewer that allows users to download the metadata of a block.

- [#3261](https://github.com/thanos-io/thanos/pull/3261) Thanos Store: Use segment files specified in meta.json file, if present. If not present, Store does the LIST operation as before.
- [#3276](https://github.com/thanos-io/thanos/pull/3276) Query Frontend: Support query splitting and retry for labels and series requests.

### Fixed
- [#3257](https://github.com/thanos-io/thanos/pull/3257) Ruler: Prevent Ruler from crashing when using default DNS to lookup hosts that results in "No such hosts" errors.

## [v0.16.0](https://github.com/thanos-io/thanos/releases) - Release in progress

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion pkg/discovery/dns/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (t ResolverType) ToResolver(logger log.Logger) ipLookupResolver {
// If empty resolver type is net.DefaultResolver.
func NewProvider(logger log.Logger, reg prometheus.Registerer, resolverType ResolverType) *Provider {
p := &Provider{
resolver: NewResolver(resolverType.ToResolver(logger)),
resolver: NewResolver(resolverType.ToResolver(logger), logger),
resolved: make(map[string][]string),
logger: logger,
resolverAddrs: extprom.NewTxGaugeVec(reg, prometheus.GaugeOpts{
Expand Down
22 changes: 19 additions & 3 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 @@ -38,11 +41,12 @@ type ipLookupResolver interface {

type dnsSD struct {
resolver ipLookupResolver
logger log.Logger
}

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

func (s *dnsSD) Resolve(ctx context.Context, name string, qtype QType) ([]string, error) {
Expand Down Expand Up @@ -71,7 +75,15 @@ func (s *dnsSD) Resolve(ctx context.Context, name string, qtype QType) ([]string
}
ips, err := s.resolver.LookupIPAddr(ctx, host)
if err != nil {
return nil, errors.Wrapf(err, "lookup IP addresses %q", host)
// We exclude error from std Golang resolver for the case of the domain (e.g `NXDOMAIN`) not being found by DNS
// server. Since `miekg` does not consider this as an error, when the host cannot be found, empty slice will be
// returned.
if dnsErr, ok := err.(*net.DNSError); !ok || !dnsErr.IsNotFound {
return nil, errors.Wrapf(err, "lookup IP addresses %q", host)
}
if ips == nil {
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 @@ -106,6 +118,10 @@ func (s *dnsSD) Resolve(ctx context.Context, name string, qtype QType) ([]string
return nil, errors.Errorf("invalid lookup scheme %q", qtype)
}

if res == nil && err == nil {
level.Warn(s.logger).Log("msg", "IP address lookup yielded no results. No host found or no addresses found", "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}
dnsSD := dnsSD{tt.resolver, log.NewNopLogger()}

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

0 comments on commit 7a9a077

Please sign in to comment.