diff --git a/Changes b/Changes index 54978a83..ad773747 100644 --- a/Changes +++ b/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 diff --git a/docs/checks/plugins/check_dns.md b/docs/checks/plugins/check_dns.md index f0326806..c89d981d 100644 --- a/docs/checks/plugins/check_dns.md +++ b/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 diff --git a/pkg/check_dns/check_dns.go b/pkg/check_dns/check_dns.go index f172c0ad..2c8aea9c 100644 --- a/pkg/check_dns/check_dns.go +++ b/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" } diff --git a/pkg/snclient/check_dns_test.go b/pkg/snclient/check_dns_test.go index 516a91fc..4b9f73d6 100644 --- a/pkg/snclient/check_dns_test.go +++ b/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) }