MINOR: mworker: replace ha_alert by ha_warning when exiting successfuly
Since commit bb86986 ("MINOR: init: report the haproxy version and
executable path once on errors") the master-worker displays its version
and path upon a successful exits of a current worker. Which is kind of
confusing upon a clean exits.
This is due to the fact that that commit displays this upon a ha_alert()
which was used during the exit of the process.
Replace the ha_alert() by an ha_warning() if the process exit correctly
and was supposed to.
It still displays the message upon a SIGINT since the workers are
catching the signal.
REGTEST: checks: Adapt SSL error message reported when connection is rejected
Depending on the SSL library version, the reported error may differ when the
connection is rejected during the handshake. An empty handshke may be detected
or just an generic handshake error. So tcp-check-ssl.vtc has been adapted to
support both error messages.
MINOR: checks/sample: Remove unnecessary tests on the sample session
A sample must always have a session defined. Otherwise, it is a bug. So it is
unnecessary to test if it is defined when called from a health checks context.
MINOR: checks: Simplify matching on HTTP headers in HTTP expect rules
Extra parameters on http-check expect rules, for the header matching method, to
use log-format string or to match full header line have been removed. There is
now separate matching methods to match a full header line or to match each
comma-separated values. "http-check expect fhdr" must be used in the first case,
and "http-check expect hdr" in the second one. In addition, to match log-format
header name or value, "-lf" suffix must be added to "name" or "value"
keyword. For intance:
http-check expect hdr name "set-cookie" value-lf -m beg "sessid=%[var(check.cookie)]"
Thanks to this changes, each parameter may only be interpreted in one way.
MEDIUM: checks: Remove dedicated sample fetches and use response ones instead
All sample fetches in the scope "check." have been removed. Response sample
fetches must be used instead. It avoids keyword duplication. So, for instance,
res.hdr() must be now used instead of check.hdr().
To do so, following sample fetches have been added on the response :
* res.body, res.body_len and res.body_size
* res.hdrs and res.hdrs_bin
Sample feches dealing with the response's body are only useful in the health
checks context. When called from a stream context, there is no warranty on the
body presence. There is no option to wait the response's body.
DOC: Fix req.body and co documentation to be accurate
Because the HTX is the only mode to represent HTTP data, "option
http-request-buffer" is no longer mandatory to have body data. Without this
option, there is no warranty on the body presence. So it is recommanded to use
it. But it is not a requirement. In addition, the note about chunked body is
removed because outdated.
MEDIUM: checks: Add matching on log-format string for expect rules
It is now possible to use log-format string (or hexadecimal string for the
binary version) to match a content in tcp-check based expect rules. For
hexadecimal log-format string, the conversion in binary is performed after the
string evaluation, during health check execution. The pattern keywords to use
are "string-lf" for the log-format string and "binary-lf" for the hexadecimal
log-format string.
MINOR: checks: Improve report of unexpected errors for expect rules
TCP and HTTP expect rules may fail because of unexpected and internal
error. Mainly during log-format strings eval. The error report is improved by
this patch.
Willy Tarreau [Tue, 5 May 2020 19:49:10 +0000 (21:49 +0200)]
[RELEASE] Released version 2.2-dev7
Released version 2.2-dev7 with the following main changes :
- MINOR: version: Show uname output in display_version()
- CI: run weekly OpenSSL "no-deprecated" builds
- CLEANUP: log: fix comment of parse_logformat_string()
- DOC: Improve documentation on http-request set-src
- MINOR: ssl/cli: disallow SSL options for directory in 'add ssl crt-list'
- MINOR: ssl/cli: restrain certificate path when inserting into a directory
- MINOR: ssl: add ssl-skip-self-issued-ca global option
- BUG/MINOR: ssl: default settings for ssl server options are not used
- MINOR: config: add a global directive to set default SSL curves
- BUG/MEDIUM: http-ana: Handle NTLM messages correctly.
- DOC: internals: update the SSL architecture schema
- BUG/MINOR: tools: fix the i386 version of the div64_32 function
- BUG/MINOR: mux-fcgi/trace: fix wrong set of trace flags in fcgi_strm_add_eom()
- BUG/MINOR: http: make url_decode() optionally convert '+' to SP
- DOC: option logasap does not depend on mode
- MEDIUM: memory: make pool_gc() run under thread isolation
- MINOR: contrib: make the peers wireshark dissector a plugin
- BUG/MINOR: http-ana: Throw a 500 error if after-response ruleset fails on errors
- BUG/MINOR: check: Update server address and port to execute an external check
- MINOR: mini-clist: Add functions to iterate backward on a list
- MINOR: checks: Add a way to send custom headers and payload during http chekcs
- MINOR: server: respect warning and alert semantic
- BUG/MINOR: checks: Respect the no-check-ssl option
- BUG/MEDIUM: server/checks: Init server check during config validity check
- CLEANUP: checks: Don't export anymore init_check and srv_check_healthcheck_port
- BUG/MINOR: checks: chained expect will not properly wait for enough data
- BUG/MINOR: checks: Forbid tcp-check lines in default section as documented
- MINOR: checks: Use an enum to describe the tcp-check rule type
- MINOR: checks: Simplify connection flag parsing in tcp-check connect
- MEDIUM: checks: rewind to the first inverse expect rule of a chain on new data
- MINOR: checks: simplify tcp expect config parser
- MINOR: checks: add min-recv tcp-check expect option
- MINOR: checks: add linger option to tcp connect
- MINOR: checks: define a tcp expect type
- MEDIUM: checks: rewrite tcp-check expect block
- MINOR: checks: Stop xform buffers to null-terminated string for tcp-check rules
- MINOR: checks: add rbinary expect match type
- MINOR: checks: Simplify functions to get step id and comment
- MEDIUM: checks: capture groups in expect regexes
- MINOR: checks: Don't use a static tcp rule list head
- MEDIUM: checks: Use a non-comment rule iterator to get next rule
- MEDIUM: proxy/checks: Register a keyword to parse tcp-check rules
- MINOR: checks: Set the tcp-check rule index during parsing
- MINOR: checks: define tcp-check send type
- MINOR: checks: define a tcp-check connect type
- MEDIUM: checks: Add implicit tcp-check connect rule
- MAJOR: checks: Refactor and simplify the tcp-check loop
- MEDIUM: checks: Associate a session to each tcp-check healthcheck
- MINOR: checks/vars: Add a check scope for variables
- MEDIUM: checks: Parse custom action rules in tcp-checks
- MINOR: checks: Add support to set-var and unset-var rules in tcp-checks
- MINOR: checks: Add the sni option for tcp-check connect rules
- MINOR: checks: Add the via-socks4 option for tcp-check connect rules
- MINOR: checks: Add the alpn option for tcp-check connect rules
- MINOR: ssl: Export a generic function to parse an alpn string
- MINOR: checks: Add the default option for tcp-check connect rules
- MINOR: checks: Add the addr option for tcp-check connect rule
- MEDIUM: checks: Support expression to set the port
- MEDIUM: checks: Support log-format strings for tcp-check send rules
- MINOR: log: Don't depends on a stream to process samples in log-format string
- MINOR: log: Don't systematically set LW_REQ when a sample expr is added
- MEDIUM: checks: Add a shared list of tcp-check rules
- MINOR: sample: add htonl converter
- MINOR: sample: add cut_crlf converter
- MINOR: sample: add ltrim converter
- MINOR: sample: add rtrim converter
- MINOR: checks: Use a name for the healthcheck status enum
- MINOR: checks: Add option to tcp-check expect rules to customize error status
- MINOR: checks: Merge tcp-check comment rules with the others at config parsing
- MINOR: checks: Add a sample fetch to extract a block from the input check buffer
- MEDIUM: checks: Add on-error/on-success option on tcp-check expect rules
- MEDIUM: checks: Add status-code sample expression on tcp-check expect rules
- MINOR: checks: Relax the default option for tcp-check connect rules
- MEDIUM: checks: Add a list of vars to set before executing a tpc-check ruleset
- MINOR: checks: Export the tcpcheck_eval_ret enum
- MINOR: checks: Use dedicated function to handle onsuccess/onerror messages
- MINOR: checks: Support custom functions to eval a tcp-check expect rules
- MEDIUM: checks: Implement redis check using tcp-check rules
- MEDIUM: checks: Implement ssl-hello check using tcp-check rules
- MEDIUM: checks: Implement smtp check using tcp-check rules
- MEDIUM: checks: Implement postgres check using tcp-check rules
- MEDIUM: checks: Implement MySQL check using tcp-check rules
- MEDIUM: checks: Implement LDAP check using tcp-check rules
- MEDIUM: checks: Implement SPOP check using tcp-check rules
- MINOR: server/checks: Move parsing of agent keywords in checks.c
- MINOR: server/checks: Move parsing of server check keywords in checks.c
- MEDIUM: checks: Implement agent check using tcp-check rules
- REGTEST: Adapt regtests about checks to recent changes
- MINOR: Produce tcp-check info message for pure tcp-check rules only
- MINOR: checks: Add an option to set success status of tcp-check expect rules
- MINOR: checks: Improve log message of tcp-checks on success
- MINOR: proxy/checks: Move parsing of httpchk option in checks.c
- MINOR: proxy/checks: Move parsing of tcp-check option in checks.c
- MINOR: proxy/checks: Register a keyword to parse http-check rules
- MINOR: proxy/checks: Move parsing of external-check option in checks.c
- MINOR: proxy/checks: Register a keyword to parse external-check rules
- MEDIUM: checks: Use a shared ruleset to store tcp-check rules
- MINOR: checks: Use an indirect string to represent the expect matching string
- MINOR: checks: Introduce flags to configure in tcp-check expect rules
- MINOR: standard: Add my_memspn and my_memcspn
- MINOR: checks: Add a reverse non-comment rule iterator to get last rule
- MAJOR: checks: Implement HTTP check using tcp-check rules
- MINOR: checks: Make resume conditions more explicit in tcpcheck_main()
- MINOR: connection: Add macros to know if a conn or a cs uses an HTX mux
- MEDIUM: checks: Refactor how data are received in tcpcheck_main()
- MINOR: checks/obj_type: Add a new object type for checks
- BUG/MINOR: obj_type: Handle stream object in obj_base_ptr() function
- MINOR: checks: Use the check as origin when a session is created
- MINOR: checks: Add a mux proto to health-check and tcp-check connect rule
- MINOR: connection: Add a function to install a mux for a health-check
- MAJOR: checks: Use the best mux depending on the protocol for health checks
- MEDIUM: checks: Implement default TCP check using tcp-check rules
- MINOR: checks: Remove unused code about pure TCP checks
- CLEANUP: checks: Reorg checks.c file to be more readable
- REGTEST: Fix reg-tests about health-checks to adapt them to recent changes
- MINOR: ist: Add a function to retrieve the ist pointer
- MINOR: checks: Use ist API as far as possible
- BUG/MEDIUM: checks: Be sure to subscribe for sends if outgoing data remains
- MINOR: checks: Use a tree instead of a list to store tcp-check rulesets
- BUG/MINOR: checks: Send the right amount of outgoing data for HTTP checks
- REGTEST: Add scripts to test based tcp-check health-checks
- Revert "MEDIUM: checks: capture groups in expect regexes"
- DOC: Add documentation about comments for tcp-check and http-check directives
- DOC: Fix the tcp-check and http-check directives layout
- BUG/MEDIUM: checks: Use the mux protocol specified on the server line
- MINOR: checks: Support mux protocol definition for tcp and http health checks
- BUG/MINOR: mux-fcgi: Be sure to have a connection as session's origin to use it
- MINOR: checks: Support list of status codes on http-check expect rules
- BUG/MEDIUM: checks: Unsubscribe to mux events when a conn-stream is destroyed
- REGTEST: Add a script to validate agent checks
- BUG/MINOR: server: Fix server_finalize_init() to avoid unused variable
- BUG/MEDIUM: checks: unsubscribe for events on the old conn-stream on connect
- BUG/MINOR: checks: Only use ssl_sock_is_ssl() if compiled with SSL support
- BUG/MINOR: checks/server: use_ssl member must be signed
- BUG/MEDIUM: sessions: Always pass the mux context as argument to destroy a mux
- BUG/MEDIUM: checks: Destroy the conn-stream before the session
- BUG/MINOR: checks: Fix PostgreSQL regex on the authentication packet
- CI: cirrus-ci: remove reg-tests/checks/tcp-check-ssl.vtc on CentOS 6
- MINOR: checks: Support HTTP/2 version (without '.0') for http-check send rules
- MINOR: checks: Use ver keyword to specify the HTTP version for http checks
- BUG/MINOR: checks: Remove wrong variable redeclaration
- BUG/MINOR: checks: Properly handle truncated mysql server messages
- CLEANUP: checks: Remove unused code when ldap server message is parsed
- MINOR: checks: Make the use of the check's server more explicit on connect
- BUG/MINOR: checks: Avoid incompatible cast when a binary string is parsed
- BUG/MINOR: checks: Remove bad call to free() when an expect rule is parsed
- BUG/MINOR: checks: Don't lose warning on proxy capability
- MINOR: log: Add "Tu" timer
- BUG/MINOR: checks: Set the output buffer length before calling parse_binary()
- BUG/MEDIUM: mux-h1: make sure we always have a timeout on front connections
- REGTEST: ssl: test the client certificate authentication
- DOC: give a more accurate description of what check does
- BUG/MEDIUM: capture: capture-req/capture-res converters crash without a stream
- BUG/MEDIUM: capture: capture.{req,res}.* crash without a stream
- BUG/MEDIUM: http: the "http_first_req" sample fetch could crash without a steeam
- BUG/MEDIUM: http: the "unique-id" sample fetch could crash without a steeam
- CLEANUP: http: add a few comments on certain functions' assumptions about streams
- BUG/MEDIUM: sample: make the CPU and latency sample fetches check for a stream
- MINOR: http-htx: Export functions to update message authority and host
- MINOR: checks: Don't support multiple host header for http-check send rule
- MINOR: checks: Skip some headers for http-check send rules
- MINOR: checks: Keep the Host header and the request uri synchronized
- CLEANUP: checks: Fix checks includes
- DOC: Fix send rules in the http-check connect example
- DOC: Add more info about request formatting in http-check send description
- REGTEST: http-rules: Require PCRE or PCRE2 option to run map_redirect script
- REGTEST: ssl: remove curl from the "add ssl crt-list" test
- REGTEST: ssl: improve the "set ssl cert" test
- CLEANUP: ssl: silence a build warning when threads are disabled
- BUG/MEDIUM: listener: mark the thread as not stuck inside the loop
- MINOR: threads: export the POSIX thread ID in panic dumps
- BUG/MINOR: debug: properly use long long instead of long for the thread ID
- BUG/MEDIUM: shctx: really check the lock's value while waiting
- BUG/MEDIUM: shctx: bound the number of loops that can happen around the lock
- MINOR: stream: report the list of active filters on stream crashes
- BUG/MEDIUM: mux-fcgi: Return from detach if server don't keep the connection
- BUG/MEDIUM: mux_fcgi: Free the FCGI connection at the end of fcgi_release()
- BUG/MEDIUM: mux-fcgi: Fix wrong test on FCGI_CF_KEEP_CONN in fcgi_detach()
- BUG/MEDIUM: connections: force connections cleanup on server changes
- BUG/MEDIUM: h1: Don't compare host and authority if only h1 headers are parsed
- BUG/MEDIUM: ssl: fix the id length check within smp_fetch_ssl_fc_session_id()
- CLEANUP: connections: align function declaration
- BUG/MINOR: sample: Set the correct type when a binary is converted to a string
- MEDIUM: checks/http-fetch: Support htx prefetch from a check for HTTP samples
- DOC: Document the log-format parameter for tcp-check send/send-binary rules
- MINOR: checks: Add support of payload-based sample fetches
- MINOR: checks: Add support of be_id, be_name, srv_id and srv_name sample fetches
- MINOR: checks: Add support of server side ssl sample fetches
- MINOR: checks: Add support of HTTP response sample fetches
- MINOR: http-htx: Support different methods to look for header names
- MINOR: checks: Set by default expect rule status to UNKNOWN during parsing
- BUG/MINOR: checks: Support multiple HTTP expect rules
- REGTEST: checks: Fix sync condition for agent-check
- MEDIUM: checks: Support matching on headers for http-check expect rules
- MINOR: lua: allow changing port with set_addr
- BUG/MINOR: da: Fix HTX message prefetch
- BUG/MINOR: wurfl: Fix HTX message prefetch
- BUG/MINOR: 51d: Fix HTX message prefetch
- MINOR: ist: add istadv() function
- MINOR: ist: add istissame() function
- MINOR: istbuf: add ist2buf() function
- BUG/MINOR: threads: fix multiple use of argument inside HA_ATOMIC_CAS()
- BUG/MINOR: threads: fix multiple use of argument inside HA_ATOMIC_UPDATE_{MIN,MAX}()
- DOC: update intro.txt for 2.2
- DOC: intro: add a contacts section
Willy Tarreau [Tue, 5 May 2020 14:13:36 +0000 (16:13 +0200)]
BUG/MINOR: threads: fix multiple use of argument inside HA_ATOMIC_UPDATE_{MIN,MAX}()
Just like in previous patch, it happens that HA_ATOMIC_UPDATE_MIN() and
HA_ATOMIC_UPDATE_MAX() would evaluate the (val) argument up to 3 times.
However this time it affects both thread and non-thread versions. It's
strange because the copy was properly performed for the (new) argument
in order to avoid this. Anyway it was done for the "val" one as well.
A quick code inspection showed that this currently has no effect as
these macros are fairly limited in usage.
It would be best to backport this for long-term stability (till 1.8)
but it will not fix an existing bug.
Willy Tarreau [Tue, 5 May 2020 13:58:00 +0000 (15:58 +0200)]
BUG/MINOR: threads: fix multiple use of argument inside HA_ATOMIC_CAS()
When threads are disabled, HA_ATOMIC_CAS() becomes a simple compound
expression. However this expression presents a problem, which is that
its arguments are evaluated multiple times, once for the comparison
and once again for the assignement. This presents a risk of performing
some side-effect operations twice in the non-threaded case (e.g. in
case of auto-increment or function return).
The macro was rewritten using local copies for arguments like the
other macros do.
Fortunately a complete inspection of the code indicates that this case
currently never happens. It was however responsible for the strict-aliasing
warning emitted when building fd.c without threads but with 64-bit CAS.
This may be backported as far as 1.8 though it will not fix any existing
bug and is more of a long-term safety measure in case a future fix would
depend on this behavior.
An additional argument has been added to smp_prefetch_htx() function in the
commit 778f5ed47 ("MEDIUM: checks/http-fetch: Support htx prefetch from a check
for HTTP samples"). But forgot to update call in 51d module.
An additional argument has been added to smp_prefetch_htx() function in the
commit 778f5ed47 ("MEDIUM: checks/http-fetch: Support htx prefetch from a check
for HTTP samples"). But forgot to update call in wurfl module.
An additional argument has been added to smp_prefetch_htx() function in the
commit 778f5ed47 ("MEDIUM: checks/http-fetch: Support htx prefetch from a check
for HTTP samples"). But forgot to update call in da module.
the name pattern (name ...) is mandatory but the value pattern (value ...) is
optionnal. If not specified, only the header presence is verified. <meth> is the
matching method, applied on the header name or the header value. Supported
matching methods are:
If not specified, exact matching method is used. If the "log-format" option is
used, the pattern (<name> or <value>) is evaluated as a log-format string. This
option cannot be used with the regex matching method. Finally, by default, the
header value is considered as comma-separated list. Each part may be tested. The
"full" option may be used to test the full header line. Note that matchings are
case insensitive on the header names.
REGTEST: checks: Fix sync condition for agent-check
agent-check.vtc script fails time to time because the 2nd cli command is sent to
early. Waiting for the connection close in the s1 server should be enough to be
sure the server state is updated.
BUG/MINOR: checks: Support multiple HTTP expect rules
For an http-check ruleset, it should be allowed to set a chain of expect
rules. But an error is triggered during the post-parsing because of a wrong
test, inherited from the evaluation mode before the refactoring.
MINOR: checks: Set by default expect rule status to UNKNOWN during parsing
The status (ok, error and timeout) of an TCP or HTTP expect rule are set to
HCHK_STATUS_UNKNOWN by default, when not specified, during the configuration
parsing. This does not change the default status used for a terminal expect rule
(ok=L7OK, err=L7RSP and tout=L7TOUT). But this way, it is possible to know if a
specific status was forced by config or not.
MINOR: checks: Add support of HTTP response sample fetches
HTPP sample fetches acting on the response can now be called from any sample
expression or log-format string in a tcp-check based ruleset. To avoid any
ambiguities, all these sample fetches are in the check scope, for instance
check.hdr() or check.cook().
MINOR: checks: Add support of server side ssl sample fetches
SSL sample fetches acting on the server connection can now be called from any
sample expression or log-format string in a tcp-check based ruleset. ssl_bc and
ssl_bc_* sample fetches are concerned.
MINOR: checks: Add support of be_id, be_name, srv_id and srv_name sample fetches
It is now possible to call be_id, be_name, srv_id and srv_name sample fetches
from any sample expression or log-format string in a tcp-check based ruleset.
MINOR: checks: Add support of payload-based sample fetches
It is now possible to call check.payload(), check.payload_lv() and check.len()
sample fetches from any sample expression or log-format string in a tcp-check
based ruleset. In fact, check.payload() was already added. But instead of having
a specific function to handle this sample fetch, we use the same than
req.payload().
These sample fetches act on the check input buffer, containing data received for
the server. So it should be part of or after an expect rule, but before any send
rule. Because the input buffer is cleared at this stage.
MEDIUM: checks/http-fetch: Support htx prefetch from a check for HTTP samples
Some HTTP sample fetches will be accessible from the context of a http-check
health check. Thus, the prefetch function responsible to return the HTX message
has been update to handle a check, in addition to a channel. Both cannot be used
at the same time. So there is no ambiguity.
William Dauchy [Mon, 4 May 2020 11:52:40 +0000 (13:52 +0200)]
CLEANUP: connections: align function declaration
srv_cleanup_connections() is supposed to be static, so mark it as so.
This patch should be backported where commit 6318d33ce625
("BUG/MEDIUM: connections: force connections cleanup on server changes")
will be backported, that is to say v1.9 to v2.1.
Fixes: 6318d33ce625 ("BUG/MEDIUM: connections: force connections cleanup
on server changes") Signed-off-by: William Dauchy <w.dauchy@criteo.com>
Dragan Dosen [Mon, 4 May 2020 07:07:28 +0000 (09:07 +0200)]
BUG/MEDIUM: ssl: fix the id length check within smp_fetch_ssl_fc_session_id()
After we call SSL_SESSION_get_id(), the length of the id in bytes is
stored in "len", which was never checked. This could cause unexpected
behavior when using the "ssl_fc_session_id" or "ssl_bc_session_id"
fetchers (eg. the result can be an empty value).
The issue was introduced with commit 105599c ("BUG/MEDIUM: ssl: fix
several bad pointer aliases in a few sample fetch functions").
This patch must be backported to 2.1, 2.0, and 1.9.
BUG/MEDIUM: h1: Don't compare host and authority if only h1 headers are parsed
When only request headers are parsed, the host header should not be compared to
the request authority because no start-line was parsed. Thus there is no
authority.
Till now this bug was hidden because this parsing mode was only used for the
response in the FCGI multiplexer. Since the HTTP checks refactoring, the request
headers may now also be parsed without the start-line.
This patch fixes the issue #610. It must be backported to 2.1.
William Dauchy [Sat, 2 May 2020 19:52:36 +0000 (21:52 +0200)]
BUG/MEDIUM: connections: force connections cleanup on server changes
I've been trying to understand a change of behaviour between v2.2dev5 and
v2.2dev6. Indeed our probe is regularly testing to add and remove
servers on a given backend such as:
-> curl on the corresponding frontend: reply from server:31255
# echo "set server be_foo/srv1 addr 10.236.139.34 port 31257" | sudo socat stdio /var/lib/haproxy/stats
IP changed from '0.0.0.0' to '10.236.139.34', port changed from '0' to '31257' by 'stats socket command'
# echo "set server be_foo/srv1 weight 256" | sudo socat stdio /var/lib/haproxy/stats
# echo "set server be_foo/srv1 check-port 8500" | sudo socat stdio /var/lib/haproxy/stats
health check port updated.
# echo "set server be_foo/srv1 state ready" | sudo socat stdio /var/lib/haproxy/stats
# echo "show servers state be_foo" | sudo socat stdio /var/lib/haproxy/stats
113 be_foo 1 srv0 10.236.139.34 2 0 1 1 105 15 3 4 6 0 0 0 - 31255 -
113 be_foo 2 srv1 10.236.139.34 2 0 256 256 2319 15 3 2 6 0 0 0 - 31257 -
-> curl on the corresponding frontend: reply for server:31257
(notice the difference of weight)
# echo "set server be_foo/srv1 state maint" | sudo socat stdio /var/lib/haproxy/stats
# echo "set server be_foo/srv1 addr 0.0.0.0 port 0" | sudo socat stdio /var/lib/haproxy/stats
IP changed from '10.236.139.34' to '0.0.0.0', port changed from '31257' to '0' by 'stats socket command'
# echo "show servers state be_foo" | sudo socat stdio /var/lib/haproxy/stats
113 be_foo 1 srv0 10.236.139.34 2 0 1 1 263 15 3 4 6 0 0 0 - 31255 -
113 be_foo 2 srv1 0.0.0.0 0 1 256 256 0 15 3 0 14 0 0 0 - 0 -
-> curl on the corresponding frontend: reply from server:31255
# echo "set server be_foo/srv1 addr 10.236.139.34 port 31256" | sudo socat stdio /var/lib/haproxy/stats
IP changed from '0.0.0.0' to '10.236.139.34', port changed from '0' to '31256' by 'stats socket command'
# echo "set server be_foo/srv1 weight 256" | sudo socat stdio /var/lib/haproxy/stats
# echo "set server be_foo/srv1 check-port 8500" | sudo socat stdio /var/lib/haproxy/stats
health check port updated.
# echo "set server be_foo/srv1 state ready" | sudo socat stdio /var/lib/haproxy/stats
# echo "show servers state be_foo" | sudo socat stdio /var/lib/haproxy/stats
113 be_foo 1 srv0 10.236.139.34 2 0 1 1 105 15 3 4 6 0 0 0 - 31255 -
113 be_foo 2 srv1 10.236.139.34 2 0 256 256 2319 15 3 2 6 0 0 0 - 31256 -
-> curl on the corresponding frontend: reply from server:31257 (!)
Here we indeed would expect to get an anver from server:31256. The issue
is highly linked to the usage of `pool-purge-delay`, with a value which
is higher than the duration of the test, 10s in our case.
a git bisect between dev5 and dev6 seems to show commit 079cb9af22da6 ("MEDIUM: connections: Revamp the way idle connections are killed")
being the origin of this new behaviour.
So if I understand the later correctly, it seems that it was more a
matter of chance that we did not saw the issue earlier.
My patch proposes to force clean idle connections in the two following
cases:
- we set a (still running) server to maintenance
- we change the ip/port of a server
This commit should be backported to 2.1, 2.0, and 1.9.
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
BUG/MEDIUM: mux-fcgi: Fix wrong test on FCGI_CF_KEEP_CONN in fcgi_detach()
When a stream is detached from its connection, we try to move the connection in
an idle list to keep it opened, the session one or the server one. But it must
only be done if there is no connection error and if we want to keep it
open. This last statement is true if FCGI_CF_KEEP_CONN flag is set. But the test
is inverted at the stage.
BUG/MEDIUM: mux-fcgi: Return from detach if server don't keep the connection
When the last stream is detached from a FCGI connection, if the server don't add
the connection in its idle list, the connection is destroyed. Thus it is
important to exist immediately from the detach function. A return statement is
missing here.
This bug was introduced in the commit 2444aa5b6 ("MEDIUM: sessions: Don't be
responsible for connections anymore.").
Willy Tarreau [Fri, 1 May 2020 14:57:02 +0000 (16:57 +0200)]
MINOR: stream: report the list of active filters on stream crashes
Now we very rarely catch spinning streams, and whenever we catch one it
seems a filter is involved, but we currently report no info about them.
Let's print the list of enabled filters on the stream with such a crash
to help with the reports. A typical output will now look like this:
[ALERT] 121/165908 (1110) : A bogus STREAM [0x7fcaf4016a60] is spinning at 2 calls per second and refuses to die, aborting now! Please report this error to developers [strm=0x7fcaf4016a60 src=127.0.0.1 fe=l1 be=l1 dst=<CACHE> rqf=6dc42000 rqa=48000 rpf=a0040223 rpa=24000000 sif=EST,10008 sib=DIS,80110 af=(nil),0 csf=0x7fcaf4023c00,10c000 ab=0x7fcaf40235f0,4 csb=(nil),0 cof=0x7fcaf4016610,1300:H1(0x7fcaf4016840)/RAW((nil))/tcpv4(29) cob=(nil),0:NONE((nil))/NONE((nil))/NONE(0) filters={0x7fcaf4016fb0="cache store filter", 0x7fcaf4017080="compression filter"}]
Willy Tarreau [Fri, 1 May 2020 11:15:32 +0000 (13:15 +0200)]
BUG/MEDIUM: shctx: bound the number of loops that can happen around the lock
Given that a "count" value of 32M was seen in _shctx_wait4lock(), it
is very important to prevent this from happening again. It's absolutely
essential to prevent the value from growing unbounded because with an
increase of the number of threads, the number of successive failed
attempts will necessarily grow.
Instead now we're scanning all 2^p-1 values from 3 to 255 and are
bounding to count to 255 so that in the worst case each thread tries an
xchg every 255 failed read attempts. That's one every 4 on average per
thread when there are 64 threads, which corresponds to the initial count
of 4 for the first attempt so it seems like a reasonable value to keep a
low latency.
The bug was introduced with the shctx entries in 1.5 so the fix must
be backported to all versions. Before 1.8 the function was called
_shared_context_wait4lock() and was in shctx.c.
The correct thing to do is to compare the value again at each loop.
This way it makes sure to mostly perform read accesses on the shared
cache line without writing too often, and to be ready fast enough to
try to grab the lock. And we must not increase the count on success
either!
Unfortunately I'd have expected to see a performance boost on the cache
with this but there was absolutely no change, so it's very likely that
these issues only happen once in a while and are sufficient to derail
the process when they strike, but not to have a permanent performance
impact.
The bug was introduced with the shctx entries in 1.5 so the fix must
be backported to all versions. Before 1.8 the function was called
_shared_context_wait4lock() and was in shctx.c.
Willy Tarreau [Fri, 1 May 2020 10:26:03 +0000 (12:26 +0200)]
BUG/MINOR: debug: properly use long long instead of long for the thread ID
I changed my mind twice on this one and pushed after the last test with
threads disabled, without re-enabling long long, causing this rightful
build warning.
This needs to be backported if the previous commit ff64d3b027 ("MINOR:
threads: export the POSIX thread ID in panic dumps") is backported as
well.
Willy Tarreau [Fri, 1 May 2020 09:28:49 +0000 (11:28 +0200)]
MINOR: threads: export the POSIX thread ID in panic dumps
It is very difficult to map a panic dump against a gdb thread dump
because the thread numbers do not match. However gdb provides the
pthread ID but this one is supposed to be opaque and not to be cast
to a scalar.
This patch provides a fnuction, ha_get_pthread_id() which retrieves
the pthread ID of the indicated thread and casts it to an unsigned
long long so as to lose the least possible amount of information from
it. This is done cleanly using a union to maintain alignment so as
long as these IDs are stored on 1..8 bytes they will be properly
reported. This ID is now presented in the panic dumps so it now
becomes possible to map these threads. When threads are disabled,
zero is returned. For example, this is a panic dump:
(gdb) info thr
Id Target Id Frame
* 1 Thread 0x7fe92b825180 (LWP 15234) 0x00007fe92ba81d6b in raise () from /lib64/libc.so.6
2 Thread 0x7fe92b6e6700 (LWP 15235) 0x00007fe92bb56a56 in epoll_wait () from /lib64/libc.so.6
3 Thread 0x7fe92a6e4700 (LWP 15237) 0x00007fe92bb56a56 in epoll_wait () from /lib64/libc.so.6
4 Thread 0x7fe92aee5700 (LWP 15236) 0x00007fe92bb56a56 in epoll_wait () from /lib64/libc.so.6
We can clearly see that while threads 1 and 2 are the same, gdb's
threads 3 and 4 respectively are haproxy's threads 4 and 3.
This may be backported to 2.0 as it removes some confusion in github issues.
Willy Tarreau [Fri, 1 May 2020 07:51:11 +0000 (09:51 +0200)]
BUG/MEDIUM: listener: mark the thread as not stuck inside the loop
We tried hard to make sure we report threads as not stuck at various
crucial places, but one of them is special, it's the listener_accept()
function. The reason it is special is because it will loop a certain
number of times (default: 64) accepting incoming connections, allocating
resources, dispatching them to other threads or running L4 rules on them,
and while all of this is supposed to be extremely fast, when the machine
slows down or runs low on memory, the expectedly small delays in malloc()
caused by contention with other threads can quickly accumulate and suddenly
become critical to the point of triggering the watchdog. Furthermore, it
is technically possible to trigger this by pure configuration by setting
a huge tune.maxaccept value, which should not be possible.
Given that each operation isn't related to the same task but to a different
one each time, it is appropriate to mark the thread as not stuck each time
it accepts new work that possibly gets dispatched to other threads which
execute it.
This looks like this could be a good reason for the issue reported in
issue #388.
Willy Tarreau [Fri, 1 May 2020 09:38:39 +0000 (11:38 +0200)]
CLEANUP: ssl: silence a build warning when threads are disabled
Building without threads now shows this warning:
src/ssl_sock.c: In function 'cli_io_handler_commit_cert':
src/ssl_sock.c:12121:24: warning: unused variable 'bind_conf' [-Wunused-variable]
struct bind_conf *bind_conf = ckchi->bind_conf;
^~~~~~~~~
This is because the variable is needed only to unlock the structure, and
the unlock operation does nothing in this case. Let's mark the variable
__maybe_unused for this, but it would be convenient in the long term if
we could make the thread macros pretend they consume the argument so that
this remains less visible outside.
Improve the test by removing the curl command and using the same proxy
chaining technique as in commit 3ed722f ("REGTEST: ssl: remove curl from
the "add ssl crt-list" test").
A 3rd request was added which must fail, to ensure that the SNI was
effectively removed from HAProxy.
This patch also adds timeouts in the default section, logs on stderr and
fix some indentation issues.
REGTEST: ssl: remove curl from the "add ssl crt-list" test
Using curl for SSL tests can be a problem if it wasn't compiled with the
right SSL library and if it didn't share any cipher with HAProxy. To
have more robust tests we now use HAProxy as an SSL client, so we are
sure that the client and the server share the same SSL requirements.
This patch also adds timeouts in the default section, logs on stderr and
fix some indentation issues.
DOC: Add more info about request formatting in http-check send description
Only one Host header can be defined and some headers are automatically skipped
(Connection, Content-Length and Transfer-Encoding). In addition, a note about
the synchronisation of the Host header value and the request uri has been added.
MINOR: checks: Keep the Host header and the request uri synchronized
Because in HTTP, the host header and the request authority, if any, must be
identical, we keep both synchornized. It means the right flags are set on the
HTX statrt-line calling http_update_host(). There is no header when it happens,
but it is not an issue. Then, if a Host header is inserted,
http_update_authority() is called.
Note that for now, the host header is not automatically added when required.
MINOR: checks: Skip some headers for http-check send rules
Connection, content-length and transfer-encoding headers are ignored for
http-check send rules. For now, the keep-alive is not supported and the
"connection: close" header is always added to the request. And the
content-length header is automatically added.
BUG/MEDIUM: sample: make the CPU and latency sample fetches check for a stream
cpu_calls, cpu_ns_avg, cpu_ns_tot, lat_ns_avg and lat_ns_tot depend on the
stream to find the current task and must check for it or they may cause a
crash if misused or used in a log-format string after commit 5f940703b3
("MINOR: log: Don't depends on a stream to process samples in log-format
string").
BUG/MEDIUM: http: the "unique-id" sample fetch could crash without a steeam
Since commit 5f940703b3 ("MINOR: log: Don't depends on a stream to process
samples in log-format string") it has become quite obvious that a few sample
fetch functions and converters were still heavily dependent on the presence
of a stream without testing for it.
The unique-id sample fetch function, if called without a stream, will result
in a crash.
This fix adds a check for the stream's existence, and should be backported
to all stable versions up to 1.7.
BUG/MEDIUM: http: the "http_first_req" sample fetch could crash without a steeam
Since commit 5f940703b3 ("MINOR: log: Don't depends on a stream to process
samples in log-format string") it has become quite obvious that a few sample
fetch functions and converters were still heavily dependent on the presence
of a stream without testing for it.
The http_first_req sample fetch function, if called without a stream, will
result in a crash.
This fix adds a check for the stream's existence, and should be backported
to all stable versions up to 1.6.
BUG/MEDIUM: capture: capture.{req,res}.* crash without a stream
Since commit 5f940703b3 ("MINOR: log: Don't depends on a stream to process
samples in log-format string") it has become quite obvious that a few sample
fetch functions and converters were still heavily dependent on the presence
of a stream without testing for it.
The capture.req.hdr, capture.res.hdr, capture.req.method, capture.req.uri,
capture.req.ver and capture.res.ver sample fetches used to assume the
presence of a stream, which is not necessarily the case (especially after
the commit above) and would crash haproxy if incorrectly used. Let's make
sure they check for this stream.
This fix adds a check for the stream's existence, and should be backported
to all stable versions up to 1.6.
BUG/MEDIUM: capture: capture-req/capture-res converters crash without a stream
Since commit 5f940703b3 ("MINOR: log: Don't depends on a stream to process
samples in log-format string") it has become quite obvious that a few sample
fetch functions and converters were still heavily dependent on the presence
of a stream without testing for it.
The capture-req and capture-res converters were in this case and could
crash the process if misused.
This fix adds a check for the stream's existence, and should be backported
to all stable versions up to 1.6.
DOC: give a more accurate description of what check does
The documentation for check implies that without an application
level check configured, it only enables simple tcp checks. What it
actually does is verify that the configured transport layer is available,
and that optional application level checks succeed.
REGTEST: ssl: test the client certificate authentication
This reg-test tests the client auth feature of HAProxy for both the
backend and frontend section with a CRL list.
This reg-test uses 2 chained listeners because vtest does not handle the
SSL. Test the frontend client auth and the backend side at the same
time.
It sends 3 requests: one with a correct certificate, one with an expired
one and one which was revoked. The client then checks if we received the
right one with the right error.
Certificates, CA and CRL are expiring in 2050 so it should be fine for
the CI.
This test could be backported as far as HAProxy 1.6
BUG/MEDIUM: mux-h1: make sure we always have a timeout on front connections
Mux-h1 currently heavily relies on the presence of an upper stream, even
when waiting for a new request after one is being finished, and it's that
upper stream that's in charge of request and keep-alive timeouts for now.
But since recent commit 493d9dc6ba ("MEDIUM: mux-h1: do not blindly wake
up the tasklet at end of request anymore") that assumption was broken as
the purpose of this change was to avoid initiating processing of a request
when there's no data in the buffer. The side effect is that there's no more
timeout to handle the front connection, resulting in dead front connections
stacking up as clients get kicked off the net.
This fix makes sure we always enable the timeout when there's no stream
attached to the connection. It doesn't do this for back connections since
they may purposely be left idle.
No backport is needed as this bug was introduced in 2.2-dev4.
BUG/MINOR: checks: Set the output buffer length before calling parse_binary()
A bug was introduced in the commit 2edcd4cbd ("BUG/MINOR: checks: Avoid
incompatible cast when a binary string is parsed"). The length of the
destination buffer must be set before call the parse_binary() function.
It can be sometimes useful to measure total time of a request as seen
from an end user, including TCP/TLS negotiation, server response time
and transfer time. "Tt" currently provides something close to that, but
it also takes client idle time into account, which is problematic for
keep-alive requests as idle time can be very long. "Ta" is also not
sufficient as it hides TCP/TLS negotiationtime. To improve that, introduce
a "Tu" timer, without idle time and everything else. It roughly estimates
time spent time spent from user point of view (without DNS resolution
time), assuming network latency is the same in both directions.
BUG/MINOR: checks: Don't lose warning on proxy capability
When a tcp-check line is parsed, a warning may be reported if the keyword is
used for a frontend. The return value must be used to report it. But this info
is lost before the end of the function.
BUG/MINOR: checks: Avoid incompatible cast when a binary string is parsed
parse_binary() function must be called with a pointer on an integer. So don't
pass a pointer on a size_t element, casting it to a pointer on a integer.
MINOR: checks: Make the use of the check's server more explicit on connect
The variable s, pointing on the check server, may be null when a connection is
openned. It happens for email alerts. To avoid ambiguities, its use is now more
explicit. Comments have been added at some places and tests on the variable have
been added elsewhere (useless but explicit).
BUG/MINOR: checks: Properly handle truncated mysql server messages
If a message is not fully received from a mysql server, depending on last_read
value, an error must be reported or we must wait for more data. The first if
statement must not check last_read.
MINOR: checks: Support HTTP/2 version (without '.0') for http-check send rules
The version is partially parsed to set the flag HTX_SL_F_VER_11 on the HTX
message. But exactly 8 chars is expected. So if "HTTP/2" is specified, the flag
is not set. Thus, the version parsing has been updated to handle "HTTP/2" and
"HTTP/2.0" the same way.
BUG/MINOR: checks: Fix PostgreSQL regex on the authentication packet
For PostgreSQL health check, there is a regex on the backend authentication
packet. It must match to succeed. But it exists 6 types of authentication
packets and the regex only matches the first one (AuthenticationOK). This patch
fixes the regex to match all authentication packets.
BUG/MEDIUM: checks: Destroy the conn-stream before the session
At the end of a tcp-check based health check, if there is still a connection
attached to the check, it must be closed. But it must be done before releasing
the session, because the session may still be referenced by the mux. For
instance, an h2 stream may still have a reference on the session.
BUG/MEDIUM: sessions: Always pass the mux context as argument to destroy a mux
This bug was introduced by the commit 2444aa5b ("MEDIUM: sessions: Don't be
responsible for connections anymore."). In session_check_idle_conn(), when the
mux is destroyed, its context must be passed as argument instead of the
connection.
BUG/MEDIUM: checks: unsubscribe for events on the old conn-stream on connect
When a new connection is established, if an old connection is still attached to
the current check, it must be detroyed. When it happens, the old conn-stream
must be used to unsubscribe for events, not the new one.
BUG/MEDIUM: checks: Unsubscribe to mux events when a conn-stream is destroyed
Since the tcp-check based heath checks uses the best multuplexer for a
connection, the mux-pt is no longer the only possible choice. So events
subscriptions and unsubscriptions must be done with the mux.
MINOR: checks: Support list of status codes on http-check expect rules
It is now possible to match on a comma-separated list of status codes or range
of codes. In addtion, instead of a string comparison to match the response's
status code, a integer comparison is performed. Here is an example:
BUG/MINOR: mux-fcgi: Be sure to have a connection as session's origin to use it
When default parameters are set in a request message, we get the client
connection using the session's origin. But it is not necessarily a
conncection. For instance, for health checks, thanks to recent changes, it may
be a check object. At this step, the client connection may be NULL. Thus, we
must be sure to have a client connection before using it.
MINOR: checks: Support mux protocol definition for tcp and http health checks
It is now possible to force the mux protocol for a tcp-check based health check
using the server keyword "check-proto". If set, this parameter overwrites the
server one.
In the same way, a "proto" parameter has been added for tcp-check and http-check
connect rules. If set, this mux protocol overwrites all others for the current
connection.
BUG/MEDIUM: checks: Use the mux protocol specified on the server line
First, when a server health check is initialized, it inherits the mux protocol
from the server if it is not already specified. Because there is no option to
specify the mux protocol for the checks, it is always inherited from the server
for now.
Then, if the connect rule is configured to use the server options, the mux
protocol of the check is used, if defined. Of course, if a mux protocol is
already defined for the connect rule, it is used in priority. But for now, it is
not possible.
Thus, if a server is configured to use, for instance, the h2 protocol, it is
possible to do the same for the health-checks.
DOC: Add documentation about comments for tcp-check and http-check directives
The documentation about the comment argument for some tcp-check and http-check
directives was missing. As well as the description of "tcp-check comment" and
"http-check comment" directives.
Captures in comment was only used when a tcp-check expect based on a negative
regex matching failed to eventually report what was captured while it was not
expected. It is a bit far-fetched to be useable IMHO. on-error and on-success
log-format strings are far more usable. For now there is few check sample
fetches (in fact only one...). But it could be really powerful to report info in
logs.
BUG/MINOR: checks: Send the right amount of outgoing data for HTTP checks
HTTP health-checks now use HTX multiplexers. So it is important to really send
the amount of outgoing data for such checks because the HTX buffers appears
always full.
MINOR: checks: Use a tree instead of a list to store tcp-check rulesets
Since all tcp-check rulesets are globally stored, it is a problem to use
list. For configuration with many backends, the lookups in list may be costly
and slow downs HAProxy startup. To solve this problem, tcp-check rulesets are
now stored in a tree.
BUG/MEDIUM: checks: Be sure to subscribe for sends if outgoing data remains
When some data are scheduled to be sent, we must be sure to subscribe for sends
if nothing was sent. Because of a bug, when nothing was sent, connection errors
are checks. If no error is found, we exit, waiting for more data, without any
subcription on send events.
Instead of accessing directly to the ist fields, the ist API is used instead. To
get its length or its pointer, to release it or to duplicate it. It is more
readable this way.