Bug 697504

I agree there is an redirection problem, but I'm not sure I agree it should be set to ${CATALINA_BASE}/logs/catalina.out, here are my thoughts:

I think there are two problems.

1) /usr/sbin/tomcat6 is a helper script (tomcat6.6.0.wrapper in the RPM) which is meant to only be called by the init script /etc/rc.d/init.d/tomcat6 (tomcat6-6.0.init in the RPM). The initscript is responsible for setting the IO redirection. It seems to me the wrapper shouldn't be redirecting at all, rather it's output should be redirected via it's caller, the initscript.

2) The initscript is designed to support different tomcat6 instances each with it's own configuration. The log file location is set via the instance configuration file and is picked up by the initscript via this line:

TOMCAT_LOG="${TOMCAT_LOG:-/var/log/tomcat6/catalina.out}"

and the output of the wrapper is redirected to $TOMCAT_LOG

Here are a couple of things to note:

TOMCAT_LOG defaults to /var/log/tomcat6/catalina.out which happens via symlinks to be identical to the suggestion of ${CATALINA_BASE}/logs/catalina.out, thus the default case would work.

However, the suggestion of ${CATALINA_BASE}/logs/catalina.out would fail in the non-default case of an instance which set it's TOMCAT_LOG to something other than the default.

From this I suggest the preferred solution would be to remove the redirection from the wrapper. The existing initscript will do the right thing then in all cases, both the default settings and when the log location is overridden by an instance.

Comments?

tomcat6-6.0.24-24.el6_0 is a zstream build which usually only gets security updates unless PM approves. 

The wrapper is run as the tomcat user and initscript is run as root, so I'd suggest that the init script write to /var/log/${NAME}/initd.log and the wrapper either not redirect (as Dennis suggests) or have it redirect to ${TOMCAT_LOG}

David makes an excellent point in comment #3 which I failed to take into consideration, the wrapper is used to execute in the context of the TOMCAT_USER. Earlier the initscript forces ownership of the TOMCAT_LOGFILE to TOMCAT_USER. That means contrary to what I said in comment #2 the IO redirection should be in the wrapper NOT the initscript (because it's the wrapper which has the right permission to write to the log file).

So it seems the real problem is the wrapper does not know what the log file is since its set in the initscript. Perhaps the right solution then would be to remove the IO redirection from the initscript, pass the TOMCAT_LOG to the wrapper and modify the wrapper to redirect to the passed TOMCAT_LOG.

Thank you David for catching this essential issue.

Does the above sound right?

Actually, what I propose is something we're already using in the EWS tomcat builds: Have the initscript redirect to /var/logs/${NAME}/initd.log. This eliminates the file sharing issue between root and tomcat user.

Since the initscript reads the config files we'll pass TOMCAT_LOG to the wrapper as Dennis suggested. It seems to be the simplest solution. 

The fix is ready for tomcat6-6.0.24-30.el6

Hi David, re comment #6

What is EWS tomcat builds?

> Have the initscript redirect to /var/logs/${NAME}/initd.log. This
> eliminates the file sharing issue between root and tomcat user.

Who creates this directory?
Who guarantees there isn't a collision on this directory/file? 

> Since the initscript reads the config files we'll pass TOMCAT_LOG to the
> wrapper as Dennis suggested. It seems to be the simplest solution.

I'm confused, isn't this in conflict with /var/logs/${NAME}/initd.log from above?

> The fix is ready for tomcat6-6.0.24-30.el6

Do you have a patch you can attach to this bz so I can review it? Maybe that will be clearer.

Thanks!

 David:

I think the problem is the directory /var/log/${NAME} is not created by the initscript. The initscript should test for the existence of /var/log/${NAME}, if present it should assure it's a directory and owned by root:root with the right permissions. If the directory /var/log/${NAME} does not exist it should be created with the right permissions and root:root ownership.

Also, in the future could you attach the patches to the bz so they can be reviewed? Thanks!

(In reply to comment #12)
>  David:
> 
> I think the problem is the directory /var/log/${NAME} is not created by the
> initscript. The initscript should test for the existence of /var/log/${NAME},
> if present it should assure it's a directory and owned by root:root with the
> right permissions. If the directory /var/log/${NAME} does not exist it should
> be created with the right permissions and root:root ownership.
> 
> Also, in the future could you attach the patches to the bz so they can be
> reviewed? Thanks!

Hi,
I'm not sure if it is okay.
If I understand it correctly, the proper directory is $CATALINA_HOME/logs
and in normal tomcat6 installation is this dir linked to /var/log/tomcat6, but tomcat should use $CATALINA_HOME/logs, because that dir know proper place for logs (in case it is link or regular direcroty).
And proper palce $CATALINA_HOME(BASE) should be read from /etc/sysconfig/$NAME file to be compatible with 
https://bugzilla.redhat.com/show_bug.cgi?id=636997
    Regards

I concur with comment #14 by Jan.

/var/log/${NAME} did not address my concern raised in my comment #7. I think Jan is correct the initscript output should be grouped together under the standard tomcat6 log directory. With this observation it shouldn't be necessary to create that directory as suggested in comment #12 as it should be created by the spec file (haven't verified this though).

Passing TOMCAT_LOG to the wrapper to support per instance log location as you have done is good and Jan also seems to concur on on this point.

(In reply to comment #7)
> Hi David, re comment #6
> 
> What is EWS tomcat builds?
> 
> > Have the initscript redirect to /var/logs/${NAME}/initd.log. This
> > eliminates the file sharing issue between root and tomcat user.
> 
> Who creates this directory?
> Who guarantees there isn't a collision on this directory/file? 
> 
> > Since the initscript reads the config files we'll pass TOMCAT_LOG to the
> > wrapper as Dennis suggested. It seems to be the simplest solution.
> 
> I'm confused, isn't this in conflict with /var/logs/${NAME}/initd.log from
> above?
> 
> > The fix is ready for tomcat6-6.0.24-30.el6
> 
> Do you have a patch you can attach to this bz so I can review it? Maybe that
> will be clearer.
> 
> Thanks!

/var/logs/${NAME} is created by the initscript if ${CATALINA_HOME} does not exist as a directory. The default installation uses 'tomcat6'. 

I don't see how there could be a collision on /var/log/${NAME}/initd.log. There can't be two services with the same name.

One area that I don't understand is in the initscript

Assuming the service name is not 'tomcat6' and this is the first time the script has been run for the new service, it looks to me as if the log file is being touched potentially before the directory is created in makeHomeDir. Touching ${TOMCAT_LOG} should proceed makeHomeDir versus precede it.

   # fix permissions on the log and pid files
    export CATALINA_PID="/var/run/${NAME}.pid"
    touch $CATALINA_PID 2>&1 || RETVAL="4"
    if [ "$RETVAL" -eq "0" -a "$?" -eq "0" ]; then 
      chown ${TOMCAT_USER}:root $CATALINA_PID
    fi
    [ "$RETVAL" -eq "0" ] && touch $TOMCAT_LOG 2>&1 || RETVAL="4" 
    if [ "$RETVAL" -eq "0" -a "$?" -eq "0" ]; then
      chown ${TOMCAT_USER}:root $TOMCAT_LOG
      chmod 775 /var/log/${NAME}
    fi
    if [ "$CATALINA_HOME" != "/usr/share/tomcat6" -a "$RETVAL" -eq "0" ]; then
        # Create a tomcat directory if it doesn't exist
        makeHomeDir
        # If CATALINA_HOME doesn't exist modify port number so that
        # multiple instances don't interfere with each other
        findFreePorts
        sed -i -e "s/8005/${randomPort1}/g" -e "s/8080/${CONNECTOR_PORT}/g" \
            -e "s/8009/${randomPort2}/g" -e "s/8443/${randomPort3}/g" \
            ${CATALINA_HOME}/conf/server.xml
    fi

Created attachment 495615 [details]
proposed patch for initscript

Created attachment 495616 [details]
proposed patch for wrapper (/usr/sbin/tomcat6)

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

re comment #16

> I don't see how there could be a collision on /var/log/${NAME}/initd.log.
> There can't be two services with the same name.

Yes there can. How? Normally RPM would prevent duplicate service names because it prevents collisions on file/directory ownership. But tomcat instances are not under the control of RPM, one need only create the instance config file in /etc/sysconfig and symlink the initscript. Presto! You now have a new service that could have file conflicts with an existing RPM package that is not currently installed. The safest way to prevent this type of collision is to isolate the files/directories under a directory owned by tomcat.

I'll follow up on the ordering issue in a minute, perhaps in bug 695284.