Bug 176613 - Review Request: Nagios - System / network monitoring application
Review Request: Nagios - System / network monitoring application
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ignacio Vazquez-Abrams
David Lawrence
Depends On:
  Show dependency treegraph
Reported: 2005-12-27 12:09 EST by Mike McGrath
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2006-01-25 22:01:04 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Mike McGrath 2005-12-27 12:09:30 EST
Spec Url: http://preview.iesabroad.org/~mmcgrath/nagios/nagios.spec
SRPM Url: http://preview.iesabroad.org/~mmcgrath/nagios/nagios-1.3-2.src.rpm
Nagios is a progam that will monitor hosts and servicees on your
network.  It has the ability to send email or page alerts when a
problem arises and when a problem is resolved.  Nagios is written
in C and is deesigned to run under Linux (and some other *NIX
variants) as a background process, intermittently running checks
on various services that you specify.

Nagios Plugins is also being reviewed:

I do not currently have anything in extras (though I have a few packages being reviewed).  I would like a sponsor.
Comment 1 Mike McGrath 2005-12-30 16:41:19 EST
I've updated the default location of nagios-plugins to /usr/lib/nagios/plugins

SPEC: http://preview.iesabroad.org/~mmcgrath/nagios/nagios.spec
SRPM: http://preview.iesabroad.org/~mmcgrath/nagios/nagios-1.3-2.src.rpm
Comment 2 Elliot Lee 2006-01-03 14:59:04 EST
I tried to find a sponsor & reviewer, but a bunch of people are having trouble
looking up that hostname, myself included. Please advise.
Comment 3 Mike McGrath 2006-01-03 15:14:54 EST
Sorry about that, our upstream provider has had an outage since about 8:00 this
morning.  I've relocated these files:

Comment 4 Ignacio Vazquez-Abrams 2006-01-03 15:42:04 EST
- Source0 should be a proper URL
- Prereq should be Requires(pre)
- -devel should also require by release
- Commented-out logrotate script refers to cacti user and group, but these are
not created anywhere nor is there a dependency on anything that does
- Typo at --libexecdir
- Typo at --enable-enbedded-perl
- Is there a reason why you disable the initscript on runlevel 4?
- Requires(pre): %{_sbindir}/useradd
- Requires(preun): /sbin/service /sbin/chkconfig
- Requires(post): /sbin/chkconfig /sbin/service %{_sbindir}/usermod
- Requires(postun): /sbin/service
- Recommend reordering %files so that the various directories are grouped together
- Any reason why %defattr is used so many times?
- Any reason why -devel doesn't just own %{_includedir}/%{name}?
Comment 5 Mike McGrath 2006-01-03 16:58:50 EST
Thanks for helping me review nagios.  I've implemented all the changes from your
comments except for one.  When I try to run chkconfig --add nagios with the
following chkconfig comment:

# chkconfig: - 345 99 01

I get:

service nagios does not support chkconfig

However, when I run chkconfig against the same comment (minus the 4) it works fine:

# chkconfig: - 35 99 01

Any ideas?

Comment 6 Jeff Carlson 2006-01-03 17:43:01 EST
(In reply to comment #5)
[trimmed for brevity]
> # chkconfig: - 345 99 01
> service nagios does not support chkconfig
> # chkconfig: - 35 99 01
> Any ideas?

Too many arguments to chkconfig.  Check what chkconfig did.  I bet it created a
S35nagios in runlevels 2, 3, 4, and 5, and a K99nagios in 0, 1, and 6, and the
01 was completely ignored.


# chkconfig: - 99 01
Comment 7 Mike McGrath 2006-01-03 17:44:50 EST
Ignore that last comment, I have no idea how I expected that to behave :-) It's
all fixed now (and disabled by default since it ships with non-working configs)


Comment 8 Ignacio Vazquez-Abrams 2006-01-04 00:07:00 EST
- Source0 should be http://dl.sourceforge.net/nagios/%{name}-%{version}.tar.gz
- Extra space on --libexecdir
- sed substitutions should be done in %build
- The sed substitution on p1.pl could be replaced with an insert (non-blocker)
- All of the files you list in the 2 %defattr sections (with the exception of
p1.pl) already have those permissions, so you can combine them and replace with
%defattr(-,root,root) and then merge redundant entries (e.g., %{_datadir}/%{name})
- The permissions on the doc dir are wrong, but I suspect that is because of the
- %{_sysconfdir}/logrotate.d/nagios is not marked %config(noreplace)
Comment 9 Mike McGrath 2006-01-04 12:20:36 EST
Thanks for your help on this, these changes have been made:

Comment 10 Enrico Scholz 2006-01-04 12:57:04 EST
* is '%{_localstatedir}/%{_lib}' really sane? nagios seems to be the
  only package using /var/lib64 and FHS 2.3 mentions only /lib64 +

  Have the libexec + cgi-bin files really have to reside under /var or
  would /usr a better place for them (dynamically created programs are
  looking a little bit hairy to me)?

* the '%{_localstatedir}' macro is a little bit problemetic because
  its value differs between distributions (e.g. mandriva uses /var/run
  for it). Not a problem when packaged for Fedora only but '%_var'
  would be more clear.

* I would add some '-p' (preserve timestamp) options to the %_install

* I would add '|| :' to some operations (server-restart + usermod) in
  the scriptlets; at least in the %postun one
Comment 11 Mike McGrath 2006-01-04 14:18:05 EST
Most of these decisions I got from examples from other spec files.  For example
the httpd.spec and php.spec files use _localstatedir instead of _var.

I've never used -p with install, what benefits would we gain from this?

I've added || : to usermod but I believe service returns true regardless of
whether httpd restarts.
Comment 12 Ignacio Vazquez-Abrams 2006-01-05 05:29:32 EST
(In reply to comment #11)
> I've added || : to usermod but I believe service returns true regardless of
> whether httpd restarts.

It will however fail if it can't find the service.
Comment 13 Mike McGrath 2006-01-05 10:28:32 EST
Thats interesting I didn't know that :-D, I've included the || : changes.

SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-6.src.rpm
SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
Comment 14 Ignacio Vazquez-Abrams 2006-01-05 11:17:26 EST
Okay, you obviously missed the memo that talked about consolidating
%{_datadir}/%{name} into a single entry. Also, since you own everything in
%{_sbindir} you can just use %{_sbindir}/*.
Comment 15 Mike McGrath 2006-01-05 11:47:07 EST
Sorry bout that, I've made that change now:

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-7.src.rpm
Comment 16 Ignacio Vazquez-Abrams 2006-01-06 06:38:12 EST
rpmlint output:

E: nagios non-standard-uid /var/log/nagios/archives nagios
E: nagios non-standard-gid /var/log/nagios/archives nagios
E: nagios non-readable /etc/nagios/private/resource.cfg-sample 0640
E: nagios non-standard-uid /var/spool/nagios nagios
E: nagios non-standard-gid /var/spool/nagios nagios
E: nagios non-standard-dir-perm /var/spool/nagios 02775
E: nagios non-standard-uid /var/log/nagios nagios
E: nagios non-standard-gid /var/log/nagios nagios
E: nagios non-standard-dir-perm /etc/nagios/private 0750

- All ignorable

E: nagios subsys-not-used /etc/rc.d/init.d/nagios

- Bogus, since the initscript does touch /var/lock/subsys/nagios

W: nagios-devel no-documentation

- Ignorable; there isn't anything appropriate for it

E: nagios configure-without-libdir-spec

- Ignorable, since the default as per configure --help is fine

- Spec looks reasonable
- File matches upstream
- Builds under mock in FC4
- The rest looks good

Comment 17 Enrico Scholz 2006-01-06 07:07:30 EST

there is still no rational for usage of '/var/lib64'
Comment 18 Ignacio Vazquez-Abrams 2006-01-06 07:12:30 EST
You're right, my bad. Further review required.
Comment 19 Mike McGrath 2006-01-06 10:34:30 EST
I've moved everything that was in %{_localstatedir}/{%_lib} to %{_libdir}.  I
should have done that earlier.

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-8.src.rpm
Comment 20 Paul Wouters 2006-01-06 11:30:01 EST
Also, be aware that people will run nagios-plugins using a single nagios config
entry on multiple machines. So it is very important that @USER1@ will work on
all architectures. This is a limitation of nagios, but the nagios-plugins should
accomodate those. What I have done is:

install in /usr/lib/nagios/plugins
create a softlink of /var/lib/nagios to /usr/lib/nagios
create a softlink of /usr/lib/nagios/plugins to /usr/lib/nagios/libexec

also: {_sbindir}/useradd -d %{_datadir}/%{name}

I am not sure what _datadir resolves to, but I believe either /usr/lib/ or
/var/lib, which means that you've put nagios' homedir in the same place as the
plugins. Since ssh checks to see if the user is owning his own homedir, this
requires nagios to have read/write to the plugins directory. I think it is
better to have nagios' homedir elsewhere (eg in /home/nagios), so that it can
have readonly access to its plugins, while not breaking automated logins with
ssh for remote plugin execution.

Additionally, /home can be NFS mounted, so that the ssh key distribution works 
over a large set of machines without the need to manually install nagios' ssh
key everywhere in /usr/lib/nagios or /var/lib/nagios (if that is not nfs
mounted, since it could be that there is a mix of linux/distro/OSes around the
network so that /usrlib/nagios cannot easilly be an NFS export)
Comment 21 Mike McGrath 2006-01-06 11:57:50 EST
In fedora %{_datadir} translates to /usr/share/  perhaps I should create
/var/lib/nagios as a rw directory for Nagios as its home directory.  I thought
about using /var/log/nagios because it's already there and nagios has rw access
to it but that doesn't seem appropriate for a home directory.  

These issues are more nagios-plugins related but I think nagios-plugins and
nagios should have the same home directory for both plugins.

Any thoughts?
Comment 22 Mike McGrath 2006-01-06 12:24:08 EST
Upon inspection of my /etc/passwd file I noticed a number of accounts that use
/var/spool/name as their home directory (mail for example). Nagios already has
rw access to its spool directory so I've made it the nagios home directory as well.

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-9.src.rpm
Comment 23 Mike McGrath 2006-01-10 17:04:58 EST
I had some duplicate build requires:
zlib-devel, libjpeg-devel and libpng-devel are provided by by gd-devel

Comment 24 Ignacio Vazquez-Abrams 2006-01-14 15:21:36 EST
Is %{_datadir}/nagios/html/docs required or could it be marked %doc?
Comment 25 Mike McGrath 2006-01-14 15:32:23 EST
The HTML Doc's are used by the webinterface.  There's a "Documentation" in the
nav. frame that uses it.  I could move it to /usr/share/doc/nagios-1.2/ but I'd
have to create an alias in the apache conf.
Comment 26 Ignacio Vazquez-Abrams 2006-01-14 16:06:36 EST
It doesn't necessarily have to be moved; it is possible to mark files and dirs
as %doc even though they're outside of the normal path for docs. The tricky part
would be splitting the %files entry.
Comment 27 Mike McGrath 2006-01-14 16:25:20 EST
I did not know that :-P  How's this:

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-11.src.rpm

Comment 28 Ignacio Vazquez-Abrams 2006-01-14 17:08:45 EST
You're missing the %{_datadir}/%{name}/html directory itself, but yes, just like
that. Although you could consolidate all those new entries into
%{_datadir}/%{name}/html/[^d]* since docs is the only one that starts with d.
Comment 29 Mike McGrath 2006-01-14 17:44:03 EST
SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-12.src.rpm

Expressions are our friends.
Comment 30 Mike McGrath 2006-01-15 11:20:35 EST
SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-13.src.rpm

I was manually stripping some of the binaries instead of letting
find-debuginfo.sh do it.
Comment 31 Roozbeh Pournader 2006-01-15 11:35:11 EST
Random comments:
There are still spelling problems in the description:
- progam, servicees (?), Nagios spelled both with capital N and small N.
- also sentences missing ending dots.

The logrotate file should also become Source1.
Comment 32 Ignacio Vazquez-Abrams 2006-01-15 11:52:37 EST
Also, the paragraph "This package provide core programs for nagios. The web
interface, documentation, and development files are built as separate packages"
is now completely wrong.
Comment 33 Mike McGrath 2006-01-15 12:22:38 EST
SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-14.src.rpm

-Fixed spelling errors and description
-Moved logrotate to a source file

I know I've said this before but thanks for everyones help on this, we'll be
using it on the Fedora Infrastructure servers soon (whenever it and the plugins
get approved).
Comment 34 Ignacio Vazquez-Abrams 2006-01-18 23:56:25 EST
Looks good to me.


Note You need to log in before you can comment on or make changes to this bug.