Remove flag and make it default behaviour

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

Author: OGKevin

cmd/thanos/query.go

@@ -261,11 +261,10 @@ func runQuery(
 	}
 
 	fileSDCache := cache.New()
-	dnsStoreProvider := dns.NewProviderWithReturnOnErrorIfNotFound(
+	dnsStoreProvider := dns.NewProvider(
 		logger,
 		extprom.WrapRegistererWithPrefix("thanos_querier_store_apis_", reg),
 		dns.ResolverType(dnsSDResolver),
-		true,
 	)
 
 	for _, store := range strictStores {
@@ -274,11 +273,10 @@ func runQuery(
 		}
 	}
 
-	dnsRuleProvider := dns.NewProviderWithReturnOnErrorIfNotFound(
+	dnsRuleProvider := dns.NewProvider(
 		logger,
 		extprom.WrapRegistererWithPrefix("thanos_querier_rule_apis_", reg),
 		dns.ResolverType(dnsSDResolver),
-		true,
 	)
 
 	var (

cmd/thanos/rule.go

@@ -334,11 +334,10 @@ func runRule(
 		}
 	}
 
-	queryProvider := dns.NewProviderWithReturnOnErrorIfNotFound(
+	queryProvider := dns.NewProvider(
 		logger,
 		extprom.WrapRegistererWithPrefix("thanos_ruler_query_apis_", reg),
 		dns.ResolverType(dnsSDResolver),
-		true,
 	)
 	var queryClients []*http_util.Client
 	for _, cfg := range queryCfg {
@@ -398,11 +397,10 @@ func runRule(
 		level.Warn(logger).Log("msg", "no alertmanager configured")
 	}
 
-	amProvider := dns.NewProviderWithReturnOnErrorIfNotFound(
+	amProvider := dns.NewProvider(
 		logger,
 		extprom.WrapRegistererWithPrefix("thanos_ruler_alertmanagers_", reg),
 		dns.ResolverType(dnsSDResolver),
-		false,
 	)
 	var alertmgrs []*alert.Alertmanager
 	for _, cfg := range alertingCfg.Alertmanagers {

pkg/cacheutil/memcached_client.go

@@ -215,11 +215,10 @@ func newMemcachedClient(
 	config MemcachedClientConfig,
 	reg prometheus.Registerer,
 ) (*memcachedClient, error) {
-	dnsProvider := dns.NewProviderWithReturnOnErrorIfNotFound(
+	dnsProvider := dns.NewProvider(
 		logger,
 		extprom.WrapRegistererWithPrefix("thanos_memcached_", reg),
 		dns.GolangResolverType,
-		true,
 	)
 
 	c := &memcachedClient{

pkg/discovery/dns/provider.go

@@ -53,20 +53,12 @@ func (t ResolverType) ToResolver(logger log.Logger) ipLookupResolver {
 	return r
 }
 
-// Deprecated. Use NewProviderWithReturnOnErrorIfNotFound instead.
-//
 // NewProvider returns a new empty provider with a given resolver type.
 // If empty resolver type is net.DefaultResolver.
 // TODO(OGKevin): Refactor code to use new method and eventually rename NewProviderWithReturnOnErrorIfNotFound back to NewProvider.
 func NewProvider(logger log.Logger, reg prometheus.Registerer, resolverType ResolverType) *Provider {
-	return NewProviderWithReturnOnErrorIfNotFound(logger, reg, resolverType, true)
-}
-
-// NewProviderWithReturnOnErrorIfNotFound returns a new empty provider with a given resolver type.
-// If empty resolver type is net.DefaultResolver.
-func NewProviderWithReturnOnErrorIfNotFound(logger log.Logger, reg prometheus.Registerer, resolverType ResolverType, returnErrOnNotFound bool) *Provider {
 	p := &Provider{
-		resolver: NewResolver(resolverType.ToResolver(logger), logger, returnErrOnNotFound),
+		resolver: NewResolver(resolverType.ToResolver(logger), logger),
 		resolved: make(map[string][]string),
 		logger:   logger,
 		resolverAddrs: extprom.NewTxGaugeVec(reg, prometheus.GaugeOpts{

pkg/discovery/dns/provider_test.go

@@ -22,7 +22,7 @@ func TestProvider(t *testing.T) {
 		"127.0.0.5:19095",
 	}
 
-	prv := NewProviderWithReturnOnErrorIfNotFound(log.NewNopLogger(), nil, "", true)
+	prv := NewProvider(log.NewNopLogger(), nil, "")
 	prv.resolver = &mockResolver{
 		res: map[string][]string{
 			"a": ips[:2],

pkg/discovery/dns/resolver.go

@@ -42,14 +42,11 @@ type ipLookupResolver interface {
 type dnsSD struct {
 	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, logger log.Logger, returnErrOnNotFound bool) Resolver {
-	return &dnsSD{resolver: resolver, logger: logger, returnErrOnNotFound: returnErrOnNotFound}
+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) {
@@ -78,13 +75,15 @@ func (s *dnsSD) Resolve(ctx context.Context, name string, qtype QType) ([]string
 		}
 		ips, err := s.resolver.LookupIPAddr(ctx, host)
 		if err != nil {
-			// https://github.com/thanos-io/thanos/issues/3186
-			// Default DNS resolver can make thanos components crash if DNS resolutions results in EAI_NONAME.
-			// the flag returnErrOnNotFound can be used to prevent such crash.
-			if dnsErr, ok := err.(*net.DNSError); !ok || !dnsErr.IsNotFound || s.returnErrOnNotFound {
+			// 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)
 			}
-			level.Error(s.logger).Log("msg", "failed to lookup IP addresses", "host", host, "err", err)
+			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)))
@@ -119,10 +118,8 @@ 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)
+		level.Warn(s.logger).Log("msg", "IP address lookup yielded no results. No host found or no addresses found", "host", host)
 	}
 
 	return res, nil

pkg/discovery/dns/resolver_test.go

@@ -186,7 +186,7 @@ func TestDnsSD_Resolve(t *testing.T) {
 
 func testDnsSd(t *testing.T, tt DNSSDTest) {
 	ctx := context.TODO()
-	dnsSD := dnsSD{tt.resolver, log.NewNopLogger(), false}
+	dnsSD := dnsSD{tt.resolver, log.NewNopLogger()}
 
 	result, err := dnsSD.Resolve(ctx, tt.addr, tt.qtype)
 	if tt.expectedErr != nil {