I have a few questions and comments (though I don't have the power to aprove or
reject your package):

- Why do you use /var/lib/nagios?  Since these are all standard scripts and not
machine specific data, i would put them in /usr. They can be shared by many
machines after all, and that is what /usr I think still intends to be.

- Would it make sense to give nagios a homedir? Most people will want to call
those scripts using a central nagios-server running plugins through
check_via_ssh. I prefer not to have the scripts *writable* by the nagios user,
so a seperate directory in /usr/lib/nagios, with files owned by root, and a
seperate nagios homnedirectory for its .ssh dirctory with authorized_keys.

- I know there is also a nagios-extras plugin set. Would it make sense to split
out all these nagios checks in seperate packages? The advantage would be that
proper Requires: can be used. The disadvantage is the enormous overhead of
packaging and installing. Should we add the nagios-extras?

- I correctly did not cause dependancies on half the Core distro (debian did
when i apt-get install'ed nagios-plugins, installing things like mysql).

- What is the purpose of /etc/nagios/ and command.cfg ?

- Should Fedora reserve a UID/GID < 500 for nagios, so we can do additional
security like not allowing password logins or change of passwords, etc

- Add a Requires: net-snmp-perl? (see below)

- Are you planning to package oreon as well? 

- sourceforge urls are usually written as http://sourceforge.net/projects/%{name}/

- Use Release: 2%{?dist}

- don't use %define nagiosname, just decide (and use nagios)

- I was told to stick to make, not %{__make}. Same for rm and install.

- I would either use $RPM_BUILD_ROOT or (imho preferably) %{buildroot}, but not
both.

- how useful are check_ifoperstatus and check_ifstatus? Perhaps make the rm a
conditional on using/defining "with-net-smtp"? (and make requires: for that
ifdef'ed as well)

- I dont think the individual files/groups in
%{_localstatedir}/%{_lib}/%{nagiosname}/libexec/ need to be seperately added if
you add the directory already? (someone else verify this?)

- add the nagios user/group upon install? What I used (from my nsd rpm):
%pre
if getent passwd nagios >/dev/null 2>&1 ; then : ; else /usr/sbin/useradd -d
/home/nagios -r -s /bin/bash nagios >/dev/null 2>&1 || exit 1 ; fi

(note that AFAIK, it requires a valid login shell for check_ssh to work. If you
get an assigned UID/GID you can specify it in this commmand. The home directory
should probably go somewhere more reasonable. I tend to think nagios files
should be in /usr/lib/nagios and its homedir in /var/lib/nagios, but perhaps
there is a clearer Fedora policy on this that I am not aware of).

Actually packaging each plugin individually is an interesting idea.  It might
encourage others to get their own plugins into Fedora Extras (and there is a
pretty active community of plugin writers at present).

I chose /var/lib/nagios for the scripts because some of them are arch specific
(check_mysql: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV), for
GNU/Linux 2.2.5, dynamically linked (uses shared libs), for GNU/Linux 2.2.5)

As far as the Nagios user goes, I'm curious as to what others think best
practice is for this.  For plugins I thought it best to leave it up to the
administrator to choose who to run the plugin as.  But it may be best practice
to give them a Nagios user by default and let them change it if they see fit. 
If we do add a Nagios user, he will need a shell for check_by_ssh.

sorry about the RPM_BUILD_ROOT (I always forget to change it from the Fedora
template...  I'll fix it for release 3)

I also found out today that Net::SNMP and perl-net-snmp are not the same thing:
https://www.redhat.com/archives/fedora-list/2005-December/msg03100.html

On second look I think command.cfg is legacy and can (and will) be safely removed.

Oreon is certainly a possibility but I'm just trying to make sure I can get
Nagios and its plugins in first.

Thanks for the feedback.

Nagios is now being reviewed:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=176613

I forgot to mention I do not currently have a sponsor.

Debian also uses /usr/lib/nagios/plugins. Deviating from this will cause
trouble, because monitoring a mix of fedora/redhat and debian machines will need
different settings for $USER1$, or additional symlinking. 

I've updated the location of nagios plugins to /usr/lib/nagios/plugins.  I've
also removed the command.cfg file.

SPEC: http://preview.iesabroad.org/~mmcgrath/nagios/nagios-plugins.spec
SRPM: http://preview.iesabroad.org/~mmcgrath/nagios/nagios-plugins-1.4.2-3.src.rpm

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.2-4.src.rpm

-I've cleaned the spec file up quite a bit
-gotten rid of some duplicate build requires

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.2-5.src.rpm

Big change here, I've split all the plugins into separate packages.  My
reasoning for this is the following:  if someone wants to install a plugin that
allows them to check how many users are logged in (check_users).  By having them
all packaged together he'd also have to install all other plugins.... and their
prereq's which are many.  I think the end user will like the ability to install
only plugins they need.

The attached srpm in comment #7 does not compile well on an x86_64 system.  The
configure run does not find the required mysql libs to create the check_mysql. 
I'm looking into it to see why this is happening.

Hey Jesse, can you attach a build log for when MySQL fails?  I build it in mock
--arch=x86_64 and it seemed to build correctly.

IIRC, configure or the Makefile hardcodes "/lib" (it uses @prefix@/lib), so it
needs to be patched to find mysql in /usr/lib64.

Why not add a dummy package called nagios-plugins which would contain no file
but Require the various separate plugin rpms ? That would ease full-blown installs.

I'm going to work with upstream to fix these issues (email's already sent)  In
the meantime I've created a nagios-plugins-all subpackage that installs all
available plugins.

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.2-7.src.rpm

Hey Aurelien or Jesse, can one of you post a build log?  Upstream has requested one.

====================== FROM EMAIL ===========================
the failed build log would be helpful.  i don't know how rh/fc handles
the split in 64/32-bit libs and sw.  in debian, iirc /usr/lib is
a symlink to /usr/lib64 and there's a seperate /usr/lib32.  or at
least, there are no such compilation errors on my amd64 machine
(which is powered off currently so i can't check :()
==============================================================

*** Bug 186921 has been marked as a duplicate of this bug. ***

Just an update on this, upstream has been working to correct the issue.  I'll 
be posting their CVS version soon for testing.

SRPM:
http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.2-8.20060216cvs.src.rpm
SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins.spec

Updates using CVS, please check if these work on a 64bit system.  Also check my
version, I think it's ok but not too sure.

Seems to build ok on x86_64 now.

Not sure what you mean by 'check my version'?

Awesome.  By version I meant my release version, I think it follows the naming
guidelines but I wanted someone else to check it because it doesn't fully match
the upstream source version which includes snapshot time as well as the date...
 Guess I'm just being careful.

In the latest srpm posted here, nagios-plugins-all requires nagios-plugins-httpd
, but the plugin package is nagios-plugins-http.

SRPM:
http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.2-9.20060216cvs.src.rpm
SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins.spec

Oops, fixed.

I've just built this package and some of the plugins wouldn't install due to a
dependency on /usr/local/bin/perl. Turns out configure has found a symlink to
/usr/bin/perl I put there to make some random perl scripts run.

It would seem sensible for the package to enforce the correct perl by adding
this to the configure line:
  --with-perl=%{_bindir}/perl

There's a few other --with parameters which could be similarly helpful.

On the other hand, seems to be working well on fc5 x86_64.

Mike,

Could you update nagios-plugins to version 1.4.3 (released last month).

TIA

Sorry about that, the Nagios people have been really busy lately :-D.  All updated:

SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.3-1.src.rpm
SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins.spec

Its also been requested that I build some i386 bin's for people to use.  

i386: http://mmcgrath.net/~mmcgrath/nagios/i386/

Sorry, I'm removing FE-NEEDSPONSOR, I've been sponsored

cool. then you should put the package in FE and close this bug. I'm happy to see
nagios-plugins got in FE now. Thanks!

I've been sponsored but this package has not yet been approved.

Mike,

Missing requirements
--------------------
Every external binary needed by a plugin needs to be on the requirement
list of that plugin.

An example, the check_fping plugin requires the /usr/sbin/fping file (or the
fping package). Right now you are only requiring it during the building process.

$ strace -f  -e trace=execve ./check_fping 127.0.0.1
execve("./check_fping", ["./check_fping", "127.0.0.1"], [/* 23 vars */]) = 0
Process 9277 attached
[pid  9277] execve("/usr/sbin/fping", ["/usr/sbin/fping", "-b", "56", "-c", "1",
"127.0.0.1"], [/* 1 var */]) = 0
Process 9277 detached
--- SIGCHLD (Child exited) @ 0 (0) ---

Other plugins will needed to be checked. Will try to check a couple more
during in the next days.


Net::SNMP
---------
Are you removing the following two plugins because
http://search.cpan.org/dist/Net-SNMP/ isn't available in Extras? 

# check_ifoperstatus and check_ifstatus require Net::SNMP
%{__rm} %{buildroot}/%{_libdir}/nagios/plugins/check_ifoperstatus
%{__rm} %{buildroot}/%{_libdir}/nagios/plugins/check_ifstatus

I will see if I can package it ASAP. 

/jpo

Comment #26: Yes the only reason I'm not packaging the check_if's is because
Net::SNMP isn't in there, I was going to package it after nagios-plugins got
approved but if you want to package it have at it :-D

As far as the individual plugin requires... it was a pain in the butt but I
think I got all the requires that rpm doesn't pick up:

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.3-2.src.rpm

(In reply to comment #27)
> Comment #26: Yes the only reason I'm not packaging the check_if's is because
> Net::SNMP isn't in there, I was going to package it after nagios-plugins got
> approved but if you want to package it have at it :-D

perl-Net-SNMP review ticket: #191628  ;)

This just documents several configure warnings. A future comment will
deal with problems in perl and bash plugins.

Configure warnings
------------------------------------------------------------
[1]
checking for rc_read_config in -lradiusclient... no
configure: WARNING: Skipping radius plugin
configure: WARNING: install radius libs to compile this plugin (see REQUIREMENTS).
[2]
checking for lmstat... no
configure: WARNING: Get lmstat from Globetrotter Software to monitor flexlm licenses
[3]
configure: WARNING: Tried /usr/bin/perl - install Net::SNMP perl module if you
want to use the perl snmp plugins
[4]
checking for quakestat... no
checking for qstat... no
configure: WARNING: Get qstat from
http://www.activesw.com/people/steve/qstat.html in order to make check_game plugin
[5]
checking for qmail-qstat... no
configure: WARNING: Could not find qmail-qstat or eqivalent
------------------------------------------------------------

[1] disables the check_radius binary plugin (not built)

[2] comercial product
    the perl plugin - check_flexlm - will not work as expected

[3] two perl plugins being removed
    perl-Net-SNMP review ticket #191628 

[4] disables the check_game binary plugin (not built)
    http://www.qstat.org/

[5] no binary distribution of qmail
    the check_mailq perl plugin won't be able to check qmail queues 

Note: Another plugin that appears to be disabled is the check_ide_smart (.c)

At least [1], [3], and [4] should be added to a TODO list.

Missing requirements in perl plugins
------------------------------------

The perl module - utils.pm - as the following lines modified by the
configure script (utils.sh also has similar lines).
--------
## updated by autoconf
$PATH_TO_RPCINFO = "/usr/sbin/rpcinfo" ;
$PATH_TO_NTPDATE = "/usr/sbin/ntpdate" ;
$PATH_TO_NTPDC   = "/usr/sbin/ntpdc" ;
$PATH_TO_NTPQ    = "/usr/sbin/ntpq" ;
$PATH_TO_LMSTAT  = "" ;
$PATH_TO_SMBCLIENT = "/usr/bin/smbclient" ;
$PATH_TO_MAILQ   = "/usr/bin/mailq";
$PATH_TO_QMAIL_QSTAT = "";

and these variables are used by some of the perl plugins. For example,
the check_ntp should require the ntp package 

    Code used by in the check_ntp plugin
    ---------
    my $ntpdate = $utils::PATH_TO_NTPDATE;
    my $ntpq    = $utils::PATH_TO_NTPQ;
    ...
    open (NTPDATE, $ntpdate . " -q $host 2>&1 |")
    ...
    open(NTPQ, $ntpq . " -np $host 2>&1 |")


Plugins with missing requirements:

* nagios-plugins-mailq
  should require the /usr/bin/mailq file (or sendmail (?))

* nagios-plugins-ntp
  should require the ntp package (or the above ntp utilities)

* nagios-plugins-disk_smb
  should require the samba-client package or the /usr/bin/smbclient binary


Directory ownership
------------------------------------
the nagios-plugins base rpm should also own the
  /usr/lib/nagios/plugins/ directory

Mike,

perl-Net-SNMP has been built for FC-4, FC-5, and devel. The devel RPMs have
already been pushed into the mirrors. The FC-4 and FC-5 will take a little a bit
longer (next push).

jpo

Next time you're in the Chicagoland area lemme know, I owe you a beer.  I'm
chomping through some of these changes and testing.  I'll post what I've got by
tomorow night.

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.3-3.src.rpm

Changelog:
- Added check_ide_smart
- Added some dependencies
- Added support for check_if* (perl-Net-SNMP now in extras)
- nagios-plugins now owns dir %{_libdir}/nagios

Changing  FE-NEW  to  FE-REVIEW.

Created attachment 129746 [details]
Inherit distro CFLAGS

Mike,

This patch does the following: 

* [Blocker] Makes the package use the distro CFLAGS 

  Most of the plugins weren't being compiled with the distro CFLAGS.
  Using the %configure macro takes care of that.

* Adds perl(Net::SNMP) to the build requirements

  One less warning message from the configure.

Suggestion: Could you also add the bugzilla ticket to one of the changelog
entries? 

Hopefully these will be the last corrections.

jpo

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.3-4.src.rpm

Changelog:
- Now using configure macro instead of ./configure
- Added BuildRequest: perl(Net::SNMP)
- For reference, this was bugzilla.redhat.com ticket# 176374

MD5SUMS:
766f257aa209759b94f68a8a37119cf5  nagios-plugins-1.4.3-4.src.rpm

2c40fc69d51cc979e85150870a1daa93  nagios-plugins-1.4.3.tar.gz
f4e1bb47d20816d128f9704be7bd6b04  nagios-plugins.spec


PUBLISH++

This is my vote for approval.  

Notes:

* The following itens should be corrected
  (maybe after importing the package to the CVS repo)
  - The last changelog entry needs to be corrected (release bump)
  - The empty export statement before the configure macro should be removed

* Could someone (re)build this package in a 64bit system and express his vote?

* I have assigned this ticket to me.  If someone wants it, please feel free to   
  reassign it.

You have to 
-Requires: nagios-plugins = %{version}-${release}
+Requires: nagios-plugins = %{version}-%{release}
in two places

These updates have been made:

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.3-5.src.rpm

Jose Pedro Oliveira: you have assigned this to yourself and voted for approval,
are you approving it?

Mike,

As in this review there are several people envolved, we should fall back
to a voting decision (as in old Fedora.us days).  At least two or three other
reviewers should express their opinion by placing a comment like PUBLISH++ or
PUBLISH--.

But if no one expresses his opinion, lets say in the next 48 hours, I will
approve this package changing the blocking ticket from FE-REVIEW to FE-ACCEPT.
(I still would love to see a comment from someone with a 64bit system.) 


My vote is for approval.

PUBLISH++

MD5SUMS:
6387568f71b4c7fd6afe62afdb5e1972  nagios-plugins-1.4.3-5.src.rpm

2c40fc69d51cc979e85150870a1daa93  nagios-plugins-1.4.3.tar.gz
6af2d874922483154a5e8adf87e066b3  nagios-plugins.spec

jpo

APPROVED.

Mike,

We have to exclude the subpackage nagios-plugins-sensors from being created for
ppc as lm_sensors is not built for it (and also exclude it from the
nagios-pluggins-all requirement list).

jpo

The build is done with these changes, I'm going to make sure it's fine in devel
before I release it in FC4/5

Mike,

Instead of just excluding the ppc arch could you try to use conditional blocks
like the one below:

...
%ifnarch ppc ppc64
%files sensors
%defattr(-,root,root,-)
%{_libdir}/nagios/plugins/check_sensors
%endif
...

I will attach a possible patch (not tested as I don't have access to ppc systems).

Created attachment 130733 [details]
Do not create the sensors plugin in ppc builds

Successfully built.  Should be availe in FC4/5 soon.  Thanks all.