Installing and removing kernel module RPMs

Question

First, the script checks to see if it is installing two kernel module packages or removing them by looking at the first parameter. If no parameter is provided it complains and exits.

If installing, it checks if each of the packages even needs to be installed by looking at the version of the existing module. If they do need to be installed, install them; If they don't, simply say so and the rest of the script runs its course.

If I am removing them just run the command to do so.

The problem is, I have a lot of if...else...fi and if...elif...fi blocks setting two variables and then testing different combinations of the two. Even though the script works as is I feel like there should be a better way to do this.

I moved one block into a function even though this just put it somewhere else. It didn't actually clean it up at all.

#!/bin/bash
awk='/bin/awk'
enic_pkg="/home/$(logname)/kmod-enic-2.1.1.75-rhel6u5.el6.x86_64.rpm"
fnic_pkg="/home/$(logname)/kmod-fnic-1.6.0.12b-1.el6.x86_64.rpm"
modinfo='/sbin/modinfo'
if [ ! 1ドル ]; then
 echo "Script must be executed with either 'install' or 'remove' as a paramter"
 exit
else
 method=1ドル
fi
test_ood() {
 if [ ! $enic = "2.1.1.75" ]; then
 enic_ood=true
 else
 echo "enic module is already up-to-date: $enic"
 fi
 if [ ! $fnic = "1.6.0.12b" ]; then
 fnic_ood=true
 else
 echo "fnic module is already up-to-date: $fnic"
 fi
}
if [ $method == "install" ]; then
 yum='/usr/bin/yum -y localinstall'
 enic=$(${modinfo} enic |grep "^version" |${awk} '{print 2ドル}')
 fnic=$(${modinfo} fnic |grep "^version" |${awk} '{print 2ドル}')
 test_ood
 if [ $enic_ood -a $fnic_ood ]; then
 echo "Both enic and fnic modules are out of date."
 echo "enic: $enic"
 echo "fnic: $fnic"
 echo "Installing kmod-enic and kmod-fnic..."
 ${yum} $enic_pkg $fnic_pkg
 elif [ $enic_ood -a ! $fnic_ood ]; then
 echo "Only the enic module is out of date: $enic"
 echo "Installing kmod-enic..."
 ${yum} $enic_pkg
 elif [ ! $enic_ood -a $fnic_ood ]; then 
 echo "Only the fnic module is out of date: $fnic"
 echo "Installing kmod-fnic..."
 ${yum} $fnic_pkg
 else
 echo "Both modules are up-to-date. Nothing to do; exiting..."
 exit
 fi
 test_ood
elif [ $method == "remove" ];then
 yum='/usr/bin/yum -y remove'
 ${yum} kmod-enic kmod-fnic
else
 echo "Unrecognized option: $method"
 exit
fi

I was thinking I could make the $method if tests case switches and put the code for each of those if blocks into methods of their own. That would at least eliminate the outer if...elif...fi block. Although, I'd only be converting it to another type of block.

• 200_success: Is there a reason you want to write a script instead of just running yum on the command line?
• theillien: It needs to be done on 60 to 70 servers. We're pushing it out and executing remotely.

Answer 1

Should this be a shell script?

You mentioned that you wrote this script to push the packages to over 60 servers. If you have that many servers, then you should have a configuration management tool to do these kinds of tasks for you and much, much more. Typical solutions include Ansible, CFengine, Chef, and Puppet.

Instead of copying the RPMs to each machine, you should probably use a YUM repository, which may be one that you operate yourself, if those are self-built RPMs.

Version check

modinfo enic |grep "^version" |awk '{print 2ドル}' would be better written as modinfo -F version enic 2>/dev/null. I recommend discarding error messages there, in case no such module is available.

However, I wonder why you bother querying modinfo at all, instead of just running yum and letting it automatically decide whether anything needs to be installed at all.

The conditionals are slightly redundant. You could just write

if [ $enic_ood -a $fnic_ood ]; then
 ...
elif [ $enic_ood ]; then
 ...
elif [ $fnic_ood ]; then
 ...
else
 ...
fi

I get the feeling, though, that nearly everything in the script is written twice. You could write a loop that handles both packages:

need_rpms=
for module in enic fnic ; do
 rpm=$HOME/kmod-$module-*.rpm
 want_version=$(rpm -qp --queryformat '%{VERSION}' $rpm)
 installed_version=$(/sbin/modinfo -F version "$module" 2>/dev/null)
 if [ "$want_version" = "$installed_version" ]; then
 echo "$module is already up-to-date: $installed_version"
 else
 need_rpms="$need_rpms $rpm"
 fi
done
if [ -n "$need_rpms" ]; then
 /usr/bin/yum -y localinstall $need_rpms
done

Alternatively, you could have one script that takes parameters, and run it once for each package.

Program structure

There should be an install() function and a remove() function.

Don't set yum='/usr/bin/yum -y localinstall' in one place and yum='/usr/bin/yum -y remove' in another place. I would expect it to be uniformly defined as yum='/usr/bin/yum -y', just like modinfo='/sbin/modinfo'.

This script always appears to succeed. Fatal errors should cause the script to exit with a non-zero status, and error messages should be printed to standard error. Exit statuses matter; even when executing the script over SSH, for example, the error status propagates all the way back to become the exit status of SSH.

install() {
 ...
}
remove() {
 ...
}
case "1ドル" in
 install|remove)
 "1ドル"
 ;;
 *)
 echo "0ドル must be executed with either 'install' or 'remove'" >&2
 exit 1
 ;;
esac

• theillien: CM is a bit of a sore spot. I've been trying to get the powers that be to allow me to install Puppet for well over three years. As for allowing yum to handle what it can, we currently have everything set up to use the Red Hat-provided modules which are steps behind what we need. They are essentially just the modules that come with the kernel so yum would assume it has to install the package anyway so if a server somehow managed to have the version it needs (not a stretch in this environment), it would be installing over it which is a bit unnecessary.
• theillien: Normally, I would upload any packages to our Satellite and allow yum to pull from there. Unfortunately, due to bureaucratic muckety-muck we have been stuck with an expired Satellite certificate with no ETA on when we'll be able to put it back into use. As a result, we're left copying the files to each server. Technically, we could set up another repo somewhere else, but that would involve configuring each server to use it anyway which doesn't really save anything.
• theillien: I do like that modinfo has that -F option. I don't know why it never occurs to me that commands have the means to provide specific information. Thank you for your input. I'll definitely be putting it to use.

Answer 2

In addition to what @200_success said.

Avoid hard-coding the target version numbers in multiple places. Put them in variables at the top of the file, and use them to derive all other uses.

The if-elif-else conditions are tedious because it seems the script is trying to be "too nice". Treating all permutations of installing these two packages or not is too much. I would simplify to treat each package individually, and simply print if they will be installed or not. No need to treat them together with words like "both" or "only".

Lastly, a very minor thing, I'm not a fan of unnecessary quotes. In all these statements the quotes are safe to drop:

awk='/bin/awk'
modinfo='/sbin/modinfo'
if [ ! $enic = "2.1.1.75" ]; then ....
if [ $method == "install" ]; then ....