[SOLVED] Shell Script not showing correct output

pliqui · 03-28-2014, 08:44 PM

Hello guys,

I'm having a little problem but i cannot pinpoint it. I'm creating a shell to put on cron for restart ngix and php processes (this is temporal, we are migrating those services to better hardware. I'm having problems with the FOR loop.

The script:

Code:

#!/bin/sh

#Variables
PGREP=/usr/bin/pgrep
KILL=/bin/kill
SERVICE_PATH=/etc/init.d
NGINX=nginx
PHP_FPM=php5-fpm
NGINX_PS=`ps -def |pgrep nginx |wc -l`

#printf "Number of NGINX processes: $NGINX_PS\n"

# Stopping Services
printf "Number of PHP_FPM processes: `$PGREP ${PHP_FPM}|wc -l`\n"
printf "Stopping PHP_FPM\n"
$SERVICE_PATH/$PHP_FPM stop
sleep 20
printf "Number of PHP_FPM processes after being stopped: `$PGREP ${PHP_FPM}|wc -l`\n"

# Validate php-fpm

if [ `$PGREP ${PHP_FPM}|wc -l` -eq 0 ] #Check if php-fpm stopped ok
then
	printf "php-fpm stopped ok\n"
else
	if [ `$PGREP ${PHP_FPM}|wc -l` -ne 0 ] #Check if php-fpm is still running
	then
		printf "php-fpm still running... waiting 2 mins\n"
		sleep 10
		if [ `$PGREP ${PHP_FPM}|wc -l` -ne 0 ] #IF php-fpm still running, kill it
		then
			printf "php-fpm is still running... killing the process\n"
			for pid in $($PGREP $PHP_FPM) #HERE IS WHERE I HAVE PROBLEMS
				do $KILL -9 $pid &> /dev/null
				(( $? ++ 0)) && echo "kill of $pid successful" || echo "kill of $pid failed"
			done
		fi
	fi
fi

If i execute this and stop the php-fpm service is ok. But during the 1st sleep i start it again to force it into the if block and this is the result

Code:

root@debian:/tmp# sh -x test.sh
+ PGREP=/usr/bin/pgrep
+ KILL=/bin/kill
+ SERVICE_PATH=/etc/init.d
+ NGINX=nginx
+ PHP_FPM=php5-fpm
+ wc -l
+ pgrep nginx
+ ps -def
+ NGINX_PS=0
+ printf Number of NGINX processes: 0\n
Number of NGINX processes: 0
+ wc -l
+ /usr/bin/pgrep php5-fpm
+ printf Number of PHP_FPM processes: 3\n
Number of PHP_FPM processes: 3
+ printf Stopping PHP_FPM\n
Stopping PHP_FPM
+ /etc/init.d/php5-fpm stop
+ sleep 20
+ wc -l
+ /usr/bin/pgrep php5-fpm
+ printf Number of PHP_FPM processes after being stopped: 3\n
Number of PHP_FPM processes after being stopped: 3
+ wc -l
+ /usr/bin/pgrep php5-fpm
+ [ 3 -eq 0 ]
+ wc -l
+ /usr/bin/pgrep php5-fpm
+ [ 3 -ne 0 ]
+ printf php-fpm still running... waiting 2 mins\n
php-fpm still running... waiting 2 mins
+ sleep 10
+ wc -l
+ /usr/bin/pgrep php5-fpm
+ [ 3 -ne 0 ]
+ printf php-fpm is still running... killing the process\n
php-fpm is still running... killing the process
+ /usr/bin/pgrep php5-fpm
+
+ 0 == 0
test.sh: 37: test.sh: 0: not found
+ echo kill of 6619 failed
kill of 6619 failed
+
+ 0 == 0
test.sh: 37: test.sh: 0: not found
+ echo kill of 6620 failed
kill of 6620 failed
+
+ /bin/kill -9 6619
+ /bin/kill -9 6620
+ 0 == 0
test.sh: 37: test.sh: 0: not found
+ echo kill of 6621 failed
kill of 6621 failed
root@debian:/tmp# + /bin/kill -9 6621
#Had to hit enter to exit the script

It kills the 3 processes but shows them as failed on the script

Code:

root@debian:/tmp# pgrep php5-fpm |wc -l
0

If i run the for loop part from the shell runs fine

Code:

/etc/init.d/php5-fpm start
root@debian:/tmp# for pid in $($PGREP $PHP_FPM); do $KILL -9 $pid &> /dev/null; (( $? == 0)) && echo "kill of $pid successful" || echo "kill of $pid failed"; done;
kill of 6558 successful
kill of 6559 successful
kill of 6560 successful

Is there anything i'm missing here?

Thanks in advance for your help

grail · 03-28-2014, 11:53 PM

Well apart from the typo in the copy of the script you showed us:

Code:

(( $? ++ 0)) && echo "kill of $pid successful" || echo "kill of $pid failed"

I cannot see any glaring issue, but I do have a suggestion which may or may not change anything.

Try using 'if':

Code:

if $KILL -9 $pid &> /dev/null; then
    echo "kill of $pid successful"
else
    echo "kill of $pid failed"
fi

Also, do you have to be so heavy handed to use SIGKILL. Just not a very clean exit, maybe you could just use SIGINT?

jlinkels · 03-29-2014, 06:23 AM

It seems that the trace output is not from the posted source code. There is no code on line 37 except for 'fi'.

Furthermore, the '==' statement does not appear even around line 37.

Please be sure that you post the source code which matches the code used for the trece output.

jlinkels

pliqui · 03-31-2014, 07:22 AM

Hello all,

Sorry for the long replay, Installed my very 1st raid0 ssd

About the ++ was a mistake made during the last round, was ==

But the solution of grail worked perfectly !!! Thank you so much. And followed your recommendation and used SIGQUIT to try to generate a core dump instead of kill -9

Here is the final script

Code:

#!/bin/sh
# Restart, version 1

PGREP=/usr/bin/pgrep           # Path to pgrep, to make it portable could be PGREP=`which pgrep`
KILL=/bin/kill                 # Path to kill, to make it portable could be KILL=`which kill`
CAT=/bin/cat                   # Cat
SERVICE_PATH=/etc/init.d       # Path to init scripts
NGINX=nginx                    # Ngix script in init.d
PHP_FPM=php-fpm                # PHP-FPM script in init.d
LOG_FILE= /tmp/restart_sh.log  #Log file of the process

#Cleaning the log
$CAT /dev/null > $LOG_FILE

# Stopping PHP-FPM Service
echo "------------- Begin PHP-FPM Stop-------------\n"
printf "Number of PHP_FPM processes: `$PGREP ${PHP_FPM}|wc -l`\n\n"
printf "Stopping PHP_FPM\n\n"
$SERVICE_PATH/$PHP_FPM stop
sleep 30
printf "Number of PHP_FPM processes after being stopped: `$PGREP ${PHP_FPM}|wc -l`\n\n"

# Validate if php-fpm still running after the stop

if [ `$PGREP ${PHP_FPM}|wc -l` -eq 0 ]
# test if the quantity of php-fpm processes are equal to 0
then
   printf "php-fpm stopped ok\n"
   echo "------------- END PHP-FPM Stop -------------\n"
else
   if [ `$PGREP ${PHP_FPM}|wc -l` -ne 0 ]
   # test if the quantity of php-fpm processes are not equal to 0, meaning the process are still running
   then
      printf "php-fpm still running... waiting 2 mins\n\n"
      sleep 120
      if [ `$PGREP ${PHP_FPM}|wc -l` -ne 0 ]
	  # test if after 2 minutes if the quantity of php-fpm processes are not equal to 0, meaning the process are still running
      then
         printf "php-fpm is still running after 2 minutes of being executed the init script... killing the process\n\n"
		 for pid in $($PGREP $PHP_FPM)
		 # For loop to do a SIGQUIT on the php-fpm processes and hopefully get a core dump
		 do 
		    if $KILL -3 $pid &> /dev/null
			then
               printf "kill of $pid successful\n"
            else
               printf "kill of $pid failed\n"
            fi
	    done
		echo "------------- END PHP-FPM Stop -------------\n\n"
      fi
   fi
fi

# Stopping NGINX Service
echo "------------- Begin NGINX Stop -------------\n"
printf "Number of NGINX processes: `$PGREP ${NGINX}|wc -l`\n\n"
$SERVICE_PATH/$NGINX stop
sleep 30
printf "\n"
printf "Number of NGINX processes after being stopped: `$PGREP ${NGINX}|wc -l`\n\n"

# Validate if nginx still running after the stop

if [ `$PGREP ${NGINX}|wc -l` -eq 0 ]
# test if the quantity of nginx process are equal to 0
then
   printf "nginx stopped ok\n"
   echo "------------- END NGIX Stop -------------\n"
else
   if [ `$PGREP ${NGINX}|wc -l` -ne 0 ]
   # test if the quantity of nginx processes are not equal to 0, meaning the process are still running
   then
      printf "nginx still running... waiting 2 mins\n\n"
      sleep 120
      if [ `$PGREP ${NGINX}|wc -l` -ne 0 ]
	  # test if after 2 minutes if the quantity of php-fpm process are not equal to 0, meaning the process are still running
      then
         printf "nginx is still running after 2 minutes of being executed the init script... killing the process\n\n"
		 for pid in $($PGREP $NGINX)
		 # For loop to do a SIGQUIT on the nginx processes and hopefully get a core dump
		    do 
			if $KILL -3 $pid &> /dev/null
			then
               printf "kill of $pid successful\n"
            else
               printf "kill of $pid failed\n"
            fi
			done
	    echo "------------- END NGINX -------------\n\n"
      fi
   fi
fi

# Starting PHP-FPM service
echo "------------- Begin PHP-FPM Start -------------\n"
printf "Starting PHP_FPM\n\n"
$SERVICE_PATH/$PHP_FPM start
printf "Number of PHP_FPM processes after being started: `$PGREP ${PHP_FPM}|wc -l`\n\n"

if [ `$PGREP ${PHP_FPM}|wc -l` -ne 0 ]
# test if the quantity of php-fpm processes are equal to 0, meaning meaning the process are running
then
   printf "PHP_FPM started ok\n"
   echo "------------- END PHP-FPM Start -------------\n\n"
else
   printf "Check processes manually (do a kill -9 and restart the services)\n"
   echo "------------- END PHP-FPM Start -------------\n\n"
fi

# Starting NGIX service
echo "------------- Begin NGINX Start -------------\n"
$SERVICE_PATH/$NGINX start
printf "\n"
printf "Number of NGINX processes after being started: `$PGREP ${NGINX}|wc -l`\n\n"

if [ `$PGREP ${NGINX}|wc -l` -ne 0 ]
# test if the quantity of nginx processes are equal to 0, meaning that they are running
then
   printf "NGINX started ok\n"
   echo "------------- End NGINX Start -------------\n\n"
else
   printf "Check processes manually (do a kill -9 and restart the services)\n"
   echo "------------- END PHP-FPM Start -------------\n\n"
fi

grail · 03-31-2014, 10:53 AM

I am curious if this script needs to be sh?

If it were able to be bash (and I am not 100% sure which parts might not work in sh (as been a while)), here are a few suggestions or ideas / notes:

1. There is a space after LOG_FILE= which will cause some issue (again may just be a typo)

2. You have created variables for what seems to be all external commands, yet not for wc

3. You only appear to use cat / CAT once in the entire script, which would seem to be pointless creating this variable for one line. You could try:

Code:

: > $LOG_FILE

4. You use echo and printf alternately throughout the script, but the printf is never really utilised (ie not using %s, %d, ...) so it would be cleaner to simply use echo

5. As an alternative to point 4 you might also wish to look at HEREDOC'S

6. Why not use the same if test, as we did for kill, for the rest of your tests of command output, ie pgrep (although you may need to look at the logic)

7. As an alternative to 6, you can use (()) for arithmetic tests

8. As yet another alternative for the pgrep lines, I would ditch the wc pipe altogether and simply test the output of pgrep, something like:

Code:

if $PGREP $PHP_FPM &> /dev/null

9. It is generally accepted (although can be a personal choice) to use $() over ``, here is some information on it

10. Is the for loop even required, maybe also have a look at pkill

11. You run, what appears, to be the exact same code twice, first for PHP_FPM and then NGINX. I would suggest looking into creating a function or maybe loop to take the arguments and process
the data in a single portion of the script.

Well I hope some of that helps. As usual you can completely ignore it if you like

pliqui · 04-01-2014, 03:52 PM

Thank you very much for your input Grail.

Definitely will look into it.

Yes, in point 1 was a typo.

This script is intended to run on cron. So to avoid any issues I declare the paths to bins.

And indeed could be more organized. Like functions, didn't thought it that night.

Again, thank you so much