HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Restart Services Automatically

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
automaticallyservicesrestart

Problem

I have been working on a script that automatically checks on the state of 2 services that I require to be running 24/7 on a server that I manage.

The script works as I need, but I would really like to optimize it if possible.
Specifically, I find that I want a way to reduce the need to type the command:

$(ps -ef | grep -v grep | grep zabbix_agentd | wc -l)


and

$(ps -ef | grep -v grep | grep zabbix_server | wc -l)


Any other feedback is greatly appreciated, I am new to bash scripting and trying to find the best way to get this to be as optimal as possible!

#!/bin/bash
zabbix_server="service zabbix-server"
zabbix_agent="service zabbix-agent"
logfile=/etc/scripts/zabbix/zabbix_auto_restart.log
zabbix_server_running=0
zabbix_agent_running=0

check_zabbix_agentd (){
        if (( $(ps -ef | grep -v grep | grep zabbix_agentd | wc -l) > $logfile
           echo "************************************************" >> $logfile

           #Send email to notify that the script ran
           echo "$(date) $zabbix_agent was restarted from zabbix_restart.sh" | mutt -s "Zabbix Auto-restart Script Just Ran" 

        else
           let zabbix_agent_running=1
        fi
}

check_zabbix_server (){
        if (( $(ps -ef | grep -v grep | grep zabbix_server | wc -l) > $logfile
           echo "************************************************" >> $logfile

           #Send email to notify that the script ran
           echo "$(date) $zabbix_server was restarted from zabbix_restart.sh" | mutt -s "Zabbix Auto-restart Script Just Ran" 

        else
           let zabbix_server_running=1
        fi
}

main_loop (){
        until ((zabbix_server_running == 1 && zabbix_agent_running == 1));
        do
            check_zabbix_agentd
            check_zabbix_server
            sleep 1.5
        done
}

main_loop

Solution

As you and I both know, what follows is the answer I posted to your question at Unix and Linux. I'm not sure if this might be considered in bad taste or not - and if so, I'll happily delete it - but at the other site the question was apparently closed as a cross-post in favor of this. And so I thought - maybe it belonged? Anyway, here goes:

What you're really doing wrong is duplicating your effort - basically every hardcoded occurrence of _agent or _server appears to be completely redundant.

For example, if this is being run on a linux system, you can completely drop the grep_...() functions, and consolidate both check_...s into a single entity which might work like:

email(){ 
    mutt -s "Zabbix Auto-restart Script Just Ran" \
}
prlog(){ 
    date +"%x %X:%tservice $1${2+%n************************}"
}
chk_run()
    while  [ "$#" -gt 0 ]
    do     if     ps -C zabbix_"$1"
           then   : "$(($1=1))"
           else   set zabbix_"$@"
                  service "$1" start || eval >&2 '
                  prlog   "$1 restart failed."  +; exit '"$?"
                  prlog   "$1 restarted."       +  >&2
                  prlog   "$1 restarted from $0." |email
           fi;    shift
    done


The key to that is you would just call chk_run with an argument list each member of which would indicate to it what it should be checking each iteration.

loop()
    until [ "$(($1&&$2))" -eq 1 ]
    do    chk_run "$@"
          sleep 2
    done  >/dev/null 2>>"$log"
agentd=0 server=0 loop agentd server


POSIXly the only thing that should need altering there is the ps command - because POSIX doesn't specify the -C switch. And so you could just change the if line to look like:

if    ps -eocomm= |
      grep -xqF zabbix_"$1"


Aside from mutt, service, and the ps optimization, it should all be standard command language. At least one advantage to that is the #!/bin/bash hash-bang is completely unnecessary - there is no anchor here to some shell-specific extension, and so it should work pretty much exactly the same in all shells which strive for POSIX-compliance. That means that #!/bin/dash is a very simple optimization in this case.

Code Snippets

email(){ 
    mutt -s "Zabbix Auto-restart Script Just Ran" \<user email\>
}
prlog(){ 
    date +"%x %X:%tservice $1${2+%n************************}"
}
chk_run()
    while  [ "$#" -gt 0 ]
    do     if     ps -C zabbix_"$1"
           then   : "$(($1=1))"
           else   set zabbix_"$@"
                  service "$1" start || eval >&2 '
                  prlog   "$1 restart failed."  +; exit '"$?"
                  prlog   "$1 restarted."       +  >&2
                  prlog   "$1 restarted from $0." |email
           fi;    shift
    done
loop()
    until [ "$(($1&&$2))" -eq 1 ]
    do    chk_run "$@"
          sleep 2
    done  >/dev/null 2>>"$log"
agentd=0 server=0 loop agentd server
if    ps -eocomm= |
      grep -xqF zabbix_"$1"

Context

StackExchange Code Review Q#94012, answer score: 3

Revisions (0)

No revisions yet.