RFR : 8196062 : Enable docker container related tests for linux ppc64le

[Baesken, Matthias]

> 
> Perhaps, you could add code to DockerTestUtils.buildJdkDockerImage()
> that does the following or similar:
>      1. Construct a name for platform-specific docker file:
>             String platformSpecificDockerfile = dockerfile + "-" +
> Platform.getOsArch();
>             (Platform is jdk.test.lib.Platform)
>

Hello,  the doc  says :

     * Build a docker image that contains JDK under test.
     * The jdk will be placed under the "/jdk/" folder inside the docker file system.
     .....
     param dockerfile    name of the dockerfile residing in the test source
     .....
    public static void   buildJdkDockerImage(String imageName, String dockerfile, String buildDirName)



It does not say anything about doing hidden insertions of  some platform names into  the dockerfile name.
So should the jtreg API doc be changed ?
If so who needs to approve this ?

(as far as I see so far only the  test  at hotspot/jtreg/runtime/containers/docker/   use this  so it should not be a big deal to change the interface?)

Best regards, Matthias




> -----Original Message-----
> From: mikhailo [mailto:mikhailo.seledtsov at oracle.com]
> Sent: Mittwoch, 24. Januar 2018 20:09
> To: Bob Vandette <bob.vandette at oracle.com>; Baesken, Matthias
> <matthias.baesken at sap.com>
> Cc: hotspot-dev at openjdk.java.net; Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com>; Langer, Christoph
> <christoph.langer at sap.com>; Doerr, Martin <martin.doerr at sap.com>
> Subject: Re: RFR : 8196062 : Enable docker container related tests for linux
> ppc64le
> 
> Hi Matthias,
> 
>     Please see my comments about the test changes inline.
> 
> 
> On 01/24/2018 07:13 AM, Bob Vandette wrote:
> > osContainer_linux.cpp:
> >
> > Can you add "return;" in each test for subsystem not found messages and
> > remove these 3 lines OR move your tests for NULL & messages inside.  The
> compiler can
> > probably optimize this but I’d prefer more compact code.
> >
> > if (memory == NULL || cpuset == NULL || cpu == NULL || cpuacct == NULL)
> {
> >
> >   342     return;
> >   343   }
> >
> >
> > The other changes in osContainer_linux.cpp look ok.
> >
> > I forwarded your test changes to Misha, who wrote these.
> >
> > Since it’s likely that other platforms, such as aarch64, are going to run into
> the same problem,
> > It would have been better to enable the tests based on the existence of an
> arch specific
> > Dockerfile-BasicTest-{os.arch} rather than enabling specific arch’s in
> VPProps.java.
> > This approach would reduce the number of changes significantly and allow
> support to
> > be added with 1 new file.
> >
> > You wouldn’t need "String dockerFileName =
> Common.getDockerFileName();”
> > in every test.  Just make DockerTestUtils automatically add arch.
> I like Bob's idea on handling platform-specific Dockerfiles.
> 
> Perhaps, you could add code to DockerTestUtils.buildJdkDockerImage()
> that does the following or similar:
>      1. Construct a name for platform-specific docker file:
>             String platformSpecificDockerfile = dockerfile + "-" +
> Platform.getOsArch();
>             (Platform is jdk.test.lib.Platform)
> 
>      2. Check if platformSpecificDockerfile file exists in the test
> source directory
>            File.exists(Paths.get(Utils.TEST_SRC, platformSpecificDockerFile)
>            If it does, then use it. Otherwise continue using the
> default/original dockerfile name.
> 
> I think this will considerably simplify your change, as well as make it
> easy to extend support to other platforms/configurations
> in the future. Let us know what you think of this approach ?
> 
> 
> Once your change gets (R)eviewed and approved, I can sponsor the push.
> 
> 
> Thank you,
> Misha
> 
> 
> 
> >
> > Bob.
> >
> >
> >
> >> On Jan 24, 2018, at 9:24 AM, Baesken, Matthias
> <matthias.baesken at sap.com> wrote:
> >>
> >> Hello,  could you please review the following change :  8196062 :  Enable
> docker container related tests for linux ppc64le  .
> >>
> >> It  adds  docker container testing   for linux ppc64 le  (little endian) .
> >>
> >> A number of things had to be done :
> >> 	• Add a  separate  docker file
> test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest-ppc64le
> for linux ppc64 le     which uses   Ubuntu ( the  Oracle Linux 7.2  used  for
> x86_64  seems not to be available for ppc64le )
> >> 	• Fix  parsing    /proc/self/mountinfo    and   /proc/self/cgroup  in
> src/hotspot/os/linux/osContainer_linux.cpp    , it could  not  handle  the
> format seen  on SUSE LINUX 12.1 ppc64le (Host)  and  Ubuntu (Docker
> container)
> >> 	• Add a bit  more logging
> >>
> >>
> >> Webrev :
> >>
> >> http://cr.openjdk.java.net/~mbaesken/webrevs/8196062/
> >>
> >>
> >> Bug :
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8196062
> >>
> >>
> >> After these adjustments I could run the    runtime/containers/docker    -
> jtreg tests successfully .
> >>
> >>
> >> Best regards, Matthias