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

[mikhailo]

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
>>
>