]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MINOR: checks: add min-recv tcp-check expect option
authorGaetan Rivet <grive@u256.net>
Fri, 7 Feb 2020 14:37:17 +0000 (15:37 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Mon, 27 Apr 2020 07:39:37 +0000 (09:39 +0200)
Some expect rules cannot be satisfied due to inherent ambiguity towards
the received data: in the absence of match, the current behavior is to
be forced to wait either the end of the connection or a buffer full,
whichever comes first. Only then does the matching diagnostic is
considered  conclusive. For instance :

    tcp-check connect
    tcp-check expect !rstring "^error"
    tcp-check expect string "valid"

This check will only succeed if the connection is closed by the server before
the check timeout. Otherwise the first expect rule will wait for more data until
"^error" regex matches or the check expires.

Allow the user to explicitly define an amount of data that will be
considered enough to determine the value of the check.

This allows succeeding on negative rstring rules, as previously
in valid condition no match happened, and the matching was repeated
until the end of the connection. This could timeout the check
while no error was happening.

[Cf: I slighly updated the patch. The parameter was renamed and the value is a
signed integer to support -1 as default value to ignore the parameter.]

doc/configuration.txt
include/types/checks.h
reg-tests/checks/tcp-check_min-recv.vtc [new file with mode: 0644]
src/cfgparse-listen.c
src/checks.c

index 3f247b6d88d0970d6553e662b684763097f2a20c..45312fd44477de7e59f4f2c7b2ca6b56759a96cd 100644 (file)
@@ -9845,12 +9845,23 @@ tcp-check connect [params*]
   See also : "option tcp-check", "tcp-check send", "tcp-check expect"
 
 
-tcp-check expect [!] <match> <pattern>
+tcp-check expect [min-recv <int>] [!] <match> <pattern>
   Specify data to be collected and analyzed during a generic health check
   May be used in sections:   defaults | frontend | listen | backend
                                no     |    no    |   yes  |   yes
 
   Arguments :
+    min-recv  is optional and can define the minimum amount of data required to
+              evaluate the current expect rule. If the number of received bytes
+              is under this limit, the check will wait for more data. This
+              option can be used to resolve some ambiguous matching rules or to
+              avoid executing costly regex matches on content known to be still
+              incomplete. If an exact string (string or binary) is used, the
+              minimum between the string length and this parameter is used.
+              This parameter is ignored if it is set to -1. If the expect rule
+              does not match, the check will wait for more data. If set to 0,
+              the evaluation result is always conclusive.
+
     <match>   is a keyword indicating how to look for a specific pattern in the
               response. The keyword may be one of "string", "rstring" or
               binary.
index 294adc6dd0f23533ad4604f2f72f4f2cea219c74..93d552ff17dff484c131ae0671ab3261b2ea6acd 100644 (file)
@@ -232,6 +232,7 @@ struct tcpcheck_rule {
        /* sent string is string */
        char *string;                           /* sent or expected string */
        int string_len;                         /* string length */
+       int min_recv;                           /* Minimum amount of data before an expect can be applied. (default: -1, ignored) */
        struct my_regex *expect_regex;          /* expected */
        int inverse;                            /* 0 = regular match, 1 = inverse match */
        unsigned short port;                    /* port to connect to */
diff --git a/reg-tests/checks/tcp-check_min-recv.vtc b/reg-tests/checks/tcp-check_min-recv.vtc
new file mode 100644 (file)
index 0000000..61ff3c8
--- /dev/null
@@ -0,0 +1,69 @@
+varnishtest "tcp-check negative bounded regex match"
+#EXCLUDE_TARGETS=freebsd,osx,generic
+#REGTEST_TYPE=slow
+#REQUIRE_VERSION=2.2
+# This test use a negative expect rule and verify that setting a required
+# minimum amount of data to match.
+feature ignore_unknown_macro
+
+syslog S1 -level notice {
+    recv
+    expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Proxy be1 started."
+    recv
+    expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server be1/srv1 failed, reason: Layer7 timeout.*at step 2 of tcp-check"
+} -start
+
+syslog S2 -level notice {
+    recv
+    expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Proxy be2 started."
+    recv
+    expect ~ "[^:\\[ ]\\[${h1_pid}\\]: Health check for server be2/srv1 succeeded"
+} -start
+
+server s1 {
+    send "valid"
+    delay 0.2
+    expect_close
+} -start
+
+server s2 {
+    send "valid"
+    recv 10
+    send "valid"
+    delay 0.2
+    expect_close
+} -start
+
+haproxy h1 -conf {
+  defaults
+    mode tcp
+    timeout connect 200ms
+    timeout check 500ms
+    timeout server 5s
+    timeout client 5s
+
+  backend be1
+    log ${S1_addr}:${S1_port} len 2048 local0
+    option tcp-check
+    option log-health-checks
+    tcp-check connect
+    tcp-check expect !rstring "^error" comment "negative check"
+    tcp-check expect string "valid" comment "positive check"
+    tcp-check send "0123456789"
+    tcp-check expect string "valid" comment "positive check"
+    server srv1 ${s1_addr}:${s1_port} check inter 200ms rise 1 fall 1
+
+  backend be2
+    log ${S2_addr}:${S2_port} len 2048 local0
+    option tcp-check
+    option log-health-checks
+    tcp-check connect
+    tcp-check expect min-recv 5 !rstring "^error" comment "negative check"
+    tcp-check expect string "valid" comment "positive check"
+    tcp-check send "0123456789"
+    tcp-check expect string "valid" comment "positive check"
+    server srv1 ${s2_addr}:${s2_port} check inter 200ms rise 1 fall 1
+} -start
+
+syslog S1 -wait
+syslog S2 -wait
index 911255b801e86d3c4de54f1dc7103ebfc7417d6d..4b099c181d03a2e6315a6439bdb0fd6f4d603ee7 100644 (file)
@@ -3192,6 +3192,7 @@ stats_error_parsing:
                }
                else if (strcmp(args[1], "expect") == 0) {
                        struct tcpcheck_rule *tcpcheck, *prev_check;
+                       long min_recv = -1;
                        const char *ptr_arg;
                        int cur_arg;
                        int inverse = 0;
@@ -3203,6 +3204,30 @@ stats_error_parsing:
                        }
 
                        cur_arg = 2;
+
+                       /* Parse potential the minimum amount of data
+                        * required before proceeding with the match.
+                        */
+                       if (strcmp(args[cur_arg], "min-recv") == 0) {
+                               if (!*(args[cur_arg + 1])) {
+                                       ha_alert("parsing [%s:%d] : '%s %s %s' expects an integer as an argument.\n",
+                                                file, linenum, args[0], args[1], args[2]);
+                                       err_code |= ERR_ALERT | ERR_FATAL;
+                                       goto out;
+                               }
+
+                               /* Use an signed integer here because of chksize */
+                               min_recv = atol(args[cur_arg + 1]);
+                               if (min_recv < -1 || min_recv > INT_MAX) {
+                                       ha_alert("parsing [%s:%d] : '%s %s %s' expects -1 or an integer from 0 to INT_MAX.\n",
+                                                file, linenum, args[0], args[1], args[2]);
+                                       err_code |= ERR_ALERT | ERR_FATAL;
+                                       goto out;
+                               }
+
+                               cur_arg += 2;
+                       }
+
                        /* consider exclamation marks, sole or at the beginning of a word */
                        while (*(ptr_arg = args[cur_arg])) {
                                while (*ptr_arg == '!') {
@@ -3221,6 +3246,7 @@ stats_error_parsing:
                        tcpcheck = calloc(1, sizeof(*tcpcheck));
                        tcpcheck->action = TCPCHK_ACT_EXPECT;
                        tcpcheck->inverse = inverse;
+                       tcpcheck->min_recv = min_recv;
 
                        if (strcmp(ptr_arg, "binary") == 0) {
                                char *err = NULL;
index ea534f5122053aa9094776a99c76d3d7ff755454..bb45026bfd7738b5ed36a8041b57e38ec31e5f34 100644 (file)
@@ -3159,14 +3159,23 @@ static int tcpcheck_main(struct check *check)
                        }
 
                tcpcheck_expect:
-                       if (!done && (check->current_step->string != NULL) && (b_data(&check->bi) < check->current_step->string_len) )
+
+                       /* The current expect might need more data than the previous one, check again
+                        * that the minimum amount data required to match is respected.
+                        */
+                       if (!done &&
+                           (((check->current_step->string != NULL) && (b_data(&check->bi) < check->current_step->string_len)) ||
+                            ((check->current_step->min_recv > 0 && (b_data(&check->bi) < check->current_step->min_recv)))))
                                continue; /* try to read more */
                        if (check->current_step->string != NULL)
                                ret = my_memmem(contentptr, b_data(&check->bi), check->current_step->string, check->current_step->string_len) != NULL;
                        else if (check->current_step->expect_regex != NULL)
                                ret = regex_exec(check->current_step->expect_regex, contentptr);
 
-                       if (!ret && !done)
+                       /* Wait for more data on mismatch only if no minimum is defined (-1),
+                        * otherwise the absence of match is already conclusive.
+                        */
+                       if (!ret && !done && (check->current_step->min_recv == -1))
                                continue; /* try to read more */
 
                        /* matched */