Re: [PATCH] MINOR: systemd: consider exit status 143 as successful

2018-06-22 Thread William Lallemand
On Fri, Jun 22, 2018 at 08:57:03PM +0200, Vincent Bernat wrote:
> The master process will exit with the status of the last worker. When
> the worker is killed with SIGTERM, it is expected to get 143 as an
> exit status. Therefore, we consider this exit status as normal from a
> systemd point of view. If it happens when not stopping, the systemd
> unit is configured to always restart, so it has no adverse effect.
> 
> This has mostly a cosmetic effect. Without the patch, stopping HAProxy
> leads to the following status:
> 
> ● haproxy.service - HAProxy Load Balancer
>Loaded: loaded (/lib/systemd/system/haproxy.service; disabled; vendor 
> preset: enabled)
>Active: failed (Result: exit-code) since Fri 2018-06-22 20:35:42 CEST; 
> 8min ago
>  Docs: man:haproxy(1)
>file:/usr/share/doc/haproxy/configuration.txt.gz
>   Process: 32715 ExecStart=/usr/sbin/haproxy -Ws -f $CONFIG -p $PIDFILE 
> $EXTRAOPTS (code=exited, status=143)
>   Process: 32714 ExecStartPre=/usr/sbin/haproxy -f $CONFIG -c -q 
> $EXTRAOPTS (code=exited, status=0/SUCCESS)
>  Main PID: 32715 (code=exited, status=143)
> 
> After the patch:
> 
> ● haproxy.service - HAProxy Load Balancer
>Loaded: loaded (/lib/systemd/system/haproxy.service; disabled; vendor 
> preset: enabled)
>Active: inactive (dead)
>  Docs: man:haproxy(1)
>file:/usr/share/doc/haproxy/configuration.txt.gz
> ---
>  contrib/systemd/haproxy.service.in | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/contrib/systemd/haproxy.service.in 
> b/contrib/systemd/haproxy.service.in
> index 7a8b6bead2df..74e66e302065 100644
> --- a/contrib/systemd/haproxy.service.in
> +++ b/contrib/systemd/haproxy.service.in
> @@ -10,6 +10,7 @@ ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
>  ExecReload=/bin/kill -USR2 $MAINPID
>  KillMode=mixed
>  Restart=always
> +SuccessExitStatus=143
>  Type=notify
>  
>  # The following lines leverage SystemD's sandboxing options to provide


Hm that's interesting, but isn't it better to consider that the SIGTERM is a
"normal" status code, and let the master leaves with zero, instead of changing
systemd configuration?

We could do the same with 130 (SIGINT). In fact those two exits codes are
already handled differently in the master.

-- 
William Lallemand



[PATCH] MINOR: systemd: consider exit status 143 as successful

2018-06-22 Thread Vincent Bernat
The master process will exit with the status of the last worker. When
the worker is killed with SIGTERM, it is expected to get 143 as an
exit status. Therefore, we consider this exit status as normal from a
systemd point of view. If it happens when not stopping, the systemd
unit is configured to always restart, so it has no adverse effect.

This has mostly a cosmetic effect. Without the patch, stopping HAProxy
leads to the following status:

● haproxy.service - HAProxy Load Balancer
   Loaded: loaded (/lib/systemd/system/haproxy.service; disabled; vendor 
preset: enabled)
   Active: failed (Result: exit-code) since Fri 2018-06-22 20:35:42 CEST; 
8min ago
 Docs: man:haproxy(1)
   file:/usr/share/doc/haproxy/configuration.txt.gz
  Process: 32715 ExecStart=/usr/sbin/haproxy -Ws -f $CONFIG -p $PIDFILE 
$EXTRAOPTS (code=exited, status=143)
  Process: 32714 ExecStartPre=/usr/sbin/haproxy -f $CONFIG -c -q $EXTRAOPTS 
(code=exited, status=0/SUCCESS)
 Main PID: 32715 (code=exited, status=143)

After the patch:

● haproxy.service - HAProxy Load Balancer
   Loaded: loaded (/lib/systemd/system/haproxy.service; disabled; vendor 
preset: enabled)
   Active: inactive (dead)
 Docs: man:haproxy(1)
   file:/usr/share/doc/haproxy/configuration.txt.gz
---
 contrib/systemd/haproxy.service.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/systemd/haproxy.service.in 
b/contrib/systemd/haproxy.service.in
index 7a8b6bead2df..74e66e302065 100644
--- a/contrib/systemd/haproxy.service.in
+++ b/contrib/systemd/haproxy.service.in
@@ -10,6 +10,7 @@ ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
 ExecReload=/bin/kill -USR2 $MAINPID
 KillMode=mixed
 Restart=always
+SuccessExitStatus=143
 Type=notify
 
 # The following lines leverage SystemD's sandboxing options to provide
-- 
2.18.0