check_dns: ai commit, add --attempts support

Changes

@@ -1,6 +1,7 @@
 This file documents the revision history for the SNClient agent.
 
 next:
+         - check_dns: resend lost packets and use the timeout and attempts settings of the resolv.conf
          - fix remaining zombie processes after automatic updates
          - check_service: add wildcard support for exclude argument
          - check_process: make parent pid (ppid) available

docs/checks/plugins/check_dns.md

@@ -67,11 +67,14 @@ Application Options:
   -w, --warning=          Return warning if elapsed time to get a successful DNS query exceeds this value in seconds.
                           Default is off.
   -c, --critical=         Return critical if elapsed time to get a successful DNS query exceeds this value in seconds.
-                          Default ist off.
+                          Default is off.
   -t, --timeout=          Global timeout in seconds. Exit early and return unknown if elapsed time to get a successful
                           DNS query exceeds this value. (default: 30)
-  -T, --query-timeout=    Timeout for each single DNS query in seconds. If exceeded, the next query is tried instead of
-                          exiting. (default: 5)
+  -T, --query-timeout=    Timeout for each single DNS query in seconds, retransmissions included. If exceeded, the next
+                          query is tried instead of exiting. Can be specified in resolv.conf file. Defaults to 5
+                          seconds.
+  -a, --attempts=         Number of packets sent for each single DNS query before it is considered unanswered. Can be
+                          specified in resolv.conf file. Defaults to 2 attempts.
 
 Help Options:
   -h, --help              Show this help message

pkg/check_dns/check_dns.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"net"
+	"os"
 	"runtime"
 	"slices"
 	"strconv"
@@ -49,11 +50,20 @@ type dnsOpts struct {
 	ResolvConfFile  string   `long:"resolv-conf-file" default:"/etc/resolv.conf" description:"Path to the resolv.conf file to use. Is not used in Windows."`
 	Verbose         bool     `short:"v" long:"vv" long:"vvv" long:"verbose" description:"Show verbose output."`
 	WarningTimeout  *int     `short:"w" long:"warning" description:"Return warning if elapsed time to get a successful DNS query exceeds this value in seconds. Default is off."`
-	CriticalTimeout *int     `short:"c" long:"critical" description:"Return critical if elapsed time to get a successful DNS query exceeds this value in seconds. Default ist off."`
+	CriticalTimeout *int     `short:"c" long:"critical" description:"Return critical if elapsed time to get a successful DNS query exceeds this value in seconds. Default is off."`
 	Timeout         int      `short:"t" long:"timeout" default:"30" description:"Global timeout in seconds. Exit early and return unknown if elapsed time to get a successful DNS query exceeds this value."`
-	QueryTimeout    int      `short:"T" long:"query-timeout" default:"5" description:"Timeout for each single DNS query in seconds. If exceeded, the next query is tried instead of exiting."`
+	QueryTimeout    *int     `short:"T" long:"query-timeout" description:"Timeout for each single DNS query in seconds, retransmissions included. If exceeded, the next query is tried instead of exiting. Can be specified in resolv.conf file. Defaults to 5 seconds."`
+	Attempts        *int     `short:"a" long:"attempts" description:"Number of packets sent for each single DNS query before it is considered unanswered. Can be specified in resolv.conf file. Defaults to 2 attempts."`
 }
 
+// same defaults as the resolver in glibc (RES_TIMEOUT / RES_DFLRETRY)
+// ref: https://github.com/bminor/glibc/blob/765325951ac5c7d072278c9424930b29657e9758/resolv/resolv.h#L68-L72
+const (
+	defaultQueryTimeout = 5
+	defaultAttempts     = 2
+	maxAttempts         = 10
+)
+
 func parseArgs(args []string) (*dnsOpts, error) {
 	opts := &dnsOpts{}
 	psr := flags.NewParser(opts, flags.HelpFlag|flags.PassDoubleDash) // default flags without flags.PrintErrors
@@ -79,8 +89,11 @@ func (opts *dnsOpts) validate() error {
 	if opts.Timeout <= 0 {
 		return fmt.Errorf("timeout must be a positive number of seconds, got: %d", opts.Timeout)
 	}
-	if opts.QueryTimeout <= 0 {
-		return fmt.Errorf("query timeout must be a positive number of seconds, got: %d", opts.QueryTimeout)
+	if opts.QueryTimeout != nil && *opts.QueryTimeout <= 0 {
+		return fmt.Errorf("query timeout must be a positive number of seconds, got: %d", *opts.QueryTimeout)
+	}
+	if opts.Attempts != nil && (*opts.Attempts < 1 || *opts.Attempts > maxAttempts) {
+		return fmt.Errorf("attempts must be between 1 and %d, got: %d", maxAttempts, *opts.Attempts)
 	}
 	if opts.WarningTimeout != nil && *opts.WarningTimeout < 0 {
 		return fmt.Errorf("warning threshold must not be negative, got: %d", *opts.WarningTimeout)
@@ -125,6 +138,29 @@ func (opts *dnsOpts) run(ctx context.Context) *checkers.Checker {
 	default:
 	}
 
+	queryTimeout := time.Duration(defaultQueryTimeout) * time.Second
+	attempts := defaultAttempts
+	if clientConfig != nil {
+		if clientConfig.Timeout > 0 {
+			queryTimeout = time.Duration(clientConfig.Timeout) * time.Second
+		}
+		if clientConfig.Attempts > 0 {
+			attempts = clientConfig.Attempts
+			if attempts > maxAttempts {
+				attempts = maxAttempts
+			}
+		}
+	}
+	if opts.QueryTimeout != nil {
+		queryTimeout = time.Duration(*opts.QueryTimeout) * time.Second
+	}
+	if opts.Attempts != nil {
+		attempts = *opts.Attempts
+	}
+	if logger != nil && opts.Verbose {
+		logger.Tracef("DNS query timeout: %s, attempts: %d", queryTimeout, attempts)
+	}
+
 	var nameservers []string
 	if len(opts.Servers) > 0 {
 		nameservers = opts.Servers
@@ -172,8 +208,9 @@ func (opts *dnsOpts) run(ctx context.Context) *checkers.Checker {
 		return checkers.Critical(fmt.Sprintf("%s is an invalid query type", opts.QueryType))
 	}
 
-	// Timeout is a builtin cumulative timeout for dial, write and read, it is applied to every single Exchange i.e. DNS query.
-	c := &dns.Client{Timeout: time.Duration(opts.QueryTimeout) * time.Second}
+	// Timeout is a builtin cumulative timeout for dial, write and read.
+	// The deadlines of the single packets are set explicitly in exchangeWithRetries.
+	c := &dns.Client{Timeout: queryTimeout}
 
 	var r *dns.Msg
 	var duration time.Duration
@@ -223,7 +260,7 @@ func (opts *dnsOpts) run(ctx context.Context) *checkers.Checker {
 
 				// Use the per-run context so either this check or an enclosing check
 				// (for example check_multi) can stop an in-flight DNS query.
-				r, duration, err = c.ExchangeContext(queryCtx, message, nameserver)
+				r, duration, err = exchangeWithRetries(queryCtx, c, message, nameserver, queryTimeout, attempts)
 
 				if err == nil {
 					if len(r.Answer) == 0 {
@@ -388,6 +425,82 @@ func (opts *dnsOpts) run(ctx context.Context) *checkers.Checker {
 	return checkers.NewChecker(checkSt, msg)
 }
 
+// exchangeWithRetries sends the query and waits for the answer.
+// Answers which do not arrive within a slice of the timeout are considered lost and the very same query is sent again over the same socket.
+// A late answer to one of the previous packets is still accepted.
+// Retries use up the given timeout instead of extending it.
+func exchangeWithRetries(ctx context.Context, client *dns.Client, message *dns.Msg, nameserver string, timeout time.Duration, attempts int) (*dns.Msg, time.Duration, error) {
+	start := time.Now()
+
+	deadline := start.Add(timeout)
+	if ctxDeadline, ok := ctx.Deadline(); ok && ctxDeadline.Before(deadline) {
+		deadline = ctxDeadline
+	}
+
+	conn, err := client.DialContext(ctx, nameserver)
+	if err != nil {
+		return nil, time.Since(start), err
+	}
+	defer func() { _ = conn.Close() }()
+
+	waitSlice := timeout / time.Duration(attempts)
+	if waitSlice <= 0 {
+		waitSlice = timeout
+	}
+	waitUntil := start.Add(waitSlice)
+	if deadline.Before(waitUntil) {
+		waitUntil = deadline
+	}
+
+	var lastErr error = os.ErrDeadlineExceeded
+	for sent := 1; sent <= attempts; sent++ {
+		if err := conn.SetWriteDeadline(waitUntil); err != nil {
+			return nil, time.Since(start), err
+		}
+		if err := conn.WriteMsg(message); err != nil {
+			return nil, time.Since(start), err
+		}
+
+		for time.Now().Before(waitUntil) {
+			if err := conn.SetReadDeadline(waitUntil); err != nil {
+				return nil, time.Since(start), err
+			}
+			r, err := conn.ReadMsg()
+			if err == nil {
+				if r.Id == message.Id {
+					return r, time.Since(start), nil
+				}
+
+				// answer to a packet sent before, keep waiting
+				continue
+			}
+			if !isTimeoutError(err) {
+				return nil, time.Since(start), err
+			}
+			lastErr = err
+
+			break
+		}
+
+		if sent == attempts || !time.Now().Before(deadline) || ctx.Err() != nil {
+			break
+		}
+
+		waitUntil = time.Now().Add(waitSlice)
+		if deadline.Before(waitUntil) {
+			waitUntil = deadline
+		}
+	}
+
+	return nil, time.Since(start), lastErr
+}
+
+func isTimeoutError(err error) bool {
+	var netErr net.Error
+
+	return errors.As(err, &netErr) && netErr.Timeout()
+}
+
 func dnsAnswer(answer dns.RR) (string, string, error) {
 	switch t := answer.(type) {
 	case *dns.A:
@@ -463,8 +576,7 @@ func emptyResultReason(rcode int) string {
 }
 
 func queryFailedReason(err error) string {
-	var netErr net.Error
-	if errors.As(err, &netErr) && netErr.Timeout() {
+	if isTimeoutError(err) {
 		return "query failed: timeout"
 	}
 

pkg/snclient/check_dns_test.go

@@ -7,7 +7,9 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
+	"sync/atomic"
 	"testing"
+	"time"
 
 	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
@@ -71,6 +73,30 @@ func startSilentDNSServer(t *testing.T, listenAddr string) string {
 	return strconv.Itoa(udpAddr.Port)
 }
 
+// startDroppingDNSServer silently drops the first drop packets and answers
+// every following query with an A record, like a lossy network path does.
+// It returns the port it has started on and a counter of received packets.
+func startDroppingDNSServer(t *testing.T, listenAddr string, drop int64) (string, *atomic.Int64) {
+	t.Helper()
+
+	var received atomic.Int64
+	port := startTestDNSServerHandler(t, listenAddr, dns.HandlerFunc(func(writer dns.ResponseWriter, req *dns.Msg) {
+		if received.Add(1) <= drop {
+			return
+		}
+
+		reply := new(dns.Msg)
+		reply.SetReply(req)
+		rr, rrErr := dns.NewRR(req.Question[0].Name + " 60 IN A 1.2.3.4")
+		if rrErr == nil {
+			reply.Answer = append(reply.Answer, rr)
+		}
+		_ = writer.WriteMsg(reply)
+	}))
+
+	return port, &received
+}
+
 func TestCheckDNS(t *testing.T) {
 	config := `
 [/modules]
@@ -388,6 +414,75 @@ CheckBuiltinPlugins = enabled
 		)
 	})
 
+	t.Run("single lost packet is retransmitted", func(t *testing.T) {
+		port, received := startDroppingDNSServer(t, "127.0.0.1:0", 1)
+		res := snc.RunCheck("check_dns", []string{
+			"-H", "lossy.example.com.",
+			"-s", "127.0.0.1", "-p", port,
+			"-T", "2",
+		})
+		assert.Equalf(t, CheckExitOK, res.State, "state ok")
+		assert.Regexpf(
+			t,
+			`^OK - lossy\.example\.com\. returns 1\.2\.3\.4 \(A\)`,
+			string(res.BuildPluginOutput()),
+			"output matches",
+		)
+		assert.Equalf(t, int64(2), received.Load(), "packets sent")
+	})
+
+	t.Run("timeout and attempts from resolv.conf", func(t *testing.T) {
+		port, received := startDroppingDNSServer(t, "127.0.0.1:0", 2)
+
+		resolvConf := filepath.Join(t.TempDir(), "resolv.conf")
+		require.NoError(t, os.WriteFile(resolvConf, []byte("nameserver 127.0.0.1\noptions timeout:1 attempts:3\n"), 0o600))
+
+		res := snc.RunCheck("check_dns", []string{
+			"-H", "lossy.example.com.",
+			"-s", "127.0.0.1", "-p", port,
+			"--resolv-conf-file", resolvConf,
+		})
+		assert.Equalf(t, CheckExitOK, res.State, "state ok")
+		assert.Equalf(t, int64(3), received.Load(), "packets sent")
+	})
+
+	t.Run("retries use up the query timeout", func(t *testing.T) {
+		port := startSilentDNSServer(t, "127.0.0.1:0")
+
+		startTimestamp := time.Now()
+		res := snc.RunCheck("check_dns", []string{
+			"-H", "silent.example.com.",
+			"-s", "127.0.0.1", "-p", port,
+			"-T", "1", "--attempts", "3",
+		})
+		elapsed := time.Since(startTimestamp)
+
+		assert.Equalf(t, CheckExitCritical, res.State, "state critical")
+		assert.Regexpf(
+			t,
+			`^CRITICAL - DNS lookup failed for host 'silent\.example\.com': 127\.0\.0\.1:`+port+`: query failed: timeout$`,
+			string(res.BuildPluginOutput()),
+			"output matches",
+		)
+		assert.Lessf(t, elapsed, 2500*time.Millisecond, "three attempts of 1 second stay within the query timeout")
+	})
+
+	StopTestAgent(t, snc)
+}
+
+func TestCheckDNSAttemptsValidation(t *testing.T) {
+	config := `
+[/modules]
+CheckBuiltinPlugins = enabled
+	`
+	snc := StartTestAgent(t, config)
+
+	t.Run("invalid attempts", func(t *testing.T) {
+		res := snc.RunCheck("check_dns", []string{"-H", "labs.consol.de", "--attempts", "0"})
+		assert.Equalf(t, CheckExitUnknown, res.State, "state unknown")
+		assert.Containsf(t, string(res.BuildPluginOutput()), "attempts must be between 1 and 10", "output matches")
+	})
+
 	StopTestAgent(t, snc)
 }