Code review comment for ~paelzer/ubuntu/+source/haproxy:bug-1848902-hang-on-exit-143-BIONIC

Revision history for this message
Bryce Harrington (bryce) wrote :

- [√] 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

« Back to merge proposal