Re: Help with script - doesn't work properly from cron

[Greg Wooledge]

On Fri, Jul 17, 2009 at 02:53:21PM -0700, Erik Olof Wahlstrom wrote:
> #!/bin/bash
> BACKUP_DIR="/media/disk/AUTOMATED_BACKUPS/DB_DAILY"
> 
> CURRENT_DIR=$BACKUP_DIR/`date +%d`

  # See how you call date here without an explicit path?  That's good.

> DATABASES="$(/usr/bin/mysql -uUsername -pPassword -Bse 'show databases')"
> echo 'Backing up databases: '$DATABASES

  echo "Backing up databases: $DATABASES"

> if [ -e "$CURRENT_DIR" ]

  if [ -d "$CURRENT_DIR" ]

  # Of course, if it's a file, you would have problems below....

> then
>         cd $CURRENT_DIR
>         /bin/rm *

         cd "$CURRENT_DIR" || exit 1
         rm -f *

         # NEVER do "rm *" after a "cd" without checking that the cd
         # succeeded!

         # You don't need an explicit path on rm.  You didn't need one
         # on date, so why put one on rm?

         # In this particular version of this particular script, you can
         # get away with not quoting the $CURRENT_DIR parameter expansion,
         # but quoting it will save your butt if someone changes it, e.g.:
         # CURRENT_DIR='/media/disk/c/Program Files'

> else
>         /bin/mkdir $CURRENT_DIR

          mkdir -p "$CURRENT_DIR"

>         cd $CURRENT_DIR

          cd "$CURRENT_DIR" || exit 1

> fi
> 
> for DB in $DATABASES
> do
>         /usr/bin/mysqldump -uroot -pHardAsMySql321 "$DB" | bzip2 >
> "$DB"_`date +%Y-%m-%d_%k.%M`".sql.bz2"

         # Long line, probably broken by your mailer.  For clarity, I'd
         # write it on two lines explicitily:

         mysqldump -uroot -pHardAsMySql321 "$DB" |
           bzip2 > "${DB}_$(date +%Y-%m-%d_%k.%M).sql.bz2"

> done
> 
> exit 0
> 
> Can any skilled eyes see why this doesn't work when it is run from the roots
> crontab?

None of the changes I suggested should really affect that.  Perhaps it's
some sort of mysql-specific thing?

> Additionally, when I run the script as root in the terminal, I get the
> following output:
> 
> Backing up databases: information_schema db1 db2 db3
> /bin/rm: cannot remove `*': No such file or directory

The "rm -f" will suppress that error message.

> Is there a better way to clear out last months files before making the
> current backups?

You could also consider writing it this way:

  cd /
  rm -rf "$CURRENT_DIR"
  mkdir -p "$CURRENT_DIR"
  cd "$CURRENT_DIR" || exit 1

Then you don't even need to check whether it already exists.


On Sat, Jul 18, 2009 at 11:25:13AM +0200, Bernd Eggink wrote:
> You could replace the whole if-then-else clause by
> 
>     mkdir -p $CURRENT_DIR
>     cd $CURRENT_DIR
>     rm -f *

Another failure to check the results of "cd" before doing "rm *".  This
can and will lead to disasters.