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

Severin Gehwolf sgehwolf at redhat.com
Wed Jul 17 17:34:18 UTC 2019


Hi Misha,

On Wed, 2019-07-17 at 10:22 -0700, 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.
> > 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.

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



More information about the core-libs-dev mailing list