RFR : 8196062 : Enable docker container related tests for linux ppc64le
Mikhailo Seledtsov
mikhailo.seledtsov at oracle.com
Wed Feb 7 05:09:11 UTC 2018
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/TestMutuallyExclusivePlatformPredicates.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