[Bug 211319] Review Request: andutteye-client - Andutteye Software Suite Monitoring agent

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: andutteye-client - Andutteye Software Suite Monitoring agent


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=211319





------- Additional Comments From faucamp at csir.co.za  2006-11-16 04:38 EST -------
Ok, some comments:
* Don't hard-code your %{andutteye_*_dir} paths - this is actually what I meant
by using the standard macros. If you want to define these directories as your
own macros (i.e. you want to you the %{andutteye_*_dir} macros), then change
their definitions so that they use the default macros. Hard-coded directories
are problematic; for example:
<snip>
%define   andutteye_cfg_dir   /etc/andutteye
%define   andutteye_doc_dir   /usr/share/doc/andutteye
</snip>
...this is incorrect, as RPM will allocate a documentation directory
automatically to your package, based on its version-release info, and it will
not be "/usr/share/doc/andutteye". And what if the system's configuration
directory isn't /etc? Unlikely, yes, but it *is* possible.

Rather do something like:
%define andutteye_cfg_dir %{_sysconfdir}/andutteye

In my opinion, though, I would not use the andutteye_*_dir macros at all, as
they add unnecessary complexity to the spec file; instead of using
%{andutteye_cfg_dir}, for instance, something like %{_syconfdir}/andutteye is
more readable for to other maintainers (and end-users recompiling your package),
since almost all other packages use this method. It definitately makes the
%files section more readable.

* For your documentation, you do not have to (and shouldn't) explicitly copy the
files to a doc dir instal %install. Just flag the files as %doc entries in the
%files section, and RPM will sort it out automagically :-). For example:
%doc opt/andutteye/LICENSE opt/andutteye/README

* When installing the application, it generates a lot of noise; this seems to be
because of the "andutteyd" and "andutteye-api-linux.pl" script calls in %post.
The final package should be quiet during installation, so rather redirect this
output to /dev/null

* On the %post scripts: I haven't checked exactly what these two scripts do, but
they seem to generate a quite a few settings-files; shouldn't these rather be
called during %install? (as Fedora systems should provide a similar baseline for
detection) You're the expert here, of course, so I might be missing the point
completely ;-).

* The %post scripts complained when I installed the program on my system:
<snip>
WARNING!! Needed binary:lspci wasnt found in the users $PATH add a path
otherwhise andutteye wont work correctly
WARNING!! Needed binary:ifconfig wasnt found in the users $PATH add a path
otherwhise andutteye wont work correctly
</snip>

On my system:
$ whereis lspci
lspci: /sbin/lspci /usr/share/man/man8/lspci.8.gz
$ whereis ifconfig
ifconfig: /sbin/ifconfig /usr/share/man/man8/ifconfig.8.gz

Also:
$ rpm -qf /sbin/ifconfig
net-tools-1.60-73
$ rpm -qf /sbin/lspci
pciutils-2.2.3-4

If these two program are needed for andutteyed to function, you need to add
those two packages (net-tools and pciutils) to "Requires" in the spec file. (I
notice you also use /bin/netstat, which is also part of the net-tools package).
The path-issue can be fixed in a variety of ways, like adding it to the
andutteye-user's PATH, or applying a patch. Also what about calling this script
in %install (as I suggested above)?

* In your %post script, for upgrades:
<snip>
/etc/init.d/andutteyed start
</snip>

The package should check whether the andutteyed daemon is currently running,
before starting it (maybe the user DOESN't want to have it running at the
moment?) Adding "reload" or "condrestart" options to /etc/init.d/andutteyed
would be the best solution (and would help to keep rpmlint quiet :-) )

* The package places a log file (andutteye.log) in /var/cache/andutteye; this
should go in /var/log

* The pid file in /var/run has a typo ;-) (/var/run/anduttyed.pid)

* From rpmlint:

E: andutteye-client sourced-script-with-shebang /etc/profile.d/aemonitoring.sh
This text file contains a shebang, but is meant to be sourced, not executed.
-- Solution: remove the shebang

E: andutteye-client executable-sourced-script /etc/profile.d/aemonitoring.sh 0755
This text file has executable bit set, but is meant to be sourced, not
executed.
-- Solution: change the file permission to 0644

W: andutteye-client no-reload-entry /etc/init.d/andutteyed
In your init script (/etc/rc.d/init.d/your_file), you don't
have a 'reload' entry, which is necessary for good functionality.
-- See comment about %post upgrade script, above


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.