LinuxQuestions.org

LinuxQuestions.org (/questions/)
-   Linux - General (http://www.linuxquestions.org/questions/linux-general-1/)
-   -   Ugly Script Looking for a Plastic Surgeon. (http://www.linuxquestions.org/questions/linux-general-1/ugly-script-looking-for-a-plastic-surgeon-785390/)

No_one_knows_me 01-28-2010 10:58 AM

Ugly Script Looking for a Plastic Surgeon.
 
I know, I know, this script is as ugly as sin, so I need some help in cleaning it up.

Code:

#!/bin/bash
#Script to find log files in /var/log directory that are 2 days old, gzip them then move them to /var/log/tmp, rename them
#to the system date + filename and move them back to /var/log.


DIR=/var/log
TMP=$DIR/tmp
HOSTNAME=`uname -n`

mkdir -p $TMP

cd $DIR

for i in $(find . -name 'cron.*' -type f -mtime +2) ; do mv $i tmp ; done
for i in $(find . -name 'ksyms.*' -type f -mtime +2) ; do mv $i tmp ; done
for i in $(find . -name 'messages.*' -type f -mtime +2) ; do mv $i tmp ; done
for i in $(find . -name 'rpmpkgs.*' -type f -mtime +2) ; do mv $i tmp ; done
for i in $(find . -name 'vmke*.*' -type f -mtime +2) ; do mv $i tmp ; done
for i in $(find . -name 'vmkw*.*' -type f -mtime +2) ; do mv $i tmp ; done
for i in $(find . -name 'secure.*' -type f -mtime +2) ; do mv $i tmp ; done
for i in $(find . -name '*.log*' -type f -mtime +2) ; do mv $i tmp ; done

cd $TMP
for i in `ls`
do
mv $i `date +%F.$HOSTNAME.$i`
done

for i in `ls`
do
mv $i $DIR
cd ..
rmdir $TMP
done

exit 0

Thanks in advance for your input, suggestions, comments.

GrapefruiTgirl 01-28-2010 11:17 AM

I don't thing the script is ugly at all; it's pretty clean, and somewhat formatted too.

I suspect that all those `find` loops commands could be reduced to something much smaller, but at this moment, I see several accomplished scripters/coders lurking in the thread ;) so I'm going to refrain from commenting further for now.

I suspect there will be improvement suggestions given shortly!

Sasha

catkin 01-28-2010 11:37 AM

Quote:

Originally Posted by GrapefruiTgirl (Post 3843837)
but at this moment, I see several accomplished scripters/coders lurking in the thread ;) so I'm going to refrain from commenting further for now.

How do you do that?

H_TeXMeX_H 01-28-2010 01:26 PM

I have a question, why do you specify the exact files in each command, and most of them are just one file, there's only one messages log file for example, and only one cron.

Well, let's just say that you need to do it in this particular way, what I would do to simplify it is:

Code:

find . -type f -mtime +2 | grep -e cron.* -e ksyms.* -e messages.*
Also, why not do 'for i in *' instead of 'for i in `ls`'. I recommend you stay away from using the old-style backticks ``` and use the new format $(), so $(ls -1), but you don't need to use ls here.

GrapefruiTgirl 01-28-2010 01:54 PM

Code:

#!/bin/bash
#Script to find log files in /var/log directory that are 2 days old,
#gzip them then move them to /var/log/tmp, rename them
#to the system date + filename and move them back to /var/log.
# note: sasha says: there's no zipping/compressing involved in this script yet..
# sasha also says: like TexmeX implied, there may be only one of each logfile
# so doing the moving/copying like this is maybe overkill. But it works.
# note: read `find` man page for security concerns regarding -exec
# and realize there's no sanity checking in here, so for example, I believe
# that `find -exec mv` will overwrite existing files in $TMP without a care.

CWD=$(pwd)
DIR=/var/log
TMP=$DIR/tmp
HOSTNAME=$(uname -n)

mkdir -p $TMP
( cd $DIR
  # first, mv most of the files:
  for log in kdm kernel cron ksyms messages crappo rpmpkgs vmke vmkw secure; do
    find . -name "$log.*" -maxdepth 1 -type f  -mtime +2 -exec mv "{}" $TMP/ \;
  done
  unset log
  # now mv any that were not mv'd the first time.
  # note: next line will possibly mv files that were already mv'd above!
  find . -name "*.log*" -maxdepth 1 -type f  -mtime +2 -exec mv "{}" $TMP/ \;
)

( cd $TMP
  # mv all files to new filename, and put back in $DIR:
  for stuff in *; do
    mv "$stuff" "$DIR/$(date +%F).$HOSTNAME.$stuff"
  done
)

cd $CWD

# uncomment next line to actually permanently delete $TMP in production
# rm -Rf $TMP

exit 0

@ catkin -- at the bottom of the thread, it says "..members viewing this thread.."

Sasha

tuxdev 01-28-2010 02:18 PM

Just a bit more quoting and some tweaks here and there..
Code:

dir="/var/log"
tmp="$DIR/tmp"
hostname="$(uname -n)"

mkdir -p "$tmp"

# first, mv most of the files:
for log in kdm kernel cron ksyms messages crappo rpmpkgs vmke vmkw secure; do
  find "$dir" -name "$log.*" -maxdepth 1 -type f -mtime +2 -exec cp "{}" "$tmp" +
done
# now mv any that were not mv'd the first time.
# note: next line will possibly mv files that were already mv'd above!
find "$dir" -name "*.log*" -maxdepth 1 -type f  -mtime +2 -exec cp "{}" "$tmp" +

# mv all files to new filename, and put back in $DIR:
for file in "$tmp"/*; do
  mv "$file" "$dir/$(date +%F).$hostname.$file"
done

# uncomment next line to actually permanently delete $TMP in production
# rm -Rf "$tmp"


GrapefruiTgirl 01-28-2010 02:24 PM

Yes! Never too many "quotes" for sure.

@ tuxdev -- can you clarify for me (us?) how the + works with -exec?

The way I understand it from the man page, is using the plus-sign will create one long, but single, command out of everything that `find` finds, and `exec` it all at once, as opposed to repeatedly doing a unique `exec` for each and every item found? Is this correct?

Sasha

tuxdev 01-28-2010 03:03 PM

Quote:

@ tuxdev -- can you clarify for me (us?) how the + works with -exec?

The way I understand it from the man page, is using the plus-sign will create one long, but single, command out of everything that `find` finds, and `exec` it all at once, as opposed to repeatedly doing a unique `exec` for each and every item found? Is this correct?
Yep, that's pretty much it. It saves on spawning processes that aren't really needed to Get The Job Done.

No_one_knows_me 01-29-2010 08:44 AM

Sasha,

I didn't realize that I hadn't implemented gzipping in the script, even though I put in the comments of the script that I would be gzipping them, so thank you for bringing that to my attention. Here is my thinking, and let me know if I'm correct or not.
Quote:

# mv all files to new filename, and put back in $DIR:
for file in "$tmp"/*; do
mv "$file" "$dir/$(date +%F).$hostname.$file" -exec gzip
done
The reason for the mv and cp of the single log files is that I am trying to implement logrotate in my audit process, and I believe I had it set up to create new files weekly (but if anyone out there knows logrotate in-depth, I could really use your expertise here).

H_TexMex_H,

I crafted this script last month, having not written a script on my own before that, so I relied on other people's existing scripts to fabricate this one, so apparently they used the old school method. Thanks for the input, and I will implement the $() way henceforth.

tuxdev,

I'm digging the + and -exec you suggested! It cleans up the script nicely, and as you mentioned, it doesn't spawn un-necessary processes, thus making it easier on the system.

catkin 01-29-2010 09:24 AM

Quote:

Originally Posted by GrapefruiTgirl (Post 3844001)
@ catkin -- at the bottom of the thread, it says "..members viewing this thread.."

Good Lord! So it does. I guess that counts me out for the Observer Corps then.

H_TeXMeX_H 01-29-2010 09:52 AM

It's funny because if everyone were to think, hey look at that, all good scripters viewing this thread, I'll let them respond ... nobody will respond.

GrapefruiTgirl 01-29-2010 11:21 AM

Quote:

Originally Posted by H_TeXMeX_H (Post 3845011)
It's funny because if everyone were to think, hey look at that, all good scripters viewing this thread, I'll let them respond ... nobody will respond.

:p that had occurred to me. I was trying an experiment:

Usually when there's a little scripting thread like this, many of us enjoy throwing our take on the situation into the mix. Which is all good :) don't get me wrong.
This time, exactly what you wrote, happened: I left, and everyone else left :D

LOL

GrapefruiTgirl 01-29-2010 11:32 AM

Quote:

Originally Posted by the OP
I didn't realize that I hadn't implemented gzipping in the script, even though I put in the comments of the script that I would be gzipping them, so thank you for bringing that to my attention. Here is my thinking, and let me know if I'm correct or not.
[code]
# mv all files to new filename, and put back in $DIR:
for file in "$tmp"/*; do
mv "$file" "$dir/$(date +%F).$hostname.$file" -exec gzip
done
[code]

The problem I see here, is that -exec is a switch used with the `find` command, not the `mv` command. mv has no such operator.

Here's what I would do:
Code:

#!/bin/bash
#Script to find log files in /var/log directory that are 2 days old,
#gzip them then move them to /var/log/tmp, rename them
#to the system date + filename and move them back to /var/log.
# sasha says: Let's add come compression now.
# so doing the moving/copying like this is maybe overkill. But it works.
# note: read `find` man page for security concerns regarding -exec
# and realize there's no sanity checking in here, so for example, I believe
# that `find -exec mv` will overwrite existing files in $TMP without a care.

CWD=$(pwd)
DIR=/var/log
TMP=$DIR/tmp
HOSTNAME=$(uname -n)

mkdir -p $TMP
( cd $DIR
  # first, mv most of the files:
  for log in kdm kernel cron ksyms messages crappo rpmpkgs vmke vmkw secure; do
    find . -name "$log.*" -maxdepth 1 -type f  -mtime +2 -exec mv "{}" $TMP/ \;
  done
  unset log
  # now mv any that were not mv'd the first time.
  # note: next line will possibly mv files that were already mv'd above!
  find . -name "*.log*" -maxdepth 1 -type f  -mtime +2 -exec mv "{}" $TMP/ \;
)

( cd $TMP
  # mv all files to new filename, and put back in $DIR:
  for stuff in *; do
  # mv "$stuff" "$DIR/$(date +%F).$HOSTNAME.$stuff" # OLD COMMAND
  gzip -c -9 "$stuff" > "$DIR/$(date +%F).${HOSTNAME}.${stuff}.gz" # NEW COMMAND
  done
)

cd $CWD

# uncomment next line to actually permanently delete $TMP in production
# rm -Rf $TMP

exit 0

NOTE: I just copied my initial version here, and added the gzip line; so you will want to re-implement tuxdev's suggestions and any other changes you've made, in your own version..

Also, if the purpose of this is to rotate logs and compress them, I think you have the right idea (if you do) that `logrotate` is probably what you want. However, there's no harm doing it this way :) plus you learn stuff..

Sasha


All times are GMT -5. The time now is 02:45 AM.