Script for loops, with multiple files

[Ratzie]

I know I need a for loop but...
This is what I have...

5 scripts, same name except it is script_1, script_2 etc.

Each of these scripts gives an output to a file... output1, output2, etc.

Instead of making variables for each file, I thought I can do a for statement...

I want to run each script, then check the output of each output and if it contains anything mail me.
I got the check for output part, thanks to S. Bhaskarla

But when I run what I have I get multiple entries and multiple emails!

I only want the 5 emails if there is data...


find $SCRIPTS_DIR/reset_pass* -print > /tmp/reset_pass.input
find $LOGS_DIR/login_entry_* -print > /tmp/login_entry.input

for i in `cat /tmp/reset_pass.input`
do
${i} #run the script
rm -r /tmp/host.not
for i in `cat /tmp/login_entry.input`
do
for HOST in $(awk '{print $1}' $SCRIPTS_DIR/sw_ip.txt |sort|uniq)
do
echo ${i}
grep ${HOST} ${i} |grep -q "changed passwd"
if [ ${?} = 1 ]
then
echo $HOST >>/tmp/host.not
mail -s "${i}, errors" $MAILTO < /tmp/host.not
fi
done
done
done

[Sridhar Bhaskarla · Accepted solution]

Hi,

If you looked at my first response in the other post, I am checking for a variable called "STAMP" to see if it is set to 1. If it sets, then I will send the mail with the contents of the temporary file. I set STAMP to 1 when I find a case. This is your modified script to add that functionality.

find $SCRIPTS_DIR/reset_pass* -print > /tmp/reset_pass.input
find $LOGS_DIR/login_entry_* -print > /tmp/login_entry.input

for i in `cat /tmp/reset_pass.input`
do
${i} #run the script
rm -r /tmp/host.not
STAMP=0
for i in `cat /tmp/login_entry.input`
do
for HOST in $(awk '{print $1}' $SCRIPTS_DIR/sw_ip.txt |sort|uniq)
do
echo ${i}
grep ${HOST} ${i} |grep -q "changed passwd"
if [ ${?} = 1 ]
then
echo $HOST >>/tmp/host.not
STAMP=1
fi
done

if [ $STAMP = 1 ]
then
mail -s "${i}, errors" $MAILTO < /tmp/host.not
fi

done
done


-Sri

[Michael Schulte zur Sur]

Hi,

for one, I would assign different loop variables for nested loops to avoid confusion. Then the return code of grep is 1 when no match was there. You have to compare to 0. Joint the two grep like grep -q "${HOST}.*changed passwd" ${i}

Michael

[Sridhar Bhaskarla]

Hi,

Michael's point is to be noted. 'i' is defined in first and second for loops. While it may work in this particular case, it is not a very good idea in general. It's better to use some meaningful names like 'for SCRIPT in `cat /tmp/reset_pass.input`, 'for ENTRY in `cat /tmp/login_entry.input` etc., Also, Posix and shell allow $() for command substitution so you don't have to worry about the ticks that get you confused sometimes with other quotes.

There is a small correction in my previous post. Put "STAMP=0" just above 'for HOST' loop.


for i in `cat /tmp/reset_pass.input`
do
${i} #run the script
rm -r /tmp/host.not
for i in `cat /tmp/login_entry.input`
do
STAMP=0
for HOST in $(awk '{print $1}' $SCRIPTS_DIR/sw_ip.txt |sort|uniq)
do

Rest remains the same.

It is a good idea to use {} while working with variables. But my personal preference is to use them only when needed. For ex., if I have to define a file name outputfile.${SCRIPT}.${DATE} to cause less confusion. However, I don't use {} convention necessarily for simple checks (if [ $? = 0 ])as it may be more confusing there.

Multiple greps can be combined into one.

-Sri

[Ratzie]

I have modified my script and now I am in an endless loop, and I recieve endless emails...

All I should be getting is 5 emails, and only showing the IPs that do not have "change passwd"

Really appreciate another pair of eyes.

[Michael Schulte zur Sur]

Hi,

rm -r /tmp/host.not # delete the -r
No need to delete a single file recursively

sort|uniq can be shortened to sort -u

$? = 1 should be $? = 0 # see man grep for return codes

I really can't see, why you should get an endless loop. May be a lot of loops, but the script should terminate.

Michael

[Elmar P. Kolkman]

The maximum number of mails you get is the number of entries in /tmp/reset_pass.input times the number of entries in /tmp/login_entry.input.

If that's not what you want, you need to check the creation of those files.

What you could do is add an echo to the mail line and then redirect the output of the script to a file and look at the size of the resulting files.

You could also skip all real actions and just print your loop variables to see if those are flowing correctly.

I see no usage of the outer loop variable in the inner loop variables, so the internal loop (with LOGIN_ENTRY) will result in the same output for all occurrences of the outer loop (with SCRIPT_EXP). Perhaps that's causing your problem?

And one more thing: you can send a mail for every LOGIN_ENTRY occurrence for every SCRIPT_EXP... Perhaps you should put that part after the 'done' line for the LOGIN_ENTRY loop, instead of before?
Because the removal of the host.not file is done before the 'for LOGIN_ENTRY' line, that seems what you want... Or you should put the removal of host.not after the 'for LOGIN_ENTRY' line.