Skip to content

Cleanup POMs#2613

Merged
rafaelweingartner merged 24 commits into
apache:masterfrom
khos2ow:clean-poms
Jul 25, 2018
Merged

Cleanup POMs#2613
rafaelweingartner merged 24 commits into
apache:masterfrom
khos2ow:clean-poms

Conversation

@khos2ow
Copy link
Copy Markdown
Contributor

@khos2ow khos2ow commented Apr 28, 2018

Description

In this PR I'm trying to cleanup POMs. The main changes are:

  • Reformat all the poms (use 4 SPACEs instead of 2)
  • Unified License Header
  • Remove obsolete mycila license-maven-plugin (as of d3cd73d)
  • Remove obsolete console-proxy/plugin project (as of e7f8c6a)
  • Move services/console-proxy-rdp/rdbconsole under services/console-proxy
  • Remove services/iam (as of ba84808) [open for discussion]
    • Removing the iam projects doesn't have any side effect, although there are lots of leftover tests
  • Remove tools/wix-cloudstack-maven-plugin
  • Alphabetic order:
    • version properties in parent pom
    • dependencies in parent pom
    • modules in engine
    • modules in framework
    • modules in plugins
    • modules in services
  • Extract all dependency version and plugin version to properties of parent pom
  • Upgrade plugins version to latest
  • Remove redundant defaultGoal
  • Adjust checkstyle plugin to run in all required projects
  • Enforce PMD plugin to be executed on all projects on enablefindbugs profile

For simplicity of reviewing, I pushed in different commits, eventually we can squash them.

TODO and followups after this PR gets merged

  • re-enable PMD in enablefindbugs profile
  • remove buildw profile and tools/wix-cloudstack-maven-plugin
  • remove services/iam project

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

How Has This Been Tested?

  • Executed mvn -P developer,systemvm -Dsimulator -Dnoredist --projects='org.apache.cloudstack:cloudstack' org.apache.rat:apache-rat-plugin:0.12:check
  • Executed mvn install -P developer -P systemvm -Dsimulator -Dnoredist
  • Executed mvn install -P enablefindbugs cobertura:cobertura
  • Executed mvn -P developer -pl developer -Ddeploydb
  • Executed mvn -P developer -pl developer -Ddeploydb-simulator
  • Built RPM packages using cloudstack-rpm-builder
  • Built DEB packages using cloudstack-deb-builder

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
    Testing
  • I have added tests to cover my changes.
  • All relevant new and existing integration tests have passed.
  • A full integration testsuite with all test that can run on my environment has passed.

@khos2ow khos2ow changed the title Clean POMs Cleanup POMs Apr 28, 2018
Copy link
Copy Markdown
Member

@marcaurele marcaurele left a comment

Choose a reason for hiding this comment

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

Good cleanup job!

Comment thread client/pom.xml Outdated
<extension>WixUIExtension</extension>
<extension>WixUtilExtension</extension>
</extensions>
<arguments>-dSourceClient=SourceDir\client -dS
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.

Isn't it going to fail the execution since the arguments are on multiple line now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a very good point, I haven't tested buildw profile!

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.

Isn't this the profile that was intended to build ACs for windows as an executable file?
I never understood this Wix project that we have.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe so. This is actually my first time trying to execute it.

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.

I would not even bother. CloudStack does not work on Windows. I mean, executing the management server on Windows (do not confuse with ACS using Hyper-V). I think this component was added to ACS in the dark ages when code was committed without much reviewing and thinking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, its' not even in here. I'll do some digging the history, then I might remove the whole thing.

Copy link
Copy Markdown
Contributor Author

@khos2ow khos2ow May 1, 2018

Choose a reason for hiding this comment

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

So the wix project and its workflow was last updated 4 years ago, and it's fully broken since Embedded Jetty commit (July 2017). although I couldn't manage to make it work after fixing that part of it.
Besides that, I wasn't able to find any guide anywhere that Cloudstack actually can be installed on Windows. (4.5, 4.6, 4.7, 4.8, 4.9, 4.11)
I'll send an email to ML, but I guess we can safely delete Wix-* and buildw altogether.

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.

This Wix project was already on my TODO list to remove it. If you look at the code base of ACS, you will notice that even though ACS is written in Java (therefore it seems it could be easily executed on Windows), there are quite a lot of dependencies on Linux operating system. Here there are some details of the port to windows that were left unfinished [1].

We have a protocol to execute the plugin/component retirement. You can read it here [2]. Thanks for picking that up!
[1] https://cwiki.apache.org/confluence/display/CLOUDSTACK/Cloudstack+Windowsfication
[2] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=68720798

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's great, although I think six months is very long for something which is already stalled. Specially that in most cases when retirement proposal happens, that piece of code must have been obsolete for months, if not years.

<dependency>
<groupId>org.apache.tomcat.embed</groupId>
<artifactId>tomcat-embed-core</artifactId>
<version>8.0.30</version>
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.

Can you move the version to the global pom into the properties section?

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented May 1, 2018

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@khos2ow 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-1988

Copy link
Copy Markdown
Contributor

@borisstoyanov borisstoyanov left a comment

Choose a reason for hiding this comment

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

LGTM. Good work @khos2ow

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented May 3, 2018

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@khos2ow 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-2003

@DaanHoogland
Copy link
Copy Markdown
Contributor

debian failed @khos2ow . not sure if it is helpful but here is the last few log lines of the job:

+ cat /jenkins/workspace/acs-pr-deb-pkg-builder/debian/changelog
+ mv /jenkins/workspace/acs-pr-deb-pkg-builder/newchangelog /jenkins/workspace/acs-pr-deb-pkg-builder/debian/changelog
+ cd /jenkins/workspace/acs-pr-deb-pkg-builder
+ dpkg-buildpackage -uc -us -b
dpkg-buildpackage: warning:     debian/changelog(l1): badly formatted heading line
LINE: cloudstack () unstable; urgency=low
dpkg-buildpackage: warning:     debian/changelog(l2): found blank line where expected first heading
dpkg-buildpackage: warning:     debian/changelog(l3): found change data where expected first heading
LINE:   * Update the version to 
Can't call method "as_string" on an undefined value at /usr/share/perl5/Dpkg/Changelog.pm line 249.
Build step 'Execute shell' marked build as failure
Stopping all containers
Finished: FAILURE

not sure if it should matter either ;)

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented May 4, 2018

Thanks @DaanHoogland, it is fixed already, I haven't pushed yet.

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented May 4, 2018

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@khos2ow 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-2011

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented May 4, 2018

@DaanHoogland it's still failing. Are you running customized command for building deb packages? I cannot reproduce this.

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented May 4, 2018

This PR should be good now. Call for reviewing: @rhtyd @rafaelweingartner @marcaurele @DaanHoogland @GabrielBrascher @wido @borisroman @swill

@DaanHoogland
Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

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

Copy link
Copy Markdown
Member

@rafaelweingartner rafaelweingartner left a comment

Choose a reason for hiding this comment

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

Great job.

@rafaelweingartner
Copy link
Copy Markdown
Member

@DaanHoogland and @rhtyd how do we merge this? I am inclined to accept it as soon as packaging is fine.
What do you guys think?

@blueorangutan
Copy link
Copy Markdown

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

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented May 8, 2018

Can any one who has access to packaging job, provide me some log to see why debian packaging fails? And also the command/script is used to trigger the job.
I cannot reproduce this myself.
/cc @DaanHoogland @rhtyd

@rafaelweingartner
Copy link
Copy Markdown
Member

I built the .deb using this PR here. Everything seems fine.

BTW: I used build-deb.sh

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented May 8, 2018

Thanks @rafaelweingartner for validating the packaging, that's why I'm guessing the way BO does the packaging is somehow broken/different/hacked, or simply does something which I haven't foreseen?

@yadvr
Copy link
Copy Markdown
Member

yadvr commented May 8, 2018

@khos2ow anyone can just rekick pkgs with bo. Sometimes jobs fail due to network, io or test related issue.
@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.

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented May 8, 2018

@rhtyd thanks for the explanation, but that would be a surprising coincidence and I would say unlikely, since both packaging job - after the debian fix - failed.

@blueorangutan
Copy link
Copy Markdown

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

@yadvr
Copy link
Copy Markdown
Member

yadvr commented May 9, 2018

@blueorangutan package

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jul 14, 2018

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

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

@borisstoyanov
Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

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

@rafaelweingartner
Copy link
Copy Markdown
Member

@khos2ow do you have an idea about why Jenkins is failing?

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented Jul 19, 2018

@rafaelweingartner hmm, not sure, let me just push an empty commit to kick it off again.

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented Jul 19, 2018

@rafaelweingartner it might be because of timeout issue of some sorts. previously PMD wasn't running for all the projects which this PR changes that, and it breaks at the end of the job while preparing PMD reports:

[PMD] Successfully parsed file /home/jenkins/jenkins-slave/workspace/cloudstack-pr-analysis/utils/target/pmd.xml with 137579 unique warnings and 127 duplicates.
<Git Blamer> Using GitBlamer to create author and commit information for all warnings.
<Git Blamer> GIT_COMMIT=40e8287b158cab4422df81946d570d5a4f74feea, workspace=/home/jenkins/jenkins-slave/workspace/cloudstack-pr-analysis
 > git rev-parse 40e8287b158cab4422df81946d570d5a4f74feea^{commit} # timeout=10
<Git Blamer> No blame results for request <plugins/network-elements/brocade-vcs/target/generated-sources/xjc3/com/cloud/network/schema/showvcs/ObjectFactory.java - [36]>.
<Git Blamer> No blame results for request <plugins/network-elements/brocade-vcs/target/generated-sources/xjc2/com/cloud/network/schema/portprofile/ObjectFactory.java - [36, 29]>.
<Git Blamer> No blame results for request <plugins/network-elements/brocade-vcs/target/generated-sources/xjc1/com/cloud/network/schema/interfacevlan/Vlan.java - [89, 75, 43, 61]>.
<Git Blamer> No blame results for request <plugins/network-elements/brocade-vcs/target/generated-sources/xjc2/com/cloud/network/schema/portprofile/PortProfile.java - [305, 69, 150, 278, 102, 198, 247, 329, 76, 126, 174]>.
<Git Blamer> No blame results for request <plugins/network-elements/brocade-vcs/target/generated-sources/xjc2/com/cloud/network/schema/portprofile/VlanProfile.java - [163, 260, 519, 233, 587, 139, 112, 337, 503, 312, 475, 412, 284]>.
<Git Blamer> No blame results for request <plugins/network-elements/brocade-vcs/target/generated-sources/xjc2/com/cloud/network/schema/portprofile/PortProfileGlobal.java - [67]>.
<Git Blamer> No blame results for request <plugins/network-elements/brocade-vcs/target/generated-sources/xjc1/com/cloud/network/schema/interfacevlan/Interface.java - [67]>.
<Git Blamer> No blame results for request <plugins/network-elements/brocade-vcs/target/generated-sources/xjc1/com/cloud/network/schema/interfacevlan/InterfaceVlan.java - [45, 67]>.
Agent went offline during the build
ERROR: Connection was broken: java.io.EOFException
        at java.io.ObjectInputStream$PeekInputStream.readFully(ObjectInputStream.java:2679)
        at java.io.ObjectInputStream$BlockDataInputStream.readShort(ObjectInputStream.java:3154)
        at java.io.ObjectInputStream.readStreamHeader(ObjectInputStream.java:862)
        at java.io.ObjectInputStream.<init>(ObjectInputStream.java:358)
        at hudson.remoting.ObjectInputStreamEx.<init>(ObjectInputStreamEx.java:48)
        at hudson.remoting.AbstractSynchronousByteArrayCommandTransport.read(AbstractSynchronousByteArrayCommandTransport.java:36)
        at hudson.remoting.SynchronousCommandTransport$ReaderThread.run(SynchronousCommandTransport.java:63)
Caused: java.io.IOException: Unexpected termination of the channel
        at hudson.remoting.SynchronousCommandTransport$ReaderThread.run(SynchronousCommandTransport.java:77)
Build step 'Publish PMD analysis results' marked build as failure
ERROR: Step ?Publish duplicate code analysis results? failed: no workspace for cloudstack-pr-analysis #7418
ERROR: Step ?Archive the artifacts? failed: no workspace for cloudstack-pr-analysis #7418
ERROR: Step ?Publish Cobertura Coverage Report? failed: no workspace for cloudstack-pr-analysis #7418
ERROR: Step ?Publish OpenClover coverage report? failed: no workspace for cloudstack-pr-analysis #7418
Putting comment on the pull request
ERROR: H31 is offline; cannot locate JDK 1.8 (latest)
ERROR: H31 is offline; cannot locate JDK 1.8 (latest)
Finished: FAILURE

The last <Git Blamer> took a long time (I didn't pay close attention, but something like 5 minutes I guess).

@rafaelweingartner
Copy link
Copy Markdown
Member

Well, I saw the message:

Agent went offline during the build
ERROR: Connection was broken: java.io.EOFException

However, if it was a job timeout, should we see a "job timeout" message?

@rafaelweingartner
Copy link
Copy Markdown
Member

BTW: Who has write access to the build "https://builds.apache.org/job/cloudstack-pr-analysis/" to check its timeout configurations?

@DaanHoogland, @wido, @swill, and @rhtyd

@DaanHoogland
Copy link
Copy Markdown
Contributor

I think I do, @rafaelweingartner . What is needed? remember there was a github webhook outage this week, btw.

@rafaelweingartner
Copy link
Copy Markdown
Member

@DaanHoogland we need to check the timeout configuration. And then, if it is not enough for us, we need to increase it.

BTW: can you give me permission to see these configuration at Jenkins as well?

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented Jul 24, 2018

@rafaelweingartner according to this, the crash happened most probably because of running out of memory and not timeout.
This is happening now, as I mentioned, because this PR enables PMD on all projects and there are huge number of problems it can find.

@marcaurele
Copy link
Copy Markdown
Member

@khos2ow would you mind moving the PMD change to another PR to help getting this one in?

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented Jul 25, 2018

@marcaurele done, let's see how much Jenkins likes this change!

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented Jul 25, 2018

@rhtyd @rafaelweingartner @marcaurele @borisstoyanov @DaanHoogland this PR should be good to be merged, I also added TODO and followups section in the description for followup PRs.

@rafaelweingartner
Copy link
Copy Markdown
Member

@khos2ow what about creating issues for those TODOs?

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented Jul 25, 2018

I was going to create those issues after this got merged and those actually became issues.

@khos2ow
Copy link
Copy Markdown
Contributor Author

khos2ow commented Jul 25, 2018

@rafaelweingartner corresponding issues have been created.

@rafaelweingartner
Copy link
Copy Markdown
Member

Danke!

@khos2ow khos2ow mentioned this pull request Aug 20, 2018
12 tasks
Sign up for free to 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