Skip to content
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

Verify RPM/Deb Behavior with Docker Tests #423

Merged
merged 26 commits into from
Mar 2, 2016

Conversation

kevinconaway
Copy link
Contributor

@gehel this PR adds a new maven module that spins up docker images for various linux distributions to test the RPM & deb packages.

It is not complete yet but I'm unable to test it in my fork due to the missing GPG information. I'm hoping that creating the PR here will successfully run the build with your travis config.

I'll update the PR when the testing is complete.

Exception in thread "main" java.lang.NoClassDefFoundError: java/nio/file/Path
at com.beust.jcommander.internal.DefaultConverterFactory.<clinit>(DefaultConverterFactory.java:66)
at com.beust.jcommander.JCommander.<clinit>(JCommander.java:167)
at com.googlecode.jmxtrans.cli.JCommanderArgumentParser.parseOptions(JCommanderArgumentParser.java:35)
at com.googlecode.jmxtrans.JmxTransformer.main(JmxTransformer.java:118)
Caused by: java.lang.ClassNotFoundException: java.nio.file.Path
at java.net.URLClassLoader$1.run(URLClassLoader.java:217)
at java.security.AccessController.doPrivileged(Native Method)
at java.net.URLClassLoader.findClass(URLClassLoader.java:205)
at java.lang.ClassLoader.loadClass(ClassLoader.java:323)
at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:294)
at java.lang.ClassLoader.loadClass(ClassLoader.java:268)
... 4 more
Exception in thread "main" java.lang.NoClassDefFoundError: java/nio/file/Path
at com.beust.jcommander.internal.DefaultConverterFactory.<clinit>(DefaultConverterFactory.java:66)
at com.beust.jcommander.JCommander.<clinit>(JCommander.java:167)
at com.googlecode.jmxtrans.cli.JCommanderArgumentParser.parseOptions(JCommanderArgumentParser.java:35)
at com.googlecode.jmxtrans.JmxTransformer.main(JmxTransformer.java:118)
Caused by: java.lang.ClassNotFoundException: java.nio.file.Path
at java.net.URLClassLoader$1.run(URLClassLoader.java:217)
at java.security.AccessController.doPrivileged(Native Method)
at java.net.URLClassLoader.findClass(URLClassLoader.java:205)
at java.lang.ClassLoader.loadClass(ClassLoader.java:323)
at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:294)
at java.lang.ClassLoader.loadClass(ClassLoader.java:268)
... 4 more

  

@kevinconaway
Copy link
Contributor Author

@gehel Looks like pull requests are having issues with the GPG keys.

On my local fork, I tried disabling the gpg profile (via -P !gpg...) but the rpm-maven-plugin looks at the gpg.passphraseServerId property and tries to load the gpg key.

@gehel
Copy link
Member

gehel commented Feb 12, 2016

This looks really interesting! I have a few comments, but I need some more time than I have right now to do your PR justice. I'll get back to you as soon as I can...

@kevinconaway
Copy link
Contributor Author

Thanks. I'm experimenting now with the travis config to see how I can get pull requests to run without signing the RPM.

@@ -354,8 +350,7 @@
fi</script>
</preremoveScriptlet>
<requires>
<require>java</require>
<require>/usr/bin/lsb_release</require>
<require>java >= 1.7</require>
Copy link
Member

Choose a reason for hiding this comment

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

We are actually still supporting Java 1.6.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like the JCommanderArgumentParser library has a java 1.7 dependency though. I saw the following error when I ran with jdk6:

Exception in thread "main" java.lang.NoClassDefFoundError: java/nio/file/Path
    at com.beust.jcommander.internal.DefaultConverterFactory.<clinit>(DefaultConverterFactory.java:66)
    at com.beust.jcommander.JCommander.<clinit>(JCommander.java:167)
    at com.googlecode.jmxtrans.cli.JCommanderArgumentParser.parseOptions(JCommanderArgumentParser.java:35)
    at com.googlecode.jmxtrans.JmxTransformer.main(JmxTransformer.java:118)
Caused by: java.lang.ClassNotFoundException: java.nio.file.Path
    at java.net.URLClassLoader$1.run(URLClassLoader.java:217)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:205)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:323)
    at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:294)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:268)
    ... 4 more
Exception in thread "main" java.lang.NoClassDefFoundError: java/nio/file/Path
    at com.beust.jcommander.internal.DefaultConverterFactory.<clinit>(DefaultConverterFactory.java:66)
    at com.beust.jcommander.JCommander.<clinit>(JCommander.java:167)
    at com.googlecode.jmxtrans.cli.JCommanderArgumentParser.parseOptions(JCommanderArgumentParser.java:35)
    at com.googlecode.jmxtrans.JmxTransformer.main(JmxTransformer.java:118)
Caused by: java.lang.ClassNotFoundException: java.nio.file.Path
    at java.net.URLClassLoader$1.run(URLClassLoader.java:217)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:205)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:323)
    at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:294)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:268)
    ... 4 more

Copy link
Member

Choose a reason for hiding this comment

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

Damn, you might well be right. Maybe it is time to drop 1.6 support. Oracle stopped public support in 2013, so it might be reasonable to upgrade to 1.7 (and try-with-resources at last!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. The jdk7 requirement is why I didn't add images for older debian/centos releases that didn't have OOTB packages for jdk7 (debian squeeze for example).

Copy link
Member

Choose a reason for hiding this comment

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

If you put a dependency on Java >= 1.7 in the packages, can you also update the configuration of the maven-compiler-plugin?

Copy link
Member

Choose a reason for hiding this comment

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

👍

@gehel
Copy link
Member

gehel commented Feb 13, 2016

Looking at the travis build, it seems that the RPM builds just fine, but there is a requirement to update Maven to more recent version than what is on Travis. Should be doable...

@kevinconaway
Copy link
Contributor Author

Looking at the travis build, it seems that the RPM builds just fine, but there is a requirement to update Maven to more recent version than what is on Travis. Should be doable...

Yes, I'm trying to get this sorted out.

The docker support on Travis is in beta and thus is using the beta Trusty environment which is only using maven 3.1.1. There is an open issue regarding the maven version so I'm hoping this gets addressed soon.

Separately, there is an open issue with the docker-maven-plugin regarding the maven version prerequisite. I'm hoping that the hard requirement for 3.2.x isn't necessary.

@gehel
Copy link
Member

gehel commented Feb 13, 2016

It should not be too hard to download and install Maven 3.3 at the start of the build and use it instead of the version coming by default on Travis. Ugly, yes, but probably working.

@kevinconaway
Copy link
Contributor Author

It should not be too hard to download and install Maven 3.3 at the start of the build and use it instead of the version coming by default on Travis. Ugly, yes, but probably working.

Glad to know thats an option. I'm going to see what progress I can make on the other 2 fronts first before approaching this one.

@kevinconaway
Copy link
Contributor Author

@gehel I'm still working on this. I've fixed the above issues but it seems that docker on travis is quite unstable, the build often fails with timeouts while the docker images are being built. I'm brainstorming on ways around that.

@gehel
Copy link
Member

gehel commented Feb 19, 2016

@kevinconaway thanks a lot for what you are doing! Just thinking about it is already an achievement! Sorry I'm not of more help, I just started a new job and I'm fairly busy at the moment. I really want to have a deeper look in what you've been doing, but it's probably going to take some time.

Let me know if there is something specific on which I can help you ... I'll do my best.

Kevin Conaway added 5 commits February 22, 2016 10:34
@kevinconaway
Copy link
Contributor Author

Build is green now and I've been running it continuously all day on my local branch. I'm not sure if that the flakiness is resolved or if today is just a good day for travis.

@gehel
Copy link
Member

gehel commented Feb 23, 2016

That sounds like something reasonable. We probably want to have it disabled by default (so that people trying to build jmxtrans locally and who do not have docker installed can build it without issue).

@kevinconaway
Copy link
Contributor Author

I'm seeing some intermittent issues with the Debian images. The apt-get updates when installing the JRE will occasionally fail. This seems to be a known issue with the debian mirrors:

https://lists.debian.org/debian-devel/2015/07/msg00132.html
http://stackoverflow.com/questions/32304631/docker-debian-apt-error-reading-from-server
http://serverfault.com/questions/690639/api-get-error-reading-from-server-under-docker

Do you think we'll get adequate coverage of installing the .deb with just testing on Ubuntu? If so, we can drop the debian images all together. Other than that, it sounds like we'll need to either use a specific mirror for pulling apt-get artifacts (not a great solution IMO) or add some basic retry logic to the apt-get update / install commands (also not that great)

Thoughts?

@gehel
Copy link
Member

gehel commented Feb 24, 2016

Debian and Ubuntu should be close enough so that testing one is sufficient. More than that, having unstable tests is the best way to learn to ignore their results. And in any case, testing just one distro will already be so much better than the no tests at all that we have at the moment!

Kevin Conaway added 2 commits February 24, 2016 15:08
… won't run by default

(cherry picked from commit 050b18a)
(cherry picked from commit b6d64a9)
@kevinconaway
Copy link
Contributor Author

OK great. I moved the debian images in to a separate profile so that they can be run manually if needed.

@kevinconaway
Copy link
Contributor Author

@gehel The build is still taking quite a while on Travis (in some cases too long which causes it to hit the hard time limit and gets killed).

I think the issue is that I'm building images from the base linux images and then running jmxtrans. It takes quite a while to pull down the JDK with all of its dependencies.

I think I can speed this up by pre-creating docker images with the JDK already installed and using that instead.

I see that there is already a jmxtrans public repo on docker hub: https://hub.docker.com/r/jmxtrans/jmxtrans/

Are you the owner for this? If so, will you grant me write access so that I can push images?

@gehel
Copy link
Member

gehel commented Feb 25, 2016

You might be able to use the official java images (https://hub.docker.com/_/java/). You are now part of the jmxtrans org on docker hub. Feel free to upload images if that make sense...

@kevinconaway
Copy link
Contributor Author

Thanks for granting me access!

You might be able to use the official java images

We can't use the official images because we need OS specific images (centos7, centos6.6, ubuntu:wheezy etc) with java on them.

The official java images appear to all use some ubuntu / debian version

Kevin Conaway added 2 commits February 25, 2016 12:54
@gehel
Copy link
Member

gehel commented Feb 25, 2016

They seem to be based on Jessie, so we might use them to test debian packages. But yes, if we want to test a large number of distro, we might need something more...

Kevin Conaway added 2 commits February 29, 2016 09:56
@kevinconaway
Copy link
Contributor Author

@gehel I'm happy with the current state of the PR. Creating specific images for each target OS really helped speed up the build and improve its stability on travis.

@gehel
Copy link
Member

gehel commented Mar 2, 2016

Great job! If we ever meet, beer is on me!

gehel added a commit that referenced this pull request Mar 2, 2016
Verify RPM/Deb Behavior with Docker Tests
@gehel gehel merged commit 7e7dbac into jmxtrans:master Mar 2, 2016
@utkarshcmu
Copy link
Contributor

👍 Awesome work !!!

@gehel
Copy link
Member

gehel commented Mar 2, 2016

And if you want to keep improving jmxtrans, feel free! It looks like we need your contributions!

@lookfirst
Copy link
Member

@kerlandsson His chocolate is better than the beer... go for that instead. ;-)

@gehel
Copy link
Member

gehel commented Mar 2, 2016

Damn, my secret weapon is now revealed...

@kevinconaway
Copy link
Contributor Author

All that praise and now the build breaks. This is the issue that I mentioned earlier where the docker container occasionally fails to stop.

It seems like its an open issue with docker:
moby/moby#12738

I've also raised the issue with the docker-maven-plugin to see if this should truly be a failure condition:
fabric8io/docker-maven-plugin#391

Re-running the build makes it go away. I'll see if I can push this along on the docker-maven-plugin side.

@gehel
Copy link
Member

gehel commented Mar 3, 2016

I just restarted the build. Let's see if it fails again or not...

@gehel
Copy link
Member

gehel commented Mar 3, 2016

Build is green again...

@kevinconaway kevinconaway deleted the issue-417 branch March 3, 2016 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants