[jdk8u-dev] RFR: 8293472: Incorrect container resource limit detection if manual cgroup fs mounts present

[Severin Gehwolf]

On Tue, 20 Dec 2022 12:14:12 GMT, Jonathan Dowland <jdowland at openjdk.org> wrote:

> This is a backport of 8293472 to 8u for cgroups v2 support. Not clean: log_debug→tty->print_cr and Files.writeString → Files.write() needed, as well as adjustments for the lack of [8266490](https://bugs.openjdk.org/browse/JDK-8266490) (PID controller support) in 8u.

Mostly good. Some comments on test code. Thanks!

hotspot/test/runtime/containers/cgroup/CgroupSubsystemFactory.java line 424:

> 422:             test.testCgroupv1MultipleControllerMounts(wb, test.cgroupv1MntInfoDoubleCpu2);
> 423:             test.testCgroupv1MultipleControllerMounts(wb, test.cgroupv1MntInfoDoublePids);
> 424:             test.testCgroupv1MultipleControllerMounts(wb, test.cgroupv1MntInfoDoublePids2);

The PIDs patch is not in 8u, but I guess this does no harm. OK.

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

> 89:             .shouldHaveExitValue(0)
> 90:             .shouldNotMatch("\\[os,container *\\]");
> 91:     }

This test has no meaning in OpenJDK 8u. There, all container related printouts are guarded with `PrintContainerInfo` which is false for this test. It makes sense for JDKs with unified logging, as there a warning was being printed to stdout/err whether or not logging was otherwise turned on or not. It's conceivable that it would catch the assert, but that would also happen in one of the cpu/memory tests with additional cgroup fs mounts.

TLDR: I suggest to skip these test changes in the backport. All it does is adding to exec time, but would always be tautologically true (especially with the `os,container` match).

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

PR: https://git.openjdk.org/jdk8u-dev/pull/216