From f2dc6885ed2816d6d29a43aff111422eaa544804 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Thu, 10 Sep 2026 15:01:33 +0200 Subject: [PATCH 1/2] check_dns: ai commit, add --attempts support it uses the same dns connection throught, but sends multiple packets if the proceeding packets do not get an answer. it uses connections WriteMsg, SetReadDeadline to add timeouts and ReadMsg with with error checks to see if timeouts were met. timeout and attemps are now also read from resolv.conf if defined. attemps divide the query timeout into equal per UDP message timeouts. this is implemented in the new exchangeWithRetries function all combinations between hostCandidate x nameserver are checked using exchangeWithRetries function now instead of previous c.ExhangeContext --- Changes | 1 + docs/checks/plugins/check_dns.md | 7 +- pkg/check_dns/check_dns.go | 128 +++++++++++++++++++++++++++++-- pkg/snclient/check_dns_test.go | 95 +++++++++++++++++++++++ 4 files changed, 221 insertions(+), 10 deletions(-) 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..a78cc345 100644 --- a/docs/checks/plugins/check_dns.md +++ b/docs/checks/plugins/check_dns.md @@ -70,8 +70,11 @@ Application Options: Default ist 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 seconds. 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..7b8af958 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" @@ -51,9 +52,18 @@ type dnsOpts struct { 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."` 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 seconds."` } +// 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) } From 00954ab5198fdd3c9ab320a7ce09233f952be5a2 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Thu, 10 Sep 2026 15:40:32 +0200 Subject: [PATCH 2/2] check_dns: fix attempts help text , 2 attempts additionally fix German ist -> is in CriticalTimeout text --- docs/checks/plugins/check_dns.md | 4 ++-- pkg/check_dns/check_dns.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/checks/plugins/check_dns.md b/docs/checks/plugins/check_dns.md index a78cc345..c89d981d 100644 --- a/docs/checks/plugins/check_dns.md +++ b/docs/checks/plugins/check_dns.md @@ -67,14 +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, 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 seconds. + 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 7b8af958..2c8aea9c 100644 --- a/pkg/check_dns/check_dns.go +++ b/pkg/check_dns/check_dns.go @@ -50,10 +50,10 @@ 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" 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 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)