Missing running check on init script

[Ariel Wainer]

[Impact]

'service haproxy stop' returns 4 if haproxy was already stopped.

[Test Case]

sudo service haproxy start
sudo killall -9 haproxy
sudo service haproxy stop
echo $?
~=> this should print 0, not 4
sudo service haproxy start
sudo service haproxy stop
~=> make sure this still correctly prints 0

[Regression Potential]

if done wrongly we could further break the haproxy service stop.

[Other Info]

Hi, this is the relevant version information:

# lsb_release -rd
Description: Ubuntu 12.04 LTS
Release: 12.04
dpkg -l haproxy|tail -1
ii haproxy 1.4.18-0ubuntu1 fast and reliable load balancing reverse proxy

When I issue a stop to haproxy via the init script, on line 49 it tries to kill the process and returns 4 in case of failure, which then after iis the exit status of the script itself.
There are many reasons why stopping a process may fail, one of them is that the process wasn't actually running. In that case, I think the exit status of stop should be 0, since the process is stopped.
This caused problems in the following scenario: haproxy governed by pacemaker if the haproxy process stops abruptly for some reason (crash, oom-killer), pacemaker will try to stop and start it again, but it will fail to stop (exit 4, expected 0).
To work around this, I added a check on the init script to see if the process was running, see patch attached.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "haproxy.diff" of this bug report has been identified as being a patch. The ubuntu-reviewers team has been subscribed to the bug report so that they can review the patch. In the event that this is in fact not a patch you can resolve this situation by removing the tag 'patch' from the bug report and editing the attachment so that it is not flagged as a patch. Additionally, if you are member of the ubuntu-reviewers team please also unsubscribe the team from this bug report.

[This is an automated message performed by a Launchpad user owned by Brian Murray. Please contact him regarding any issues with the action taken in this bug report.]

[Jim Howell (jimbocoder)]

+1. In a very related vein, the init script exits with status 4 when it tries to start an already-running haproxy. Both the stop and start behaviors are non-standard LSB: http://linux-ha.org/wiki/LSB_Resource_Agents

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Ante Karamatić (ivoks)]

This still won't be enough. I suggest something like this:

--- haproxy.old 2014-09-22 10:52:07.599718882 +0000
+++ haproxy.new 2014-09-22 10:51:34.815653158 +0000
@@ -46,7 +46,9 @@
   return 0
  fi
  for pid in $(cat $PIDFILE) ; do
- /bin/kill $pid || return 4
+ if [ ! kill -0 $pid ]; then
+ /bin/kill $pid || return 4
+ fi
  done
  rm -f $PIDFILE
  return 0

[Ante Karamatić (ivoks)]

Patch

[Serge Hallyn (serge-hallyn)]

Updated init patch which I'll be pushing.

--- haproxy.old 2014-09-22 10:52:07.599718882 +0000
+++ haproxy.new 2014-09-22 10:51:34.815653158 +0000
@@ -46,7 +46,9 @@
   return 0
  fi
  for pid in $(cat $PIDFILE) ; do
- /bin/kill $pid || return 4
+ if kill -0 $pid 2>/dev/null; then
+ /bin/kill $pid || return 4
+ fi
  done
  rm -f $PIDFILE
  return 0

[Launchpad Janitor (janitor)]

This bug was fixed in the package haproxy - 1.5.4-1ubuntu1

---------------
haproxy (1.5.4-1ubuntu1) utopic; urgency=medium

  * haproxy.init: return 0 on stop if haproxy was not running. (LP: #1038139)
 -- Serge Hallyn <email address hidden> Tue, 23 Sep 2014 12:06:17 -0500

[Brian Murray (brian-murray)]

Hello Ariel, or anyone else affected,

Accepted haproxy into trusty-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/haproxy/1.4.24-2ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Brian Murray (brian-murray)]

Hello Ariel, or anyone else affected,

Accepted haproxy into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/haproxy/1.4.18-0ubuntu1.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Ubuntu Foundations Team Bug Bot (crichton)]

The fix for this bug has been awaiting testing feedback in the -proposed repository for precise for more than 90 days. Please test this fix and update the bug appropriately with the results. In the event that the fix for this bug is still not verified 15 days from now, the package will be removed from the -proposed repository.

[Steve Langasek (vorlon)]

The version of haproxy in the proposed pocket of Precise that was purported to fix this bug report has been removed because the bugs that were to be fixed by the upload were not verified in a timely (105 days) fashion.

[Steve Langasek (vorlon)]

The version of haproxy in the proposed pocket of Trusty that was purported to fix this bug report has been removed because the bugs that were to be fixed by the upload were not verified in a timely (105 days) fashion.