Missing running check on init script

Bug #1038139 reported by Ariel Wainer on 2012-08-17
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
haproxy (Ubuntu)
High
Unassigned
Precise
High
Unassigned
Trusty
High
Unassigned
Utopic
High
Unassigned

Bug Description

[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.

Ariel Wainer (ariel-cafelug) wrote :

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.]

tags: added: patch
Jim Howell (jimbocoder) wrote :

+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) wrote :

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

Changed in haproxy (Ubuntu):
status: New → Confirmed
James Page (james-page) on 2014-09-22
Changed in haproxy (Ubuntu Trusty):
status: New → Confirmed
importance: Undecided → High
Changed in haproxy (Ubuntu Utopic):
importance: Undecided → High
Ante Karamatić (ivoks) wrote :

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) wrote :

Patch

Serge Hallyn (serge-hallyn) wrote :

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

description: updated
Changed in haproxy (Ubuntu Precise):
importance: Undecided → High
status: New → Confirmed
Launchpad Janitor (janitor) wrote :

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

Changed in haproxy (Ubuntu Utopic):
status: Confirmed → Fix Released

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!

Changed in haproxy (Ubuntu Trusty):
status: Confirmed → Fix Committed
tags: added: verification-needed
Brian Murray (brian-murray) wrote :

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!

Changed in haproxy (Ubuntu Precise):
status: Confirmed → Fix Committed

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.

tags: added: removal-candidate
tags: added: precise
description: updated

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.

Changed in haproxy (Ubuntu Precise):
status: Fix Committed → Won't Fix
Steve Langasek (vorlon) wrote :

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.

Changed in haproxy (Ubuntu Trusty):
status: Fix Committed → Won't Fix
tags: removed: removal-candidate verification-needed
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers