RFR: 8227642: [TESTBUG] Make docker tests podman compatible

Thu Jul 18 01:38:11 UTC 2019

On 7/17/19 11:37 AM, Igor Ignatyev wrote:
>
> Hi Severin,
>
> the updated webrev looks good to me, please see a couple comments below.
>
> Cheers,
> -- Igor
>
>> On Jul 17, 2019, at 10:34 AM, Severin Gehwolf <sgehwolf at redhat.com 
>> <mailto:sgehwolf at redhat.com>> wrote:
>>
>> Hi Misha,
>>
>> On Wed, 2019-07-17 at 10:22 -0700,mikhailo.seledtsov at oracle.com 
>> <mailto:mikhailo.seledtsov at oracle.com>wrote:
>>> Hi Severin,
>>>
>>> On 7/17/19 5:44 AM, Severin Gehwolf wrote:
>>>> Hi Igor, Misha,
>>>>
>>>> On Tue, 2019-07-16 at 11:49 -0700, Igor Ignatyev wrote:
>>>>> Hi Severin,
>>>>>
>>>>> I don't think that tests (or test libraries for that matter) should
>>>>> be responsible for setting correct PATH value, it should be a part of
>>>>> host configuration procedure (tests can/should check that all
>>>>> required bins are available though). in other words, I'd prefer if
>>>>> you remove 'env.put("PATH", ...)' lines from both DockerTestUtils and
>>>>> TestJFREvents. the rest looks good to me.
>>>> Updated webrev:
>>>> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227642/02/webrev/
>>>>
>>>> No more additions to PATH are being done.
>>>>
>>>> I've discovered that VMProps.java which defines "docker.required", used
>>>> the "docker" binary even for podman test runs. This ended up not
>>>> running most of the tests even with -Djdk.test.docker.command=podman
>>>> specified.
>>> Good catch.
> should we rename docker.support and DOCKER_COMMAND to something more 
> abstract?

Now that more container technologies are coming online we could consider 
more generic names for these properties/variables.

Here are some thoughts:

   - container.support (CONTAINER_COMMAND) - may be too generic

   - linux.container.support (LINUX_CONTAINER_COMMAND) - more narrow

   - even more narrow/specific: oci.container.support 
(OCI_CONTAINER_COMMAND)

      OCI in this case is " Open Container Initiative", ( Linux 
Foundation project to design open standards for Linux Container technology)

      I believe both Docker and Podman are OCI compliant.

However, I would recommend to do this work as part of a new RFE. If we 
agree, I will create an RFE, and we can continue discussion in the 
context of that RFE.

Thank you,

Misha

>
>>>> I've fixed that by moving DOCKER_COMMAND to Platform.java so
>>>> that it can be used in both places.
>>>
>>> Sounds good to me.
>>>
>>> (of course, the alternative would be to import
>>> jdk.test.lib.containers.docker.DockerTestUtils into VMProps.java -- not
>>> sure if there are any potential problems doing it this way)
>>
>> I've tried that but for some reason this was a problem and VMProps
>> failed to compile. I don't know exactly how those jtreg extensions work
>> and went with the Platform approach. Hope that's OK.
>
> all files needed for VMProps (or other @requires expression class) 
> have to be listed in requires.extraPropDefns or 
> requires.extraPropDefns.bootlibs property in TEST.ROOT file in all the 
> test suites which use these extensions. we are trying to be very 
> cautious in what is used by VMProps (directly and indirectly) so these 
> lists won't grow and we won't require any modules other than 
> java.base, given DockerTestUtils has dependencies on a number of other 
> library classes, the Platform approach is much better from that point 
> of view.
>
>>
>>>> Testing: Container tests with docker daemon running on Linux x86_64,
>>>> container tests without docker daemon running (podman is daemon-less)
>>>> via the podman binary on Linux x86_64 (with -e:PATH). All pass.
>>>
>>> Sounds good.
>>>
>>>
>>> Overall looks good to me.
>>
>> Thanks for the review!
>>
>>> One minor nit: DockerTestUtils.java does not need "import
>>> java.util.Map;" (no need to post updated webrev for this change)
>>
>> OK, good catch. Fixed locally.
>>
>> Thanks,
>> Severin
>>
>>>
>>> Thank you,
>>>
>>> Misha
>>>
>>>> More thoughts?
>>>>
>>>> Thanks,
>>>> Severin
>>>>
>>>>> Thanks,
>>>>> -- Igor
>>>>>
>>>>>> On Jul 16, 2019, at 5:36 AM, Severin Gehwolf <sgehwolf at redhat.com 
>>>>>> <mailto:sgehwolf at redhat.com>> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I believe I still need a *R*eviewer for this. Any takers?
>>>>>>
>>>>>> Thanks,
>>>>>> Severin
>>>>>>
>>>>>> On Fri, 2019-07-12 at 15:19 -0700, mikhailo.seledtsov at oracle.com 
>>>>>> <mailto:mikhailo.seledtsov at oracle.com> wrote:
>>>>>>> Hi Severin,
>>>>>>>
>>>>>>>   The change looks good to me. Thank you for adding support for 
>>>>>>> Podman
>>>>>>> container technology.
>>>>>>>
>>>>>>> Testing: I ran both HotSpot and JDK container tests with your patch;
>>>>>>> tests executed on Oracle Linux 7.6 using default container 
>>>>>>> engine (Docker):
>>>>>>>
>>>>>>>     test/hotspot/jtreg/containers/   AND
>>>>>>> test/jdk/jdk/internal/platform/docker/
>>>>>>>
>>>>>>> All PASS
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Misha
>>>>>>>
>>>>>>>
>>>>>>> On 7/12/19 11:08 AM, Severin Gehwolf wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> There is an alternative container engine which is being used by 
>>>>>>>> Fedora
>>>>>>>> and RHEL 8, called podman[1]. It's mostly compatible with 
>>>>>>>> docker. It
>>>>>>>> looks like OpenJDK docker tests can be made podman compatible 
>>>>>>>> with a
>>>>>>>> few little tweaks. One "interesting" one is to not assert 
>>>>>>>> "Successfully
>>>>>>>> built" in the build output but only rely on the exit code, 
>>>>>>>> which seems
>>>>>>>> to be OK for my testing. Interestingly the test would be skipped in
>>>>>>>> that case.
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8227642
>>>>>>>> webrev: 
>>>>>>>> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227642/01/webrev/
>>>>>>>>
>>>>>>>> Adjustments I've done:
>>>>>>>>  * Don't assert "Successfully built" in image build output[2].
>>>>>>>>  * Add /usr/sbin to PATH as the podman binary relies on 
>>>>>>>> iptables for it
>>>>>>>>    to work which is in /usr/sbin on Fedora
>>>>>>>>  * Allow for Metrics.getCpuSystemUsage() and 
>>>>>>>> Metrics.getCpuUserUsage()
>>>>>>>>    to be equal to the previous value. I've found those counters 
>>>>>>>> to be
>>>>>>>>    slowly increasing, which made the tests unreliable.
>>>>>>>>
>>>>>>>> Testing:
>>>>>>>>
>>>>>>>> Running docker tests with docker as engine. Did the same with 
>>>>>>>> podman as
>>>>>>>> engine via -Djdk.test.docker.command=podman on Linux x86_64. Both
>>>>>>>> passed (non-trivially).
>>>>>>>>
>>>>>>>> Thoughts?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Severin
>>>>>>>>
>>>>>>>> [1] https://podman.io/
>>>>>>>> [2] Image builds with podman look
>>>>>>>>     like ("COMMIT" over "Successfully built"):
>>>>>>>> STEP 1: FROM fedora:29
>>>>>>>> STEP 2: RUN dnf install -y java-11-openjdk-devel &&     dnf 
>>>>>>>> clean all
>>>>>>>> --> Using cache 
>>>>>>>> 96f8b1a0dfe7dba581a64fc67a27002ddf52e032af55f9ddc765182a690afd9d
>>>>>>>> STEP 3: COPY TestMetrics.class  TestMetrics.java /opt/
>>>>>>>> 269042160f7a4e6a06789cd19640ea658a8f941bc53de0fd40a574dc3bdb49a8
>>>>>>>> STEP 4: CMD /usr/lib/jvm/java-11-openjdk/bin/java -cp /opt 
>>>>>>>> --add-modules java.base --add-exports 
>>>>>>>> java.base/jdk.internal.platform=ALL-UNNAMED TestMetrics
>>>>>>>> STEP 5: COMMIT fedora-metrics-11
>>>>>>>> d749088d6ce4510f212820ad4eca55a9b05e5c5c245f2372b6cfe91926e8cd7e
>