Bug 168836

Description of problem:

Bill Nottingham<notting> reported some issues with the
tog-pegasus.spec file of release 2.4.1-4.2.rhel4 :

Here's the laundry list.

Correctness/Functionality:
   
1) %pre scriptlet requires coreutils, not listed in spec
2) %post scriptlet requires ldconfig, not listed in spec
3) %post scriptlet requires shadow-utils, not listed in spec
4) %preun scriptlet requires procps, grep, remove_initd, not listed in spec
5) %postun scriptlet requires shadow-utils, not listed in spec
6) Makes directories in %pre, %post without checking/setting umask
7) condrestart is in %post, it should be in %postun
8) condrestart is triggered off of "$1" -gt  "1", it should be -ge
9) Uses ps and grep to check for running cimservers, instead of checking
   a pid file (may kill cimservers in chroots)
10) Kills cimserver by hand instead of stopping the server
11) Makes a directory in %post that it ships with the package
12) Runs by default. Makes me nervous. :)
13) The %post parts that create SSL certs don't actually seem to run
    correctly on a test install:
Preparing...                ########################################### [100%]
   1:tog-pegasus            ########################################### [100%]
   chmod: cannot access /etc/Pegasus/*.pem': No such file or directory
   cat: /etc/Pegasus/key.pem: No such file or directory
   cat: /etc/Pegasus/cert.pem: No such file or directory
   cat: /etc/Pegasus/cert.pem: No such file or directory

This is due to:
Mon Sep 19 15:38:24 EDT 2005
error on line 11 of /etc/Pegasus/ssl.cnf
23242:error:0E066065:configuration file routines:CONF_load_bio:missing equal
sign:conf_def.c:366:line 11

because the 'host' invocation mishandles it when it returns MX records.

   Ergo, a default installation won't actually work.

Won't break it, but probably isn't right:

1) Provides tog-pegasus-2.4, an (apparently) superflous provide
2) -devel obsoletes tog-pegasus-sdk, a package we've never shipped - superfluous.
3) Package requires lsb and uses lsb install/remove tools, instead of
   the standard tools. It should directly use chkconfig
4) Installation of the test package stops the service. (thankfully, 
   we don't build this.)
5) It generates SSL certs in %post. openssh does this at startup time
   if necesary, for example.
6) Realistically, the file perms on /etc/Pegasus/ssl.cnf should be set
   before writing to it, but I doubt that really matters much.
7) Shipping the lib<blah>.so in the main package isn't technically right.

Potential improvements:

1) Could ghost certs & ssl config file for checking perms



> > Correctness/Functionality:
> 
> Yes, these missing Requires: should be added to the next release,
> but have no impact on correct package functionality:

Wrong. You're getting correct packaging order on install,
and your scriplets are working, merely by luck.

> > 9) Uses ps and grep to check for running cimservers, instead of checking
> >    a pid file (may kill cimservers in chroots)
> 
> You mean this in %preun:
>  # Check if the cimserver is running
> isRunning=`ps -el | grep cimserver | grep -v "grep cimserver"`
> if [ "$isRunning" ]; then
>         %PEGASUS_SBIN_DIR/cimserver -s  
> fi
> 
> "cimserver -s" will only kill the cimserver listening on
> the /var/run/tog-pegasus/cimxml.socket ( there can be only one ).
> No chroot environment for cimserver is supported - all third party
> apps / libraries DEPEND on the /var/run/tog-pegasus location .

Then... why not test for the lock file, or the socket file?
ps|grep is a hack. Even 'pidof /usr/sbin/cimserver' would
be much better.

> > 10) Kills cimserver by hand instead of stopping the server
> 
> No, "cimserver -s" is the way to kill the cimserver; that is the 
> method used by the initscript.

If you're stopping the service, you stop the service with the
init script, you don't duplicate what the init script does.

> > 11) Makes a directory in %post that it ships with the package
> 
> No, the %install makes the %PEGASUS_LOG_DIR which is in the %files
> section.

And the %post attempts to remake it.

> > 12) Runs by default. Makes me nervous. :)
> 
> NO, IT DOES NOT RUN cimserver BY DEFAULT!!!
> 
> The initscript is copied to its correct location - that is all .
> There is an optional '--define "AUTOSTART 1"' rpm variable, which 
> defaults to '0', which, if > 0, causes the %post script to 'chkconfig
> --add'
> and start up tog-pegasus, but this is NOT the default.

Right, I missed the fact that it doesn't run --add. Which just leads to
more problems:

- Not --adding itself by default is broken - the way to change whether
  or not something starts by default is to change the default in
  the script, not by not running -add.
- Defining the --add or not based on a RPM variable is silly.

> > 13) The %post parts that create SSL certs don't actually seem to run
> >     correctly on a test install:
> > Preparing...                ########################################### [100%]
> >    1:tog-pegasus            ########################################### [100%]
> >    chmod: cannot access /etc/Pegasus/*.pem': No such file or directory
> >    cat: /etc/Pegasus/key.pem: No such file or directory
> >    cat: /etc/Pegasus/cert.pem: No such file or directory
> >    cat: /etc/Pegasus/cert.pem: No such file or directory
> > 
> > This is due to:
> > Mon Sep 19 15:38:24 EDT 2005
> > error on line 11 of /etc/Pegasus/ssl.cnf
> > 23242:error:0E066065:configuration file routines:CONF_load_bio:missing equal
sign:conf_def.c:366:line 11
> > 
> > because the 'host' invocation mishandles it when it returns MX records.
> > 
> >    Ergo, a default installation won't actually work.
> > 
> 
> I cannot reproduce this problem - nor could our QA testers, or the IBM
> or HP testers.
> What glibc / openssl version were you using ? I've just tested this
> again with the 
> RHEL-4-U2 current glibc-2.3.4-2.13 and openssl-0.9.7a-43.2 and all
> the /etc/Pegasus/*.pem 
> files were created OK .
> 
> What OS / glibc / openssl version were you using ?

This was off the devel tree, not RHEL 4, as it's what I had in front
of me at the time. This does appear to be an issue with using
later bind-utils.

(Speaking of which, this means the %post requires net-tools & bind-utils
too)

> > Won't break it, but probably isn't right:
> > 
> > 1) Provides tog-pegasus-2.4, an (apparently) superflous provide
> > 2) -devel obsoletes tog-pegasus-sdk, a package we've never shipped -
superfluous.
> 
> Not quite - tog-pegasus-sdk was in the Beta channels, so we need to get
> tog-pegasus-devel
> to replace it.

Do we normally support upgrading from cruft in the beta channel?

> > 3) Package requires lsb and uses lsb install/remove tools, instead of
> >    the standard tools. It should directly use chkconfig
> 
> By default, no scriptlet uses any initscript installation tools.

Which, as stated above, is wrong... (and by default, it *does*
call remove_initd)

> /usr/lib/lsb/{install,remove}_initd are symbolic links to chkconfig ,
> so the scriptlets would use chkconfig correctly.

That's not the point. The point is that pulling in redhat-lsb implies
a *ton* of unnecessary dependencies; your package now relies on
X libraries, a print server, rsync, ... etc.

> > 5) It generates SSL certs in %post. openssh does this at startup time
> >    if necessary, for example.
> 
> In future, the initscript should do this if the files don't exist, I
> agree.

Instead of %post, actually; it's much cleaner.




Version-Release number of selected component (if applicable):
2.4.1-4.2.rhel4