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

[mikhailo]

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