Check disk space against a threshold

Question

The below script is one I made to check the disk space on mounted partitions under Debian Wheezy. I tried adding a -d switch to force printing out the used data, but its main purpose is to run as a cron job, and email the administrator if any disks are below a certain threshold.

What I'm looking for is to make sure it's POSIX compliant (in case it needs to be called by another script/application), and that it's rock solid. It works on my servers as is, with and without the -d. It requires that if an argument is given, it's a mount point. I am trying to get the proper coding practices done properly, as well.

#!/bin/sh
MOUNTPOINTS="/ /var /home" #The mount points to always check; the user can
 #specify more via the command line, or changing this.
THRESHOLD=85 #The amount of disk space used before printing the warning.
DEBUG=false #If TRUE when running (-d switch) lists all mount points, even if
 #the threshold has not been met.
while getopts ":dh" opt; do
 case $opt in
 d)
 DEBUG=true
 ;;
 h)
 echo USAGE: 0ドル \[-d\] \[\/mount\/point\/1 ...\]
 exit 0
 ;;
 \?)
 echo Incorrect syntax
 ;;
 esac
 shift $((OPTIND - 1))
done
if [ "$#" -gt 0 ]; then
 for var in $@; do
 MOUNTPOINTS="${MOUNTPOINTS} ${var} "
 done
fi
for MOUNT in ${MOUNTPOINTS}; do
 CURRENT=$(df ${MOUNT} | grep / | awk '{ print 5ドル}' | sed 's/%//g')
 if ($DEBUG); then
 printf "%20s\t%s\n" ${MOUNT} ${CURRENT}%
 fi
 if [ "${CURRENT}" -gt "${THRESHOLD}" ] ; then
 echo "Your ${MOUNT} partition\'s remaining space is critically low. Used: ${CURRENT}%"
 fi
done

• Etan Reisner: That shift is before the done of the while loop. And yes, spaces and other shell glob characters *, [] and ?.
• Canadian Luke: What I meant was that in my script, I moved it (I was told not to "edit" original scripts on Code Review, but create a new question after taking many helpful hints into account)
• Etan Reisner: Ah, I misunderstood your comment then. Sorry.

Answer 1

Using a string for MOUNTPOINTS means you can't have any mount points with spaces or glob characters in the names. I realize you likely can't use an array if you want real portability but that's a severe limitation that you might want to check for.

I don't know what you can do (without using an array) that will allow directories like that to work but you can check the input arguments for compatibility and disallow any that don't fit those limitations.

As I said on some other question (of yours I believe) that shift in the while loop is incorrect. You are shifting off (an increasing number of arguments) during the getopts processing. You want to do that shift once and after getopts is done.

Try this and see what I mean:

$ cat test-getopts.sh
#!/bin/sh
while getopts ":dh" opt; do
 case $opt in
 d) echo DEBUG=true;;
 h) echo HELP;;
 \?) echo error;;
 esac
 printf 'Shifting %d\n' $((OPTIND - 1))
 shift $((OPTIND - 1))
done
$ /bin/sh go.sh -d -h -d -h -d -h -d -h -d -h -d -h
DEBUG=true
Shifting 1
DEBUG=true
Shifting 2
HELP
Shifting 3
HELP
Shifting 4

There is almost never a reason to pipe grep to awk or sed to awk or awk to sed. awk can almost certainly do everything you need.

So the grep | awk | sed part of $(df ${MOUNT} | grep / | awk '{ print 5ドル}' | sed 's/%//g') can be replaced by a single awk call:

`$(df "${MOUNT}" | awk '/\/{gsub(/%/, ""); print}')`

You'll also notice I quoted "${MOUNT}" there. In general you always want to quote shell variable expansions. In this case it can't actually matter since anything these quotes would protect will already have been expanded/word-split by the for MOUNT in ${MOUNTPOINTS} loop but it is the right thing to do anyway.

Also related to df if you are going to try to field-parse the output you want to use the -P/--portability option. It keeps df (at least from coreutils) from wrapping long lines to get columns to line up (and breaking field counts).

The syntax of if tests in the shell does not require wrapping parentheses. In fact, adding them spawns a sub-shell. So if ($DEBUG); then is running the value of $DEBUG as a command in a sub-shell. If you were to have set DEBUG=debug instead of DEBUG=true you'd have gotten an error from that because debug isn't a valid command. Drop them and use a non-empty or value checking test instead. Either if [ -n "$DEBUG" ]; or if [ "$DEBUG" = DEBUG ];.

You don't need to quote single quotes in a double quoted string so:

echo "Your ${MOUNT} partition\'s remaining space is critically low. Used: ${CURRENT}%"

should be echoing partition\'s out which probably isn't what you want.

for var in $@; do can be simplified to for var; do if you wanted and if you don't then you should use for var in "$@"; do though again given that spaces/glob characters are going to be a problem later this isn't as useful as it might be (though it does let you test for them correctly and avoid them as discussed above).

Also, if you haven't already, you should run your code through shellcheck.

• Canadian Luke: That shellcheck thing is pretty cool, book marked! I'll admit, I know very little about awk or sed yet, and I'm trying to incorporate small bits at a time. I appreciate your help, here and on SO

Answer 2

The other answers made very good points, in the following I will mention only what was missed.

---

First of all, don't be afraid to use Bash instead of /bin/sh. Bash must exist in any decent system today, and it will enable you to use arrays, which would simplify the handling of mount points with spaces.

That said, for the record, there is a way to make the script work with spaces in mount point names without arrays, like this:

for path; do
 MOUNTPOINTS="$MOUNTPOINTS \"$path\""
done
eval "set -- $args"

A few things to note here:

• Don't do for arg in $@. If you have arguments "a" and "b c", then this will iterate over "a", "b", "c". You almost always want to do for arg in "$@" instead, which will correctly iterate over "a" and "b c".
• for arg; do is the same as for arg in "$@"; do, so you can use the shorter form
• The trick to make spaces in arguments work is to append to MOUNTPOINTS the arguments double-quoted, and then use an eval.
• set -- a "b c" command sets the positional arguments (the value of $@) to "a", "b c". We need to wrap this in an eval so the shell interprets the double-quotes that we added inside MOUNTPOINTS, otherwise they would be taken literally. However, as @etan-reisner pointed out in a comment, although this is safe for spaces, it isn't safe for double quotes. It also allows for arbitrary command execution if the input does contain them.

I used this technique for many years to work around crappy old systems that didn't have Bash. Today, it's an unnecessary awkwardness. I recommend to use proper arrays instead. Also keep in mind that eval is generally considered evil and a last resort in all languages that have it.

If in some of the systems Bash is not at /bin/bash, then you can use this shebang instead:

#!/usr/bin/env bash

---

The indentation is inconsistent. In the beginning you used a tab width of 8 spaces, while at the end you used a tab width of 2 spaces. It's better to be consistent throughout. (I use a width of 4.) If you use vim, an easy way to reindent the entire file is with the gg=G command.

---

In some languages it is preferred to not use inline comments, but put them on the line before the statement they refer to. I like that idea because it often reduces the length of the line. Keep in mind that long lines can be hard to read, so it's good to keep line length short.

In some languages, when you use inline comments, it's preferred to put at least two spaces in front of the comment marker, and one space after the comment marker. So instead of this:

MOUNTPOINTS="/ /var /home" #The mount points to always check; ...

This is preferred:

MOUNTPOINTS="/ /var /home" # The mount points to always check; ...

---

Too many escape symbols hurt readability here:

echo USAGE: 0ドル \[-d\] \[\/mount\/point\/1 ...\]

Better to use quotes so you don't need to escape:

echo "USAGE: 0ドル [-d] [/mount/point/1 ...]"

Btw, even in the first version, you didn't need to escape /.

---

In this part:

 \?)
 echo Incorrect syntax
 ;;

Maybe it would be better to exit with an error, or to make the message more descriptive:

 \?)
 echo Incorrect option: $OPTARG
 ;;

---

if [ "$#" -gt 0 ]; then

You don't need to quote $#, as it's never empty.

---

 MOUNTPOINTS="${MOUNTPOINTS} ${var} "

This could be simplified:

 MOUNTPOINTS="$MOUNTPOINTS $var"

That is, the {} and the trailing space were unnecessary. If the {} make you feel more comfortable, and perhaps more readable, then it's fine, there's nothing wrong with them, I'm just lazy to type unnecessary things.

Putting it together

Using some of the suggestions from this and other answers, I recommend this implementation:

#!/usr/bin/env bash
# The mount points to always check; the user can
# specify more via the command line, or changing this.
MOUNTPOINTS=(/ /var /home)
# The amount of disk space used before printing the warning.
THRESHOLD=65
# If TRUE when running (-d switch) lists all mount points, even if
# the threshold has not been met.
DEBUG=false
while getopts ":dh" opt; do
 case $opt in
 d)
 DEBUG=true
 ;;
 h)
 echo "USAGE: 0ドル [-d] [/mount/point/1 ...]"
 exit 0
 ;;
 \?)
 echo Incorrect option: "$OPTARG"
 ;;
 esac
done
shift $((OPTIND - 1))
for path; do
 MOUNTPOINTS+=("$path")
done
set -- "${MOUNTPOINTS[@]}"
for MOUNT; do
 test -d "$MOUNT" || continue
 CURRENT=$(df "$MOUNT" | awk '/\// {gsub(/%/, ""); print 5ドル}')
 if [ $DEBUG = true ]; then
 printf "%20s\t%d%%\n" $MOUNT "$CURRENT"
 fi
 if [ "$CURRENT" -gt "$THRESHOLD" ] ; then
 echo "Your $MOUNT partition's remaining space is critically low. Used: $CURRENT%"
 fi
done

• Etan Reisner: The eval "set -- $args" trick may be safe for spaces but isn't safe for double quotes (and as such allows for arbitrary command execution if the input does contain them).
• janos: Thanks @EtanReisner, I amended my answer, paraphrasing your comment.

Answer 3

There are a number of concerns I have here, some of them style related, but mostly about the actual functionality ...

mounts

/var and /home may not necessarily be actual mount points. You should first check to see whether it is a mount point before testing it. This is because it may inaccurately reflect the / space if it is not actually a mount.

Additionally, it is (remotely) possible that, even if it is normally a mount point, that the partition may be unmounted too, but that's a remote problem.

The program /bin/mountpoint is a useful one for identifying whether a folder is in fact a mountpoint. It returns a useful exit code (0 if it is).

/bin/mountpoint -q / && echo YES

I would also possibly recommend that you just check all mounted filesystems. Something like this will do you well:

# Get mounts that come from non-kernel "magic" places.
grep -e '^/' /proc/mounts | cut -d' ' -f2

/boot

Additionally, I highly recommend that you test the /boot folder. More than any other folder, this one has been a problem for me personally.

/boot is used for your kernel images, and it is relatively common to apply system updates, and then forget to remove old images. The /boot folder is quite small, and often can have less than 10 kernel images ... This has resulted in a few problems for me. Add /boot to your 'default' list of partitions to check. It will remind you to remove old and unused kernel versions, and prevent failed updates.

Why shell? Got Perl?

df -h | perl -ne 'chomp; $line = $_; $_ =~ s/.*?\s+(\S+)\%.*/1ドル/; next unless $_ eq "Use" or $_ > 80; print "$line\n";'

For me that outputs:

Filesystem Size Used Avail Use% Mounted on
/dev/mapper/mediavg-medialv 1.8T 1.7T 17G 100% /multimedia
/dev/mapper/snapvg-snaplv 2.7T 2.4T 349G 88% /snapshotng

(Filesystems with > 80% usage).

You are aware that the df command will go a long way to solving the whole thing for you, right?

grep/awk/sed

df ${MOUNT} | grep / | awk '{ print 5ドル}' | sed 's/%//g'

This is a complicated line. I’m pretty sure you can get awk to do the whole thing for you ...

• Etan Reisner: Good point about checking for a real mountpoint and the mountpoint command. Also about just munging the df output directly for this instead of faffing about with finding mountpoints.
• Canadian Luke: Currently, it works even without the mount points being actual mount points - As in, if I have one large / partition only, it still works with any other folder. You're right, I should do error checking as well. As for Perl, I want to learn only a few languages at a time, then try other languages. Eventually, I'd like to have a .conf file to read from, and probably store the variables there, but that's for a later date
• rolfl: The perl section is only an indication that parsing the output from df as a group is probably better than building your own df-with-a-threshold system. df can do all the heavy lifting for you.... use it.