-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLOUDSTACK-9456: Migrate master to Spring 4.x #1638
Conversation
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✖centos6 ✖centos7 ✖debian repo: http://packages.shapeblue.com/cloudstack/pr/1638 |
@blueorangutan I see, slaves need Java8 installed. Fixing that. |
Packaging result: ✖centos6 ✖centos7 ✖debian repo: http://packages.shapeblue.com/cloudstack/pr/1638 |
c1716cd
to
4a0d5c0
Compare
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
<cs.jstl.version>1.2</cs.jstl.version> | ||
<cs.jstl-api.version>1.2.1</cs.jstl-api.version> | ||
<cs.selenium.server.version>1.0-20081010.060147</cs.selenium.server.version> | ||
<cs.vmware.api.version>6.0</cs.vmware.api.version> | ||
<org.springframework.version>3.2.16.RELEASE</org.springframework.version> | ||
<org.springframework.version>4.2.7.RELEASE</org.springframework.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason to not use the version 4.3.2 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcaurele 4.3 is slightly different than 4.2; there were several unit test failures with 4.3 so for now I've avoided switching to 4.3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've bumped the version to 4.3.2 now, there are two failing unit tests now that need fixing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed a PR on your branch with the fixes
Packaging result: ✔centos6 ✖centos7 ✖debian repo: http://packages.shapeblue.com/cloudstack/pr/1638 |
4a0d5c0
to
98d0f3f
Compare
Packaging result: ✔centos6 ✔centos7 ✔debian repo: http://packages.shapeblue.com/cloudstack/pr/1638 |
@blueorangutan package |
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Thanks @marcaurele for your fix, it has been included in the PR. |
Packaging result: ✔centos6 ✔centos7 ✔debian repo: http://packages.shapeblue.com/cloudstack/pr/1638 |
Shouldn't we consider doing a 5.0.0 release that would support Java 8 or we want to go Java8 at 4.10.0 ? Otherwise I can't review, but I should be able to help on testing. |
@pdion891 we're neither breaking any backward compatibility wrt APIs, or schema, nor we've made any core change (mgmt server core, cmd-answer patterns, rpcs/serialization, or the plugin-system). So, I think there is no need to do a 5.0.0 for this (yet). Thanks @pdion891 testing would be much needed, especially around plugins. |
Make sense, thanks, I hope it will get thru then to add support for Ubuntu 16.04 for the management server host and KVM. |
@rhtyd @pdion891 I apologize for my delayed reply as I am on vacation ATM. We had previously agreed not to require Java8 until 5.0.0 which is scheduled for the end of 2016. 4.x should be able to run on Java7 and Java8. Furthermore, Spring4 does not require Java8 so I don't see why such a Java upgrade would conflated with a Spring upgrade. @rhytd I think it makes sense to reduce scope of this PR to Soring only. In a separate PR, we can address running tests for 4.x on both Java7 and Java8 which needs to be done for LTS as well. Does that make sense? |
@jburwell Running tests against JRE 1.7, 1.8 is do-able. But I really want our master to use JDK 1.8. If we want to be able to run ACS mgmt server, agents etc against JRE 1.8 on both 4.9/lts and master branches why not use JDK 1.8 for at least master moving forward? Starting 4.10 why not make JRE 1.8 default for CloudStack mgmt server and agents? |
@rhtyd there is a difference between supporting Java8 and using it as the default version. By changing the POMs, Maven will require that Java8 in order to use CloudStack dependencies. Furthermore, making Java8 ripples into supported Tomcat versions and Linux distributions. Therefore, we agreed sometime ago that 4.x would support Java8, and 5.0.0 would require/default to it. Personally, I would like to adopt Java8 as soon as possible, but we have to recognize that many of our users can move so quickly. I would also like to do it once we have Jetty embedded so that the upgrade is much simpler for users of Linux distributions because they will no longer be concerned with version matching Tomcat. |
c397509
to
b3adbd1
Compare
@jburwell fixed, it's jdk7+spring4 now |
Packaging result: ✔centos6 ✔centos7 ✔debian repo: http://packages.shapeblue.com/cloudstack/pr/1638 |
Trillian test result (trillian-pr1638-46-kvm-centos68-cs410):
Trillian env - trillian-pr1638-46-kvm-centos68-cs410, Job ID 46 |
b3adbd1
to
d0e22a2
Compare
@rhtyd have you had a chance to review the test cases with errors and skips? |
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-204 |
@blueorangutan package |
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✖debian. JID-266 |
@blueorangutan test |
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-466)
|
76e85b4
to
3cc3463
Compare
@blueorangutan package |
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-285 |
@blueorangutan test |
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
- Bump spring-framework version to 4.x and Jetty to version that runs with JDK8 - Bump servet dependency version - Migrate spring xmls to version 4, fixes schema locations that are 3.0 dependent in various xmls. - Fix failing tests due to spring upgrade (Thanks @marcaurele Marc-Aurèle Brothier for fixing them) * Fix test DeploymentPlanningManagerImplTest * Fix GloboDNS test Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
3cc3463
to
0dce1c5
Compare
LGTM, and test LGTM (Travis). |
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-398 |
@blueorangutan package |
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-411 |
@blueorangutan test centos7 kvm-centos6 |
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos6) has been kicked to run smoke tests |
Trillian test result (tid-723)
|
Merging this based on test results. |
CLOUDSTACK-9456: Migrate master to Spring 4.xThis changes makes CloudStack use spring 4: ``` - Bump spring-framework version to 4.x and Jetty to version that runs with JDK7 - Bump servet dependency version - Migrates various xmls to use version independent schema uris ``` Outstanding issue: - Testing of various non-standard plugins such as network and storage plugins etc. Since, this is a big change pinging for review -- @jburwell @karuturi @wido @murali-reddy @abhinandanprateek @DaanHoogland @GaborApatiNagy @JayapalUradi @kishankavala @K0zka @nvazquez @rafaelweingartner @pyr and others @blueorangutan package * pr/1638: CLOUDSTACK-9456: Update Spring version in maven poms Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-423 |
This changes makes CloudStack use spring 4:
Outstanding issue:
- Testing of various non-standard plugins such as network and storage plugins etc.
Since, this is a big change pinging for review -- @jburwell @karuturi @wido @murali-reddy @abhinandanprateek @DaanHoogland @GaborApatiNagy @JayapalUradi @kishankavala @K0zka @nvazquez @rafaelweingartner @pyr and others
@blueorangutan package