Consolidate script

[allanm77]

Hi,

Have the following script which  looks up a few html status pages on a load balancer and reports out if nodes (instances) are down, I want to consolidate the code so that it looks cleaner. Each environment has different variables but the logic remains the same.

I execute the script like this -

./script ENV

I want to have a function which has the central logic and can be called by each case statements.

   PROD)
      export ENVIRONMENT=prod
      INSTANCES=`some-command | awk -F: '{print $2}'`
      PORT=`some-command | awk -F: '{print $3}'|head -1`
      count_errs=0
      for i in $INSTANCES
      do
         echo "" >> /tmp/var.tmp
         curl --connect-timeout 2 -s --retry 1 --retry-delay 20 --max-time 5 "http://$i:$PORT/dest" |sed 's/<[^>]*>//'g  | egrep -A1 "DOWN|DISABLE" >> /tmp/var.tmp
         status=$?
         if [ $status -eq 0 ]; then
            (( count_errs += 1 ))
         fi
      done
      egrep -B1 -A1 "DOWN|DISABLE" /tmp/var.tmp > /tmp/var.1.tmp
      count_instance_errors=`egrep "DOWN|DISABLE" /tmp/var.1.tmp |wc -l`
      
      if [ $count_errs -ne 0 -a $count_instance_errors -gt 1 ]; then
         (echo "Subject:$1:Instances down"; echo "To:${EMAIL}"; echo ""; cat /tmp/var.1.tmp; ) | /usr/sbin/sendmail -t
      fi
      if [ $count_errors -ne 0 -a $count_inst_errors -eq 1 ]; then
         EMAIL="foobar@foobar.com"
         (echo "Subject:$1:Instances down"; echo "To:${EMAIL}"; echo ""; cat /tmp/var.1.tmp; ) | /usr/sbin/sendmail -t
         rm -f /tmp/var.tmp /tmp/var.1.tmp
      fi

   ;;

 Thanks,

Allan.

[Matti_Kurkela]

Yeah,  that incomplete script fragment you posted looks pretty ugly as it is.

Back when I was at the university, the lecturer on our basic programming course taught me this:

"Whenever you need to do something twice or more times, you should make it a function, so you can define it once and then call it as many times as you need. That guarantees each repetition will behave exactly the same.

If you don't use a function and instead copy/paste the code that would be the function definition, you may later end up with two pieces of code that look identical at first glance but are in fact subtly different, either because of a copy/paste mistake or because someone has applied some extra changes later. This tends to cause bugs and unexpected behavior."

For example, your script fragment has two copies of this part:

# first copy
if [ $count_errs -ne 0 -a $count_instance_errors -gt 1 ]; then # more than one error?
        (echo "Subject:$1:Instances down"; echo "To:${EMAIL}"; echo ""; cat /tmp/var.1.tmp; ) | /usr/sbin/sendmail -t
fi

# second copy
if [ $count_errors -ne 0 -a $count_inst_errors -eq 1 ]; then # just one error?
        EMAIL="foobar@foobar.com" # later addition?
        (echo "Subject:$1:Instances down"; echo "To:${EMAIL}"; echo ""; cat /tmp/var.1.tmp; ) | /usr/sbin/sendmail -t
        rm -f /tmp/var.tmp /tmp/var.1.tmp # this too?
fi

This looks like it will send a message to some previously-defined $EMAIL and leave the temporary files around when $count_instance_errors is more than 1. But if $count_instance_errors is exactly 1, it will change the value of $EMAIL, send a message and remove the temporary files. This seems odd to me: maybe this is a bug?

Sending the email could be defined as a function, like this:

mailReportDown () {
# name the positional parameters for clarity
INSTNAME="$1"
EMAIL="$2"
MAILTEXT="$3"

( echo "Subject:${INSTNAME}:Instances down"
  echo "To: ${EMAIL}"
  cat "${MAILTEXT}" ) | /usr/sbin/sendmail -t
}

 Note that within a function, the positional parameters ($1, $2, $3 etc.) will refer to the parameters given to the function, not the parameters given to the main script. If your function needs any of the parameters given to the main script, you must either store the required parameters into a global variable before calling the function, or explicitly pass the main script's parameter to the function too (that's exactly what I'm doing with $1 below).

Later you could call the function multiple times if required:

# send to ${EMAIL} as defined in some earlier point
mailReportDown "$1" "${EMAIL}" /tmp/var.1.tmp
...
# special case: send to a special unique recipient
mailReportDown "$1" "foobar@foobar.com" /tmp/var.1.tmp

You should be able to use this example of defining and using a function to write your own functions as you see fit. Please ask if something was unclear in my example.

By the way, in your script fragment, you also use temporary files with fixed names. If two users happen to run this script simultaneously, their temporary files will overlap, resulting in confusion. Or if files with the same names already exist, your script will try to append to them, possibly mixing old results with the new ones.

A safer method would be to use the "mktemp" command to generate an unique name for your temporary file, or if you need more than one temporary file, use a uniquely-named directory for them.

You can also use the "trap" command to ensure that your temporary files will always be removed when the script exits, whether your script exits normally or is interrupted. (However, if the script is interrupted with "kill -9", the trap actions won't run.)

# set up a single temporary file
MYTEMPFILE=$(mktemp)
trap "rm -f $MYTEMPFILE" EXIT

# then use it
echo "some stuff" >>$MYTEMPFILE # or whatever...

# set up a directory for multiple temporary files
MYTEMPDIR=$(mktemp)
mkdir $MYTEMPDIR 
trap "rm -rf $MYTEMPDIR" EXIT

# then create and use as many temporary files as you need
echo "some stuff" >> $MYTEMPDIR/var.tmp 
echo "some other stuff" >> $MYTEMPDIR/var.1.tmp

[allanm77]

Thanks Matti,

The if conditions are not a bug but rather since my original variable had a pager listed so I had to make adjustment that when the issue is lesser in extent it just mails to a mail id and not to the pager.

Can the logic be contructed into a function?

Rgrds,

Allan.

[Dennis Handly]

>The if conditions are not a bug, so I had to make adjustment that when the issue is lesser in extent ...

>Can the logic be constructed into a function?

You would have to pass in a "severity" parm and it could reset the mail address?

Or pass to another function that will reset the address and still have that one mailReportDown function.