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 180092
Summary: | Review Request: NRPE - Monitoring agent for Nagios | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Mike McGrath <imlinux> |
Component: | Package Review | Assignee: | Dennis Gilmore <dennis> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | drees76, jose.p.oliveira.oss, rmo, wtogami |
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-12-06 04:23:44 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
2006-02-05 18:13:50 UTC
Upstream has a newer version SPEC: https://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.4-1.src.rpm Good: + Local build worked fine. + Mock build wored fine. Bad: - Missing SMP flags. If it doesn't build with it, please add a comment (wiki: PackagingGuidelines#parallelmake) - %{?dist} missing. - init script doesn't have a reload action. SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.4-2.src.rpm SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec - Added %{?dist} - Parallel build added and works. - Added reload section (via HUP) to the init script Good: + Local build worked fine. + Mock build worked fine. TODO: 1. Please register a fixed uid on http://fedoraproject.org/wiki/Packaging/UserRegistry?action=show&redirect=PackageUserRegistry and use it in your rpm. Please see: http://fedoraproject.org/wiki/Packaging/UserCreation "Using 'fedora-usermgmt' is optional and not required by packaging guidelines." Yes, put you should assign a fixed uid to the user which will created by your package. Sorry, I think I'm missing something. I don't see in the packaging guidelines nor in the review checklist a UID registration requirement. I have thought about switching this to the "damon" user. Perhaps I should do that? (In reply to comment #7) > Sorry, I think I'm missing something. I don't see in the packaging guidelines > nor in the review checklist a UID registration requirement. I have thought > about switching this to the "damon" user. Perhaps I should do that? There is no requirement to register or assign a fixed UID. Doing so only makes sense for applications that may share data over NFS or a similar arrangement that uses the same UIDs on multiple machines. There are many packages in Extras that create users but do not assign fixed UIDs (or attempt to do so using fedora-usermgmt). I don't think reusing a different UID (did you mean "daemon"?) is a good idea though. Can't say more without looking at the spec, and mmcgrath.net is unreachable from here at the moment. mmcgrath.net should be back up now. Terrible storm blew through central Illinois last night. I did mean daemon but will stick with useradd -r nrpe in my spec file. I don't really even know what services use daemon. (In reply to comment #9) > mmcgrath.net should be back up now. Terrible storm blew through central > Illinois last night. I did mean daemon but will stick with useradd -r nrpe in > my spec file. Looks reasonable to me. I'd just add a '-c "Description of what account is for"' option to the useradd command so that people will know what the nrpe account in their passwd file is for. SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.4-3.src.rpm Updated useradd with a -c Bad: - You don't allocate a fixed uid/gid. Bad: - You don't allocate a fixed uid/gid. Looks like you need a new reviewer Mike, unless Jochen can justify his requirement for a fixed uid/gid. (In reply to comment #13) > Bad: > > - You don't allocate a fixed uid/gid. Elaborate in depth why this package requires a fixed uid/gid. Please read: http://fedoraproject.org/wiki/PackageDynamicUserCreationConsideredBad?highlight=%28user%29 (In reply to comment #16) > Please read: > > http://fedoraproject.org/wiki/PackageDynamicUserCreationConsideredBad?highlight=%28user%29 This answer of yours is simply inacceptable. A) Fedora User Managment is optionsl B) Removing users after removing a package is bad. Notice NRPE does not remove the user. This prevents other people from becoming that user. Many packages (including httpd) don't remove the user they create when they get uninstalled. But nobody prohihed root for deleting such an user. It may be right, that in older package the system user didn't assigned to a fixed user if, but this is from by point of view an argument for new packages. Thats fine, but its not required for Fedora Extras packages. If you'd like to make it a requirement you'll have to convince the FESCo to make it manditory. In the meantime its optional and not in the review guidelines anywhere. Last comment from Warren (Due to bz db loss) During configure: checking for Kerberos include files... could not find include files SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.4-4.src.rpm - Added --with-kerberos-inc to configure Mike, Could you update to version 2.5.1 ? tia, jpo SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.5.2-1.src.rpm Sorry for the delay, here's 2.5.2-1 Good: + Local build works fine. + Mock build works fine. + rpmlint quite on binary rpm. Bad: - rpmlint complaints the binary rpm: E: nrpe configure-without-libdir-spec - Can't sheck source tarball agains upstream during a technical problem on download the upstream source. The RPM lint error is safe to ignore because there is nothing that would go in a lib dir. No more rpmlint errors: SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.5.2-2.src.rpm Mike, I've found two more problems: * missing build requirement: tcp_wrappers configure output ---------------- ... checking for socket in -lsocket... no checking for main in -lwrap... yes checking for strdup... yes ... nrpe linking ------------ gcc ... -o nrpe nrpe.c utils.c -L/usr/lib -lssl -lcrypto -lnsl -lwrap * package nagios-plugins-nrpe shouldn't own /usr/lib/nagios /usr/lib/nagios/plugins as they are owned by the nagios-plugins package (which is listed as a requirement) $ rpm -qf /usr/lib/nagios nagios-plugins-1.4.3-14.fc5 $ rpm -qf /usr/lib/nagios/plugins nagios-plugins-1.4.3-14.fc5 jpo Blocker: * should not require nagios-plugins; see bug #203689 This is by no means blocked. The comments listed in 203689 are not required in the packaging guidelines anywhere (that I found anyway) If I'm mistaken please let me know. Sorry, I thought I posted these: SPEC: http://mmcgrath.net/~mmcgrath/nrpe/nrpe.spec SRPM: http://mmcgrath.net/~mmcgrath/nrpe/nrpe-2.5.2-3.src.rpm Changelog: - no longer owns libdir/nagios - buildrequires tcp_wrappers ill take this over package meets naming and packaging guidelines. specfile is properly named, is cleanly written and uses macros consistently. dist tag is present. build root is correct. %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) license field matches the actual license. license is open source-compatible. GPL License text is not included in package. source files match upstream: 22afa197db8e4e5b13fac48636917b6d ../SOURCES/nrpe-2.5.2.tar.gz 22afa197db8e4e5b13fac48636917b6d nrpe-2.5.2.tar.gz latest version is being packaged. BuildRequires are proper. package builds in mock ( fc5,fc6,devel on x86_64 and i386 ). rpmlint is silent. final provides and requires are sane: nagios-plugins-nrpe-2.5.2-3.fc6.x86_64.rpm check_nrpe nagios-plugins-nrpe = 2.5.2-3.fc6 libcrypto.so.6()(64bit) libnsl.so.1()(64bit) libssl.so.6()(64bit) nagios-plugins nrpe-2.5.2-3.fc6.x86_64.rpm config(nrpe) = 2.5.2-3.fc6 nrpe = 2.5.2-3.fc6 /sbin/chkconfig /sbin/service /usr/sbin/useradd config(nrpe) = 2.5.2-3.fc6 libcrypto.so.6()(64bit) libnsl.so.1()(64bit) libssl.so.6()(64bit) libwrap.so.0()(64bit) no shared libraries are present. package is not relocatable. owns the directories it creates. doesn't own any directories it shouldn't. no duplicates in %files. file permissions are appropriate. %clean is present. code, not content. documentation is small, so no -docs subpackage is necessary. %docs are not necessary for the proper functioning of the package. no headers. no pkgconfig files. no libtool .la droppings. not a GUI app. not a web app. APPROVED Hey Mike. I don't see this package in owners.list. Can you please add it? See: http://fedoraproject.org/wiki/Extras/Contributors#head-f6f080b4c48fe519c98a29364a740953f90179e7 I'm terrible about that, its added now. Also imported and build. Closing bug. Mike, Could you also build it for FC-5 and FC-6 ? Tia, jpo Sure thing, I'll have them built by tonight. Should be on the mirrors in the next couple of days. |