Spec URL: http://rocha.web.cern.ch/rocha/fedora/nagios-plugins-lcgdm.spec SRPM URL: http://rocha.web.cern.ch/rocha/fedora/nagios-plugins-lcgdm-0.4.0-1.src.rpm Description: This package provides the nagios probes for LCGDM components (DPM and LFC). The Disk Pool Manager (DPM) is a lightweight grid storage component, allowing access to data using commonly used grid protocols. The LCG File Catalog (LFC) is the main catalog being used by grid communities for both file bookkeeping and metadata. This is part of my first set of packages, and I am looking for a sponsor. Other packages i'm submitting in this first bunch cover the bits of the DPM and LFC components not yet available in Fedora/EPEL - the core components are already built in Fedora (see lcgdm). This is part of a more general effort to get software already available and packaged in the EMI project (http://www.eu-emi.eu/) directly available in the main distributions.
A quick first parse. Some similar comments to bug #749132. 1) Add details about making the tar ball. 2) CFLAGS It seems that Fedora and RHEL6 do have a %{cmake} macro to do all this for you. See $(rpm -E '%{cmake}' Assuming RHEL5 as well then you can case the dist tag to do it by hand. http://fedoraproject.org/wiki/Packaging:DistTag and effectively implement the same thing by hand. 3) rpmlint nagios-plugins-dpm-disk.x86_64: W: only-non-binary-in-usr-lib nagios-plugins-dpm-head.x86_64: W: only-non-binary-in-usr-lib okay this seems to be normal for nagios-plugins even it seems wrong to me, precedent is there so fine. 4) There are directories such as /etc/nrpe.d/ /usr/lib64/nagios/plugins/lcgdm /usr/lib64/nagios/plugins /usr/lib64/nagios ... that you create but are not part of your package nor owned by something you pull in. It makes sense as you have done not to require nagios to make the probes easily available to other monitoring systems so you should at least own the directories. http://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership 5) When you require a sub package it should be exactly matched Requires: nagios-plugins-lcgdm-common%{?_isa} = %{version}-%{release} 6) You duplicate %doc LICENSE README RELEASE-NOTES but they are only needed in just one package with the exception of the LICENSE which should be in all packages that can be installed in isolation as defined by the inter requires of your sub packages.
For the purposes of process bug #749132 tracks the sponsorship process.
(In reply to comment #1) > A quick first parse. > > Some similar comments to bug #749132. > > 1) Add details about making the tar ball. Fixed. > 2) CFLAGS > It seems that Fedora and RHEL6 do have a %{cmake} macro to do all this > for you. See $(rpm -E '%{cmake}' > Assuming RHEL5 as well then you can case the dist tag to do it by hand. > http://fedoraproject.org/wiki/Packaging:DistTag and effectively implement > the same thing by hand. I simply started using the cmake macro and it's looking fine - even for RHEL5. > 3) rpmlint > > nagios-plugins-dpm-disk.x86_64: W: only-non-binary-in-usr-lib > nagios-plugins-dpm-head.x86_64: W: only-non-binary-in-usr-lib > > okay this seems to be normal for nagios-plugins even it seems wrong > to me, precedent is there so fine. Yes, i had followed the feedback from: https://bugzilla.redhat.com/show_bug.cgi?id=423821#c6 The other one is E: no-binary, but i couldn't make it noarch to have it going in /usr/lib64 along with the rest. > 4) There are directories such as > /etc/nrpe.d/ > /usr/lib64/nagios/plugins/lcgdm > /usr/lib64/nagios/plugins > /usr/lib64/nagios > ... > > that you create but are not part of your package nor owned > by something you pull in. > > It makes sense as you have done not to require nagios to make > the probes easily available to other monitoring systems so you should > at least own the directories. > > http://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership Thanks for the pointer, i had a new look. I'll add the ownership of /etc/nrpe.d. However looking at the other nagios plugins packages, they all seem to require both nagios-common and nagios-plugins, so i was thinking of adding that to nagios-plugins-lcgdm-common. They provide the remaining dirs. The packages are called nagios-plugins-*, so maybe thinking that other monitoring systems might use them is not needed? > > 5) When you require a sub package it should be exactly matched > > Requires: nagios-plugins-lcgdm-common%{?_isa} = %{version}-%{release} Fixed. > 6) You duplicate > %doc LICENSE README RELEASE-NOTES > but they are only needed in just one package with the exception > of the LICENSE which should be in all packages that can be installed in > isolation as defined by the inter requires of your sub packages. Just to make sure... should i just put the %doc LICENSE in the 3 packages depending on nagios-plugins-lcgdm-common? If i don't put a %doc at all, rpmlint complains of no-documentation for package. After the doubts above, i'll provide a new spec/srcrpm. Thanks.
Hi. New spec and source rpm. Spec URL: http://rocha.web.cern.ch/rocha/fedora/nagios-plugins-lcgdm.spec SRPM URL: http://rocha.web.cern.ch/rocha/fedora/nagios-plugins-lcgdm-0.4.0-2.src.rpm Some open questions on usage of %doc and dependencies (rpmlint spitting no-documentation after removal of %doc from subpackages), and directory ownership (explained in the previous comment). Apart from that fixed previous issues. Thanks.
Found an issue with the currently src rpm in F16 regarding byte-compiled python files. New spec and src rpms: Spec URL: http://rocha.web.cern.ch/rocha/fedora/nagios-plugins-lcgdm.spec SRPM URL: http://rocha.web.cern.ch/rocha/fedora/nagios-plugins-lcgdm-0.4.0-3.src.rpm Release number bumped, koji builds available (successful): https://koji.fedoraproject.org/koji/taskinfo?taskID=3504587 (dist-5E-epel) https://koji.fedoraproject.org/koji/taskinfo?taskID=3504609 (dist-6E-epel) https://koji.fedoraproject.org/koji/taskinfo?taskID=3504554 (f16)
Review of nagios-plugins-lcgdm https://bugzilla.redhat.com/show_bug.cgi?id=749232 Builds okay in Fedora 16 x86_64 +:ok, =:needs attention, -:needs fixing MUST Items: [+] MUST: rpmlint must be run on every package. $ rpmlint ./nagios-plugins-lcgdm.spec ./nagios-plugins-lcgdm.spec: W: invalid-url Source0: nagios-plugins-lcgdm-0.4.0.tar.gz which is expected. $ rpmlint ./nagios-plugins-* nagios-plugins-dpm-disk.x86_64: W: only-non-binary-in-usr-lib nagios-plugins-dpm-head.x86_64: W: only-non-binary-in-usr-lib nagios-plugins-lcgdm.x86_64: E: no-binary nagios-plugins-lcgdm.x86_64: W: only-non-binary-in-usr-lib nagios-plugins-lcgdm-common.x86_64: W: only-non-binary-in-usr-lib nagios-plugins-lfc.x86_64: W: only-non-binary-in-usr-lib Odd but it's a nagios plugin and that that has noconcept of a noarch path in typical configuration so okay. nagios-plugins-dpm-disk.x86_64: W: no-documentation nagios-plugins-dpm-head.x86_64: W: no-documentation Okay nagios-plugins-lfc.x86_64: W: no-documentation 6 packages and 0 specfiles checked; 1 errors, 9 warnings. [+] MUST: The package must be named according to the Package Naming Guidelines. Based on other nagios plugins so fine. [+] MUST: The spec file name must match the base package %{name} [+] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. ASL 2.0 [+] MUST: The License field in the package spec file must match the actual license. Source code is clearly licensed as ASL2.0 [+] MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc. LICENSE file present. [+] MUST: The spec file must be written in American English. [+] MUST: The spec file for the package MUST be legible. [+] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Sources match: diff -r --brief nagios-plugins-lcgdm-0.4.0 ../SPECS/nagios-plugins-lcgdm-0.4.0/ [+] MUST: The package must successfully compile and build into binary rpms on at least one supported architecture. mock and koji okay. [+] MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. builds in koji. [+] MUST: All build dependencies must be listed in BuildRequires Builds in mock. [+] MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. No locales. [+] MUST: Every binary RPM package which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. No shared libs. [+] MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review Not relocatable. [=] MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. /etc/nagios.d is now owned or required but see below. Also you should probably own. /usr/share/pnp4nagios /usr/share/pnp4nagios/lcgdm-templates [+] MUST: A package must not contain any duplicate files in the %files listing. [+] MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line. [+] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [+] MUST: Each package must consistently use macros, as described in the macros section of Packaging Guidelines. [+] MUST: The package must contain code, or permissible content. This is described in detail in the code vs. content section of Packaging Guidelines. [+] MUST: Large documentation files should go in a doc subpackage. [+] MUST: If a package includes something as %doc, it must not affect the runtime of the application. [+] MUST: Header files must be in a -devel package. No headers. [+] MUST: Static libraries must be in a -static package. No statics. [+] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). None present. [+] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. No libs. [] MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} No devels. [+] MUST: Packages must NOT contain any .la libtool archives, these should be removed in the spec. Nope. [+] MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. No guis. [+] MUST: Packages must not own files or directories already owned by other packages. [+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [+] MUST: All filenames in rpm packages must be valid UTF-8. SHOULD Items: [+] SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [+] SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. They don't but up to you, you support another language. [+] SHOULD: The reviewer should test that the package builds in mock. It does. [+] SHOULD: The package should compile and build into binary rpms on all supported architectures. See koji builds [notchecked] SHOULD: The reviewer should test that the package functions as described. [+] SHOULD: If scriptlets are used, those scriptlets must be sane. Why is this a SHOULD? [+] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. No devel packages. Comments: 1) The layout of files between packages does not make sense to me. e.g. nagios-plugins-lcgdm-0.4.0-3.fc16.x86_64.rpm contains /usr/share/pnp4nagios/lcgdm-templates/check_dpm_perf.php and all the php4nagios files but nagios-plugins-dpm-head-0.4.0-3.fc16.x86_64.rpm contains /usr/lib64/nagios/plugins/lcgdm/check_dpm_perf there should be some correlation between files for the same probe or alternativley probes in one and php4nagios files in another though the first is a better option. 2) You have %define debug_package %{nil} this must be justified with a comment or removed. 3) I don't understand the /etc/nagios.d directory at least on Centos6 where I am looking nagios does not contain this directory, is this something you are introducing? I presume these are probe configuration files. 4) arch vs noarch, I appreciate the problem that the path is architecture dependent when it comes to nagios locations reguardless of the file contents. If after reording the files between packages you end up with out a non-architecture path then that sub package can be marked noarch except for on .el5. Someone really should tackle this in a future nagios version but not your problem for now. 5) Adding nagios-common and nagios-plugins is probably fine. I think neither of these actually require nagios which is worth leaving behind. 6) As long as LICENSE is pulled in by every package that can be installed in isolation to the others you are fine, more over you should not supply it elsewhere.
Hi. See explanations below before i provide a new package (if one is needed). > [=] MUST: A package must own all directories that it creates. If it does not > create a directory that it uses, then it should require a package which does > create that directory. > /etc/nagios.d is now owned or required but see below. > > Also you should probably own. > > /usr/share/pnp4nagios > /usr/share/pnp4nagios/lcgdm-templates I'll double check this one. It looks line pnp4nagios provides /usr/share/nagios/html/pnp4nagios, so maybe it's more logical to add a lcgdm directory there. > Comments: > 1) The layout of files between packages does not make sense to me. > e.g. > nagios-plugins-lcgdm-0.4.0-3.fc16.x86_64.rpm > contains > /usr/share/pnp4nagios/lcgdm-templates/check_dpm_perf.php > and all the php4nagios files > but > nagios-plugins-dpm-head-0.4.0-3.fc16.x86_64.rpm > contains > /usr/lib64/nagios/plugins/lcgdm/check_dpm_perf > > there should be some correlation between files for the same probe > or alternativley probes in one and php4nagios files in another > though the first is a better option. I didn't put them with the probe, as the probe is to be run on the monitored host, while the template is needed in the monitoring host (where the web interface is running). Unless we also install all the probes in the Nagios host, but they're really not needed as they're run remotely with nrpe. The current layout is pretty much one rpm per 'node type' - dpm-head, dpm-disk, lfc and lcgdm (lcgdm meaning the nagios master itself, nagios-plugins-lcgdm-nagios being a weird name). The second option you suggest is splitting the templates into additional packages following the probe layout? Something like: nagios-templates-dpm-head|dpm-disk|lfc? I couldn't find any existing packages with names suggesting this. > 2) You have > %define debug_package %{nil} > this must be justified with a comment or removed. If i remove it i get from rpmlint: ... nagios-plugins-lcgdm-debuginfo.x86_64: E: empty-debuginfo-package and as it was an error i added it. There's nothing in the packages generating debuginfo, i guess that's why it's empty (they're all python probes). > 3) I don't understand the /etc/nagios.d directory at least on > Centos6 where I am looking nagios does not contain this directory, > is this something you are introducing? I presume these > are probe configuration files. I guess it is. Nagios gives the option of a confdir (or several), and /etc/nagios.d seemed logical, although it does not seem to be standard. Should we call it something else or simply own it like this? Maybe /etc/nagios.lcgdm.d instead just to make sure we don't clash? > 4) arch vs noarch, I appreciate the problem that the path is > architecture dependent when it comes to nagios locations > reguardless of the file contents. > If after reording the files between packages you end > up with out a non-architecture path then that sub package > can be marked noarch except for on .el5. > > Someone really should tackle this in a future nagios version but > not your problem for now. All the packages have probes, so i guess for now they can't be noarch? > 5) Adding nagios-common and nagios-plugins is probably > fine. I think neither of these actually require nagios which is > worth leaving behind. Yes, confirmed. > 6) As long as LICENSE is pulled in by every package that can > be installed in isolation to the others you are fine, more > over you should not supply it elsewhere. Only provided in lcgdm-common, guess it is ok? I'll provide a new version right after these issues are cleared up. Thanks!
Okay, I think just one iteration needed then. Will try and answer above: > 2) You have > %define debug_package %{nil} > this must be justified with a comment or removed. Remove it and live with the rpmlint error. >> 3) I don't understand the /etc/nagios.d directory at least on >> Centos6 where I am looking nagios does not contain this directory, >> is this something you are introducing? I presume these >> are probe configuration files. >I guess it is. Nagios gives the option of a confdir (or several), and >etc/nagios.d seemed logical, although it does not seem to be standard. Should >we call it something else or simply own it like this? Maybe /etc/nagios.lcgdm.d >instead just to make sure we don't clash? It makes sense that all nagios configuration files remain in /etc/nagios/ add a directory in there. Assuming these are okay for you will approve.
>> Also you should probably own. >> >> /usr/share/pnp4nagios >> /usr/share/pnp4nagios/lcgdm-templates >I'll double check this one. It looks line pnp4nagios provides >/usr/share/nagios/html/pnp4nagios, so maybe it's more logical to add a lcgdm >directory there. Same logic here I think as nagios. If you are providing config files for another piece of software putting them in the directory of that package makes sense rather than a new one.
Hi again. Changes as requested: - removed debug_package nil - nagios config goes into /etc/nagios - pnp4nagios templates go into /usr/share/nagios/html/pnp4nagios/templates.lcgdm Two reasons i think it might be better to keep our own pnp4nagios templates dir (but now moved under the default path): - according to http://docs.pnp4nagios.org/pnp-0.4/doc_complete (look for update), they recommend not to put anything under templates.dist on danger of being overwritten - we could put them under templates, as suggested on the page, but some probes have very generic names, there's a probability of clash with another package It's an additional config hassle, but maybe its better? I've updated the package to the latest upstream released version (fixes above + two new probes), removed unnecessary build deps. One last thing related to a previous comment: /etc/nrpe.d is provided by nrpe, so i added it as a requires instead of owning the dir. New spec and src rpms: Spec URL: http://rocha.web.cern.ch/rocha/fedora/nagios-plugins-lcgdm.spec SRPM URL: http://rocha.web.cern.ch/rocha/fedora/nagios-plugins-lcgdm-0.5.0-1.src.rpm Koji builds: https://koji.fedoraproject.org/koji/taskinfo?taskID=3525643 (5E) https://koji.fedoraproject.org/koji/taskinfo?taskID=3525647 (6E) https://koji.fedoraproject.org/koji/taskinfo?taskID=3525651 (F16)
>I've updated the package to the latest upstream released version (fixes above + >two new probes), removed unnecessary build deps. One last thing related to a >previous comment: /etc/nrpe.d is provided by nrpe, so i added it as a requires >instead of owning the dir. this is your choice. It's perfectly possible to execute the probes with a local nagios instance, icinga or even collectd and then of course nrpe is not needed. I see no harm in just owning the directory. Make that choice and import. APROVED.
Thanks! I'll keep the nrpe at least for now, all the template config files rely on check_nrpe.
New Package SCM Request ======================= Package Name: nagios-plugins-lcgdm Short Description: Nagios probes to be run remotely against DPM / LFC nodes Owners: rocha Branches: f16 el5 el6 InitialCC:
Git done (by process-git-requests).
nagios-plugins-lcgdm-0.5.0-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/nagios-plugins-lcgdm-0.5.0-1.el6
nagios-plugins-lcgdm-0.5.0-1.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/nagios-plugins-lcgdm-0.5.0-1.el5
nagios-plugins-lcgdm-0.5.0-1.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/nagios-plugins-lcgdm-0.5.0-1.fc16
nagios-plugins-lcgdm-0.5.0-1.fc16 has been pushed to the Fedora 16 testing repository.
nagios-plugins-lcgdm-0.5.0-1.fc16 has been pushed to the Fedora 16 stable repository.
nagios-plugins-lcgdm-0.5.0-1.el5 has been pushed to the Fedora EPEL 5 stable repository.
nagios-plugins-lcgdm-0.5.0-1.el6 has been pushed to the Fedora EPEL 6 stable repository.