RFR(S): 8181592: [TESTBUG] Docker test utils and docker jdk basic test

[Mikhailo Seledtsov]

Here is the updated webrev:
     http://cr.openjdk.java.net/~mseledtsov/8181592.02/


Leonid, George - thank you for your comments.
In addition to addressing your feedback, I also did:
   - implemented @requires docker.support
   - added dockerRunJava() method and associated data structure 
DockerRunOptions, for running Java tests inside
      docker environment, and to account for java opts, test java opts, 
docker opts, classes and class params
   - added a simple HelloWorld test case that runs HelloWorld inside a 
container
   - ran jtreg with extra Java options, make sure they are added 
correctly to the docker run command
   - added docker image cleanup after testing is done


Please review.

Misha


On 9/27/17, 11:00 AM, mikhailo wrote:
> Leonid,
>
> Thank you for review and constructive feedback. See my comment in line.
>
>
> On 09/26/2017 11:19 AM, Leonid Mesnik wrote:
>> Misha
>>
>> http://cr.openjdk.java.net/~mseledtsov/8181592.00/test/hotspot/jtreg/runtime/containers/docker/DockerBasicTest.java.html 
>> <http://cr.openjdk.java.net/%7Emseledtsov/8181592.00/test/hotspot/jtreg/runtime/containers/docker/DockerBasicTest.java.html> 
>>
>> Copyright is incorrect, need to updated it for GPL.
> Fixed
>>
>> The Hotspot is Oracle VM name only so test might fail for OpenJDK. I 
>> think you need to fix this check.
> I see. I fixed this by using Platform.vmName which should be correct 
> in all cases. I double-checked with OpenJDK also.
>>
>> The requires checks only that test is executed only on the 64-bit 
>> linux. Does it make a sense to introduce more docker-specific check?
> I agree this is a better way. I will do some prototyping; if such 
> check is feasible and efficient in at requires then I will add it.
>>
>>
>> http://cr.openjdk.java.net/~mseledtsov/8181592.00/test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest.html 
>> <http://cr.openjdk.java.net/%7Emseledtsov/8181592.00/test/hotspot/jtreg/runtime/containers/docker/Dockerfile-BasicTest.html> 
>>
>> Could you please explain why oraclelinux 7.0 is used as a base image 
>> for test.
> I have upgraded to Oracle Linux 7.2. If we have specific requirement I 
> will change it to that. If we have requirements in the future to 
> support multiple OS, I can add Dockerfile generation.
> For this basic sanity tests I think this should suffice.
>>
>> http://cr.openjdk.java.net/~mseledtsov/8181592.00/test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java.html 
>> <http://cr.openjdk.java.net/%7Emseledtsov/8181592.00/test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java.html> 
>>
>> The content looks fine.
>>
>> I don’t see anything to clean up docker images on the system. Could 
>> you please explain how tests are going to cleanup images.
> To clean up containers I will add "--rm" to the 'docker run' command. 
> This should ensure that container data is removed after container stops.
> As for the image - I use the same image name. The image will stay in 
> the local registry unless manually removed. I should probably do 
> 'docker rmi' at the end of the test to clean this up.
>
>
> Once I implement these changes I will send the updated webrev.
>
> Thank you,
> Misha
>>
>> Leonid
>>
>>
>>> On Sep 21, 2017, at 5:58 PM, mikhailo <mikhailo.seledtsov at oracle.com 
>>> <mailto:mikhailo.seledtsov at oracle.com>> wrote:
>>>
>>> Please review this initial drop of Docker test utils and a sanity 
>>> test. This change lays ground
>>> for further test development and test utils improvement in this area.
>>>
>>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8181592
>>>     Webrev: http://cr.openjdk.java.net/~mseledtsov/8181592.00/ 
>>> <http://cr.openjdk.java.net/%7Emseledtsov/8181592.00/>
>>>     Testing:
>>>        - run this test on machine with Docker enabled - works
>>>        - run this test on Linux-x64 with no Docker engine or Docker 
>>> disabled - test skipped (as expected)
>>>        - run this test on automated system - in progress
>>>
>>>
>>> Thank you,
>>> Misha
>>>
>>
>