[Bug 693608] Review Request: icinga - System Monitoring Application

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=693608

--- Comment #3 from Ken Dreyer <ktdreyer at ktdreyer.com> 2011-04-26 00:32:17 EDT ---
Hi Mike, thanks for putting this together. Here are some initial comments to
get you into shape for a formal review:

1. Latest upstream seems to be 1.3.1. Please update your package from 1.3.0 to
the latest upstream version.

2. The proper license is GPLv2. Please update the License field.

3. Please remove gcc, glibc, glibc-common from BuildRequires. These packages
are installed in every buildroot in mock.
https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions

4. net-snmp-devel will pull in net-snmp, so no need to BuildRequires both.
Please remove BuildRequires: net-snmp.

5. When you copy in README.Fedora, please try to preserve the timestamp.
https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps

6. This portion makes me think that you should probably not use the "nagios"
username at all:

 # For when the user already exists
 /usr/sbin/usermod -c "nagios" -s /sbin/nologin -r -d /var/icinga -G icinga-cmd
-g nagios nagios 2> /dev/null || :

In this case, you're reaching in and modifying another package's username. It
would be better to just use an "icinga" username. Your spec contains a few
comments that indicate you're trying to preserve compatibility with
nagios-plugins, but nagios-plugins ought to work ok without requiring a
"nagios" username on the system. Please elaborate in this ticket if you think
that you need to use the "nagios" username.

7. Please use %{_sbindir}/usermod instead of /usr/sbin/usermod .

8. /var/icinga should probably not be used. Nagios uses /var/spool/nagios, so
can you use /var/spool/icinga ?

9. --with-lockfile="%{_localstatedir}/icinga/icinga.pid" should be
%{_localstatedir}/run/%{name}.pid like nagios. If it really needs its own
sub-folder, %{_localstatedir}/run/%{name}/%{name}.pid.

10. Can you please clean up some of the "non standard perm" rpmlint errors, eg.
/usr/bin/icingastats 0774L

11. Please do not start the services by default:
icinga.i686: W: service-default-enabled /etc/rc.d/init.d/icinga
icinga-idoutils.i686: W: service-default-enabled /etc/rc.d/init.d/ido2db

Use "-" as the default runlevel in the icinga init script's "chkconfig:" line,
and remove the "Default-Start:" LSB keyword in the ido2db init script.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.