Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 176374
Summary: | Review Request: nagios-plugins | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Mike McGrath <imlinux> | ||||||
Component: | Package Review | Assignee: | Jose Pedro Oliveira <jose.p.oliveira.oss> | ||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | medium | ||||||||
Version: | rawhide | CC: | dcantrell, drees76, fedora, gauret, jose.p.oliveira.oss, matteo, paul, pmacedo, redhat, steve | ||||||
Target Milestone: | --- | ||||||||
Target Release: | --- | ||||||||
Hardware: | All | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2006-06-15 00:12:12 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | |||||||||
Bug Blocks: | 163779 | ||||||||
Attachments: |
|
Description
Mike McGrath
2005-12-21 21:47:01 UTC
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. |