RFR: JDK-8293472: Potentially incorrect container resource limit detection if manual cgroup fs mounts present [v5]

[Severin Gehwolf]

On Thu, 8 Sep 2022 03:22:04 GMT, 王超 <duke at openjdk.org> wrote:

>> Similar to [JDK-8253435](https://bugs.openjdk.org/browse/JDK-8253435), when setting the mount path of cgroup controller "memory/cpu/cpuacct/pids", it should also discard duplicate items.
>
> 王超 has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Change warning to debug log

We definitely need regression tests for memory/cpu limits with those additional cgroup mounts. 
Please take a look at `test/hotspot/jtreg/containers/docker/TestCPUAwareness.java` and `test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java`

For cgroups v2 the issues isn't fixed. The paths for cg v2 are being set here:
https://github.com/openjdk/jdk/blob/98da03af50e2372817a7b5e381eea5ee6f2cb919/src/hotspot/os/linux/cgroupSubsystem_linux.cpp#L303..L313.

Example of running (with your patch on a cg v2 system):


$ sudo podman run --rm -ti --memory=300M --memory-swap=300M -v /sys/fs/cgroup:/cgroup-in:ro -v $(pwd)/jdk20-jdk:/opt/jdk:z fedora:36 /opt/jdk/bin/java -Xlog:os+container=trace -version 2>&1 | grep 'Memory Limit'
[0.001s][trace][os,container] Memory Limit is: -2
[0.055s][trace][os,container] Memory Limit is: -2


Expected:


[0.001s][trace][os,container] Memory Limit is: 314572800
[0.068s][trace][os,container] Memory Limit is: 314572800

test/hotspot/jtreg/containers/docker/DockerBasicTest.java line 91:

> 89:             new DockerRunOptions(imageNameAndTag, "/jdk/bin/java", "-version")
> 90:             .addJavaOpts("-Xlog:os+container=trace")
> 91:             .addDockerOpts("-v", "/sys/fs/cgroup:/cgroups-in:ro");

I'd suggest, to first add a test which verifies that without `-Xlog:os+container=trace` option, `java -version` on such a container doesn't print **any** `os+container` logs (it used to print a warning in the` -v /sys/fs/cgroup:/cgroups-in:ro` case). That's all this test should verify.

test/hotspot/jtreg/containers/docker/DockerBasicTest.java line 102:

> 100:             .shouldContain("Path to /cpu.cfs_quota_us is /sys/fs/cgroup/cpu,cpuacct/cpu.cfs_quota_us")
> 101:             .shouldContain("Path to /cpu.cfs_period_us is /sys/fs/cgroup/cpu,cpuacct/cpu.cfs_period_us")
> 102:             .shouldContain("Path to /memory.limit_in_bytes is /sys/fs/cgroup/memory/memory.limit_in_bytes");

Those are all cgroups v1 specific paths, so the test will fail when run on cgroups v2. I don't think that's necessary. Just remove those and rely on new test cases in `test/hotspot/jtreg/containers/docker/TestCPUAwareness.java` and `test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java`

-------------

Changes requested by sgehwolf (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10193