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

mikhailo mikhailo.seledtsov at oracle.com
Wed Feb 7 21:42:09 UTC 2018 

Pre-integration tests executed; no new failures; change integrated.


Best regards,

Misha


On 02/07/2018 10:24 AM, mikhailo wrote:
> Thank you for the update. I will import your latest webrev, and start 
> the testing.
>
>
> Regard,
>
> Misha
>
>
> On 02/07/2018 08:12 AM, Baesken, Matthias wrote:
>> Hi Mikhailo,  sorry for causing the issue.
>> Looks like  the change to test/lib/jdk/test/lib/Platform.java    had 
>> unexpected consequences .
>> I created a new webrev   without the  Platform.java change :
>>
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8196062.3/
>>
>> Best regards, Matthias
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: Mikhailo Seledtsov [mailto:mikhailo.seledtsov at oracle.com]
>>> Sent: Mittwoch, 7. Februar 2018 06:09
>>> To: Baesken, Matthias <matthias.baesken at sap.com>
>>> Cc: hotspot-dev at openjdk.java.net
>>> Subject: Re: RFR : 8196062 : Enable docker container related tests 
>>> for linux
>>> ppc64le
>>>
>>> Hi Matthias,
>>>
>>> Unfortunately one test failed during the pre-integration testing.
>>> The following test failed during the pre-integration testing:
>>> open/test/hotspot/jtreg/testlibrary_tests/TestMutuallyExclusivePlatformPr 
>>>
>>> edicates.java
>>>
>>>
>>> Reproducible: 100%, Linux and MAC
>>>
>>> Failure: TEST RESULT: Failed. Execution failed: `main' threw exception:
>>> java.lang.RuntimeException: All Platform's methods with signature 
>>> '():Z'
>>> should be tested. Missing: isPPC64le: expected true, was false
>>>
>>> See my comments in the RFE for details, and a suggested fix.
>>>
>>>
>>> Thank you,
>>> Misha
>>>
>>> On 2/6/18, 7:28 PM, Mikhailo Seledtsov wrote:
>>>> I am running pre-integration testing; will push unless the testing
>>>> finds any issues.
>>>>
>>>> Regards,
>>>> Misha
>>>>
>>>> On 2/5/18, 11:50 PM, Baesken, Matthias wrote:
>>>>> I only had to correct some whitespace changes  found by hg jcheck ,
>>>>> updated :
>>>>>
>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8196062.2/
>>>>>
>>>>> The fix has been reviewed by goetz, dsamersoff, bobv  .
>>>>> Feedback was added,  Summary updated too  (suggested by Goetz).
>>>>>
>>>>> Tested with docker on SLES 12.1  / Ubuntu based container .
>>>>>
>>>>> Best regards, Matthias
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Mikhailo Seledtsov [mailto:mikhailo.seledtsov at oracle.com]
>>>>>> Sent: Samstag, 3. Februar 2018 00:01
>>>>>> To: Baesken, Matthias<matthias.baesken at sap.com>
>>>>>> Cc: Bob Vandette<bob.vandette at oracle.com>; Lindenmaier, Goetz
>>>>>> <goetz.lindenmaier at sap.com>; hotspot-dev at openjdk.java.net;
>>> Langer,
>>>>>> Christoph<christoph.langer at sap.com>; Doerr, Martin
>>>>>> <martin.doerr at sap.com>; Dmitry Samersoff<dmitry.samersoff at bell-
>>>>>> sw.com>
>>>>>> Subject: Re: RFR : 8196062 : Enable docker container related tests
>>>>>> for linux
>>>>>> ppc64le
>>>>>>
>>>>>> Hi Matthias,
>>>>>>
>>>>>>      I can sponsor your change if you'd like.
>>>>>> Once you addressed all the feedback from code review, please sync to
>>>>>> the
>>>>>> tip, build and test.
>>>>>> Then export the changeset and send it to me (see:
>>>>>> http://openjdk.java.net/sponsor/)
>>>>>>
>>>>>> I will import your change set, run all required testing and push the
>>>>>> change.
>>>>>>
>>>>>>
>>>>>> Thank you,
>>>>>> Misha
>>>>>>
>>>>>> On 2/2/18, 12:39 AM, Baesken, Matthias wrote:
>>>>>>> Thanks for the reviews .
>>>>>>>
>>>>>>> I added info about the fix for /proc/self/cgroup and
>>>>>>> /proc/self/mountinfo
>>>>>> parsing to the bug :
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8196062
>>>>>>>
>>>>>>> Guess I need a sponsor now to get it pushed ?
>>>>>>>
>>>>>>>
>>>>>>> Best regards, Matthias
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Bob Vandette [mailto:bob.vandette at oracle.com]
>>>>>>>> Sent: Donnerstag, 1. Februar 2018 17:53
>>>>>>>> To: Lindenmaier, Goetz<goetz.lindenmaier at sap.com>
>>>>>>>> Cc: Baesken, Matthias<matthias.baesken at sap.com>; mikhailo
>>>>>>>> <mikhailo.seledtsov at oracle.com>; hotspot-dev at openjdk.java.net;
>>>>>> Langer,
>>>>>>>> Christoph<christoph.langer at sap.com>; Doerr, Martin
>>>>>>>> <martin.doerr at sap.com>; Dmitry Samersoff<dmitry.samersoff at bell-
>>>>>>>> sw.com>
>>>>>>>> Subject: Re: RFR : 8196062 : Enable docker container related tests
>>>>>>>> for linux
>>>>>>>> ppc64le
>>>>>>>>
>>>>>>>> Looks good to me.
>>>>>>>>
>>>>>>>> Bob.
>>>>>>>>
>>>>>>>>> On Feb 1, 2018, at 5:39 AM, Lindenmaier, Goetz
>>>>>>>> <goetz.lindenmaier at sap.com>   wrote:
>>>>>>>>> Hi Matthias,
>>>>>>>>>
>>>>>>>>> thanks for enabling this test. Looks good.
>>>>>>>>> I would appreciate if you would add a line
>>>>>>>>> "Summary: also fix cgroup subsystem recognition"
>>>>>>>>> to the bug description.  Else this might be mistaken
>>>>>>>>> for a mere testbug.
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>>     Goetz.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Baesken, Matthias
>>>>>>>>>> Sent: Mittwoch, 31. Januar 2018 15:15
>>>>>>>>>> To: mikhailo<mikhailo.seledtsov at oracle.com>; Bob Vandette
>>>>>>>>>> <bob.vandette at oracle.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>;
>>>>>>>>>> Dmitry Samersoff<dmitry.samersoff at bell-sw.com>
>>>>>>>>>> Subject: RE: RFR : 8196062 : Enable docker container related
>>>>>>>>>> tests for
>>>>>> linux
>>>>>>>>>> ppc64le
>>>>>>>>>>
>>>>>>>>>> Hello , I created a second webrev :
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8196062.1/8196062.1/webr
>>>>>>>>>> ev/
>>>>>>>>>>
>>>>>>>>>> - changed DockerTestUtils.buildJdkDockerImage in the suggested
>>>>>>>>>> way
>>>>>>>> (this
>>>>>>>>>> should be extendable to linux s390x soon)
>>>>>>>>>>
>>>>>>>>>>>>>> Can you add "return;" in each test for subsystem not found
>>>>>>>> messages
>>>>>>>>>> - added returns in the tests for the subsystems in
>>>>>> osContainer_linux.cpp
>>>>>>>>>> - moved some checks at the beginning of  subsystem_file_contents
>>>>>>>>>> (suggested by Dmitry)
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Best regards, Matthias
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: mikhailo [mailto:mikhailo.seledtsov at oracle.com]
>>>>>>>>>>> Sent: Donnerstag, 25. Januar 2018 18:43
>>>>>>>>>>> To: Baesken, Matthias<matthias.baesken at sap.com>; Bob
>>> Vandette
>>>>>>>>>>> <bob.vandette at oracle.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,
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 01/25/2018 12:15 AM, Baesken, Matthias wrote:
>>>>>>>>>>>>> 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 ?
>>>>>>>>>>> Thank you for your concerns about the clarity of API and
>>>>>> corresponding
>>>>>>>>>>> documentation. This is a test library API, so no need to file
>>>>>>>>>>> CCC or CSR.
>>>>>>>>>>>
>>>>>>>>>>> This API can be changed via a regular RFR/webrev review
>>>>>>>>>>> process, as
>>>>>>>> soon
>>>>>>>>>>> as on one objects. I am a VM SQE engineer covering the docker
>>> and
>>>>>>>> Linux
>>>>>>>>>>> container area, I am OK with this change.
>>>>>>>>>>> And I agree with you, we should update the javadoc header on
>>> this
>>>>>>>>>> method
>>>>>>>>>>> to reflect this implicit part of API contract.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thank you,
>>>>>>>>>>> Misha
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> (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
>

More information about the hotspot-dev mailing list