Bug 707349

Summary: tomcat plugin: get support for Tomcat 7 fully working
Product: [Other] RHQ Project Reporter: Ian Springer <ian.springer>
Component: PluginsAssignee: John Mazzitelli <mazz>
Status: CLOSED WONTFIX QA Contact: Mike Foley <mfoley>
Severity: medium Docs Contact:
Priority: medium    
Version: 4.5CC: asantos, ccrouch, dawebster, hrupp, jclere, kenrumer, loleary, mazz, remm
Target Milestone: ---Keywords: FutureFeature
Target Release: RHQ 4.6   
Hardware: All   
OS: All   
See Also: https://bugzilla.redhat.com/show_bug.cgi?id=921261
https://bugzilla.redhat.com/show_bug.cgi?id=921194
Whiteboard:
Fixed In Version: Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-06-20 13:58:27 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 921194, 921261    
Attachments:
Description Flags
issue 1)
none
issue 2)
none
prototype patch to fix some things
none
jopr-tomcat-plugin
kenrumer: review?
Tomcat 7 support none

Description Ian Springer 2011-05-24 15:26:45 EDT
I looked into whether the tomcat plugin would autodiscover and manage Tomcat 7 instances. Autodiscovery worked fine for the server itself and most (all?) of the various child services. However, there appear to be a few minor kinks to work out:

1) Connectors are discovered w/ Resource name e.g. "?-6080" and are DOWN. The plugin returns the following error:

org.rhq.core.pluginapi.inventory.InvalidPluginConfigurationException: Failed to start component for resource Resource[id=11404, type=Tomcat Connector, key=Catalina:port=8009,type=Connector, name=?-6080, parent=Tomcat (6080)].
	at org.rhq.core.pc.inventory.InventoryManager.activateResource(InventoryManager.java:1589)
	at org.rhq.core.pc.inventory.InventoryManager.refreshResourceComponentState(InventoryManager.java:2678)
	at org.rhq.core.pc.inventory.InventoryManager.processSyncInfo(InventoryManager.java:2456)
	at org.rhq.core.pc.inventory.InventoryManager.processSyncInfo(InventoryManager.java:2462)
	at org.rhq.core.pc.inventory.InventoryManager.processSyncInfo(InventoryManager.java:2462)
	at org.rhq.core.pc.inventory.InventoryManager.synchInventory(InventoryManager.java:1022)
	at org.rhq.core.pc.inventory.InventoryManager.handleReport(InventoryManager.java:996)
	at org.rhq.core.pc.inventory.RuntimeDiscoveryExecutor.call(RuntimeDiscoveryExecutor.java:120)
	at org.rhq.core.pc.inventory.RuntimeDiscoveryExecutor.call(RuntimeDiscoveryExecutor.java:56)
	at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
	at java.util.concurrent.FutureTask.run(FutureTask.java:139)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:98)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:206)
	at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:909)
	at java.lang.Thread.run(Thread.java:662)
Caused by: org.rhq.core.pluginapi.inventory.InvalidPluginConfigurationException: The connector is not listening for requests on the configured port. This is most likely due to the configured port being in use at Tomcat startup. In some cases (AJP connectors) Tomcat will assign an open port. This happens most often when there are multiple Tomcat servers running on the same platform. Check your Tomcat configuration for conflicts: Catalina:port=8009,type=Connector
	at org.jboss.on.plugins.tomcat.TomcatConnectorComponent.start(TomcatConnectorComponent.java:108)
	at sun.reflect.GeneratedMethodAccessor34.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at org.rhq.core.pc.inventory.ResourceContainer$ComponentInvocationThread.call(ResourceContainer.java:525)
	at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
	at java.util.concurrent.FutureTask.run(FutureTask.java:139)
	... 3 more

This error seems bogus because I am able to connect to http://localhost:6080/ fine using a browser.


2) Web Application Resources are discovered but are all DOWN.


There may also be some other issues. These are just what I saw after a brief amount of investigation. I did not check whether the metrics, operations, and resource configs all worked on the various Resource types.

Screenshots demonstrating both issues 1) and 2) are attached.
Comment 1 Ian Springer 2011-05-24 15:27:13 EDT
Created attachment 500678 [details]
issue 1)
Comment 2 Ian Springer 2011-05-24 15:27:35 EDT
Created attachment 500679 [details]
issue 2)
Comment 3 John Mazzitelli 2012-06-19 09:38:04 EDT
*** Bug 833059 has been marked as a duplicate of this bug. ***
Comment 4 John Mazzitelli 2012-06-19 09:48:51 EDT
Here's my server.xml snippets that are relevent for looking at APR Connectors and using shared executors with Connectors (note, I used Tomcat 7 on Windows since it already had the JNI native wrappers pre-built out of box):

  # BZ change - SSLEngine was changed from on to "off"
  <Listener className="org.apache.catalina.core.AprLifecycleListener" SSLEngine="off" />
...
    # BZ change - uncommented this thread pool executor for use in the 8081 connector
    <!--The connectors can use a shared executor, you can define one or more named thread pools-->
    <Executor name="tomcatThreadPool" namePrefix="catalina-exec-"
        maxThreads="150" minSpareThreads="4"/>
...
    # BZ change - the port 8080 was unchanged but the 8081 was added to use a shared executor
    <Connector port="8080" protocol="HTTP/1.1"
               connectionTimeout="20000"
               redirectPort="8443" />

    <!-- A "Connector" using the shared thread pool-->
    <Connector executor="tomcatThreadPool"
               port="8081" protocol="HTTP/1.1"
               connectionTimeout="20000"
               redirectPort="8444" />

    # BZ change - SSLEnabled="false" rather than "true" - no need to use SSL for this test
    <!-- Define a SSL HTTP/1.1 Connector on port 8443
         This connector uses the JSSE configuration, when using APR, the
         connector should be using the OpenSSL style configuration
         described in the APR documentation -->
    <Connector port="8443" protocol="HTTP/1.1" SSLEnabled="false"
               maxThreads="150" scheme="https" secure="true"
               clientAuth="false" sslProtocol="TLS" />

I see these MBeans:

   Catalina:type=GlobalRequestProcessor,name="ajp-apr-8009"
   Catalina:type=GlobalRequestProcessor,name="http-apr-8080"
   ...and others...

Notice the literal double quotes wrapping the name attribute value. Our parsing code chokes on this (see the constructor for TomcatConnectorDiscoveryComponent.ConfigInfo).

Without APR configured (comment out the APR lifecycle listener in server.xml) you will see these global request proc MBeans:

   Catalina:type=GlobalRequestProcessor,name="ajp-bio-8009"
   Catalina:type=GlobalRequestProcessor,name="http-bio-8443"
   ... and others...

Notice "-bio" (and from the APR enabled Tomcat) "-apr" in the object names now. This appears to be new in Tomcat 7. Our parsing code in that ConfigInfo constructor needs to take this into account as well.

When specifying "address" attribute on the Connectors in server.xml, MBean names are like:

   Catalina:type=GlobalRequestProcessor,name="http-bio-192.168.1.10-8080"

When using APR connectors with "address", the same kind of thing is used:

   Catalina:type=GlobalRequestProcessor,name="ajp-apr-192.168.1.10-8009"

This is true (uses an IP address) if address is a hostname, too (Tomcat seems to do a DNS lookup and uses the IP for the hostname in the MBean ObjectName).

However, with "address" in the Connector, the Connector MBean ObjectName uses quotes, too:

   Catalina:type=Connector,port=8009,address="192.168.1.10"

I think this is not expected in the current Tomcat plugin parser code, either.
Comment 5 John Mazzitelli 2012-06-19 15:49:26 EDT
Another warning in the agent log that indicates a problem in another area (this is re: configuration of a connector)

2012-06-19 15:33:32,168 WARN  [ConfigurationManager.threadpool-1] (rhq.core.pc.configuration.ConfigurationCheckExecutor)- Plugin Error: Invalid Tomcat Connector resource configuration returned by Tomcat plugin - Required property 'emptySessionPath' has a null value in Configuration[id=0]
Comment 6 John Mazzitelli 2012-06-19 16:00:47 EDT
i think i found out why the wars are always down.

Catalina:j2eeType=WebModule,J2EEApplication=none,J2EEServer=none,name=//localhost/docs

is an example of an MBean ObjectName - this is still valid. But we are looking for an attribute called "state". I do not see an attribute with that name, but I do see "stateName" whose value is the string "STARTED" (no quotes).
Comment 7 John Mazzitelli 2012-06-19 16:16:35 EDT
Created attachment 593063 [details]
prototype patch to fix some things

attaching bz707349.patch as a prototype to fixing SOME things but not all. This doesn't take into account the new "-apr" or "-bio" strings in the name attribute of the ObjectName. But it does seem to fix the problem where there are now quotes in Tomcat 7's MBean name.

This patch is really only here for documentation purposes to see where in the code things need to be fixed for some of these issues. We can incorporate this patch in a real fix if appropriate.
Comment 8 Ken Rumer 2012-09-30 12:16:58 EDT
I completed the update for Tomcat 7.
Changes:
Connector - Storing the name from discovery and using the full name (with quotes in 7), not rebuilding for each request
Cache Discovery - Try to query for 'path', if 0 returns, try with 'context'
War - Changed all availability items to stateName and using String instead of int

I don't have any datasources deployed, so I don't know if there were any changes.  I tested against Tomcat 6 and 7.  Let me know how you want me to get the code submitted.
Comment 9 Ken Rumer 2012-09-30 16:00:28 EDT
I put in a datasource and found that there is a change there as well.  Almost identical to the Cache changes:
Datasource Discovery - Try to query for 'path', if 0 returns, try with 'context'
Comment 10 John Mazzitelli 2012-10-08 09:18:04 EDT
(In reply to comment #8)
> I completed the update for Tomcat 7.
> Changes:
> Connector - Storing the name from discovery and using the full name (with
> quotes in 7), not rebuilding for each request
> Cache Discovery - Try to query for 'path', if 0 returns, try with 'context'
> War - Changed all availability items to stateName and using String instead
> of int
> 
> I don't have any datasources deployed, so I don't know if there were any
> changes.  I tested against Tomcat 6 and 7.  Let me know how you want me to
> get the code submitted.

Ken - please rebase the patch and any of your fixes on the current master branch, then attach the new patch, which includes your fixes, to this BZ. I'll peer review and push if appropriate. Thanks -John
Comment 11 Ken Rumer 2012-10-09 11:22:06 EDT
Created attachment 624151 [details]
jopr-tomcat-plugin

Comments in previous post.

Thanks,
Ken Rumer
Comment 12 John Mazzitelli 2012-10-09 11:33:22 EDT
(In reply to comment #11)
> Created attachment 624151 [details]
> jopr-tomcat-plugin
> 
> Comments in previous post.

Ken - it appears this attachment includes everything - all tomcat plugin source, not just your change diffs. Did you mean to attach the source code for the entire tomcat plugin here?

As RHQ community work is performed in git - you'd just need to git rebase on the current master branch, then use git to produce a patch diff (preferrably via git format-patch) which should include the changes being proposed. Otherwise, it would force someone to go through the entire source attachment to discern what you did or did not change and what those changes actually were. This is what we want to use git for. Thanks.
Comment 13 Ken Rumer 2012-10-15 17:33:26 EDT
Created attachment 627712 [details]
Tomcat 7 support

Here is the commit patch.
Comment 14 John Mazzitelli 2012-10-15 18:27:44 EDT
(In reply to comment #13)
> Created attachment 627712 [details]
> Tomcat 7 support
> 
> Here is the commit patch.

I created a remote branch bug/707349, rebased off of current master branch, and applied this patch to it. The git commit is e7d48240474fba87f1a3c4118de4618fd2c8b32d

We'll need someone to peer review and test the patch before we cherry pick over to the master branch.

(note: for some reason "git am" could not apply the patch, so I applied it via "git apply" thus losing the author tag. It also told me there are tabs in the code - I applied it anyway, but please supply patches in the future that do not have any tabs in the source. The RHQ Eclipse project has the code formatting settings in it to help)
Comment 15 John Mazzitelli 2012-10-16 10:06:34 EDT
Related message sent to rhq-devel mailing list:

https://lists.fedorahosted.org/pipermail/rhq-devel/2012-October/002227.html

Here's the content:

---
There is a community contribution to the Tomcat plugin:

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

I created a branch off of master yesterday and put the patch in it:

http://git.fedorahosted.org/cgit/rhq/rhq.git/log/?h=bug/707349

In order to be merged into master and get into the next RHQ release, we need someone to peer review this and test this. Is anyone available?

We do not have any unit tests, but I was asked to get some. I'll ask the submitter to see if a unit test(s) is possible (this might be one of those that is hard to test today because it will probably require integration tests with a running Tomcat instance).
---

Ken, is there unit test(s) you can create to verify these changes?
Comment 16 Ken Rumer 2012-10-16 10:38:06 EDT
I will try, but i can't get anything done for a couple of days.
Comment 17 Remy Maucherat 2012-11-05 11:07:35 EST
I tested it with Tomcat 7 (current build), and it works properly. The branch can be merged.
Comment 18 John Mazzitelli 2012-11-12 09:03:43 EST
the patch is git commit: e7d48240474fba87f1a3c4118de4618fd2c8b32d

I merged this into master: 99a1543f266710c7136844ae269ff944dad3f289
Comment 19 Mike Foley 2012-11-12 11:22:33 EST
adding a link which documents the qualification of this plugin 

https://tcms.engineering.redhat.com/plan/536/ews-plugin#testruns
Comment 20 Larry O'Leary 2013-03-18 14:58:19 EDT
Seems that metric collection fails for Tomcat 7 for both connectors and web applications. 

Also, it is unclear on how this commit impacts Tomcat 5 and Tomcat 6.

I would strongly encourage that in the future, large chunks of work such as this be done as individual bugs/tasks instead of one BZ representing unrelated changes. However, considering this commit is tied to this BZ only, I have no choice but to put this entire thing back on dev for further work to "tomcat plugin: get support for Tomcat 7 fully working."
Comment 21 Jean-frederic Clere 2013-06-17 05:46:43 EDT
Also a duplicate of https://bugzilla.redhat.com/show_bug.cgi?id=921194
Comment 22 Larry O'Leary 2013-06-20 13:58:27 EDT
Marking as closed/won't fix. This bug is too broad and although some work had been done in an attempt to get Tomcat 7 from EWS working with the managed plug-in, it was unsuccessful and had sense been broken out into separate tasks.

The most relevant:

    Bug 921261
    Bug 921194


If additional work is needed to get the managed plug-in working with Tomcat 7 from EWS, please create new BZs for appropriate tacking int he EWS BZ.