Bash script for log rotation

Question

Okay, first things first:

Yes, I am aware of the logrotate tool, which would usually be used for something like this. However, since the logfile i want to rotate already contains a timestamp in the filename, logrotate will never delete old logs.

This script is supposed to:

1. tell the application to create a new logfile (via SIGHUP)
2. find all existing, unprocessed logfiles of the application
3. compress all logfiles except the one currently in use by the application
4. keep the most recent 7 logs and delete the rest.

From what I can tell, everything seems to work just fine, but I'm curious if anything could be improved.

#!/usr/bin/bash
procname="foo"
srcpath="/var/log"
srcname="*${procname}.log" # example: 2021年02月05日_1200_foo.log
count=7
echo "rotate $procname logs: ${srcpath}/${srcname} ($count rotations)"
pid="$(pidof $procname)"
if [[ ! $? == 0 ]]
then
 # don't rotate anything if the application is not running
 echo "$procname process not running"
 exit 1
fi
# ask the application to create a new logfile
kill -HUP "$pid"
sleep 1 #probably not necessary?
# get array of all logfiles (should be exactly 2 in most cases)
mapfile -t list < <(find "$srcpath" -maxdepth 1 -name "$srcname" | sort)
size=${#list[@]}
# don't do anything unless there are at least 2 files
if [[ $size -lt 2 ]]
then
 echo "nothing to do"
 exit 0
fi
# find the active logfile (should be the last one) and delete it from the array
for ((i=size-1; i>=0; i--));
do
 if [[ $(find -L /proc/"$pid"/fd -inum "$(stat -c '%i' "${list[i]}")") ]]
 then
 unset "list[i]"
 break
 fi
done
# compress all remaining files (usually just one)
gzip "${list[@]}"
unset list
unset size
# get array of all compressed logfiles
mapfile -t list < <(find "$srcpath" -maxdepth 1 -name "$srcname.gz" | sort)
size=${#list[@]}
if [[ $size -gt $count ]]
then
 idx=("${!list[@]}")
 # remove $count most recent files from $list
 for i in "${idx[@]: -$count:$count}"
 do
 unset "list[$i]"
 done
 # delete the remaining old files
 rm -f "${list[@]}"
fi
exit 0

Answer 1

Shellcheck points out this one:

pid="$(pidof $procname)"
if [[ ! $? == 0 ]]
then

We can write that more clearly

if ! pid="$(pidof $procname)"
then

Shortly after that is:

# don't rotate anything if the application is not running
echo "$procname process not running"

That looks like an error message - redirect that to &2.

I'm a little concerned about

# find the active logfile (should be the last one) and delete it from the array

If the daemon didn't get around to opening a new file by the time we reach here, then we'll be out of sync. That said, it probably doesn't matter - we'll just have one more unrotated file around until we next run, which isn't a big concern.

Take care with commands like this:

gzip "${list[@]}"

It's a good idea to use -- so we can be sure that filenames beginning with - aren't interpreted as options.

Also, commands such as gzip can fail - do we really want to keep going if they do? (This question isn't so simple to answer, as one of the likely reasons is "no space on device", and in that case we do want to delete some files!)

• Felix G: Thanks! I didn't know you could have an assignment inside an if clause, that's much nicer than the $? version. Concerning the out-of-sync problem, that's why i added the sleep 1, which of course isn't a guarantee either, but hopefully better than nothing. Didn't think about the --, i will add it to gzip and rm to be safe (even though the filenames shouldn't begin with -). I'll need to think about the error handling... to be honest, i'm not quite sure what i want to do if gzip fails
• Toby Speight: Yes, I saw the sleep, with its comment, so did know you're aware of it. Sorry I don't have anything better to suggest there.
• Toby Speight: I thought the argument lists wouldn't start with -, but it's often easier just to drop -- into the command to save precious brainpower reasoning about it!