Merge ~paelzer/ubuntu/+source/haproxy:bug-1848902-hang-on-exit-143-BIONIC into ubuntu/+source/haproxy:ubuntu/bionic-devel

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

PPA: https://launchpad.net/~paelzer/+archive/ubuntu/bug-1848902-haproxy-hang

[Bryce Harrington (bryce)]

- [√] changelog entry correct, targeted to correct codename
- [√] no major upstream changes to consider
- [√] debian changes look safe
- [√] update-maintainer has been run
- [-] changes forwarded upstream/debian (if appropriate)
- [-] nothing else to drop
- [√] patches match what was proposed upstream
- [√] patches correctly included in debian/patches/series?
- [√] patches have correct DEP3 metadata

== Testing ==

$ lxc launch ubuntu:18.04/amd64 bug-1848902-haproxy-hang
$ lxc exec bug-1848902-haproxy-hang -- bash

== Existing (broken) case ==
# apt-get install haproxy=1.8.8-1ubuntu0.7
# systemctl status haproxy | grep Active:
   Active: active (running) since Sat 2019-11-16 20:41:51 UTC; 1min 32s ago

# pkill -TERM -x haproxy
# systemctl status haproxy
   Active: active (running) since Sat 2019-11-16 20:43:52 UTC; 1s ago

# for x in {1..100}; do pkill -TERM -x haproxy ; sleep 0.1 ; done
# systemctl status haproxy | grep Active:
   Active: failed (Result: exit-code) since Sat 2019-11-16 20:44:53 UTC; 18s ago

== PPA Version ==
# add-apt-repository ppa:paelzer/bug-1848902-haproxy-hang
# apt install haproxy
# apt-cache policy haproxy | grep Installed:
  Installed: 1.8.8-1ubuntu0.8~ppa1
# systemctl status haproxy | grep Active:
   Active: active (running) since Sat 2019-11-16 20:46:52 UTC; 29s ago

# pkill -TERM -x haproxy
# systemctl status haproxy | grep Active:
   Active: active (running) since Sat 2019-11-16 20:49:10 UTC; 917ms ago

# for x in {1..100}; do pkill -TERM -x haproxy ; sleep 0.1 ; done
# systemctl status haproxy | grep Active:
   Active: failed (Result: start-limit-hit) since Sat 2019-11-16 20:50:18 UTC; 9s ago

---

I think the test case would be clearer if the end result was haproxy remained actively running, rather than fail just for a different reason, however it is failing for the documented reason (start limit hit) instead of the original problem (exit code), so technically seems to be correct and behaving as designed, so +1.

review: Approve

[Bryce Harrington (bryce)]

Figured out a way to hackaround the rate limiter in the test case for the SRU template:

for x in {1..100}; do pkill -TERM -x haproxy ; sleep 0.1 ; systemctl reset-failed haproxy.service; done

With that, the test case should result in success with the fixed case.

Otherwise, once you hit x==6 in the loop systemd just kills off the service and you don't exercise the test coverage for the remaining 95 steps of the counter. I couldn't find a way to configure this per-service (I experimented with StartLimitBurst=101 and StartLimitIntervalSec=<N> but no effect), and didn't spot a way to configure this in systemd globally), but the brute force reset-failed should be sufficient for the test case.

review: Needs Fixing

[Christian Ehrhardt  (paelzer)]

Thanks for the modified test bryce, but with it also the old code remains an active service.
I extended the steps in the SRU template.
I admit the test isn't perfect at all, but that is the nature with races.

At least the fix is accepted upstream and in addition fact that active users already run the accepted RC in the field convinces me to go forward.

[Christian Ehrhardt  (paelzer)]

To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/haproxy
 * [new tag] upload/1.8.8-1ubuntu0.8 -> upload/1.8.8-1ubuntu0.8

$ dput ubuntu ../haproxy_1.8.8-1ubuntu0.8_source.changes
Checking signature on .changes
gpg: ../haproxy_1.8.8-1ubuntu0.8_source.changes: Error checking signature from BA3E29338280B242: SignatureVerifyError: 0
Checking signature on .dsc
gpg: ../haproxy_1.8.8-1ubuntu0.8.dsc: Error checking signature from BA3E29338280B242: SignatureVerifyError: 0
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading haproxy_1.8.8-1ubuntu0.8.dsc: done.
  Uploading haproxy_1.8.8-1ubuntu0.8.debian.tar.xz: done.
  Uploading haproxy_1.8.8-1ubuntu0.8_source.buildinfo: done.
  Uploading haproxy_1.8.8-1ubuntu0.8_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 8470612..a388302 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+haproxy (1.8.8-1ubuntu0.8) bionic; urgency=medium
+
+  * d/p/lp-1848902-MINOR-systemd-consider-exit-status-143-as-successful.patch:
+    fix potential hang in haproxy (LP: #1848902)
+
+ -- Christian Ehrhardt <[e-mail]>  Tue, 12 Nov 2019 13:16:22 +0100
+
 haproxy (1.8.8-1ubuntu0.7) bionic-security; urgency=medium
 
   * SECURITY UPDATE: Messages with transfer-encoding header missing "chunked"
diff --git a/debian/patches/lp-1848902-MINOR-systemd-consider-exit-status-143-as-successful.patch b/debian/patches/lp-1848902-MINOR-systemd-consider-exit-status-143-as-successful.patch
new file mode 100644
index 0000000..1d782f9
--- /dev/null
+++ b/debian/patches/lp-1848902-MINOR-systemd-consider-exit-status-143-as-successful.patch
@@ -0,0 +1,61 @@
+From fdc6c62dbebf4b646b4f80c383e3b00f34b0440f Mon Sep 17 00:00:00 2001
+From: Vincent Bernat <[e-mail]>
+Date: Fri, 22 Jun 2018 20:57:03 +0200
+Subject: [PATCH] MINOR: systemd: consider exit status 143 as successful
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+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
+
+(cherry picked from commit 3b479bd5f5f50ce91cabed32bb26556313552d23)
+Signed-off-by: William Lallemand <[e-mail]>
+
+Origin: upstream, http://git.haproxy.org/?p=haproxy-1.8.git;a=commit;h=fdc6c62dbebf4b646b4f80c383e3b00f34b0440f
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1848902
+Applied-Upstream: 1.8.13
+Last-Update: 2019-11-12
+
+---
+ 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 7a8b6bead..74e66e302 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.24.0
+
diff --git a/debian/patches/series b/debian/patches/series
index 295e83d..ac811fe 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -14,3 +14,4 @@ stksess-align.patch
 lp-1841936-CLEANUP-ssl-make-ssl_sock_load_dh_params-handle-errc.patch
 lp-1841936-BUG-MEDIUM-ssl-tune.ssl.default-dh-param-value-ignor.patch
 CVE-2019-18277.patch
+lp-1848902-MINOR-systemd-consider-exit-status-143-as-successful.patch