Bug 693608 - Review Request: icinga - Open Source host, service and network monitoring program
Summary: Review Request: icinga - Open Source host, service and network monitoring pro...
Keywords:
Status: CLOSED DUPLICATE of bug 1055378
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard: NotReady
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-04-05 04:44 UTC by Mike Kirton
Modified: 2014-01-20 06:33 UTC (History)
25 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2014-01-20 06:33:12 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Spec-file icinga permissions unchanged (14.59 KB, text/plain)
2011-06-20 06:16 UTC, Dirk Götz
no flags Details
Spec-file icinga permissions changed (14.93 KB, text/plain)
2011-06-20 06:17 UTC, Dirk Götz
no flags Details
current pre 1.7 icinga spec file (18.05 KB, text/x-rpm-spec)
2012-04-23 09:05 UTC, Michael Friedrich
no flags Details
final icinga 1.7 spec file epel prepared (19.47 KB, text/x-rpm-spec)
2012-05-15 18:22 UTC, Michael Friedrich
no flags Details
fix build test suite (1.87 KB, patch)
2014-01-20 06:20 UTC, Shawn Starr
no flags Details | Diff

Description Mike Kirton 2011-04-05 04:44:32 UTC
Spec URL: http://matrix.senecac.on.ca/~mkirton/SBR/icinga.spec
SRPM URL: http://matrix.senecac.on.ca/~mkirton/SBR/icinga-1.3.0-4.fc14.src.rpm
SOURCE URL: http://matrix.senecac.on.ca/~mkirton/SBR/icinga-1.3.0.tar.gz

Description: Icinga - System Monitoring Application

First packaged created/released. Thank you for any assistance.

Comment 1 Chris Tyler 2011-04-11 16:53:42 UTC
rpmlint is throwing 72 errors and 2646 warnings on your files - please clean that up before review. Most of these are due to file being owned by nagios (most should be owned by root:root). Use the -i option to get verbose information about any errors which seem unclear. Also, I noticed that your package is trying to own /etc/httpd/conf.d which is owned by httpd.

Comment 2 Mike Kirton 2011-04-23 03:45:43 UTC
Sorry about that, here are some updated versions of the files.

Spec URL: http://matrix.senecac.on.ca/~mkirton/SBR/latest/icinga.spec
SRPM URL: http://matrix.senecac.on.ca/~mkirton/SBR/latest/icinga-1.3.1-4.fc14.src.rpm
SOURCE URL: http://sourceforge.net/projects/icinga/files/icinga/1.3.1/icinga-1.3.1.tar.gz/download

Please note that while rpmlint throws a "subsys-not-used" error, it is actually being used and rpmlint is simply not detecting it.

Comment 3 Ken Dreyer 2011-04-26 04:32:17 UTC
Hi Mike, thanks for putting this together. Here are some initial comments to get you into shape for a formal review:

1. Latest upstream seems to be 1.3.1. Please update your package from 1.3.0 to the latest upstream version.

2. The proper license is GPLv2. Please update the License field.

3. Please remove gcc, glibc, glibc-common from BuildRequires. These packages are installed in every buildroot in mock. https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions

4. net-snmp-devel will pull in net-snmp, so no need to BuildRequires both. Please remove BuildRequires: net-snmp.

5. When you copy in README.Fedora, please try to preserve the timestamp. https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps

6. This portion makes me think that you should probably not use the "nagios" username at all:

 # For when the user already exists
 /usr/sbin/usermod -c "nagios" -s /sbin/nologin -r -d /var/icinga -G icinga-cmd -g nagios nagios 2> /dev/null || :

In this case, you're reaching in and modifying another package's username. It would be better to just use an "icinga" username. Your spec contains a few comments that indicate you're trying to preserve compatibility with nagios-plugins, but nagios-plugins ought to work ok without requiring a "nagios" username on the system. Please elaborate in this ticket if you think that you need to use the "nagios" username.

7. Please use %{_sbindir}/usermod instead of /usr/sbin/usermod .

8. /var/icinga should probably not be used. Nagios uses /var/spool/nagios, so can you use /var/spool/icinga ?

9. --with-lockfile="%{_localstatedir}/icinga/icinga.pid" should be %{_localstatedir}/run/%{name}.pid like nagios. If it really needs its own sub-folder, %{_localstatedir}/run/%{name}/%{name}.pid.

10. Can you please clean up some of the "non standard perm" rpmlint errors, eg. /usr/bin/icingastats 0774L

11. Please do not start the services by default:
icinga.i686: W: service-default-enabled /etc/rc.d/init.d/icinga
icinga-idoutils.i686: W: service-default-enabled /etc/rc.d/init.d/ido2db

Use "-" as the default runlevel in the icinga init script's "chkconfig:" line, and remove the "Default-Start:" LSB keyword in the ido2db init script.

Comment 4 Michael Schwendt 2011-05-03 09:00:45 UTC
> you're reaching in and modifying another package's username.

Already using the same special user/group as another package isn't entirely nice and also isn't best practice security-wise (even if apparently it just tries to access nagios-plugins).
https://fedoraproject.org/wiki/Packaging:UsersAndGroups

> %attr(2755,nagios,icinga-cmd) %{_localstatedir}/icinga/rw/


Along the same lines, the following is also going too far:

> Provides: nagios

"nagios" is not a package capability, but the name of an existing package in the collection. Even if "icinga" has started as a fork of nagios, it must not disturb the package namespace of nagios and must not claim that it provides "nagios".


> %package gui
> ...
> Requires: %{name}-doc

Caution! A strict dependency on a documentation package asks for a rationale in the spec file just like other explicit dependencies:
https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

Note that %doc files may be absent with --excludedocs installs, so either the documentation really is required by the -gui pkg at run-time, or it may be optional. What is it?

Looking at the %files tree, currently the -gui package provides a directory that is needed by the -doc package. Same for the -api package.
https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership


> %package doc
> Summary: documentation %{name}
> Group: Applications/System

Cut'n'paste error. Not the appropriate group.


> %prep
> %setup -qn %{name}-%{version}

"-n %{name}-%{version}" is the default.


> %config(noreplace) %{_sysconfdir}/icinga/objects/commands.cfg
> %{_libdir}/icinga/cgi

https://fedoraproject.org/wiki/Packaging:UnownedDirectories
There could be more unowned directories.

Comment 5 Dirk Götz 2011-05-05 07:21:11 UTC
Found this bug while discussing the future use of icinga instead of nagios for monitoring and just wanted to add a comment on idoutils.

Idoutils are provided for mysql, postgresql and oracle. If it is build with --enable-idoutils like in the spec it is build for use with mysql. If --with-pgsql is also added it is build for postgresql. --with-oracle would build for oracle but needs ocilib or oracle instant client, both not use able for build because of licensing. 
See: http://docs.icinga.org/1.3.0/en/quickstart-idoutils.html

I can see the difficulties in building both mysql and postgresql support, but I think it would be good to do so and split idoutils in three packages:
- idoutils providing the common files
- idoutils-mysql providing the files for mysql
- idoutils-postgresql providing the files for postgresql
and delete the oracle specific files.

Also I hope it could provided for epel!

Comment 6 Ken Dreyer 2011-05-09 21:07:27 UTC
Mike, if you incorporate the changes in the last three comments and post your updated files, I'll do a formal review.

Comment 7 Michael Friedrich 2011-06-03 13:07:43 UTC
> Idoutils are provided for mysql, postgresql and oracle. If it is build with
> --enable-idoutils like in the spec it is build for use with mysql. If
> --with-pgsql is also added it is build for postgresql. --with-oracle would
> build for oracle but needs ocilib or oracle instant client, both not use able
> for build because of licensing. 
> See: http://docs.icinga.org/1.3.0/en/quickstart-idoutils.html

allow me to correct that as I am the developer who added the --enable-pgsql flag in order to use libpq instead of libdbi's abstraction layer for postgresql.

current implementation of idoutils focuses on 2 dependencies, which are used productively and are fully implemented:

* --enable-idoutils
needs libdbi, whereas you can load either libdbd-mysql or libdbd-pgsql as a rdbms driver being detected as approriate.
so this flag as is will provide *either* mysql *or* pgsql - no further configure option needed during build

* --enable-idoutils --enable-oracle
requires ocilib as oracle rdbms abstraction layer. as there are no builds available, i have created a spec file myself in order to ship that in our environment. https://wiki.icinga.org/display/Dev/ocilib
but due to the dependency on commercial, not fully licensed rpms (oracle instantclient) this can't be built into by default. so this was made an exclusive configure option for those demanding a seperated build (gcc will create another binary, ido2db.cfg will need an adapted config whilst depending on tnsnames.ora and such. 


> 
> I can see the difficulties in building both mysql and postgresql support, but I
> think it would be good to do so and split idoutils in three packages:
> - idoutils providing the common files
> - idoutils-mysql providing the files for mysql
> - idoutils-postgresql providing the files for postgresql
> and delete the oracle specific files.

that sounds good to me, but keep in mind that rpmforge spec file (which is the default we provide within the official Icinga tar.gz) already built

- icinga-idoutils (for libdbi usage with mysql and pgsql)
- icinga-idoutils-oracle is never found anywhere, but can be contributed somewhere for those needing it (e.g. Icinga Wiki)

Even though, to conclude with the original discussion - my packaging skills are work-in-progress, especially on rpmlint/selinux. So if you have any suggestions especially for RHEL/Fedora on the spec file which could fit to upstream (side-by-side for SuSE), then feel free to get in touch.
Either open a new issue on the dev tracker, get aboard the mailinglists or join the IRC community - https://dev.icinga.org https://www.icinga.org/support

Kind regards,
Michael

Comment 8 Dirk Götz 2011-06-20 06:16:26 UTC
Created attachment 505534 [details]
Spec-file icinga permissions unchanged

Comment 9 Dirk Götz 2011-06-20 06:17:19 UTC
Created attachment 505535 [details]
Spec-file icinga permissions changed

Comment 10 Dirk Götz 2011-06-20 06:29:01 UTC
I have created two Spec-files. The first one I have left the permissions unchanged so icinga can read and write its configs, but I don't see why this should be needed so I have created the second one.

It fixes the most issues mentioned above I think. Most warnings come from using user icinga and groups icinga and icingacmd. 

> icinga.x86_64: W: non-executable-in-bin /usr/bin/p1.pl 0644
> icinga.x86_64: E: script-without-shebang /usr/bin/p1.pl
> icinga.x86_64: E: non-executable-script /usr/bin/p1.pl 0644 None
If my perl knowledge is right, it is a perl module so I gave no execution permissions, but did not move it because I think it will break something.

> icinga-idoutils-mysql.x86_64: E: explicit-lib-dependency libdbi-dbd-mysql
> icinga-idoutils-pgsql.x86_64: E: explicit-lib-dependency libdbi-dbd-pgsql
It is not really the lib, it is the driver needed by the lib.

Building the package works fine on rhel5, rhel6 and f14 also i386 and x86_64, install is only tested on rhel5 and it works.

I hope the provided files are useful, but I am sorry I am not allow to do packaging and maintain packages by my employer, so someone else has to step in.

Comment 11 Michael Friedrich 2011-06-20 16:05:19 UTC
(In reply to comment #10)
> I have created two Spec-files. The first one I have left the permissions
> unchanged so icinga can read and write its configs, but I don't see why this
> should be needed so I have created the second one.

Thanks, I'll have a look what I can merge into the upstream available icinga.spec file.

https://dev.icinga.org/issues/1666

2 more topic to discuss ... 

1) the so called "icinga api" is mainly a php based abstraction layer on the idoutils rdbms. shouldn't this be renamed into "icinga-phpapi" in their respective package name (if you consider adding this from scratch)? i can't change that in the official spec file but e.g. debian maintainers did due to the nature of its behaviour.

2) the newly introduced "module definition" in 1.4.0 - https://dev.icinga.org/issues/162 can now be used to load neb modules without enabling them in icinga.cfg explicitely.
1.4.x still contains the modules/idoutils.cfg which contains commented sample configs. this is known bug (make install overrides existing source installs) - https://dev.icinga.org/issues/1625 - and it will be replaced by modules/idoutils.cfg-sample allowing the same procedure as you know about idomod/ido2db.cfg

https://git.icinga.org/?p=icinga-core.git;a=commit;h=6e9e78f686c0aa0de436e66b840984a6a122ab66

https://git.icinga.org/?p=icinga-core.git;a=commit;h=65a76c55df7052884354de4884684e8fc0803981


(all recent changes can be found in this git branch until they hit the release branches 
https://git.icinga.org/?p=icinga-core.git;a=shortlog;h=refs/heads/test/core
whilst cgis and ido do have their own)

> It fixes the most issues mentioned above I think. Most warnings come from using
> user icinga and groups icinga and icingacmd. 
> 
> > icinga.x86_64: W: non-executable-in-bin /usr/bin/p1.pl 0644
> > icinga.x86_64: E: script-without-shebang /usr/bin/p1.pl
> > icinga.x86_64: E: non-executable-script /usr/bin/p1.pl 0644 None
> If my perl knowledge is right, it is a perl module so I gave no execution
> permissions, but did not move it because I think it will break something.

p1.pl is only used if embedded perl is enabled (by configure/compile) and if enabled explicitely in icinga.cfg. this is intentionally disabled in future releases as the embedded perl interpreter could cause memory leaks which haven't been fixed / cannot be fixed for some years now. don't ask me about the details, i'm no expert at this stage of coding.

even more, nearly each know distribution moans about the location of p1.pl in $bindir so the recent git commit introduces default $libdir and a new configure option to be named '--with-p1-file-dir' and is targetted to be used by packagers who don't like to do the normal mv and sed stuff.

https://git.icinga.org/?p=icinga-core.git;a=commit;h=08b0437c5648bfe014e096c14be73b784e2d4f02

so expect older packages to be having an old p1.pl floating around, but that's something to be mentioned seperately in the changelogs.

due to these changes, the icinga-spec provided with the official releases will change too.
and some more changes, because the target of p1.pl in (new_)mini_epn.c in contrib/ (which can be used to manually execute and test perl plugins) have a lot of hardcoded stuff which is now to be replaced to be more packager friendly.

the intended location for an official distribution package is $libdir/icinga/p1.pl which is enabled in configure and the filelist itsself.

https://dev.icinga.org/issues/1569

https://git.icinga.org/?p=icinga-core.git;a=commit;h=15d9ce78b907811b2048a8fb99983d08be2de6e8

though i'm not sure if introducing such changes would make sense in 1.4.2 as it will remain a bugfix release.


> > icinga-idoutils-mysql.x86_64: E: explicit-lib-dependency libdbi-dbd-mysql
> > icinga-idoutils-pgsql.x86_64: E: explicit-lib-dependency libdbi-dbd-pgsql
> It is not really the lib, it is the driver needed by the lib.

probably a change which can only be done with a fresh package start. existing rpmbuilds will suffer from revoking icinga-idoutils, but that's fine if it will be kept as an alternative over here.


> Building the package works fine on rhel5, rhel6 and f14 also i386 and x86_64,
> install is only tested on rhel5 and it works.
> 
> I hope the provided files are useful, but I am sorry I am not allow to do
> packaging and maintain packages by my employer, so someone else has to step in.

i'm doing internal packaging stuff (still learning!), but my daily job is partly icinga core development, so i'd be very happy to see an official maintainer. also possible as part of team icinga as we are trying to keep communication channels easy and changes - if they fit - mostly upstream.

Comment 12 Bill McGonigle 2011-06-20 16:54:22 UTC
If the p1.pl relocation patch:

https://git.icinga.org/?p=icinga-core.git;a=commitdiff;h=08b0437c5648bfe014e096c14be73b784e2d4f02;hp=d362c03fd78789dfb7e1a24164cf656ab5dd6ce1

applies cleanly to 1.4.2, then I'd advocate carrying the patch for moving p1.pl in the Fedora spec to avoid trying to move it in the next release (and putting it in a silly place to start with).

Thanks for all of your work, guys.

Comment 13 Michael Friedrich 2011-06-20 17:24:01 UTC
currently it won't apply cleanly against 1.4.x release target, as this was intentionally built against latest dev branches for 1.5 to be released in August.

the reason i'm asking in advance is that this will introduce a change several packagers have been asking for, but i am not very keen on doing this within a bug fix release as it is still a rather major change (although it would just affect those actually using embedded perl, and that's mostly within packages, not within the quickstart guides).

so the question remains, where to put that logically. the idoutils.cfg-sample fix is now included for 1.4.2 upstream release which is planned next week or similar.

https://git.icinga.org/?p=icinga-core.git;a=shortlog;h=refs/heads/r1.4

Comment 14 Michael Friedrich 2011-06-29 09:58:40 UTC
i've taken the liberty to move some proposed patches on icinga.spec for upcoming 1.4.2 release.

there's a small bug in the spec file over here on the logging directory for the cgis.

initially, %{logdir}/gui is created, but the cgi.cfg reflects that as %{logdir}/cgis

this only is in effect, if you enable gui logging, but should be changed if this spec is taken upstream.


 %{__perl} -pi -e '
-        s|cgi_log_file.*|cgi_log_file=%{logdir}/cgi/icinga-cgi.log|;
+        s|cgi_log_file.*|cgi_log_file=%{logdir}/gui/icinga-cgi.log|;
         s|cgi_log_archive_path=.*|cgi_log_archive_path=%{logdir}/archives|;
    ' %{buildroot}%{_sysconfdir}/icinga/cgi.cfg


for 1.5 i am planning to add a configure option for that.

Comment 15 Bill McGonigle 2011-08-22 02:59:18 UTC
Is there a revised package to test?

Comment 16 Michael Friedrich 2011-08-22 21:33:42 UTC
you could get the 1.5.0-beta where most of the things for wednesday's 1.5.0 are already included. the icinga.spec does not contain everything mentioned in this topic but some suggestions have been taken over. so creating a fully compliant spec file will need an updated version from the one Dirk Götz provided.

Comment 17 Michael Friedrich 2011-11-07 20:36:35 UTC
just a thought on the libdbi dependencies and creation of different idoutils packages - it does not matter if you install icinga-idoutils-mysql or icinga-idoutils-pgsql then, because it matters which driver is being used via config directive and not explizit package dependency. so by just naming the package "icinga-idoutils" you can reserve the name logic for some different implementations (e.g. icinga-idoutils-pgsql for a compile based on libpq, if it might come).

icinga-api is not a dependency for icinga-web anymore, so there's actually no deep need to create rpms out of that too. 

all recent changes have been documented into changelog as well as developer wiki
https://wiki.icinga.org/display/Dev/Changes

still, if someone's willing to update the existing spec file provided with icinga tarball (and used for rpmforge), i'd be happy help wherever i can.

Comment 18 Dirk Götz 2011-11-08 07:30:24 UTC
My thought was not only having the correct libdbi-driver installed, I also wanted to reduce the installed files. Can the idoutils be compiled with support for both libdbi and pgsql-specific support? Perhaps the naming should then be icinga-idoutils-libdbi-pgsql for installing libdbi with pgsql-driver and icinga-idoutils-pgsql for pgsql-specific? Or will this get too complicated?

I did not find any time to update the spec-files, but I have convinced my supervisor to pay the guys from netways to build the packages for us, just need to get the budget from finance to start the project. So I am looking forward to get spec-files for all parts of icinga and eventdb which can pass the review process. Next thing to do after having packages build will be convincing my supervisor to sign a support contract so netways guys are payed for updating or allow me to do so.

Comment 19 Michael Friedrich 2011-11-08 11:33:22 UTC
actually the point in naming it genericly "icinga-idoutils" is to use the libdbi version, and allow the user to decide

* which driver is being used
* which driver is loaded

one could install all possible libdbd-* drivers, and just select that by changing ido2db.cfg entry. i would not recommend to make the driver for libdbi an actual dependency of icinga idoutils. it should be made a user decision only.

regarding the libdbi and libpq compiles - it wouldn't make sense to add that doubled dependency to a package. either use libdbi (which is stable implemented) or use libpq (which is not yet implemented, experimental). 
so i do think it's fine to support binary builds for each rdbms layer. if the would be oracle available, one could also build idoutils with ocilib dependency.

Comment 20 Dirk Götz 2011-11-08 12:45:31 UTC
I do not see the problem with the user decision. 

If the user has to decide between icinga-idoutils-mysql and icinga-idoutils-pgsql (or installing both if he for some unknown reasons likes) and this package installs the idoutils with libdbi-support and the corresponding libdbi-driver, the user has the possiblity to decide and does not need to know which further packages to install. Also the user benefit is less (unnecessary) files on the system.

If the user has one generic icinga-idoutils with all files, he installs the package and also needs to know that he has to install the needed libdbi-driver and to decide which one he actually needs. I think the first one is more user-friendly.

I just asked the question if compiling with support for both is possible, because I do not know how to handle multiple make && make install with different parameters for one source in one rpmbuild. If it is possible, I am just not experienced enough to know how! If not, we perhaps need to make one decision for the packages on a technical limitation or need multiple specfiles.

Comment 21 Jeremy Katz 2012-02-23 16:41:12 UTC
Is anyone still actually trying to push this through as the packager?  If not, I'll try to take a look at what's needed to get them into shape and reviewed.

Comment 22 Michael Friedrich 2012-02-23 16:58:44 UTC
not as a packager - i'd love to see someone (you? :) )actually catching up with this (as i said already on twitter - @dnsmichi).

for what it's worth, i've got a ping by someone revamping the spec file a bit - from here and from upstream, so maybe we can merge that discussion.

should i put you on cc and summarize offlist? current discussion is being done in german only.

thing is, for the discussion above my final proposal to name the idoutils package like this:

* icinga-idoutils-libdbi-mysql or -pgsql for the libdbi db layer
=> needs ./configure --enable-idoutils

* icinga-idoutils-oracle (will never happen as it requires oracle rpms)
=> needs ./configure --enable-idoutils --enable-oracle

* icinga-idoutils-pgsql (will happen in 2012 based on libpq)
=> needs ./configure --enable-idoutils --enable-pgsql

libdbi does not scale well. it's not threadsafe, does not support parameter bindings, and can get deprecated if it gets worse.

Comment 23 Michael Friedrich 2012-02-23 21:35:51 UTC
ok, some updates. in current git branch "dev/core" are some mostly breaking changes, which should make packing even more easier.

this will hit the 1.7 release then, so if we could work out a mostly package upstream <=> source upstream spec file until then, it would be great.

* add --with-plugin-dir to configure for setting the plugins path accordingly https://dev.icinga.org/issues/2344

* add configure option --with-temp-file=<filepath> to set temp_file for icinga.cfg https://dev.icinga.org/issues/2121

furthermore, the most significant break will be moving the icinga idoutils idomod.o neb module from $bindir to $libdir by default. this has been suggested by $every packager but i feared to introduce such a breaking change.

* change default target location of idomod.o from $bindir to $libdir https://dev.icinga.org/issues/2346

and last but not least, the package install of icinga-idoutils should install the "idoutils.cfg" containing a module definition by default (/etc/icinga/modules)

this will allow installs without modifying the icinga.cfg for automatically loading the event broker module. just like apache handles config integration, but hereby based on a new object definition.

https://dev.icinga.org/issues/2348
http://docs.icinga.org/latest/en/objectdefinitions.html#id1345546

and just for once - the "provides: nagios" is required in regard of other packagers requiring nagios implicitely - if there aren't any, feel free to drop it.

anyhow. if there are other suggestions or needs for making the package building cleaner, either contact me directly at michael.friedrich(at)univie.ac.at or open a new issue on https://dev.icinga.org in the core section.

Comment 24 Michael Friedrich 2012-04-23 08:45:43 UTC
i've now cleaned the spec file by comments over here, by the nagios.spec on epel, and other input i found necessary to compete with the issues.

it's now called

icinga-idoutils-libdbi-mysql
icinga-idoutils-libdbi-pgsql

and got the proper requires and conflicts.

furthermore, htpasswd is automated like the nagios.spec as well as a logrotate example is added to upstream.
plus it's not /var/icinga anymore, but /var/spool/icinga so a rather huge change for now.


https://git.icinga.org/?p=icinga-core.git;a=shortlog;h=refs/heads/r1.7

this is currently on git r1.7, so in order to get a testable build, do the following.

$ git clone git://git.icinga.org/icinga-core.git
$ cd icinga-core
$ ./configure ; make create-tarball
$ cp ../icinga-1.7.0.tar.gz* <yoursourcesdir>
$ cp icinga.spec <yourspecdir>

a pre 1.7.0 changelog can be found here
https://wiki.icinga.org/display/Dev/Icinga+Core+Changelog

rpmlint only moans about the "provides: nagios" which would possibly be a discussion point.

please keep in mind that this is still in development, so if you happen to have proposals or fixes, we can do it.

Comment 25 Michael Friedrich 2012-04-23 09:05:23 UTC
Created attachment 579454 [details]
current pre 1.7 icinga spec file

Comment 26 Michael Friedrich 2012-05-15 18:22:00 UTC
Created attachment 584757 [details]
final icinga 1.7 spec file epel prepared

this is the final version. it contains all valuable patches, plus a clean upgrade path for those having built previous versions on their systems. the full changelog can be seen inside the spec - upstream 1.7.0 tarball was released a few hours ago.

so i'd appreciate it if someone could review the spec file and tell what else is needed to bring icinga into EPEL.

thanks,
michael

Comment 27 Dagan McGregor 2012-05-18 05:25:06 UTC
 Just one question about the changes for 1.7 - does this make Icinga work with SELinux, or are some rules still required?

 I don't like the need to disable (or set to permissive) SELinux to run this, but the changes appear to better fit what SELinux expects with file locations and such.

Comment 28 Michael Friedrich 2012-05-18 09:51:23 UTC
mh tbh i have forgotten that issue. all my systems don't use selinux, and i am not really an expert regarding this.

so if anyone applicable to help out adding rules (maybe copy them from nagios rpm?) i would really appreciate that.

Comment 29 Bill McGonigle 2012-07-09 22:57:26 UTC
The practical extent of my SELinux knowledge is attending a nice talk Dan Walsh gave about 5 years ago ...  that said, has anybody here tried running the 2012-05-15 version and has the selinux errors handy?

I ask because what I see in the nagios SPEC is a 1-liner to run restorecon on the nagios .pid file.  I'm wondering if icinga's needs are more complex.

If nobody has this, I'll install a VM and give it a whirl.  I've been maintaining my own nagios SPEC with a local patch anyway (one that icinga picked up a couple years ago) so I'd just as well be done with that. :)

Comment 30 Roland Wolters 2012-07-18 15:06:19 UTC
Hej Michael,
I fooled around with your spec file and think that, for using the nagios-plugins (especially nagios-plugins-icmp), it would make sense to add the user icinga to the group "nagios".

Cheers,
Roland

Comment 31 Michael Friedrich 2012-07-30 22:10:02 UTC
@Bill
Chris prepared some selinux parts within selinux/ (which are rather old, as only el5 covered). if you have any other special findings, please keep us updated here, and if possible, at that issue as well: https://dev.icinga.org/issues/2921

@Roland
true that. that should mainly be the reason why Debian packages still use the nagios user. but imho, icinga should use it's own. either way, as this is important, marked as upstream issue here https://dev.icinga.org/issues/2922

i will provide an updated spec file later on. the -devel packages was introduced as well on git (https://dev.icinga.org/issues/2634)

Comment 32 Joshua Hoblitt 2012-07-30 22:53:57 UTC
Out of all of the nagios-plugins-\* packages, only 4 utilities have a mode other than 0755:

-rwsr-x--- 1 root nagios  50640 Jul  1 15:26 check_dhcp
-rwsr-x--- 1 root nagios  49008 Jul  1 15:26 check_fping
-rwsr-x--- 1 root nagios  56584 Jul  1 15:26 check_icmp
-rwsr-x--- 1 root nagios  40400 Jul  1 15:26 check_ide_smart

I think it would be reasonable for the icinga RPM to add the icinga user to the nagios group.  However, since it's just a few sgid files I also think it would be completely reasonable for the user to have to manually add icinga to the nagios group.  That action would only need to be done on the server running icinga as nrpe clients will already being in the nagios group.

Comment 33 Joshua Hoblitt 2012-07-30 22:58:31 UTC
One could also question the rational for the *ping check_* utils to not be o+rx since ping & fping are world executable by default:

-rwsr-xr-x 1 root root 27824 May 18  2010 /usr/sbin/fping
-rwsr-xr-x. 1 root root 40760 Mar 22  2011 /bin/ping

Comment 34 Jose Pedro Oliveira 2012-08-03 02:20:26 UTC
(In reply to comment #33)
> One could also question the rational for the *ping check_* utils to not be
> o+rx since ping & fping are world executable by default:
> 
> -rwsr-xr-x 1 root root 27824 May 18  2010 /usr/sbin/fping
> -rwsr-xr-x. 1 root root 40760 Mar 22  2011 /bin/ping

Feel free to open a bug against the nagios-plugins package.

/jpo

Comment 35 Joshua Hoblitt 2013-06-11 21:33:55 UTC
I see that there is now a package owner for Icinga 2.x (which is not yet released).  https://fedoraproject.org/wiki/Features/Icinga However, that release is still a ways off.

What is blocking the icinga 1.x srpms from being included into EPEL? Are there any technical objects or does a packager just need to step up?  If the later is the case, I will volunteer to become a packager if I can find a mentor.

Comment 36 Bill McGonigle 2013-06-13 15:43:08 UTC
(In reply to Joshua Hoblitt from comment #35)
> What is blocking the icinga 1.x srpms from being included into EPEL?

check out the above problems listed, but if you just want a running icinga 1.x, the RPM's from rpmforge seem to work great on el:

 http://pkgs.repoforge.org/icinga/

The issues I ran into with those are the same as listed above: adding the icinga user to the nagios group so pings work (essential for most icinga installs, I'd imagine) and figuring out that I needed idoutils (coming over from nagios) and which idoutils packages I'd need (postgresql on my machines).  For users running selinux, a more comprehensive policy seems to be needed to meet EPEL muster.

My input is that adding the icinga user to the nagios group should not be a user task, because icinga is re-using nagios-plugins* and the group membership is needed for compatibility with the nagios-plugins packages.  Re-bundling nagios-plugins makes less sense, and icinga should work out-of-the-box.

The idoutils dependency should be handled with the documentation.  I use a puppet script, but Fedora* still handles these manually.

As I understand it, the selinux cycle is a matter of running icinga through its capabilities, finding denials, and running the selinux tools to map those to policies.  Is there a test suite that will exercise the majority of icinga to force these issues out?

Comment 37 Dirk Götz 2013-06-13 19:39:23 UTC
Testing the policy provided in https://dev.icinga.org/issues/2921 and adding feedback there would be helpfull. If it does not work for you, please describe your usecase and the avc from /var/log/audit/audit.log. 

You can additional comment in the permissive lines in the te-file. This will run icinga and/or ido permissive while keeping the system in enforcing mode. This will create more avcs, but perhaps not all privileges are necessary.

For idoutils: The backend is not needed for running the core, but for the most addons. And the database to use is user’s choice. So dependency from ido packages to core package is good if the user knows he has to choose one of those and gets installed all needed. ;-) 

For user: Adding icinga to the nagios group would be nice but I do not think packaging guidelines allow it.

Comment 38 Michael Friedrich 2013-06-16 10:19:23 UTC
Summary from my point of view, having so many different people asking and offering help:

- Rene Koch asked on icinga-devel mailinglists [0], and we had a short chat on osdc. We had the conclusion that we need a sponsor and selinux policies for the package [no further updates]

- On 27.4.2013 Christopher Meng wrote me a mail asking about Icinga2. While I think this is a really good idea, I've asked about bringing 1.x also into Fedora. He told me to ask the original bug creator here. [no further updates]

- Dirk Goetz already added some SELinux Policies to the bug on the Icinga Dev Tracker [1]. I've added these files into upstream's git already, so they ship with the Icinga tarball itsself - and eveverything else can and will be, if a sponsor/packager is found.

- A User named "Corey" (with the cloak "staff/freenode") joined last week on #icinga at freenode irc, where a missing SELinux policy prevented him from installing the package "icinga-cgi" (because it adds the apache user to the icinga-cmd group, which selinux prohibits). Maybe he will/can provide an updated policy then.

- Some others offered help too ("palli" on irc, etc).

- Packages on repoforge are not automated, but manually. The spec files on git are 1:1 the same as shipped with the Icinga tarball and mostly updated within the release week. But the package build itsself takes a rather long time. Users are not really satisfied by that. Of course, there's an icinga repo on the (long) todo list, but having Icinga on the official Fedora/EPEL repositories would be much appreciated - as many distributions already have it [2] (and icinga-web even).


Conclusion to that:

- A package sponsor is needed. I don't know who to ping/grab/convince/sendbeer/etc but I would appreciate it if we find someone willing to help us out here.

- A packager & coordinator is needed. While I think that my package skills evolved over time, I would really appreciate it if someone else (Dirk? Joshua?) could take the hat. My development ressources are flattering between 1.x and 2.x right now. 

- If there are changes required for the Icinga source (e.g. adding something to make packaging easy), just create a patch and/or issue on the icinga dev tracker and i will make sure that it gets into the next upstream tarball release.

- regarding the icinga user to nagios group for plugin execution: please provide a README.RHEL patch for that with manual instructions for the user.

thanks,
Michael



[0] http://sourceforge.net/mailarchive/message.php?msg_id=30509241
[1] https://dev.icinga.org/issues/2921#note-1
[2] https://www.icinga.org/download/packages/

Comment 39 Michael Friedrich 2013-08-09 16:11:39 UTC
Hi,

Status Update

- David Ressman is now working on the selinux and systemd patches. 
- systemd support is already in git 'next' - https://git.icinga.org/?p=icinga-core.git;a=tree;h=refs/heads/next;hb=refs/heads/next (requires tests)
- selinux is pending - icinga assigned issue: https://dev.icinga.org/issues/2921
- possible package reviewer/sponsor, chatted now with Shawn Starr
- the nagios plugins issue with the icinga user being in the nagios group - to be discussed

kind regards,
Michael

Comment 40 Michael Friedrich 2013-08-09 16:20:33 UTC
Forgot that Wiki entry:

http://fedoraproject.org/wiki/Features/Icinga

- should be updated for 1.x too

Comment 41 Shawn Starr 2013-08-09 16:39:54 UTC
Hello, 

I can help you with getting Icinga into Fedora. If the SELinux issues are sorted then it's a matter of now just making sure the package .spec file passes rpmlint, builds in mock and rpmlint shows no serious issues.

There is no reason this should be blocked from being brought into Fedora.

The nagios-plugins issue

For user: Adding icinga to the nagios group would be nice but I do not think packaging guidelines allow it.

I don't know of any policy regarding this from what I can see. It would not make sense to have to repackage nagios-plugins JUST to change the groups in .spec file.


Thanks,
Shawn

Comment 42 Shawn Starr 2013-08-12 14:15:57 UTC
I have asked the maintainer for permission to use the nagios group in nagios-plugins. If they say yes, this will unblock that issue.

Comment 43 Shawn Starr 2013-08-13 12:26:00 UTC
Peter Lemenkov has said it's fine with him to let icinga use the nagios group as long as we check for it.

As long as we are depending on nagios-plugins the group should be created prior to us being installed, in the unlikely event the nagios group isn't created, we can still create it as nagios does this:

getent group nagios >/dev/null || groupadd -r nagios
getent passwd nagios >/dev/null || useradd -r -g nagios -d %{_localstatedir}/spool/%{name} -s /sbin/nologin nagios

It is not using any UID/GID hard coded or specific numbers just the name, we just use the same name and no conflicts will occur.

rpmdb doesn't store the UID/GID as I just checked only the name. So with everyone using the fixed name there can be no situation where the UID/GID for nagios (previously installed) would be overwritten by a new UID/GID breaking things.

Comment 44 Michael Friedrich 2013-09-05 19:25:37 UTC
the current selinux files are on git next - yet someone needs to tell if they're ok and/or integrate them (haven't figured out how the nagios package does it).
https://git.icinga.org/?p=icinga-core.git;a=tree;f=selinux;hb=refs/heads/next

imho it would be best to work on pre 1.10 packages having systemd and selinux inside (october will be rushing in anyways when release is scheduled) and commit possible changes back into git next. we may even create pre-release tarballs for that specific reason.

david will update us soon-ish with the latest and greatest changes here.

Comment 45 Shawn Starr 2013-09-10 14:37:31 UTC
I'm no SELinux expert to verify if those are right or not. I'll wait til thats sorted then we can begin the package review process. 

Then I need the .spec file and will do Fedora specific cleanups accordingly, finally someone else will use the fedora-review tool to do vetting of the spec and  package requirements.

You are welcome to run fedora-review also, which can make grabbing your .spec file easy if there's less changes I need to make (if any).

Comment 46 Shawn Starr 2013-10-22 14:48:13 UTC
Package has sponsor, me :)

Waiting for an update from Icinga folks please.

Comment 47 Shawn Starr 2013-10-28 01:45:49 UTC
Upstream has provided me builds, we will begin the re-review this week. I need to go over their stuff first.

Comment 48 Shawn Starr 2013-11-04 15:20:16 UTC
reset owner, reviewer will change this.

Comment 49 Shawn Starr 2014-01-20 06:10:17 UTC
Aside from any SELinux issues which we will deal with in this ticket, upstream provides some SELinux policy files.

Here is the latest spec and SRPM, first pass mock pass, rpmlint pretty good (with exceptions)

SPEC: http://spstarr.fedorapeople.org/packages/icinga/icinga.spec
SRPM: http://spstarr.fedorapeople.org/packages/icinga/icinga-1.10.2-1.fc20.src.rpm

Of note inside SRPM:

icinga-0001-Apache-2.4-configuration-fix-for-Fedora.patch
icinga-0002-Added-several-images-to-the-sample-config-revb.patch
icinga-0003-fix-tests.patch

There are three patches I've included, two from Nagios (the first one modified, second one left as-is) and the third one to fix building the Icinga test suite. If upstream can take any of the patches this would be great!

As with Nagios there are funny permissions in this package, I'd like to make sure we've got this all covered so we don't have any issues.

Let's begin this kickoff review.

Comment 50 Shawn Starr 2014-01-20 06:20:05 UTC
Created attachment 852626 [details]
fix build test suite

Comment 51 Christopher Meng 2014-01-20 06:24:15 UTC
Shawn, please submit a new review and mark this one as obsolete because the initial submitter was not you.

Comment 52 Shawn Starr 2014-01-20 06:33:12 UTC
will do

*** This bug has been marked as a duplicate of bug 1055378 ***


Note You need to log in before you can comment on or make changes to this bug.