3
\$\begingroup\$

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
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

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!)

\$\endgroup\$
3
  • \$\begingroup\$ 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 \$\endgroup\$
    – Felix G
    Feb 5, 2021 at 14:19
  • \$\begingroup\$ 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. \$\endgroup\$ Feb 5, 2021 at 14:24
  • \$\begingroup\$ 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! \$\endgroup\$ Feb 5, 2021 at 14:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.