RFR(S): 8195809: [TESTBUG] jps and jcmd -l support for Docker containers is not tested

[Severin Gehwolf]

Hi Misha,

One question:

Is it expected for a root user (outside a container) to see *all* Java
processes inside a container?

As far as I understand it, these tests assert that the same user
(outside) and inside a container can see each other's Java processes.

Review is below...

On Mon, 2019-07-29 at 20:46 -0700, mikhailo.seledtsov at oracle.com wrote:
> Please review this change that:
>    - adds test case for "jcmd -l" and "jcmd <pid> help" where jcmd is 
> executed on a host/node outside the container,
>      while a targeted JVM is running inside a container
>    - factors out some common functionality to DockerTestUtils and 
> docker.Common
> 
> Please note:
>    - the "jcmd -l" works in this configuration, however the JCMD's and 
> Target's username and UID have to match
>      (per design)
>    - the "jcmd help", "jcmd JFR.start" or any other JCMD command besides 
> "jcmd -l" does not work in this configuration
>      (Filed "JDK-8228343: JCMD and attach fail to work across Linux 
> Container boundary")
>      The test case is commented out, however can be used for reproducing 
> the issue, and will be enabled
>      once the bug is fixed.
> 
> 
>      JBS: https://bugs.openjdk.java.net/browse/JDK-8195809
>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8195809.00/

+    // find process ID from JCMD output
+    public static long findPidFromJcmdOutput(OutputAnalyzer out, String name) throws Exception {
+        List<String> l = out.asLines()
+            .stream()
+            .filter(s -> s.contains(name))
+            .collect(Collectors.toList());
+        if (l.isEmpty()) {
+            throw new RuntimeException("Could not find matching process");
+        }
+        String psInfo = l.get(0);

This seems to assume there is exactly one matching line. In that case,
I'd suggest to amend the "l.isEmpty()" condition to:

if (l.isEmpty() or l.size() > 1) ...

     public static void
-        buildDockerImage(String imageName, Path dockerfile, Path buildDir) throws Exception {
+        buildDockerImage(String imageName, Path dockerfile,
+                         Path buildDir, String additionalDockerfileContent) throws Exception {
 
         generateDockerFile(buildDir.resolve("Dockerfile"),
                            DockerfileConfig.getBaseImageName(),
-                           DockerfileConfig.getBaseImageVersion());
+                           DockerfileConfig.getBaseImageVersion(),
+                           additionalDockerfileContent);
         try {
             // Build the docker
             execute(Container.ENGINE_COMMAND, "build", "--no-cache", "--tag", imageName, buildDir.toString())
-                .shouldHaveExitValue(0);
+                .shouldHaveExitValue(0)
+                .shouldContain("Successfully built");

If you assert "Successfully built" in the output it'll break podman
support. See 
https://hg.openjdk.java.net/jdk/jdk/rev/709913d8ace9#l4.38

TestJcmd:

+        // In order for jcmd to work, the USER name and UID of the observer
+        // need to match the USERNAME/UID of the observed JVM process
+        String additionalDockerFileContent =
+            String.format("RUN useradd %s --uid %d \n", getCurrentUserName(), getCurrentUserId()) +
+            String.format("USER %s \n", getCurrentUserName());

Perhaps the test should have a more telling class name. Perhaps
TestJcmdMatchingUsernames. Failing that, add this info the @summary tag
of the test?

TestJcmdWithSideCar:

  80             // JCMD does not work in sidecar configuration, except for "jcmd -l".
  81             // Including this test case to assist in reproduction of the problem.
  82             // t.assertIsAlive();
  83             // testCase03(mainProcPid);

Shouldn't this refer to JDK-8228343 as well? I believe a fix for JDK-
8228343 should cover both test cases.

Thanks,
Severin


>      Testing:
>        - ran the new test multiple times on Linux-x64
>        - ran TestJCMDWithSideCar multiple times on Linux-x64
>        - ran all Docker/Container tests (HotSpot and JDK)
>      All PASS
> 
> Thank you,
> Misha
>