RFR(S): 8227122: [TESTBUG] Create Docker sidecar test cases

Fri Jul 12 09:12:37 UTC 2019

Hi Misha,

On Thu, 2019-07-11 at 17:58 -0700, mikhailo.seledtsov at oracle.com wrote:
> Hi Severin,
> 
>    Thank you for taking a look at this change.
> 
> On 7/10/19 10:40 AM, Severin Gehwolf wrote:
> > Hi Misha,
> > 
> > On Tue, 2019-07-02 at 15:24 -0700, mikhailo.seledtsov at oracle.com wrote:
> > > Please review this new test that uses a Docker sidecar pattern to
> > > manage/monitor JVM running in the main payload container.
> > > 
> > > Sidecar is a common pattern used in the cloud environments for
> > > monitoring among other uses. In side car pattern the main
> > > application/service container that runs the payload is paired with a
> > > sidecar container. It is achieved by sharing certain namespace
> > > aspects
> > > between the two containers such as PID namespace, specific
> > > sub-directories, IPC and more.
> > > 
> > > This test implements the following cases:
> > >     - "jcmd -l" to list java processes running in "main" container
> > > from
> > > the "sidecar" container
> > >     - "jhsdb jinfo" in the sidecar configuration
> > >     - jcmd <some-command>
> > > 
> > > This change also builds a basis for more test cases in the future.
> > > 
> > > Minor changes were done to DockerTestUtils:
> > >     - changing access to DOCKER_COMMAND constant to public
> > >     - minor spelling and terminology corrections
> > > 
> > > 
> > >       JBS: https://bugs.openjdk.java.net/browse/JDK-8227122
> > >       Webrev: http://cr.openjdk.java.net/~mseledtsov/8227122.00/
> > >       Testing:
> > >           1. ran Docker tests on Linux-x64 - PASS
> > >           2. Running Docker tests in test cluster - in progress
> > > 
> > 
> > // JCMD does not work in sidecar configuration, except for "jcmd -l".
> > // Including this test case to assist in reproduction of the problem.
> > // t.assertIsAlive();
> > // testCase03(mainProcPid);
> > 
> > FWIW, "jcmd -l" doesn't work in this case either. It only sees itself
> > as far as I can tell.
> 
> In my experiment it does work. Here are parts of the test log, first the 
> command that runs jcmd in a sidecar container, then the output of that 
> container:
> 
> """
> 
> [COMMAND]
> 
> /usr/local/bin/docker run --tty=true --rm --cap-add=SYS_PTRACE 
> --sig-proxy=true --pid=container:test-container-main 
> --ipc=container:test-container-main -v 
> /ws/playArea/sidecar-jcmd-8227122/JTwork/scratch/.:/tmp/ 
> jdk-internal:test-jfr-jcmd /jdk/bin/jcmd -l
> 
> [2019-07-12T00:26:29.083764Z] Gathering output for process 8703
> [ELAPSED: 5 ms]
> [STDERR]
> 
> [STDOUT]
> 1 EventGeneratorLoop 15
> 23 jdk.jcmd/sun.tools.jcmd.JCmd -l
> 
> """
> 
> The output shows 2 processes, one is EventGeneratorLoop with PID of 1 
> (as expected). This is possible because the containers share certain 
> namespaces and mounted volumes in a 'sidecar' configuration. In this 
> case, containers share the PID namespace 
> (--pid=container:test-container-main) and share volumes mounted as 
> "/tmp" inside the container (-v 
> /ws/playArea/sidecar-jcmd-8227122/JTwork/scratch/.:/tmp/)
> 

Right, sorry. Perhaps this code should get a comment that sharing /tmp
between sidecar and host container is needed for jvmstat - used
internally by the attach mechanism - to work. See
HotSpotAttachProvider.testAttachable():

+        String[] command = new String[] {
+            DockerTestUtils.DOCKER_COMMAND, "run",
+            "--tty=true", "--rm",
+            "--cap-add=SYS_PTRACE", "--sig-proxy=true",
+            "--pid=container:" + MAIN_CONTAINER_NAME,
+            "--pid=container:" + MAIN_CONTAINER_NAME,
+            "--ipc=container:" + MAIN_CONTAINER_NAME,
+            "-v", WORK_DIR + ":/tmp/",

I believe -XX:+UsePerfData would be in order too as I don't think
things would work if that default changed.

> > What's more, this seems to be a case of AttachListener::is_init_trigger[1] and
> > VirtualMachineImpl.createAttachFile[2] disagreeing. The former looks in
> > $(pwd)/.attach_pid<pid> or /tmp/.attach_pid<pid> and the latter creates
> > it in /proc/<pid>/root/tmp/.attach_pid<ns_pid>.

This seems to be the cause for why testCase03 doesn't work. Perhaps
this deserves a bug and I can help fix it.

While looking at that, I discovered what I said below, which is a
different case I know.

> > There seems to be more issues involved. As attaching to a JVM inside a
> > container doesn't seem to work from outside which is supposed to be
> > fixed with JDK-8179498. That alone seems to warrant a bug.
> 
> You are describing a slightly different use case / pattern, but I agree 
> it does not seem to work. I am happy to hear confirmation of that.

I was pointing out that JDK-8179498 seems to have regressed. It's
unrelated but should be taken into account when fixing the above issue.

> The pattern addressed in this test is a side car, where both the 
> observer and observee run in containers; the containers are 'friendly' 
> by sharing certain apsects of namespaces.

Yes.

> The use case you are describing is somewhat different, if I understand 
> correctly: the observer runs on a host machine, and obsrvee runs in a 
> container. Observer tries to use jcmd to list the java processes running 
> in container(s), and issue commands, but that fails. I can create a bug 
> for that, and a simple test case.

There should be a bug and a test so that it cannot again regress.

JDK-8193710 is also related, but the fix for that bug didn't have a
test either :( That's this one which needs fixing:
https://bugs.openjdk.java.net/browse/JDK-8195809

> >      private static DockerThread startMainContainer() throws Exception {
> >          // start "main" container (the observee)
> >          DockerRunOptions opts = commonDockerOpts("EventGeneratorLoop");
> >          opts.addDockerOpts("--cap-add=SYS_PTRACE", "--ipc=shareable")
> > 
> > Is '--ipc=shareable' really needed? It's not a supported option for my
> > docker here :-(
> 
> I have removed the '--ipc=shareable' and the test still works. I think 
> this is extra stuff that is not necessary for this test case, so I will 
> remove it.

Excellent!

> 
> I will incorporate changes from your and Bob's review, run some testing, 
> and post an updated webrev.

Thanks,
Severin

> > [1] http://hg.openjdk.java.net/jdk/jdk/file/ba72dac556c3/src/hotspot/os/linux/attachListener_linux.cpp#l500
> > [2] http://hg.openjdk.java.net/jdk/jdk/file/ba72dac556c3/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java#l295
> > 