[SOLVED] Bash wrapper script

[Starting Minter]

Hello Forum,

Re: Checking for a script crash, kill crashed script and restart again in background

On boot, crontab starts a wrapper bash script in the background. This wrapper script periodically checks wether an "I am alive" file from another script exists, and if it doesn't, that other script is killed and restarted in the background by the wrapper. If the "I am alive" file does exist, the wrapper deletes it. The other script creates it again upon completion of the next cycle of an infinite loop.

A little log gets updated.

Below I have added a few "\" to ease the reading of the script.

My wrapper BASH script is as follows:

Code: Select all

while true ; do
        if [ ! -f "/home/kodi-fileserver/watchkodi.alive" ] ; then
                        while /usr/bin/pkill -9 -f "/home/kodi-fileserver/bin/watchkodi" ; \
                        	do /usr/bin/sleep 1 >/dev/null 2>&1 ; \
                        done
                        (/home/kodi-fileserver/bin/watchkodi &) >/dev/null 2>&1 && \
                        	/usr/bin/echo "$(date +%Y%m%d-%H:%M:%S) restarted" >> /home/kodi-fileserver/log_watchkodi 2>/dev/null \
                                ||  /usr/bin/echo "$(date +%Y%m%d-%H:%M:%S) restart failed" >> /home/kodi-fileserver/log_watchkodi 2>/dev/null
                else
                        /usr/bin/rm -f "/home/kodi-fileserver/watchkodi.alive" >/dev/null 2>&1
        fi
        /usr/bin/sleep 15 >/dev/null 2>&1
done

Is this the correct way of doing things?

I am particularly intersted in whether the while loop with the pkill is correct or not - it does seem to work, but not always and it's hard to nail down how/when it goes wrong.




Thanks for your time and help.

S.M.

[Welcome]

I think I see something...

Code: Select all

while /usr/bin/pkill -9 -f "/home/kodi-fileserver/bin/watchkodi" ;

Maybe this?

Code: Select all

while true ; do
    /usr/bin/pkill -9 -f "/home/kodi-fileserver/bin/watchkodi" >/dev/null 2>&1
    if [ $? != 0 ] ; then
        break
    fi
    /usr/bin/sleep 1
done

You might need to use 'sync' to flush file writes....

As an alternative, have you considered using a date and time as the control? For example, you could write the date and time to a file, and then read that date and time to test it. If more than x minutes have past (based on that date/time in that file), then process....

[Welcome]

Here's a simple sample (using bash, not dash) for use of the time values:

server

Code: Select all

#!/bin/bash

echo "Starting server..."
for ((i = 1; i <= 10; ++i)) ; do
    date > syncdate
    sync
    echo -n .
    sleep 14
done
echo -e "\nExiting server..."
#eof

client

Code: Select all

#!/bin/bash

echo "Starting client..."
while true; do
    # Check for the sync date
    if [[ -e ./syncdate ]]; then
        filedate=`cat ./syncdate`
    else
        echo "File not found! Aborting!"
        break
    fi
    elapsedsec=$(( $( date +%s ) - $( date --date="$filedate" +%s ) ))
    echo -n "$elapsedsec "
    if [[ $elapsedsec -gt 14 ]] ; then
        echo -e "\n\nServer is down!"
        break
    fi
    sleep 1
done
echo -e "\nExiting client..."    
#eof

[Starting Minter]

Thanks for the replies Welcome; I'm having a look.

( I had modified some of the code according "shellcheck"-script's hints...)

Errrm, I have to excuse myself. I just noted that my wrapper does not seem to start my wrapped script properly ! Rather, the wrapped script turns up in "ps aux", but does not actually seem to behave as expected, i.e. behaving differently than when started directly from the command line, bypassing the wrapper script.

My wrapper, as listed above, gets started by cron at boot as follows:

Code: Select all

@reboot /home/kodi-fileserver/bin/wrapper &

and has the regular shebang #! /bin/bash as does the wrapped script.

I initially suspected my logic in the wrapped script to be the cause of it not working properly, but have since just-about ruled that out. I then suspected the wrapper script to be faulty in killing an unresponsive wrapped script, but now I am fairly sure that the wrapped script gets started in a wrong way, somehow, so that it does not run properly. I am on thin ice here. Where could my issue be?

Thanks for all your time and help.

B.R.
Starting Minter

[Welcome]

Where could my issue be?

With cron, there can be a typical problem of an environment variable needing to be set. For example, a graphical program might require "env DISPLAY=:0" as a part of the crontab command line.

With starting something at boot, the usual problem is that something isn't ready for a program to be run. The normal fix is to add a delay with something like "sleep 30 && ..." to the crontab command line.

Wish I could help more at this time, but I've got to go now and I'll probably be tied up until late tomorrow.

[Starting Minter]

Hello Welcome,

I appreciate your help and will add that delay to my cron @ boot, then fault-find further if needed.

In any case, I will report back here.

B.R.
Starting Minter

[Starting Minter]

Hello again, I'm still trying to get going with my cron @reboot, wrapper and wrapped script and have made some progress after Welcome talked about cron.

Adding sleep 30 && to the crontab @reboot made no noticable difference unfortunately, but I left it in after reading-up on the crontab man.

I also added PATH ( as given by echo $PATH in the terminal), and SHELL lines to crontab to 'force' it to use BASH rather than SH.

The part of interest of my crontab is now as follows:

Code: Select all

# use /bin/bash to run commands, instead of the default /bin/sh
SHELL=/bin/bash

PATH=/home/kodi-fileserver/bin:/home/kodi-fileserver/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/local/games:/usr/games

@reboot sleep 30 && /home/kodi-fileserver/bin/wrapper &

Furthermore, I have inspected the wrapper and wrapped scripts and, just in case, where still missing, added full paths to all external commands like echo, cut, grep etc.

This has improved the working of the wrapped script, when started by the wrapper that gets started by cron @ reboot.

However, what I now see below, I do not understand. This is right after a boot:

Code: Select all

 ps aux | grep wrapper
..   425  0.0  0.0   7676  1720 ?        S    10:30   0:00 /bin/bash -c sleep 30 && /home/kodi-fileserver/bin/wrapper &
..   586  0.0  0.0   7676  2536 ?        S    10:30   0:00 /bin/bash /home/kodi-fileserver/bin/wrapper

It seems like wrapper is running both in the foreground (PID 586) started by "I-do-not-know-what", as well as in the background as started by cron @reboot (PID 425). (Previously, without the PATH and SHELL lines in the crontab, wrapper would only show up once above.)
Is this to be expected, is there something not quite right ?


B.R.
Starting Minter


EDIT
I just ran kill -9 586 (586 being the PID of what seemed like wrapper running in foreground), and re-ran ps aux | grep wrapper.
Killing this one process removed both wrapper processes !? I reckon this behaviour is to be expected, but do not understand it.

[Welcome]

Got a few minutes this morning...

Code: Select all

..   425  0.0  0.0   7676  1720 ?        S    10:30   0:00 /bin/bash -c sleep 30 && /home/kodi-fileserver/bin/wrapper &
..   586  0.0  0.0   7676  2536 ?        S    10:30   0:00 /bin/bash /home/kodi-fileserver/bin/wrapper

My guess: Process 425 starts process 586 and is waiting for it's completion. Might try ...

Code: Select all

/bin/bash -c "sleep 30 && /home/kodi-fileserver/bin/wrapper" &

[Termy]

Some improvements* to logic and overall readability:

Code: Select all

#!/usr/bin/env bash

LogFile='/home/kodi-fileserver/log_watchkodi'
AliveFile='/home/kodi-fileserver/watchkodi.alive'
WatchExec='/home/kodi-fileserver/bin/watchkodi'

while true; do
    if [ ! -f "$AliveFile" ] ; then
        while /usr/bin/pkill -9 -f "$WatchExec"; do
            /usr/bin/sleep 1 &>/dev/null
        done

        printf -v Date '%(%Y%m%d-%H:%M:%S)T' -1
        if ("$WatchExec" &) &> /dev/null; then
            echo "$Date restarted" >>"$LogFile" 2>/dev/null
        else
            echo "$Date restart failed" >>"$LogFile" 2>/dev/null
        fi  
    else
        /usr/bin/rm -f "/home/kodi-fileserver/watchkodi.alive" &>/dev/null
    fi  

    /usr/bin/sleep 15 &>/dev/null
done

* Untested, and the original code was quite fragmented, so I apologise if there are any typos or misinterpretations.

Some pointers:

1. BASH's printf builtin can already display the time, so there's no need for date(1), here.
2. You might be interested to know that : is shorthand for true.
3. Don't be afraid to use variables to make your code more readable.
4. Sensible indentation of your code is very important for readability.
5. You're using BASH, so you can use the bashism: &> (STDOUT & STDERR)
6. /usr/bin/echo is a GNU alternative, but echo is already a BASH builtin.
7. Lastly, but still incredibly important, be careful how you use: CMD_1 && CMD_2 || CMD_3

Regarding 7, I'm assuming you're using it as shorthand for if CMD; then CMD; else CMD; fi, in which case I went for a typical if statement, because not only did it make the code a lot easier to read (because of otherwise very long lines), but it avoids a likely-unwanted result. If you use the 7th example, it's a potential problem if CMD_2 fails, because then even if CMD_1 is true, CMD_3 will still execute.

This approach is better left for something very simple, such as a basic shell variable assignment, or the use of something which is incredibly unlikely to fail. In your case, date(1) was very unlikely to fail, but due to the long lines and hampered readability, I went for the if statement, regardless. In some cases, not accounting for CMD_2 failing, when using this syntax, can be disastrous, which is why I urge you to try to understand all of this.

Actually, I thought of an eighth: do you need that subshell (the one in the if statement)? Something tells me you don't. If you run a command like that, it's already going to be separate, so using a subshell is both unnecessary and wasteful, unless I'm missing something.

You might also want to be more careful where you use --force or -f with commands like rm(1). As a general rule, I recommend that if you don't need to force, then don't use those flags; it might just save your bacon some day.

[Starting Minter]

Thank you both.

Currently trying Welcome's idea concerning the quotes for cron @reboot.

BR
SM

[Starting Minter]

Hello Welcome,

Thanks for the suggestion, /bin/bash -c "sleep 30 && /home/kodi-fileserver/bin/wrapper" & in crontab @reboot did the trick. The wrapper script does not show up in ps aux twice any more.

Hello Termy,

I appreciate your comments about the wrapper script. As a noob, I tried to write that script in such a way that it was as simple and as robust as possible, and so did without variables. That's also why I ran the script through shellcheck several times, changing syntax until I only had green-coloured comments left. The result was posted here.

Thank you both for your time and help.

I will mark this post as solved.

Best Regards,
S.M.

PS To summarize the changes I made to solve this thread's issue:

• Adding sleep 30 && to the crontab @reboot made no noticable difference unfortunately, but I left it in after reading-up on the crontab man.

• Modifying the crontab @reboot commands to be wrapped in quotes entirely, then suffixing with & to background. /bin/bash -c "sleep 30 && /home/kodi-fileserver/bin/wrapper" &

• I also added PATH ( as given by echo $PATH in the terminal), and SHELL=/bin/bash
lines to crontab to 'force' it to use BASH rather than SH.

• In both wrapper and wrapped script add full paths to external commands as listed by which command

(Possibly one of these might have solved the issue, or a combination of more than one.)