You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by miguelaferreira <gi...@git.apache.org> on 2015/10/19 15:24:47 UTC
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
GitHub user miguelaferreira opened a pull request:
https://github.com/apache/cloudstack/pull/950
Load mysql driver before connecting to db
When deploying ACS as a WAR file in a vanilla tomcat installation, I always get an error reporting that the JDBC driver for MySQL can't be found. This PR loads said driver before establishing a connection.
I tried to figure out how this is done in the ACS rpm but I couldn't. Therefore I open this PR to enable anyone that wants to deploy ACS as a WAR file.
The PR also adds a dependency on mysql connector in the developer profile. Without this dependency the build will fail because the driver manager won't be able to load the driver class.
I have tested this PR by deploying the resulting WAR several times, and I have to say to reviewers, that the war needs to be rename to client.war, otherwise the UI will not work.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/miguelaferreira/cloudstack load-mysql-driver-before-connecting-to-db
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/950.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #950
----
commit e9ab28f6a2de81d8596fdb4b8e6d4aa967acfe1a
Author: Miguel Ferreira <mi...@me.com>
Date: 2015-10-19T09:29:23Z
Add MySQL connector dependency to developer profile
commit 8b6bf567f988c0b362441bc53f19da9df9e226d9
Author: Miguel Ferreira <mi...@me.com>
Date: 2015-10-18T13:42:19Z
Load MySQL driver before creating datasource
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-151057998
It's already included as a dependency in the `noredist` profile. This PR includes it as a dependency in the `developer` profile, and forces the MySQL driver to be loaded before initialising any connection to the DB. However, travis and Jenkins don't seem to be building the project with any profile that includes the dependency, therefore the failed unit tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-150670213
@miguelaferreira Could you force push your commit again? So we're all green? :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-150638411
LGTM, build several times and it succeeds. The resulting war now works in tomcat, which is awesome.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-149911276
Right. I'm including the dependency in the `developer` profile. And by the way the dependency is already included in the `noredist` profile.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-151037118
@miguelaferreira Can you please have a look why both Travis and Jenkins are unhappy again? Thx!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-151050165
The erros, travis and Jenkins report are due to missing mysql connector jar. Shall I make Travis build with the developer profile (i.e. `-Pdeveloper`)? Or just close this PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-153390623
@remibergsma @borisroman I'm closing this PR since I've been able to figure out how the mysql driver is being loaded in the RPM install. It happens that it's not enough to add the jar to tomcat's common class loader, nor the webapp class loader. For it to work the jar must be in the class path of tomcat itself. That is, the `CLASSPATH` variable in the environment that starts tomcat. In tomcat terminology that is the `System` loader (https://tomcat.apache.org/tomcat-7.0-doc/class-loader-howto.html)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-151055701
Can't we include it as dependency? Or are there licencing issues?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-150900045
@borisroman @remibergsma I've rebased on latest master and force pushed it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-153289366
I've searched the legal section of Apache's JIRA and found several posts about including GPL licensed products in builds and test runs. One in particular (https://issues.apache.org/jira/browse/LEGAL-19) mentions "runtime dependency". My commit "ccb2cd9568fd7d2febfe6de08e43c43e460c0bee" makes the mysql connector an explicit runtime dependency, by requiring that the class is loaded.
I would argue that mysql connector is already a runtime dependency because without it the application won't run. My commit makes that explicit. Which could be a problem in legal-land.
I've tried an failed miserably to understand how is the mysql connector loaded in tomcat when installing from RPM. Does anyone know how this is done?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-153174098
@miguelaferreira Could you look at this again? It would be nice to have it in!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-153287336
That seems to be LGPL and as such not allowed by apache policy: http://www.apache.org/legal/resolved.html
The issue to me is more on where do the restrictions apply. We cannot distribute this dependency but we can use it in development and test. Therefore, why not include in the development profile?
I don't know enough about where and how these restrictions apply to make a call. The people that might know are not contributing to the discussion, so I'm loosing hope.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-153396280
@miguelaferreira Shame, though I understand ;). Just for reference; here it loads the mysql-connector-jar [0]. It is loaded before the main app because it's in the common.loader definition. The jar itself is installed because we require mysql-connector-java here [1]. And if you look at the RPM here [3] you can see it is actually installed there.
Don't know if you we're actually searching for that...
[0] https://github.com/apache/cloudstack/blob/master/packaging/centos7/tomcat7/catalina.properties#L47
[1] https://github.com/apache/cloudstack/blob/master/packaging/centos7/cloud.spec#L74
[2] http://mirror.centos.org/centos/7.1.1503/os/x86_64/Packages/mysql-connector-java-5.1.25-3.el7.noarch.rpm
Ping @remibergsma
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-153286535
@miguelaferreira @remibergsma Would this be of any help? https://mariadb.com/kb/en/mariadb/about-mariadb-connector-j/
It has a GNU license. Are there any compatability issues?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-153259670
Hi @borisroman
The travis buld fails ont he unit tests because it would need the developer profile (`-Pdeveloper`) in this line https://github.com/apache/cloudstack/blob/master/tools/travis/install.sh#L52.
Jenkins is returning a 404!
I could update the travis config, but I don't see how that would help clarify what to do with this PR. @ke4qqq stepped in to make a remark that causes doubts on whether to merge this or not, @remi and myself have asked for clarification, but got no answer.
Unfortunately, I don't see what else can I do to get this PR merged :(
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira closed the pull request at:
https://github.com/apache/cloudstack/pull/950
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-153468603
@borisroman That's what I thought, but no, the `common.loader` property (in [0] is not what does the trick). I've tried that too and that did not work, not because the jar isn't "loaded" by the JVM but because the driver class itself is not loaded into the memory area that holds classes.
Tomcat has 4 layers of class loaders: Bootstrap, System, Common and each webapp.
In the `catalina.properties` file there is a property called `common.loader` that defines which jars are available to tomcat's Common class loader. The fact that a jar is available to the class loader does not mean all classes it defines will be loaded. It only means that when a class is requested, the class loader will be able to search that jar. And, by the way, if a class is defined in multiple jars, only the first one that is found will be loaded.
**So, why is it that having the jar with the right driver class available, the DriverManager still doesn't find a suitable driver?**
It turns out that the DriverManager is not actively searching for all available drivers (as specified in the Java Docs [1]). It so happens that for the driver class to be available for the DriverManager, that class has to be explicitly loaded, either by an `import com.mysql.jdbc.Driver;` statement, or a `Class.forName("com.mysql.jdbc.Driver")` statement.
I've made a very simple webapp that asks the DriverManager for a JDBC MySQL connector, and even though the correct driver is sitting in a jar that is in the class path, the DriverManager still throws a "no suitable driver found" exception. To make that work, all we need is to add a line of code saying `Class.forName("com.mysql.jdbc.Driver");` right before asking for the connection. Adding that line proves the jar with the driver is correctly defined in the class path, and when requested the correct driver class is loaded by the JVM.
What happens in CloudStack is that the mysql connector jar is not only searchable by the Common class loader (via `common.loader` in [0]), but it is also searchable by tomca's System class loader set in [2]. And that's the class loader that is actually loading the driver class. I'm not sure why, but my best guess is that when tomcat is booting, one of it's components (maybe tomcat-jdbc [3]) sweeps the available jars for any class implementing `java.sql.Driver` and actually finds the one we need.
So, if you want to drop a CloudStack WAR in a tomcat instance, make sure that the MySQL connector jar is the class path of the process that starts JVM.
Finally, just for documentation sake, @ke4qqq, if you even read this, please grep the codebase for `Class.forName("com.mysql.jdbc.Driver")`. You will find two files that contain that line, and those make the "runtime dependency" on a class licensed under GPL very explicit. I wonder how does that play out with Apache's policy?
[0] https://github.com/apache/cloudstack/blob/master/packaging/centos7/tomcat7/catalina.properties#L47
[1] http://docs.oracle.com/javase/7/docs/api/java/sql/DriverManager.html
[2] https://github.com/apache/cloudstack/blob/master/packaging/centos7/cloud-management.sysconfig#L48
[3] https://tomcat.apache.org/tomcat-7.0-doc/jdbc-pool.html
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-149296878
What do you mean by the default build?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by ke4qqq <gi...@git.apache.org>.
Github user ke4qqq commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-149296707
There's a reason why the MySQL connector is not a dependency - it is Cat-X licensed, which means we may not depend on it in the default build.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-150638131
@ke4qqq Are you OK with including it in the Developer profile as well. This obviously is not the default build. This would allow for war drop deployments in tomcat, making this a whole lot easier.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Load mysql driver before connecting to db
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/950#issuecomment-149839957
@miguelaferreira I think that is without specifying -P. You only include it in the developer profile, right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---