RFR: 8221221: [testbug] make test TEST="jtreg:test/hotspot/jtreg/runtime/containers" failed on Ubuntu 18.04

[Severin Gehwolf]

Hi,

On Fri, 2019-03-22 at 11:26 +0100, Severin Gehwolf wrote:
> Hi Misha,
> 
> On Thu, 2019-03-21 at 15:37 -0700, mikhailo.seledtsov at oracle.com wrote:
> > Hi Jie,
> > 
> >      In general I agree its time to make the docker file in HotSpot 
> > tests more dynamic. When the tests were originally created, there was no 
> > need to make them dynamic, and possibly dockerfile did not support the 
> > build args (do not remember for sure).
> > 
> > I was the original author of runtime Docker tests, and would like to 
> > provide additional suggestion. I like the idea of using the build args 
> > in the docker file. In general in hotspot testing we usually use 
> > properties in such situations to parameterize the tests. I propose the 
> > following properties:
> > 
> > - jdk.test.docker.image.name for image name
> > 
> > - jdk.test.docker.image.version for image version
> > 
> > 
> > Here are the proposed prototype code changes to implement this:
> > ======================================
> > --- a/test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest
> > +++ b/test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest
> > @@ -1,4 +1,7 @@
> > -FROM oraclelinux:7.6
> > +ARG IMAGE_NAME=oraclelinux
> > +ARG IMAGE_VERSION=7.6
> > +
> > +FROM $IMAGE_NAME:$IMAGE_VERSION
> > 
> > 
> > --- a/test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java
> > +++ b/test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java
> > @@ -46,6 +46,10 @@
> >       private static boolean isDockerEngineAvailable = false;
> >       private static boolean wasDockerEngineChecked = false;
> > 
> > +    // Image name and version
> > +    private static final String IMAGE_NAME = 
> > System.getProperty("jdk.test.docker.image.name", "oraclelinux");
> > +    private static final String IMAGE_VERSION = 
> > System.getProperty("jdk.test.docker.image.version", "7.6");
> > +
> >       // Diagnostics: set to true to enable more diagnostic info
> >       private static final boolean DEBUG = false;
> > 
> > @@ -162,7 +166,10 @@
> >           Files.copy(dockerfile, buildDir.resolve("Dockerfile"));
> > 
> >           // Build the docker
> > -        execute("docker", "build", "--no-cache", "--tag", imageName, 
> > buildDir.toString())
> > +        execute("docker", "build", "--no-cache",
> > +                "--build-arg", "IMAGE_NAME=" + IMAGE_NAME,
> > +                "--build-arg", "IMAGE_VERSION=" + IMAGE_VERSION,
> > +                "--tag", imageName, buildDir.toString())
> >               .shouldHaveExitValue(0)
> >               .shouldContain("Successfully built");
> >       }
> > 
> > ======================================
> > 
> > Please let me know if this works for you, and if you have any questions 
> > or comments.
> 
> This looks reasonable. I'll test it and will let you know.

This doesn't seem to work for me as ARG before FROM seems to require a
docker version which is likely too new (v17.06) for most users to be
prohibitive. But since the docker tests more or less preprocess the
Dockerfile already we might be able to work around it by generating the
Dockerfile on the fly and get rid of Dockerfile-BasicTest-<OS-ARCH>
versions. The idea with the properties would be the same.

Thanks,
Severin

>  One comment:
> 
> Some more files will need similar treatment:
> 
> $ find test -type f -name Dockerfile-BasicTest\*
> test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest-ppc64le
> test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest-s390x
> test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest-aarch64
> test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest
> test/jdk/jdk/internal/platform/docker/Dockerfile-BasicTest-ppc64le
> test/jdk/jdk/internal/platform/docker/Dockerfile-BasicTest-s390x
> test/jdk/jdk/internal/platform/docker/Dockerfile-BasicTest-aarch64
> test/jdk/jdk/internal/platform/docker/Dockerfile-BasicTest
> 
> Thanks,
> Severin
> 
> 
> > Best regards,
> > Misha
> > 
> > 
> > On 3/21/19 2:50 AM, Jie Fu wrote:
> > > Good suggestions!
> > > 
> > > Thanks Severin.
> > > 
> > > 
> > > On 2019/3/21 下午4:57, Severin Gehwolf wrote:
> > > > Hi,
> > > > 
> > > > On Thu, 2019-03-21 at 16:23 +0800, Jie Fu wrote:
> > > > > Hi all,
> > > > > 
> > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8221221
> > > > > 
> > > > > make test TEST="jtreg:test/hotspot/jtreg/runtime/containers" failed on
> > > > > Ubuntu 18.04.
> > > > > 
> > > > > The issue can be fixed by
> > > > > -----------------------------------------------
> > > > > diff -r ce78fac1f6d5
> > > > > test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest
> > > > > --- a/test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest
> > > > > Thu Mar 21 01:49:27 2019 +0100
> > > > > +++ b/test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest
> > > > > Thu Mar 21 15:44:39 2019 +0800
> > > > > @@ -1,4 +1,4 @@
> > > > > -FROM oraclelinux:7.6
> > > > > +FROM fedora:29
> > > > >    MAINTAINER mikhailo.seledtsov at oracle.com
> > > > > 
> > > > >    COPY /jdk /jdk
> > > > > -----------------------------------------------
> > > > > 
> > > > > The change has been tested with
> > > > > -----------------------------------------------
> > > > > make test TEST="test/jdk/jdk/modules/scenarios/container
> > > > > test/hotspot/jtreg/vmTestbase/vm/gc/containers
> > > > > test/hotspot/jtreg/runtime/containers"
> > > > > -----------------------------------------------
> > > > > on Linux/x86-64 (Ubuntu 18.04).
> > > > > 
> > > > > Could you please review it?
> > > > > Thanks a lot.
> > > > > 
> > > > > Best regards,
> > > > > Jie
> > > > This fix will be inadequate as the expectation is to have the
> > > > (reasonably?) same base image as the build platform. If you had built
> > > > the JDK on say, centos 7, then the test would likely work.
> > > > 
> > > > So if anything, I agree with David who commented on the bug, there
> > > > should be a way to set the to-be-used base image via some option in the
> > > > test using the current value as a default.
> > > > 
> > > > Here is what AdoptOpenJDK does:
> > > > 
> > > > ARG IMAGE_NAME=adoptopenjdk/openjdk8
> > > > ARG IMAGE_VERSION=latest
> > > > 
> > > > FROM $IMAGE_NAME:$IMAGE_VERSION
> > > > 
> > > > Example usage:
> > > > docker build --build-arg IMAGE_NAME=fedora --build-arg 
> > > > IMAGE_VERSION=29 [...]
> > > > 
> > > > Thanks,
> > > > Severin
> > > >