-
Notifications
You must be signed in to change notification settings - Fork 504
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
Conversation
… its built on (to allow for dev on osx). Force files to be owned by jmxtrans user
… testing on travis
@gehel Looks like pull requests are having issues with the GPG keys. On my local fork, I tried disabling the gpg profile (via |
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... |
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> |
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.
We are actually still supporting Java 1.6.
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.
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
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.
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!)
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.
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).
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.
If you put a dependency on Java >= 1.7 in the packages, can you also update the configuration of the maven-compiler-plugin?
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.
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. |
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. |
@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. |
@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. |
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. |
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). |
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 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? |
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! |
OK great. I moved the debian images in to a separate profile so that they can be run manually if needed. |
@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? |
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... |
Thanks for granting me access!
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 |
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... |
@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. |
Great job! If we ever meet, beer is on me! |
Verify RPM/Deb Behavior with Docker Tests
👍 Awesome work !!! |
And if you want to keep improving jmxtrans, feel free! It looks like we need your contributions! |
@kerlandsson His chocolate is better than the beer... go for that instead. ;-) |
Damn, my secret weapon is now revealed... |
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: I've also raised the issue with the docker-maven-plugin to see if this should truly be a failure condition: Re-running the build makes it go away. I'll see if I can push this along on the docker-maven-plugin side. |
I just restarted the build. Let's see if it fails again or not... |
Build is green again... |
@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.

On my local fork, I tried disabling the gpg profile (via
-P !gpg...
) but the rpm-maven-plugin looks at thegpg.passphraseServerId
property and tries to load the gpg key.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.
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.
Let me know if there is something specific on which I can help you ... I'll do my best.
I've asked the docker-maven-plugin maintainer to release a new version. That should come out in a few days, when it does, I will update the version and remove the plugin repo
Again, thanks a lot for the work! That's really going to help!
Yes, definitely!
On a different topic, what are your thoughts on adding a travis envvar to control whether these tests run or not? That way, if something goes wrong later down the line they can be skipped by modifying the travis envvar, rather than changing the code.
In that case, I would modify the mvn invocation to include something like -Ddocker.skip=false which could be made true as necessary. Is that what you had in mind?
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?
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?
It seems like its an open issue with docker:
Cannot stop container XXXXXXXXXXXX: [2] Container does not exist: container destroyed moby/moby#12738
I've also raised the issue with the docker-maven-plugin to see if this should truly be a failure condition:
Build Fails When Docker Container Cannot be Stopped 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.
JCommanderArgumentParser
library has a java 1.7 dependency though. I saw the following error when I ran with jdk6:/etc/init.d/jmxtrans start
should probably not bee required.chkconfig --add
which doesn't start the service. The man page seems to indicate that--add
just registers the service with chkconfig but does not start it.