RFR(XS): 8227317: [TESTBUG] jdk docker/TestDockerMemoryMetrics.java fails on systems w/o kernel memory accounting

[mikhailo.seledtsov at oracle.com]

Hi Severin,

On 10/23/19 2:08 AM, Severin Gehwolf wrote:
> Hi Misha,
>
> On Tue, 2019-10-22 at 11:50 -0700, mikhailo.seledtsov at oracle.com wrote:
>> Please review this change that adds extra check for test case
>> testKernelMemoryLimit(). If system does not support kernel memory
>> accounting then verification for this test case is skipped.
>>
>>       JBS: https://bugs.openjdk.java.net/browse/JDK-8227317
>>       Webrev: http://cr.openjdk.java.net/~mseledtsov/8227317.01/
>   123         OutputAnalyzer oa = DockerTestUtils.dockerRunJava(opts);
>   124         if (oa.getStderr().contains("kernel memory accounting disabled")) {
>   125             System.out.println("Kernel memory accounting disabled on this system, " +
>   126                                        "skipping the test case");
>   127             return;
>   128         }
>   129         oa.shouldHaveExitValue(0).shouldContain("TEST PASSED!!!");
>   130     }
>
>  From what I understand this message comes from some container runtimes
> (runc in this case), right? It seems to be a case where the system
> potentially has kernel memory accounting, but the container runtime was
> compiled without support for it. Have you checked whether systems where
> the test fails also have 'cgroup.memory=nokmem' as kernel
> commandline[1]?

I am not sure, could be either case. In any case, the tests should be 
able to handle this type of error.

Perhaps, I should change the message to simply echo the original message 
from the docker command: "Kernel memory accounting disabled, skipping 
test case",

that is w/o mentioning "on this system".


> In either case it seems diligent to add a comment prior the if.
>
> Suggestion:
>
>         OutputAnalyzer oa = DockerTestUtils.dockerRunJava(opts);
>         // Some container runtimes (e.g. runc, docker 18.09)
>         // have been built without kernel memory accounting. In
>         // that case, the runtime issues a message on stderr saying
>         // so. Skip the test in that case.
>         if (oa.getStderr().contains("kernel memory accounting disabled")) {

Thanks. I will add the comment that you recommend.

Thank you,

Misha

>
> Thanks,
> Severin
>
> [1] cat /proc/cmdline | grep cgroup.memory=nokmem
>
>>       Testing:
>>          1. Ran the test on systems with disabled kernel memory
>> accounting before and after the fix - fix works
>>          2. Ran the test case 60 times on multiple Linux-x64 systems - no
>> failures
>>
>>
>> Thank you,
>> Misha
>>