You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by rsafonseca <gi...@git.apache.org> on 2015/06/08 19:18:14 UTC
[GitHub] cloudstack pull request: Embedded Tomcat & Jetty
GitHub user rsafonseca opened a pull request:
https://github.com/apache/cloudstack/pull/372
Embedded Tomcat & Jetty
Well, this spruces things up a bit :)
Some of the features/fixes (i may have forgotten a couple as this involved a lot of changes ;) )
- Embedded Tomcat support
- Embedded Jetty support
- Reduce management .deb package to roughly half the size
- Reduce common .rpm package to roughly half the size
- Transparent support for more distros like Fedora 21
- Reduced dependencies on the packages, such as mysql-connector-java which are now bundled
- Upgraded Jetty version used by Travis from oldie and discontinued morbay 6.x version to current eclipse 9.x version
- Tidy up debian packaging under /packaging and include support on package.sh
- Cleanup of a lot of duplicate files which made it harder to maintain and caused some fixes to be implemented only for some distros
- Consolidates a few configs that were different across distros and shouldn't be
- Enables running with any version of openjdk runtime and compiling with any version of openjdk devel as long as they meet the minimum requirements (Java 7+) without conflicting with other packages.
The package.sh script packages ASF Tomcat by default, you can package it with Jetty instead by passing -j or --jetty :)
There's still a couple more things to consolidate and a couple more places to remove hardcoding of configs like the cloud user, but i'll take care of that on a later PR :)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rsafonseca/cloudstack fixpackaging2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/372.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 #372
----
commit decde6d129ce8c87a82d8aa00a6723b91c322e02
Author: Rafael da Fonseca <rs...@gmail.com>
Date: 2015-05-03T18:59:53Z
Tomcat packaging
Reduced cloudstack-management_4.6.0-snapshot_all.deb size from 211Mb to 94Mb (CentOS is 92Mb)
This deb file was packing two copies of the systemvm (which is alread packed in cloudstack-common), so removed like in CentOS RPM specs
Fedora 21 support for management :)
Removed Fedora specific branches (symlink fedora2x points to centos7 so, using same installer :)
Small cleanup of duplicate files
Added embedded jetty
Support for choosing tomcat or jetty on package.sh
Removed mysql driver distro dependency
Replaced old mortbay jetty 6 with eclipse jetty 9
Removed duplicate files
Tidy up debian packaging
Make things work on debian
Cleanup, fixes, distro abstraction, etc
Fix simulator for new jetty plugin
Fix servlet engines on older mvn version
Config jetty version
Fix juli logging manager error on tomcat
Fix dep problem with mysqlha plugin
----
---
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: Embedded Tomcat & Jetty
Posted by serverchief <gi...@git.apache.org>.
Github user serverchief commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110242990
I'm with Wilder on this. Thank you for being active and submitting really useful patches - its much appreciated. However, it would be best if you can split the commits into multiples - as 1 fix per commit (or more if related).
Mainly, it would be easier to revert a commit - in case something is wrong and loose 1 functionality VS reverting 1 commit because 1 item is broken, but as cascade effect, 10 fixes will be reverted as well.
Regards
ilya
---
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: Embedded Tomcat & Jetty
Posted by rsafonseca <gi...@git.apache.org>.
Github user rsafonseca commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-121889370
Hi @wido
I've been waiting for feedback from Pyr on a thread in the dev mailing list with subject "4.6" if you want to look it up.
I will nevertheless do what @wilderrodrigues suggested and break it down, and will also remove the mysql bit, or make it non-default to be compliant with Apache licensing rules, regardless of the Oracle licensing exception.
Unfortunately i've not had any time to work on ACS for the past couple of weeks since i've just relocated to Dublin, and it might take a couple more before I have the time to pick it up.
Cheers,
Rafael
---
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: Embedded Tomcat & Jetty
Posted by rsafonseca <gi...@git.apache.org>.
Github user rsafonseca commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110151397
Hi @terbolous
I'm in no way a licensing expert, but don't think there's any incompatibility with the licensing.. but I'll leave that for the experts to say their .2c
I guess that mainly it was not shipped because it was just not working when you bundled it in the webapp, it was missing the driver instantiation which it relied on the servlet engine to do.. that is taken care of now :)
About the testing, I've tested manually deploying a datacenter with this package in:
Ubuntu 12.04.5
Centos 6.6
Centos 7
Fedora 21
Sadly, i've done no testing on upgrades, but i can easily do so in a few days and fix whatever issue may arise... although i don't foresee any issues.
I've also tested running marvin locally, and it's working fine :)
All distros were tested with both tomcat and jetty, except Fedora 21 which i only tested with tomcat, but should be all the same.
---
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: Embedded Tomcat & Jetty
Posted by rsafonseca <gi...@git.apache.org>.
Github user rsafonseca commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110255489
Hi @wilderrodrigues , @serverchief
Thank you for your feedback :)
I understand your point, but since there are a lot of changes and most are interconnected or change the same files, breaking it down would make it take a long time to get things done and i'm used to working fast with a lot of changes, sorry about that ;) . This is still not all the cleaning up i'd like to do in packaging, and packing it all together makes it also a lot faster to test for issues, than to do complete tests for a big bunch of PRs individually, although a complete code review will be a bit taxing of course, either way, reviewing 5x 200 lines of code, should be no faster than to review 1000 line of code once, although testing packaging and functionality once is a lot faster than doing it 5x, don't you agree?
I will try to break down a some things in the coming days like i did with the last packaging PR, but ultimately, the work goes a lot faster if i'm not waiting for each individual change to get reviewed and merged before moving on to the next step to improve the same thing. This is a particular case because all the changes revolve around the same things: packaging and distribution.
Either way, this allows for people to test once and make sure everything is working.. if any issues are reported i'll be sure to fix them, and not go back in time ;)
I will also be sure to review all the changes myself and add anything relevant i may have forgotten about, if any, to the initial comment to make it easier to follow :)
Merging the whole PR would speed things up considerably and allow me to continue improving packaging with a new base, but i'm not expecting to see this through overnight of course :)
Ultimately it was easier and faster to get everything done in the same branch and fix issues as they arose in my testing. This just gets it all out there at once for the community to comment on and test as a whole.
Other than this packaging PR which is something that needed an overhaul in my opinion, I will try to keep my PRs as small as possible as you have suggested.
Cheers,
Rafael
---
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: Embedded Tomcat & Jetty
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-121140056
Any progress on this PR? Since I really do like the proposal, but my knowledge of Jetty/Tomcat is indeed to limited.
I can test the packaging, but I have no idea how to test the rest.
---
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: Embedded Tomcat & Jetty
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-121150878
Hi @wido ,
On this comment https://github.com/apache/cloudstack/pull/372#issuecomment-110239298 I suggested that the PR should be split so we could tackle the changes in an independent way.
Unfortunately, I don't know the current status but I hope that @rsafonseca could bring some light on the subject.
Cheers,
Wilder
---
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.
---
Re: [GitHub] cloudstack pull request: Embedded Tomcat & Jetty
Posted by Marcus <sh...@gmail.com>.
Looks like that is LGPL
http://www.apache.org/legal/resolved.html#category-x
On Mon, Jun 8, 2015 at 2:57 PM, creategui <gi...@git.apache.org> wrote:
> Github user creategui commented on the pull request:
>
> https://github.com/apache/cloudstack/pull/372#issuecomment-110153681
>
> How about switching to https://downloads.mariadb.org/client-java/
>
>
> ---
> 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: Embedded Tomcat & Jetty
Posted by creategui <gi...@git.apache.org>.
Github user creategui commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110153681
How about switching to https://downloads.mariadb.org/client-java/
---
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: Embedded Tomcat & Jetty
Posted by terbolous <gi...@git.apache.org>.
Github user terbolous commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110145052
By the way, what kind of testing has this undergone? In particular I'm interested in upgrade testing, as we don't want to break any old installations.
---
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: Embedded Tomcat & Jetty
Posted by runseb <gi...@git.apache.org>.
Github user runseb commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-134929830
@bhaisaab this came out from a roadmap item suggested by @pyr, but it seems he has not had time to review and comment. So this is a bit stuck I am afraid.
---
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: Embedded Tomcat & Jetty
Posted by rsafonseca <gi...@git.apache.org>.
Github user rsafonseca commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110265493
Hi @terbolous
I'm here to help, so i'll try to break it down a bit in the coming days as i mentioned :)
Testing this will take some time anyway, and if an issue arises with the mysql connector change, i'll just update the PR, nothing will get stuck because of that ;)
Btw, the change with mysql has 2 objectives:
1 - Reduce distro specific dependencies (different distros have different package names an versions) and thus increase distro abstraction.
2 - Make the management webapp self contained (currently, if you take the war file from this PR and put it in your own servlet engine all you need to do is install the RPM and add /etc/cloudstack/management to the context class loader's classpath )
Either way, do you all mean easier to review? I don't see how testing a bunch of PRs would be faster or easier to test a single one :)
---
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: Embedded Tomcat & Jetty
Posted by terbolous <gi...@git.apache.org>.
Github user terbolous commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110144112
I'm by no means an expert on this, but isn't the reason we didn't ship the mysql connector previously due to licensing? @ke4qqq or @runseb wanna chime 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: Embedded Tomcat & Jetty
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-134928152
anyone working on progressing this PR, I really like it and it has some good things; if no one is I can help take over some things in future (cc @rsafonseca)
---
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: Embedded Tomcat & Jetty
Posted by terbolous <gi...@git.apache.org>.
Github user terbolous commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110320351
Thanks @rsafonseca, your efforts are well appreciated, don't think otherwise.
Small changes equals, atleast usually, a smaller set of testing.
The CloudStack community consists of a lot of different people (and their companies), who all have different needs, agendas, and time available.
So, while some might have the time and willingness to test packaging, they might not have time, the experience or willingness to test the other changes.
For instance, I have no idea how to test the embedded jetty/tomcat changes.
The bigger a change is, the harder it is to track why it was done, and it's implications, especially when it's not related to a Jira issue/FS or similar.
This is not to say that your PR cannot be merged, but waiting for someone to either have tested all the changes, or for enough people to have tested each subset could take time.
---
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: Embedded Tomcat & Jetty
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110239298
Hi there @rsafonseca
Cool to see you contribution, thanks for that, but I think the PRs should be broken down. You listed 11 changes and said that you might have forgotten another few. That's a bit dangerous and also make testing the PR way to expansive for the reviewers.
We should have 1 PR per feature/fix, or perhaps a couple of fixes, depending on their size (fixing findbugs is a good example).
Another good practice is to add the environment where the things were tested, and also the steps you took to test it, in the PR. Adding those afterwards, as comments, it's also not very good to follow.
Reading your list I would say that you have at least 4 PRs.
1. Tomcat/Jetty changes
2. Packaging
3. Configs and Distros
4. JDK version
In addition, the a PR should be independent of any other. Thus, we could for example test 4 and merge it without waiting for 2 or 3.
If PR follows that structure I think we can get them merge way faster than now.
I hope you appreciate the feedback. We are working to achieve the same goal.
Cheers,
Wilder
---
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: Embedded Tomcat & Jetty
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-216187512
tag:pickup
---
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: Embedded Tomcat & Jetty
Posted by runseb <gi...@git.apache.org>.
Github user runseb commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-134929897
We should keep it open though.
---
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.
---
Re: [GitHub] cloudstack pull request: Embedded Tomcat & Jetty
Posted by Marcus <sh...@gmail.com>.
I assume the ASF turns their nose up to this? Have we discussed this before?
https://www.mysql.com/about/legal/licensing/foss-exception/
On Mon, Jun 8, 2015 at 3:16 PM, creategui <gi...@git.apache.org> wrote:
> Github user creategui commented on the pull request:
>
> https://github.com/apache/cloudstack/pull/372#issuecomment-110160470
>
> bummer that Apache does not allow LGPL.
>
>
> ---
> 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: Embedded Tomcat & Jetty
Posted by creategui <gi...@git.apache.org>.
Github user creategui commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110160470
bummer that Apache does not allow LGPL.
---
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: Embedded Tomcat & Jetty
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-129040864
@rsafonseca Can you close this PR? If I understand correctly the work will be split over multiple new PRs. Hope you're doing OK, talk to you soon.
---
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: Embedded Tomcat & Jetty
Posted by ke4qqq <gi...@git.apache.org>.
Github user ke4qqq commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110152435
We can't bundle mysql-connector-java. It is licensed as GPL which is CatX and thus verboten. We can make it a system dependency.
---
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: Embedded Tomcat & Jetty
Posted by terbolous <gi...@git.apache.org>.
Github user terbolous commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110257923
I'm with Wilder & co on this.
If you break it down, it's easier to test and thus merge.
For instance, the mysql licensing could now potentially hold the complete
PR, while tomcat/jetty changes are irrelevant and could've been merged (if
tested and verified working).
--
Erik
On Tue, Jun 9, 2015 at 9:10 AM, Rafael da Fonseca <no...@github.com>
wrote:
> Hi @wilderrodrigues <https://github.com/wilderrodrigues> , @serverchief
> <https://github.com/serverchief>
>
> Thank you for your feedback :)
>
> I understand your point, but since there are a lot of changes and most are
> interconnected or change the same files, breaking it down would make it
> take a long time to get things done and i'm used to working fast with a lot
> of changes, sorry about that ;) . This is still not all the cleaning up i'd
> like to do in packaging, and packing it all together makes it also a lot
> faster to test for issues, than to do complete tests for a big bunch of PRs
> individually, although a complete code review will be a bit taxing of
> course, either way, reviewing 5x 200 lines of code, should be no faster
> than to review 1000 line of code once, although testing packaging and
> functionality once is a lot faster than doing it 5x, don't you agree?
>
> I will try to break down a some things in the coming days like i did with
> the last packaging PR, but ultimately, the work goes a lot faster if i'm
> not waiting for each individual change to get reviewed and merged before
> moving on to the next step to improve the same thing. This is a particular
> case because all the changes revolve around the same things: packaging and
> distribution.
> Either way, this allows for people to test once and make sure everything
> is working.. if any issues are reported i'll be sure to fix them, and not
> go back in time ;)
>
> I will also be sure to review all the changes myself and add anything
> relevant i may have forgotten about, if any, to the initial comment to make
> it easier to follow :)
> Merging the whole PR would speed things up considerably and allow me to
> continue improving packaging with a new base, but i'm not expecting to see
> this through overnight of course :)
> Ultimately it was easier and faster to get everything done in the same
> branch and fix issues as they arose in my testing. This just gets it all
> out there at once for the community to comment on and test as a whole.
>
> Other than this packaging PR which is something that needed an overhaul in
> my opinion, I will try to keep my PRs as small as possible as you have
> suggested.
>
> Cheers,
> Rafael
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/cloudstack/pull/372#issuecomment-110255489>.
>
---
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: Embedded Tomcat & Jetty
Posted by rsafonseca <gi...@git.apache.org>.
Github user rsafonseca commented on the pull request:
https://github.com/apache/cloudstack/pull/372#issuecomment-110161431
How about the FOSS license exception?
https://www.mysql.com/about/legal/licensing/foss-exception/
FOSS License List
License Name Version(s)/Copyright Date
...
Apache Software License 1.0/1.1/2.0
...
---
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.
---