Linux HDD/CPU Temperature Logging Bash Script to run every minute in crontab

Question

I wrote a simple script to log HDD and CPU temps to a log file. If those temperatures reach a threshold, it will shutdown the server. I also made the script make sure the log files don't get over a certain file size. Any ways to improve the code would be greatly appreciated, as I am a noob to programming.

#!/bin/bash
# Check HDD Temps, CPU temp.. shut down server if any exceed acceptable limit
# Using hddtemp to check HDD temps, sensors command to check CPU temp.
date=$(TZ='America/Los_Angeles' date)
HDD1=$(sudo hddtemp /dev/sdb | cut -d":" -f1,2)
HDD2=$(sudo hddtemp /dev/sdc | cut -d":" -f1,2)
T1=$(sudo hddtemp /dev/sdb | cut -d":" -f3 | cut -d" " -f2 | cut -c1-2)
T2=$(sudo hddtemp /dev/sdc | cut -d":" -f3 | cut -d" " -f2 | cut -c1-2)
CPU1=$(sensors | grep "Core" | head -1 | cut -d"+" -f2 | cut -d"." -f1)
CPU2=$(sensors | grep "Core" | tail -1 | cut -d"+" -f2 | cut -d"." -f1)
base_file='/home/brandon/logs/temp1.log'
file='/home/brandon/logs/temp'
log='.log'
error='.error'
find_lowest_file_num() {
 local lowest_file_num=$(ls /home/brandon/logs/ | grep temp | cut -d"p" -f2 | cut -d"." -f1 | sort -rn | tail -1)
 if [ -z "$lowest_file_num" ]
 then
 touch $base_file
 echo 1
 else
 echo $lowest_file_num
 fi
}
find_highest_file_num() {
 local highest_file_num=$(ls /home/brandon/logs/ | grep temp | cut -d"p" -f2 | cut -d"." -f1 | sort -rn | head -1)
 # if highest_file_num is empty, that means there is no temp files, so create one
 if [ -z "$highest_file_num" ]
 then
 touch $base_file
 echo 1
 #else return highest_file_num
 else
 echo $highest_file_num 
 fi
}
highest_file_num=$(find_highest_file_num)
lowest_file_num=$(find_lowest_file_num)
get_curr_highest_file() {
 local h=${file}${highest_file_num}${log}
 echo $h
}
get_curr_lowest_file() {
 local l=${file}${lowest_file_num}${log}
 echo $l
}
curr_file=$(get_curr_highest_file)
echo "HDD1: ${HDD1} Temp:${T1}C ${date} " >> $curr_file
echo "HDD2: ${HDD2} Temp:${T2}C ${date} " >> $curr_file
echo "CPU Core 1 Temp:${CPU1}C ${date} " >> $curr_file
echo "CPU Core 2 Temp:${CPU2}C ${date} " >> $curr_file
#if files get bigger than 1000k, make a new one
if [ $(du -k $curr_file | cut -f1) -gt 1000 ]
then
 let "highest_file_num++"
 touch $(get_curr_highest_file)
fi
if [ $(du -k /home/brandon/logs | cut -f1) -gt 100000 ]
then
 rm -f $(get_curr_lowest_file)
fi
if [ $T1 -gt 50 ] || [ $T2 -gt 50 ] || [ $CPU1 -gt 80 ] || [ $CPU2 -gt 80 ]
then
 echo "HIGH TEMPS DETECTED!!! HDD1 Temp: ${T1}C HDD2 Temp: ${T2}C ${date}" >> ${curr_file}${error}
 echo "CPU1: ${CPU1}C, CPU2: ${CPU2} ${date}" >> ${curr_file}${error}
 echo "Shutting down..." >> ${curr_file}${error}
 sudo shutdown now
fi

• RomanPerekhrest: can you post a sample raw output of hddtemp /dev/sdb and sensors commands?

Answer 1

Here is my revision. I changed it that instead of hardcoding the user's home directory the script can just get that from env. Also, I would run this directly as a root account chron task rather than configuring sudo to be able to run shutdown and hddtemp. Finally, instead of using if -z to check for a file's existance, I would use test -f or something. Anyway, not bad at all for a 'noob', I'd give yourself more credit! Here is my version:

#!/bin/bash
# Check HDD Temps, CPU temp.. shut down server if any exceed acceptable limit
# Using hddtemp to check HDD temps, sensors command to check CPU temp.
date=$(TZ='America/Los_Angeles' date)
# Consider running as root directly instead of requiring sudo (unless you want \
# to configure a nonroot user to be able to use `hddtemp` and `shutdown`) \ 
# which would work, but since you're running as chron, I would just run \
# it as a root cron job
test $(id -u) == 0 || {echo 'This script must be run as root'; exit 1}
HDD1=$(hddtemp /dev/sdb | cut -d":" -f1,2) 
HDD2=$(hddtemp /dev/sdc | cut -d":" -f1,2)
T1=$(sudo hddtemp /dev/sdb | cut -d":" -f3 | cut -d" " -f2 | cut -c1-2)
T2=$(sudo hddtemp /dev/sdc | cut -d":" -f3 | cut -d" " -f2 | cut -c1-2)
CPU1=$(sensors | grep "Core" | head -1 | cut -d"+" -f2 | cut -d"." -f1)
CPU2=$(sensors | grep "Core" | tail -1 | cut -d"+" -f2 | cut -d"." -f1)
logdir="/home/$USER/templogs"
base_file="${logdir}/temp1.log"
file="${logdir}/temp"
log='.log'
error='.error'
test -d $logdir||mkdir $logdir 2>/dev/null
find_lowest_file_num() {
 local lowest_file_num=$(ls ${logdir}/ | grep temp | cut -d"p" -f2 | cut -d"." -f1 | sort -rn | tail -1)
 if [ -z "$lowest_file_num" ]
 then
 touch $base_file
 echo 1
 else
 echo $lowest_file_num
 fi
}
find_highest_file_num() {
 local highest_file_num=$(ls ${logdir}/ | grep temp | cut -d"p" -f2 | cut -d"." -f1 | sort -rn | head -1)
 # How about using `test` instead of ` if -z`? 
 test -f "$highest_file_num" || { >"$highest_file_num" ; echo 'Created temp file' ;} && echo "highest_file_num is present"
}
highest_file_num=$(find_highest_file_num)
lowest_file_num=$(find_lowest_file_num)
get_curr_highest_file() {
 local h=${file}${highest_file_num}${log}
 echo $h
}
get_curr_lowest_file() {
 local l=${file}${lowest_file_num}${log}
 echo $l
}
curr_file=get_curr_highest_file
echo "HDD1: ${HDD1} Temp:${T1}C ${date} " >> $curr_file
echo "HDD2: ${HDD2} Temp:${T2}C ${date} " >> $curr_file
echo "CPU Core 1 Temp:${CPU1}C ${date} " >> $curr_file
echo "CPU Core 2 Temp:${CPU2}C ${date} " >> $curr_file
#if files get bigger than 1000k, make a new one
if [ $(du -k $curr_file | cut -f1) -gt 1000 ]
then
 let "highest_file_num++"
 touch $(get_curr_highest_file)
fi
if [ $(du -k $logdir | cut -f1) -gt 100000 ]
then
 rm -f $(get_curr_lowest_file)
fi
if [ $T1 -gt 50 ] || [ $T2 -gt 50 ] || [ $CPU1 -gt 80 ] || [ $CPU2 -gt 80 ]
then
 echo "HIGH TEMPS DETECTED!!! HDD1 Temp: ${T1}C HDD2 Temp: ${T2}C ${date}" >> ${curr_file}${error}
 echo "CPU1: ${CPU1}C, CPU2: ${CPU2} ${date}" >> ${curr_file}${error}
 echo "Shutting down..." >> ${curr_file}${error}
 shutdown now # I would run this as root directly
else
 echo "Temps ok: HDD1: ${T1}, HDD2: ${T2}, CPU1: ${CPU1}, CPU2: ${CPU2}"
fi