Opened 5 years ago

Closed 5 years ago

#12881 closed enhancement (implemented)

Set Java version to 7

Reported by: iwakeh Owned by:
Priority: Medium Milestone:
Component: Metrics/Onionoo Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Especially in order to avoid test related code in the production environment.
(see #12869 about util.Time).

I currently cannot point to an exploit or similar, but having something
(non-static) standing in between java.lang.System.millisec and the application
does not feel right.

So, a Mocking Framework might be a way to solve this. The framework should
be compatible with the Guidelines.

(I could look into it. So, I have a reason to study the Guidelines thoroughly.)

Child Tickets

Change History (12)

comment:1 Changed 5 years ago by iwakeh

Wheezy main stable contains four java mocking frameworks:

  • jmock
  • mockito
  • mockobjects
  • easymock

Now the one most suited to onionoo needs to be identified.
Unfortunately, we need to mock System.millisec which will need non-wheezy extensions.

Last edited 5 years ago by iwakeh (previous) (diff)

comment:2 Changed 5 years ago by iwakeh

When introducing a third party dependency anyway, jmockit 1.10 would be the way to go.

Example for setting the time (e.g. in UptimeDocumentWriterTest with its TEST_TIME) inside a test method:

    @Test
    public void testMockTime() {
        long justNow = System.currentTimeMillis();


        new MockUp<System>() {
            @Mock
            public long currentTimeMillis() {
                return TEST_TIME;
            }
        };

        assertTrue( (new Date(System.currentTimeMillis())).before(new Date(justNow)));
    }

That would save several test classes and the application code for the util.Time class.

The frameworks mentioned in comment 1 are quite ancient and cannot mock system classes
without extensions e.g. like PowerMock for easymock.

Would it be that troublesome to use a non-debian lib for testing?
Secondly, the java version for onionoo should be set to 1.6 or bigger.
Does that pose a problem?

Last edited 5 years ago by iwakeh (previous) (diff)

comment:3 Changed 5 years ago by karsten

Setting the Java version to 1.6 is not a problem. Should I do that, regardless of the mocking framework?

But requiring a library that is not in Debian stable is a problem, because it doesn't play well with the development guidelines. It seems that sticking with the Time class is the lesser pain.

comment:4 Changed 5 years ago by iwakeh

Yeah, you're right. The mocking frameworks are quite old in debian,
and other than for Time it is not that necessary right now.
Wait for better versions.

I really would advocate for java 7.
Would that be fine with you?

comment:5 Changed 5 years ago by karsten

Summary: Mocking framework for testingSet Java version to 7

Okay, let's revisit this when the next Debian stable is out.

Java 7 is fine with me. This is blocking on #13002, but once that ticket is closed, I'm happy to switch. Updating the summary.

comment:6 Changed 5 years ago by karsten

I'm having trouble upgrading the Vagrant environment to Java 7. Here's what I did, based on current master (a6abbba):

diff --git a/build.xml b/build.xml
index 0d04892..657a37f 100644
--- a/build.xml
+++ b/build.xml
@@ -38,8 +38,8 @@
           depends="metrics-lib, init">
     <javac destdir="${classes}"
            srcdir="${javasources}"
-           source="1.5"
-           target="1.5"
+           source="1.7"
+           target="1.7"
            debug="true"
            deprecation="true"
            optimize="false"
@@ -51,8 +51,8 @@
   <target name="test" depends="compile">
     <javac destdir="${classes}"
            srcdir="${tests}"
-           source="1.5"
-           target="1.5"
+           source="1.7"
+           target="1.7"
            debug="true"
            deprecation="true"
            optimize="false"
diff --git a/vagrant/bootstrap.sh b/vagrant/bootstrap.sh
index f3f0118..4659bc5 100644
--- a/vagrant/bootstrap.sh
+++ b/vagrant/bootstrap.sh
@@ -9,7 +9,10 @@ apt-get -y upgrade
 echo "Installing required packages."
 apt-get install -y libcommons-codec-java libcommons-compress-java \
 libcommons-lang-java libgoogle-gson-java junit4 libservlet3.0-java \
-openjdk-6-jdk ant liblog4j1.2-java tomcat6
+openjdk-7-jdk ant liblog4j1.2-java tomcat6
+
+echo "Switching to Java 7."
+update-java-alternatives -s java-1.7.0-openjdk-amd64
 
 echo "Setting up paths and creating symbolic links."
 mkdir -p /srv/onionoo.torproject.org/onionoo/

Here are Tomcat's logs after deploying onionoo.war:

Sep 4, 2014 1:37:05 AM org.apache.catalina.startup.HostConfig deployWAR
INFO: Deploying web application archive onionoo.war
Sep 4, 2014 1:37:05 AM org.apache.tomcat.util.modeler.Registry registerComponent
SEVERE: Null component Catalina:type=JspMonitor,name=jsp,WebModule=//localhost/onionoo,J2EEApplication=none,J2EEServer=none
Sep 4, 2014 1:37:05 AM org.apache.catalina.startup.HostConfig deployWAR
SEVERE: Error deploying web application archive onionoo.war
java.lang.UnsupportedClassVersionError: org/torproject/onionoo/server/NodeIndexer : Unsupported major.minor version 51.0 (unable to load class org.torproject.onionoo.server.NodeIndexer)
        at org.apache.catalina.loader.WebappClassLoader.findClassInternal(WebappClassLoader.java:2822)
        at org.apache.catalina.loader.WebappClassLoader.findClass(WebappClassLoader.java:1159)
        at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1647)
        at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1526)
        at org.apache.catalina.startup.WebAnnotationSet.loadClassAnnotation(WebAnnotationSet.java:145)
        at org.apache.catalina.startup.WebAnnotationSet.loadApplicationListenerAnnotations(WebAnnotationSet.java:73)
        at org.apache.catalina.startup.WebAnnotationSet.loadApplicationAnnotations(WebAnnotationSet.java:56)
        at org.apache.catalina.startup.ContextConfig.applicationAnnotationsConfig(ContextConfig.java:297)
        at org.apache.catalina.startup.ContextConfig.start(ContextConfig.java:1078)
        at org.apache.catalina.startup.ContextConfig.lifecycleEvent(ContextConfig.java:261)
        at org.apache.catalina.util.LifecycleSupport.fireLifecycleEvent(LifecycleSupport.java:142)
        at org.apache.catalina.core.StandardContext.start(StandardContext.java:4612)
        at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:799)
        at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:779)
        at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:601)
        at org.apache.catalina.startup.HostConfig.deployWAR(HostConfig.java:943)
        at org.apache.catalina.startup.HostConfig.deployWARs(HostConfig.java:778)
        at org.apache.catalina.startup.HostConfig.deployApps(HostConfig.java:504)
        at org.apache.catalina.startup.HostConfig.check(HostConfig.java:1385)
        at org.apache.catalina.startup.HostConfig.lifecycleEvent(HostConfig.java:306)
        at org.apache.catalina.util.LifecycleSupport.fireLifecycleEvent(LifecycleSupport.java:142)
        at org.apache.catalina.core.ContainerBase.backgroundProcess(ContainerBase.java:1389)
        at org.apache.catalina.core.ContainerBase$ContainerBackgroundProcessor.processChildren(ContainerBase.java:1653)
        at org.apache.catalina.core.ContainerBase$ContainerBackgroundProcessor.processChildren(ContainerBase.java:1662)
        at org.apache.catalina.core.ContainerBase$ContainerBackgroundProcessor.run(ContainerBase.java:1642)
        at java.lang.Thread.run(Thread.java:701)

Restarting Tomcat seems to fix this problem, but doing that in the bootstrap script is rather ugly.

Any idea how we could fix this properly?

comment:7 Changed 5 years ago by iwakeh

Using the apt-get install order above, starts tomcat using java6.
After that java 7, becomes default; thus, you have to restart tomcat in order to fix things.

I didn't verify the following (since I have a jdk7 virtual box for using vagrant).
I would purge jdk6 first:

apt-get purge -y openjdk-6-jdk default-jre
apt-get install -y openjdk-7-jdk ant \
    libcommons-codec-java libcommons-compress-java \
    libcommons-lang-java libgoogle-gson-java junit4  \
    libservlet3.0-java tomcat6

That should make jdk7 default even without the update alternatives line.
Hope this works!

PS:
If you want to keep jdk6, it might also work to place the tomcat install after
the update-alternatives.

Last edited 5 years ago by iwakeh (previous) (diff)

comment:8 Changed 5 years ago by karsten

Oh well, no luck. Here's what I did:

diff --git a/vagrant/bootstrap.sh b/vagrant/bootstrap.sh
index d4b1724..cc7ab57 100644
--- a/vagrant/bootstrap.sh
+++ b/vagrant/bootstrap.sh
@@ -6,10 +6,19 @@ echo "Updating package list and upgrading existing packages."
 apt-get update
 apt-get -y upgrade
 
+echo "Removing Java 6 packages."
+apt-get purge -y openjdk-6-jdk default-jre
+
 echo "Installing required packages."
 apt-get install -y libcommons-codec-java libcommons-compress-java \
 libcommons-lang-java libgoogle-gson-java junit4 libservlet3.0-java \
-openjdk-6-jdk ant tomcat6 libslf4j-java liblogback-java
+openjdk-7-jdk ant tomcat6 libslf4j-java liblogback-java
+
+echo "Switching to Java 7."
+#update-java-alternatives -s java-1.7.0-openjdk-amd64
+
+echo "Restarting Tomcat, so that it runs with Java 7."
+#service tomcat6 restart
 
 echo "Setting up paths and creating symbolic links."
 mkdir -p /srv/onionoo.torproject.org/onionoo/

Still, I get:

vagrant@vagrant:~$ java -version
java version "1.6.0_32"
OpenJDK Runtime Environment (IcedTea6 1.13.4) (6b32-1.13.4-1~deb7u1)
OpenJDK 64-Bit Server VM (build 23.25-b01, mixed mode)
vagrant@vagrant:~$ javac -version
javac 1.7.0_65

vagrant@vagrant:~$ dpkg -l | egrep "jdk|jre"
ii  default-jre-headless                1:1.6-47                      amd64        Standard Java or Java compatible Runtime (headless)
ii  icedtea-6-jre-cacao:amd64           6b32-1.13.4-1~deb7u1          amd64        Alternative JVM for OpenJDK, using Cacao
ii  icedtea-6-jre-jamvm:amd64           6b32-1.13.4-1~deb7u1          amd64        Alternative JVM for OpenJDK, using JamVM
ii  openjdk-6-jre-headless:amd64        6b32-1.13.4-1~deb7u1          amd64        OpenJDK Java runtime, using Hotspot JIT (headless)
ii  openjdk-6-jre-lib                   6b32-1.13.4-1~deb7u1          all          OpenJDK Java runtime (architecture independent libraries)
ii  openjdk-7-jdk:amd64                 7u65-2.5.1-5~deb7u1           amd64        OpenJDK Development Kit (JDK)
ii  openjdk-7-jre:amd64                 7u65-2.5.1-5~deb7u1           amd64        OpenJDK Java runtime, using Hotspot JIT
ii  openjdk-7-jre-headless:amd64        7u65-2.5.1-5~deb7u1           amd64        OpenJDK Java runtime, using Hotspot JIT (headless)

vagrant@vagrant:/srv/onionoo.torproject.org/onionoo$ ant war
Unable to locate tools.jar. Expected to find it in /usr/lib/jvm/java-6-openjdk-amd64/lib/tools.jar
[...]

But let's try something else: what virtual box are you using? Ideally, we should use the same development environment, so that we don't have to fight problems like this. And ideally, the development environment is as close to the production environment as possible, which means Debian stable. So, if you're using a different Debian stable image, I'd like to try that. And if you're using a different system, please consider switching to Debian stable.

comment:9 Changed 5 years ago by iwakeh

The installation order makes the difference:
jdk7 must be there first,
otherwise junit4 or ant draw the default jre/jdk 6 again.
Might be even necessary to have separate apt-get lines to force the
installation order.

apt-get purge -y openjdk-6-jdk default-jre
apt-get install -y openjdk-7-jdk 
apt-get install -y  ant \
    libcommons-codec-java libcommons-compress-java \
    libcommons-lang-java libgoogle-gson-java junit4  \
    libservlet3.0-java tomcat6

I do use the virtual box you suggested. In order to save time I updated it
and installed java7 and now use this local vbox-image for new vagrant machines.
So, only the updates are polled every time I setup a new machine.
This might resemble the production environment even closer?

Last edited 5 years ago by iwakeh (previous) (diff)

comment:10 Changed 5 years ago by karsten

Status: newneeds_review

I didn't try changing the package order, but two separate apt-get lines did the trick! Finally.

Regarding using a local vbox image, I think the preferred way is to start with the exact same image as other developers. All changes to that image should be written to the provisioning script, so that everyone ends up with the same development environment. Imagine another contributor comes along and wants to write a patch; all we want to tell them is vagrant up and write the patch.

Can you try out the provisioning script in my task-12881 branch? That is, can you vagrant halt && vagrant destroy your current image and start from scratch? (If you want to keep your /srv/onionoo.torproject.org/onionoo/[out|status] directories, be sure to copy them to /vagrant/ first. Thanks!

comment:11 Changed 5 years ago by iwakeh

Yeah, java 7, finally :-)

I verified the vagrant installation (from the task-12881 branch), and everything works
running on java 7 without any manual intervention.

Using vagrant is really nice for testing, still, i have some concerns
regarding the setup (Potential new contributors might have similar concerns.):
Why replay the debian setup again and again?
The current third party image is not tailored to Onionoo/Tor, is it?
Why does Onionoo need the puppet packages? Is puppetlabs.com in any affiliated with Tor?

I would find it really helpful to download an Onionoo test-vbox from
the Tor project, readily equipped with everything that's needed. And,
with some way of verifying that it is 'kosher'.
This would enable everyone to test with the same machine.

You have such a standard box anyway, it should be easy to make it available.
Updates could be announced in the client development mailing list.

(Actually, clients should use such a box for testing there clients before they
run against the Onionoo server?)

comment:12 Changed 5 years ago by karsten

Resolution: implemented
Status: needs_reviewclosed

Awesome, merged to master!

Regarding the base machine for Vagrant, I think it's overkill to maintain an image that people use for Onionoo development. And using a standard machine has the advantage that people don't have to trust us that we're setting up things right, but they can trust an image used by hundreds or thousands of other people. In theory, provisioning should only be done once, and whenever we change dependencies, so maybe half a dozen times per year.

Why Puppet Labs? Because I started using Puppet to provision the machine and then switched to shell scripts for simplicity. We can switch to another image, for example, one that is used by more people, or smaller, or in another way better. But let's discuss that in a separate ticket if you want.

Closing this ticket, because we now have Java 7! Thanks! :)

Note: See TracTickets for help on using tickets.