cartridge control scripts: Fix exit value handling

Fix some problems with how we handle exit values in control scripts.

Most control scripts have `#!/bin/bash -e` or `set -e` so that they exit
immediately if a command exits with a non-zero return value if that
command is not part of a compound command or conditional statement.
Thus if we anticipate that a command may fail, we must put it in a compound
command or a conditional statement.  This commit makes the following
changes:

• Replace `foo; [ $? -eq 0 ] && bar` with `local rc=0; foo || rc=$?;
  [[ "$rc" = 0 ]] && bar` where foo is an especially long command.

• Replace `foo; [ $? -eq 0 ] || bar` with `if ! foo; then bar; fi`.

• Replace `foo; ret=$?; if [ $ret -eq 0 ]; then` with `if foo; then`.

• Replace `foo; ret=$?; if [ $ret -ne 0 ]; then` with `if ! foo; then`.

• Replace `foo; bar $?` with `local rc=0; foo || rc=$?; bar "$rc"`.

• Replace `foo; return $?` with `local ret=0; foo || ret=$?; return
  $ret`.

• If a script runs with `/bin/bash -e` and ends with `exit $?`, delete
  the `exit $?` because it is redundant.

Moreover, several control scripts had inverted logic around the
/bin/kill command: These scripts would run /bin/kill, check its exit
value, and retry /bin/kill if the first attempt returned 0.  However,
an exit value of 0 from /bin/kill means that it succeeded, so this code
was retrying exactly when it did not need to do so.  This commit fixes
this logic.

Author: Miciah

cartridges/openshift-origin-cartridge-haproxy/bin/control

@@ -134,9 +134,7 @@ function _stop_haproxy_service() {
     _stop_haproxy_ctld
     [ -f $HAPROXY_PID ]  &&  pid=$( /bin/cat "${HAPROXY_PID}" )
     if `ps -p $pid > /dev/null 2>&1`; then
-        /bin/kill $pid
-        ret=$?
-        if [ $ret -eq 0 ]; then
+        if ! /bin/kill $pid; then
             TIMEOUT="$STOPTIMEOUT"
             while [ $TIMEOUT -gt 0 ] && [ -f "$HAPROXY_PID" ]; do
                 /bin/kill -0 "$pid" >/dev/null 2>&1 || break

cartridges/openshift-origin-cartridge-mariadb/bin/control

@@ -65,9 +65,7 @@ function stop {
     if [ -f $pidfile ]; then
       echo "Stopping MariaDB cartridge"
       pid=$( /bin/cat $pidfile )
-      /bin/kill $pid
-      ret=$?
-      if [ $ret -eq 0 ]; then
+      if ! /bin/kill $pid; then
         TIMEOUT="$STOPTIMEOUT"
         while [ $TIMEOUT -gt 0 ] && [ -f "$pidfile" ]
         do

cartridges/openshift-origin-cartridge-mongodb/bin/control

@@ -39,8 +39,9 @@ function _repair_mongod() {
         client_result "Attempting to repair MongoDB ..."
         tmp_config="/tmp/mongodb.repair.conf"
         grep -ve "fork\s*=\s*true" $OPENSHIFT_MONGODB_DIR/conf/mongodb.conf > $tmp_config
-        mongodb_context "mongod ${authopts} -f ${tmp_config} --repair"
-        client_result "MongoDB repair status = $?"
+        local rc=0
+        mongodb_context "mongod ${authopts} -f ${tmp_config} --repair" || rc=$?
+        client_result "MongoDB repair status = $rc"
         rm -f $tmp_config
     else
         echo "MongoDB already running - not running repair"
@@ -84,16 +85,10 @@ function stop() {
 
     if [ -n "$pid" ]; then
         echo "Stopping MongoDB cartridge"
-        set +e
-        /bin/kill $pid 2>/dev/null
-        set -e
-        ret=$?
-        if [ $ret -eq 0 ]; then
+        if ! /bin/kill $pid 2>/dev/null; then
             TIMEOUT="$STOPTIMEOUT"
             while [ $TIMEOUT -gt 0 ] && [ -f "$OPENSHIFT_MONGODB_DIR/pid/mongodb.pid" ]; do
-                set +e
                 /bin/kill -0 "$pid" >/dev/null 2>&1 || break
-                set -e
                 sleep 1
                 let TIMEOUT=${TIMEOUT}-1
             done
@@ -125,8 +120,10 @@ function pre_snapshot {
    _wait_for_mongod_to_startup
    #  Work in a temporary directory (create and cd to it).
    umask 077
-   dumpdir=$(mktemp -d /tmp/mongodumpXXXXXXXX)
-   [ $? -eq 0 ] || die 0 "ERROR" "Failed to create working directory."
+   if ! dumpdir=$(mktemp -d /tmp/mongodumpXXXXXXXX)
+   then
+     die 0 "ERROR" "Failed to create working directory."
+   fi
    pushd $dumpdir > /dev/null
 
    #  Take a "dump".
@@ -187,8 +184,10 @@ WARNING: You may have possibly encountered the mongorestore bugs related to
 function restore_from_mongodb_snapshot {
    #  Work in a temporary directory (create and cd to it).
    umask 077
-   dumpdir=$(mktemp -d /tmp/mongodumpXXXXXXXX)
-   [ $? -eq 0 ] || die 0 "ERROR" "Failed to create working directory."
+   if ! dumpdir=$(mktemp -d /tmp/mongodumpXXXXXXXX)
+   then
+     die 0 "ERROR" "Failed to create working directory."
+   fi
    pushd $dumpdir > /dev/null
 
    #  Extract dump from the snapshot.

cartridges/openshift-origin-cartridge-mysql/bin/control

@@ -80,9 +80,7 @@ function stop {
     if [ -f $pidfile ]; then
       echo "Stopping MySQL ${OPENSHIFT_MYSQL_VERSION} cartridge"
       pid=$( /bin/cat $pidfile )
-      /bin/kill $pid
-      ret=$?
-      if [ $ret -eq 0 ]; then
+      if ! /bin/kill $pid; then
         TIMEOUT="${stop_timeout}"
         while [[ "${TIMEOUT}" > 0 && -f "${pidfile}" ]]
         do

cartridges/openshift-origin-cartridge-nodejs/bin/control

@@ -126,10 +126,7 @@ function stop() {
 
       echo "`date +"$FMT"`: Stopping application '$OPENSHIFT_APP_NAME' ..."
 
-      /bin/kill $cart_pid
-      ret=$?
-
-      if [ $ret -eq 0 ]; then
+      if ! /bin/kill $cart_pid; then
         TIMEOUT="$STOPTIMEOUT"
         while [ $TIMEOUT -gt 0 ]  && is_cartridge_running ; do
           /bin/kill -0 "$cart_pid" >/dev/null 2>&1 || break

cartridges/openshift-origin-cartridge-perl/bin/control

@@ -28,10 +28,11 @@ function start() {
   ensure_valid_httpd_process "$HTTPD_PID_FILE" "$HTTPD_CFG_FILE"
   # Force httpd into its own pgroup, as httpd is hard-coded to TERM everything in
   # its pgroup during shutdown (even while foregrounded)
+  local rc=0
   set -m
-  eval "nohup /usr/sbin/httpd $HTTPD_CMD_CONF -D FOREGROUND |& /usr/bin/logshifter -tag perl &"
+  eval "nohup /usr/sbin/httpd $HTTPD_CMD_CONF -D FOREGROUND |& /usr/bin/logshifter -tag perl &" || rc=$?
   set +m
-  [ "$?" == "0" ] && wait_for_pid_file $HTTPD_PID_FILE
+  [[ "$rc" = 0 ]] && wait_for_pid_file $HTTPD_PID_FILE
 }
 
 function stop() {

cartridges/openshift-origin-cartridge-php/bin/control

@@ -25,37 +25,45 @@ function pre_start_httpd_config {
 }
 
 function start() {
+    local ret=0
+
     echo "Starting PHP ${OPENSHIFT_PHP_VERSION} cartridge (Apache+mod_php)"
     pre_start_httpd_config
     # Force httpd into its own pgroup, as httpd is hard-coded to TERM everything in
     # its pgroup during shutdown (even while foregrounded)
     set -m
     php_context "nohup /usr/sbin/httpd $HTTPD_CMD_CONF -D FOREGROUND |& /usr/bin/logshifter -tag php &" \
-      && wait_for_pid_file $HTTPD_PID_FILE
+      && wait_for_pid_file $HTTPD_PID_FILE || ret=$?
     set +m
-    return $?
+
+    return $ret
 }
 
 function reload() {
+    local ret=0
+
     echo "Reloading PHP ${OPENSHIFT_PHP_VERSION} cartridge (Apache+mod_php)"
     pre_start_httpd_config
     httpd_pid=`cat "$HTTPD_PID_FILE" 2> /dev/null`
-    kill -USR1 $httpd_pid && wait_for_pid_file $HTTPD_PID_FILE
-    return $?
+    kill -USR1 $httpd_pid && wait_for_pid_file $HTTPD_PID_FILE || ret=$?
+
+    return $ret
 }
 
 function restart() {
+    local ret=0
+
     echo "Restarting PHP ${OPENSHIFT_PHP_VERSION} cartridge (Apache+mod_php)"
     ensure_httpd_restart_succeed "$HTTPD_PID_FILE" "$HTTPD_CFG_FILE"
     if [ -f "$HTTPD_PID_FILE" ]; then
         pre_start_httpd_config
         httpd_pid=`cat "$HTTPD_PID_FILE" 2> /dev/null`
-        kill -HUP $httpd_pid
+        kill -HUP $httpd_pid || ret=$?
     else
-        start
+        start || ret=$?
     fi
 
-    return $?
+    return $ret
 }
 
 function stop() {
@@ -69,9 +77,12 @@ function stop() {
 }
 
 function configtest() {
+    local ret=0
+
     echo "Testing Apache *.conf files"
-    php_context "/usr/sbin/httpd $HTTPD_CMD_CONF -t"
-    return $?
+    php_context "/usr/sbin/httpd $HTTPD_CMD_CONF -t" || ret=$?
+
+    return $ret
 }
 
 function status() {
@@ -185,5 +196,3 @@ case "$1" in
   build)           build ;;
   *)               exit 0
 esac
-
-exit $?

cartridges/openshift-origin-cartridge-postgresql/bin/control

@@ -163,9 +163,7 @@ function post_restore {
     local rexp="s#\(DROP DATABASE\) \(.*\)#\\1 IF EXISTS \\2#g;"
 
     # Restore old postgres database
-    zcat $dump_file | sed "${rexp}" | postgresql_context "psql -U postgres -d postgres" &> /dev/null
-
-    if [ $? -ne 0 ]
+    if ! zcat $dump_file | sed "${rexp}" | postgresql_context "psql -U postgres -d postgres" &> /dev/null
     then
       warning "Error: Could not import Postgres Database!  Continuing..."
     fi

cartridges/openshift-origin-cartridge-python/usr/versions/shared/bin/control

@@ -68,10 +68,11 @@ function start_apache() {
     ensure_valid_httpd_process "$HTTPD_PID_FILE" "$HTTPD_CFG_FILE"
     # Force httpd into its own pgroup, as httpd is hard-coded to TERM everything in
     # its pgroup during shutdown (even while foregrounded)
+    local rc=0
     set -m
-    eval "nohup /usr/sbin/httpd $HTTPD_CMD_CONF -DFOREGROUND |& /usr/bin/logshifter -tag python &"
+    eval "nohup /usr/sbin/httpd $HTTPD_CMD_CONF -DFOREGROUND |& /usr/bin/logshifter -tag python &" || rc=$?
     set +m
-    [ "$?" == "0" ] && wait_for_pid_file $HTTPD_PID_FILE
+    [[ "$rc" = 0 ]] && wait_for_pid_file $HTTPD_PID_FILE
 }
 
 function start() {

cartridges/openshift-origin-cartridge-ruby/bin/control

@@ -41,10 +41,11 @@ function start() {
     update_passenger_performance
     # Force httpd into its own pgroup, as httpd is hard-coded to TERM everything in
     # its pgroup during shutdown (even while foregrounded)
+    local rc=0
     set -m
-    ruby_context "umask 002 &>/dev/null ; RAILS_ENV=${RAILS_ENV:-production} nohup /usr/sbin/httpd $HTTPD_CMD_CONF -D FOREGROUND |& /usr/bin/logshifter -tag ruby &"
+    ruby_context "umask 002 &>/dev/null ; RAILS_ENV=${RAILS_ENV:-production} nohup /usr/sbin/httpd $HTTPD_CMD_CONF -D FOREGROUND |& /usr/bin/logshifter -tag ruby &" || rc=$?
     set +m
-    [ "$?" == "0" ] && wait_for_pid_file $HTTPD_PID_FILE
+    [[ "$rc" = 0 ]] && wait_for_pid_file $HTTPD_PID_FILE
 }
 
 function stop() {