Embedded Tomcat & Jetty

[rsafonseca]

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 :)

[asfbot]

cloudstack-pull-requests #341 FAILURE
Looks like there's a problem with this pull request

[eriweb]

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?

[eriweb]

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.

[rsafonseca]

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.

[ke4qqq]

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.

[creategui]

How about switching to https://downloads.mariadb.org/client-java/

[asfbot]

Marcus on [email protected] replies:
Looks like that is LGPL

http://www.apache.org/legal/resolved.html#category-x

[creategui]

bummer that Apache does not allow LGPL.

[rsafonseca]

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
...

[asfbot]

Marcus on [email protected] replies:
I assume the ASF turns their nose up to this? Have we discussed this before?

https://www.mysql.com/about/legal/licensing/foss-exception/

[wilderrodrigues]

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

[serverchief]

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

[rsafonseca]

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

[eriweb]

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 [email protected]
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
#372 (comment).

[rsafonseca]

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 :)

[eriweb]

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.

[wido]

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.

[wilderrodrigues]

Hi @wido ,

On this comment #372 (comment) 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

[rsafonseca]

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

[remibergsma]

@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.

[rohityadavcloud]

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)

[sebgoa]

@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.

[sebgoa]

We should keep it open though.

[rohityadavcloud]

tag:pickup

[DaanHoogland]

this is old, the writer has left the project it seems and it combines many changes; closing

[DaanHoogland]

Just discussed with @rhtyd, We still want this functionality so I will open a smaller PR with a reference to this.