Skip to content

CLOUDSTACK-10012: Towards CloudStack fat jar - #2226

Merged
yadvr merged 2 commits into
masterfrom
CLOUDSTACK-10012
Nov 3, 2017
Merged

CLOUDSTACK-10012: Towards CloudStack fat jar#2226
yadvr merged 2 commits into
masterfrom
CLOUDSTACK-10012

Conversation

@yadvr

@yadvryadvr commented Aug 7, 2017

Copy link
Copy Markdown
Member

From #2207
I've moved the branch to ASF remote to allow cross-collaboration between ACS committers. Feel free to discuss and push your changes in this branch.

  • Fix init.d/system.d scripts
  • Remove tomcat related files and config
  • Fix development mode
  • Fix CentOS6, 7, Ubuntu packaging
  • Validate scripts backward compatibility, run Trillian tests
  • Improve ServerDaemon
  • Single yml/properties file based configuration
  • HTTPs/keystore management
  • Fix overlapping/shaded jar issues, missing classes
  • Allow webapp override, a separate UI/assets folder
  • Cleanup code, refactor build scripts/files
  • Fix docs, run upgrade tests

@blueorangutan package

@yadvryadvr mentioned this pull request Aug 7, 2017
4 tasks
@yadvr

yadvr commented Aug 7, 2017

Copy link
Copy Markdown
MemberAuthor

Travis green, so at least the mvn side of the world works: https://travis-ci.org/apache/cloudstack/builds/261836210 :)

@yadvr

yadvr commented Aug 7, 2017

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@marcaurele

Copy link
Copy Markdown
Member

For postmortem, there are comments on this change in the linked PR #2207

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✖centos6 ✖centos7 ✔debian. JID-928

@yadvr

yadvr commented Aug 7, 2017

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-930

@marcaurele

marcaurele commented Aug 18, 2017

Copy link
Copy Markdown
Member

I came across another change that is required for the fatjar to work. We have to change the DbUpgrade process to not load File but return an array of InputStream since we have to load those files from inside the JAR. I'll push a change on the branch today

@marcaurele

Copy link
Copy Markdown
Member

@rhtyd The DB files are kept out of the jar. Any opinion on that one if we should keep them outside (due to some possible customization for providers/plugins) or should we include them in the META-INF directory of the jar?

@yadvr

yadvr commented Aug 18, 2017

Copy link
Copy Markdown
MemberAuthor

@marcaurele I think we need to consider and discuss three things;

  • Whether to bundle the UI within the jar, or provider a webapp path to say /usr/share/cloudstack-management/webapp, in this directory we copy the UI asset files. This would allow admins to put their customizations. A solution could be to do both, so the fat jar has the UI files and however ServerDaemon would set the webapp path to the /usr path by default (if available) and fallback to assets in the jar when not found.
  • The DB files are kept out of the jar in case someone want to fix any upgrade failure issue. It's kind of subjective. My opinion is to bundle them in the jar, which reduces file management and opens scope for changes to the sql paths.
  • In addition, a lot of people may want to consume additional jars (plugins etc) which is why bounty castle and other jars may be put at /usr/share/cloudstack-management/lib. When mgmt server starts, we scan and include all the jars at this path in the classpath, with them first in the order in case someone want to overide any bundled jar etc.

What do you think?

@marcaurele

Copy link
Copy Markdown
Member

@rhtyd

  1. I will add a startup parameter -Dwebappdir=... to let the user change the path location instead of trying to guess things. Then if the path is incorrect at least it will return an error instead of loading the default UI. IMO it's better.
  2. I'm in favor to bundle them inside the JAR. It's best for people customizing the installation to have an extra module where they do what they need, instead of editing files from the official releases.
  3. +1 for the JARs ordering, that make sense for customization of the install with extra jars.

I'll push some of the changes today.

setJavaHome

JARS=$(ls /usr/share/cloudstack-management/lib/*.jar | tr '\n' ':' | sed s'/.$//')
CLASSPATH="/etc/cloudstack/management:/usr/share/cloudstack-common:/usr/share/cloudstack-management/setup:/usr/share/cloudstack-management:/usr/share/cloudstack-management/cloud-management-server.jar:$JARS"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:$JARS should be put in front here to first load extra JARs

@marcaurele

Copy link
Copy Markdown
Member

@rhtyd I cannot push to this branch

@yadvr

Copy link
Copy Markdown
MemberAuthor

@marcaurele perhaps you don't have commit rights yet, can you send a PR to the asf branch CLOUDSTACK-10012 and ping me I'll merge your changes right away

@marcaurele

Copy link
Copy Markdown
Member

Related to 42d9297, I forgot to remove the jetty. in the class javadoc of ServerDaemon- if you could do it, thanks (PR seems overkill)

@yadvr

Copy link
Copy Markdown
MemberAuthor

Thanks @marcaurele, sure will do that. These days I'm down with some £dayjob project work but I've planned to resume working on this PR after next ~2 weeks.

@yadvr

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@yadvr

yadvr commented Oct 16, 2017

Copy link
Copy Markdown
MemberAuthor

@marcaurele I've resumed working on this now. I've squashed changes by authors, after painfully going through 27+ commits while rebasing against the latest master, hope you don't mind. I did not see any classes being overwritten for bountycastle and hence removed it from the exclusion list, do you think that could cause any issues? I'm exploring some ways to further improve the ServerDaemon launcher by using a customer properties or yml file that can be used as a single point to configure ports, listening interfaces, https, keystore etc. I've also upgraded several dependencies, notably Spring, bountycastle, and jetty.

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1164

@yadvr

yadvr commented Nov 1, 2017

Copy link
Copy Markdown
MemberAuthor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@wido

wido commented Nov 1, 2017

Copy link
Copy Markdown
Contributor

I just checked, building DEB files worked flawlessly. cloudstack.jar is 73M in size. Good!

@yadvr

yadvr commented Nov 2, 2017

Copy link
Copy Markdown
MemberAuthor

Thanks @wido, is that a LGTM on this PR?

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-1606)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41902 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2226-t1606-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_host_annotations.py
Intermitten failure detected: /marvin/tests/smoke/test_iso.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_ssvm.py
Intermitten failure detected: /marvin/tests/smoke/test_templates.py
Intermitten failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Test completed. 54 look OK, 9 have error(s)

TestResultTime (s)Test File
test_07_resize_failFailure15.34test_volumes.py
test_10_attachAndDetach_isoFailure1513.31test_vm_life_cycle.py
test_09_destroy_ssvmFailure0.04test_ssvm.py
test_07_reboot_ssvmFailure0.04test_ssvm.py
test_05_stop_ssvmFailure0.04test_ssvm.py
test_03_ssvm_internalsFailure0.03test_ssvm.py
test_01_list_sec_storage_vmFailure0.04test_ssvm.py
test_04_rvpc_privategw_static_routesFailure223.27test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanupFailure117.63test_privategw_acl.py
test_02_vpc_privategw_static_routesFailure117.59test_privategw_acl.py
test_01_vpc_privategw_aclFailure46.57test_privategw_acl.py
test_04_extract_IsoFailure5.08test_iso.py
ContextSuite context=TestVpcSite2SiteVpn>:setupError0.00test_vpc_vpn.py
ContextSuite context=TestVpcRemoteAccessVpn>:setupError0.00test_vpc_vpn.py
ContextSuite context=TestRVPCSite2SiteVpn>:setupError0.00test_vpc_vpn.py
ContextSuite context=TestVPCRedundancy>:setupError0.00test_vpc_redundant.py
test_06_download_detached_volumeError20.28test_volumes.py
test_05_create_template_with_no_checksumError65.61test_templates.py
test_04_extract_templateError5.09test_templates.py
test_04_create_template_with_checksum_md5Error65.51test_templates.py
test_03_delete_templateError5.09test_templates.py
test_03_create_template_with_checksum_sha256Error65.51test_templates.py
test_02_create_template_with_checksum_sha1Error65.48test_templates.py
test_01_create_templateError65.66test_templates.py
test_05_add_annotation_for_invalid_entityTypeError0.07test_host_annotations.py
test_change_service_offering_for_vm_with_snapshotsSkipped0.00test_vm_snapshots.py
test_09_copy_delete_templateSkipped0.02test_templates.py
test_06_copy_templateSkipped0.00test_templates.py
test_static_role_account_aclsSkipped0.02test_staticroles.py
test_11_ss_nfs_version_on_ssvmSkipped0.02test_ssvm.py
test_01_scale_vmSkipped0.00test_scale_vm.py
test_01_primary_storage_iscsiSkipped0.09test_primary_storage.py
test_vm_nic_adapter_vmxnet3Skipped0.00test_nic_adapter_type.py
test_03_nic_multiple_vmwareSkipped1.06test_nic.py
test_nested_virtualization_vmwareSkipped0.00test_nested_virtualization.py
test_06_copy_isoSkipped0.00test_iso.py
test_list_ha_for_host_validSkipped0.04test_hostha_simulator.py
test_list_ha_for_host_invalidSkipped0.04test_hostha_simulator.py
test_list_ha_for_hostSkipped0.03test_hostha_simulator.py
test_hostha_enable_feature_without_setting_providerSkipped0.02test_hostha_simulator.py
test_hostha_enable_feature_validSkipped0.02test_hostha_simulator.py
test_hostha_disable_feature_validSkipped0.02test_hostha_simulator.py
test_hostha_configure_invalid_providerSkipped0.02test_hostha_simulator.py
test_hostha_configure_default_driverSkipped0.03test_hostha_simulator.py
test_ha_verify_fsm_recoveringSkipped0.03test_hostha_simulator.py
test_ha_verify_fsm_fencedSkipped0.02test_hostha_simulator.py
test_ha_verify_fsm_degradedSkipped0.02test_hostha_simulator.py
test_ha_verify_fsm_availableSkipped0.03test_hostha_simulator.py
test_ha_multiple_mgmt_server_ownershipSkipped0.03test_hostha_simulator.py
test_ha_list_providersSkipped0.03test_hostha_simulator.py
test_ha_enable_feature_invalidSkipped0.02test_hostha_simulator.py
test_ha_disable_feature_invalidSkipped0.02test_hostha_simulator.py
test_ha_configure_enabledisable_across_clusterzonesSkipped0.02test_hostha_simulator.py
test_configure_ha_provider_validSkipped0.03test_hostha_simulator.py
test_configure_ha_provider_invalidSkipped0.03test_hostha_simulator.py
test_deploy_vgpu_enabled_vmSkipped0.04test_deploy_vgpu_enabled_vm.py
test_3d_gpu_supportSkipped0.04test_deploy_vgpu_enabled_vm.py

- Changes packaging to shaded jar
- Introduced embedded jetty
- Updates jetty dependencies version 9.2 (java 7+)
- Initial changes to debian config files
- Initial removal of tomcat dependency
- Introduce ServerDaemon class with customizations
@borisstoyanov

Copy link
Copy Markdown
Contributor

@rhtyd after upgrading from 4.5 I wasn't able to start management server, got the following:

[root@trl-1020-v-cs45-bstoyanov-mgmt1 ~]# service cloudstack-management restart
ls: cannot access /usr/share/cloudstack-management/lib/*.jar: No such file or directory
Stopping CloudStack Management Server cloudstack-management[ OK ]
cat: /var/run/cloudstack-management.pid: No such file or directory]
[FAILED]

@yadvr

yadvr commented Nov 3, 2017

Copy link
Copy Markdown
MemberAuthor

Rebased to latest master. This is ready for merge now.
@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@yadvr

yadvr commented Nov 3, 2017

Copy link
Copy Markdown
MemberAuthor

@borisstoyanov okay I'll look into it

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1201

@wido

wido commented Nov 3, 2017

Copy link
Copy Markdown
Contributor

Yes, LGTM from me.

I tested a clean install on Ubuntu 16.04 with MySQL 5.7 ( https://blog.widodh.nl/2017/04/apache-cloudstack-and-mysql-5-7/ )

May I however ask that in server.properties this is set:

# The binding interface used by the management server
bind.interface=::

This will make sure that the server also listens on IPv6

Comment threadclient/conf/server.properties.in Outdated
# under the License.

# The binding interface used by the management server
bind.interface=0.0.0.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change 0.0.0.0 to :: so that it listens on IPv6 by default?

I tested this on Ubuntu 16.04 and works fine:

root@cloudstack-man-test:~# curl -I http://127.0.0.1:8080/client/
HTTP/1.1 200 OK
Content-Type: text/html
ETag: W/"114557-1499056602000
Content-Length: 114557
Server: Jetty(9.2.z-SNAPSHOT)
root@cloudstack-man-test:~# curl -I http://[::1]:8080/client/
HTTP/1.1 200 OK
Content-Type: text/html
ETag: W/"114557-1499056602000
Content-Length: 114557
Server: Jetty(9.2.z-SNAPSHOT)
root@cloudstack-man-test:~#

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @wido I think the standard I've seen in mysql and elsewhere is to use this. It's a matter of config issue, and ipv6 may not be enabled on all systems.

For me it fails when JAVA_OPT has -Djava.net.preferIPv4Stack=true set, and bind.interface has :: set. Exception seen is:

WARN [o.e.j.u.c.AbstractLifeCycle] (main:null) (logid:) FAILED ServerConnector@4e9ba398{HTTP/1.1}{:::8080}: java.net.SocketException: Protocol family unavailable
java.net.SocketException: Protocol family unavailable
at sun.nio.ch.Net.bind0(Native Method)

Command ran to test this case (build cloudstack and cd to client/target, fix conf/server.properties): java -Dlog4j.configuration=conf/log4j-cloud.xml -cp cloud-client-ui-4.11.0.0-SNAPSHOT.jar:lib/bcpkix-jdk15on-1.58.jar:lib/bcprov-jdk15on-1.58.jar:.:/usr/share/java/mysql-connector-java.jar:conf:common:utilities/scripts/db/ -Djava.net.preferIPv4Stack=true org.apache.cloudstack.ServerDaemon

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's true. However, all modern Linux systems have IPv6 enabled. It's a manual action to disable IPv6 on a system.

You don't need a public IPv6 address though to listen on ::, you already have your localhost ::1 to listen on.

Don't forget that the current Tomcat6/7 already listen on IPv6 by default. So it would be a change from 4.10 to 4.11 if we wouldn't listen on IPv6 anymore.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wido I agree with you, however, for the default I needed to put some configuration that would work out of the box on most/all systems. I know several installations where they have put -Djava.net.preferIPv4Stack=true in their global JAVA opts (or in cloudstack specific env/init/systemd files). That said, we can have a compromise where I can remove the default value set to empty, i.e. bind.interface=, that way the system picks up the default interface that could work automatically.

With this approach, on systems without ipv6, this listens on:
tcp 0 0 127.0.0.1:8080 0.0.0.0:* LISTEN

And with ipv6 enabled, it listens on:
tcp6 0 0 127.0.0.1:8080 :::* LISTEN

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wido check now, I've made the above change.

@yadvr

yadvr commented Nov 3, 2017

Copy link
Copy Markdown
MemberAuthor

Thanks for testing and reviewing @wido.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1203

@yadvr

yadvr commented Nov 3, 2017

Copy link
Copy Markdown
MemberAuthor

I've confirmed manual upgrade (in-place) testing, from ACS 4.9.3.0 -> 4.11.0.0-SNAPSHOT on CentOS6, CentOS7 and Ubuntu 14.04. Worked as expected, other than seeding systemvmtemplates, the upgrade worked without issues, the old tomcat files are not removed. I'll update upgrade/release notes on the same after this PR gets merged.

- Migrate to embedded Jetty server.
- Improve ServerDaemon implementation.
- Introduce a new server.properties file for easier configuration.
- Have a single /etc/default/cloudstack-management to configure env.
- Reduce shaded jar file, removing unnecessary dependencies.
- Upgrade to Spring 5.x, upgrade several jar dependencies.
- Does not shade and include mysql-connector, used from classpath instead.
- Upgrade and use bountcastle as a separate un-shaded jar dependency.
- Remove tomcat related configuration and files.
- Have both embedded UI assets in uber jar and separate webapp directory.
- Refactor systemd and init scripts, cleanup packaging.
- Made cloudstack-setup-databases faster, using `urandom`.
- Remove unmaintained distro packagings.
- Moves creation and usage of server keystore in CA manager, this
deprecates the need to create/store cloud.jks in conf folder and
the db.cloud.keyStorePassphrase in db.properties file. This also
remove the need of the --keystore-passphrase in the
cloudstack-setup-encryption script.
- GZip contents dynamically in embedded Jetty
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

@borisstoyanovborisstoyanov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgrade was successful on:
CentOS7 4.9 -> master
CentOS6 4.5 -> master
LGTM

@yadvr

yadvr commented Nov 3, 2017

Copy link
Copy Markdown
MemberAuthor

LGTM, merging based on reviews, tests and upgrade-tests. Thank you everyone for helping with reviews and feedback.

@yadvr
yadvr merged commit eda3b35 into masterNov 3, 2017
asfgit pushed a commit that referenced this pull request Nov 4, 2017
Per @wido's comment on PR #2226, this adds default server configuration
to make embedded Jetty listen on all interfaces ipv4 and ipv6. This
also fixes default deployment and mgmt server start issues on Trillian
and other CI systems.
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

@borisstoyanovborisstoyanov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've setup and configured rabbitmq broker and it worked out of the box, following the official documentation.

@rafaelweingartner
rafaelweingartner deleted the CLOUDSTACK-10012 branch April 3, 2018 21:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@yadvr@blueorangutan@marcaurele@DaanHoogland@borisstoyanov@wido@kiwiflyer