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 176613
Summary: | Review Request: Nagios - System / network monitoring application | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Mike McGrath <imlinux> |
Component: | Package Review | Assignee: | Ignacio Vazquez-Abrams <ivazqueznet> |
Status: | CLOSED NEXTRELEASE | QA Contact: | David Lawrence <dkl> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-extras-list, sopwith, 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-01-26 03:01:04 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 |
Description
Mike McGrath
2005-12-27 17:09:30 UTC
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 I tried to find a sponsor & reviewer, but a bunch of people are having trouble looking up that hostname, myself included. Please advise. Sorry about that, our upstream provider has had an outage since about 8:00 this morning. I've relocated these files: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.2-3.src.rpm - 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}? 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? -Mike (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. Use # chkconfig: - 99 01 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) http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-4.src.rpm http://mmcgrath.net/~mmcgrath/nagios/nagios.spec - 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 %defattr - %{_sysconfdir}/logrotate.d/nagios is not marked %config(noreplace) Thanks for your help on this, these changes have been made: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-5.src.rpm * is '%{_localstatedir}/%{_lib}' really sane? nagios seems to be the only package using /var/lib64 and FHS 2.3 mentions only /lib64 + /usr/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 operations * I would add '|| :' to some operations (server-restart + usermod) in the scriptlets; at least in the %postun one 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. (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. 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 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}/*. 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 rpmlint output: RPMS/nagios-1.3-7.i386.rpm: 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 RPMS/nagios-debuginfo-1.3-7.i386.rpm: RPMS/nagios-devel-1.3-7.i386.rpm: W: nagios-devel no-documentation - Ignorable; there isn't anything appropriate for it SRPMS/nagios-1.3-7.src.rpm: 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 APPROVED STOP STOP STOP... there is still no rational for usage of '/var/lib64' You're right, my bad. Further review required. 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 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) 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? 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 I had some duplicate build requires: zlib-devel, libjpeg-devel and libpng-devel are provided by by gd-devel http://mmcgrath.net/~mmcgrath/nagios/nagios.spec http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-10.src.rpm Is %{_datadir}/nagios/html/docs required or could it be marked %doc? 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. 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. 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 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. SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-12.src.rpm Expressions are our friends. 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. 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. 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. 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). Looks good to me. APPROVED |